diff --git a/.ai/spec/how/reconciler.md b/.ai/spec/how/reconciler.md index 611966a0..2f90a466 100644 --- a/.ai/spec/how/reconciler.md +++ b/.ai/spec/how/reconciler.md @@ -24,7 +24,7 @@ Audience: AI agents. Behavioral rules and phase semantics live in **what/** spec | File | Types / primary responsibilities | Key functions / methods | |------|----------------------------------|-------------------------| | `reconciler.go` | `AgenticRunReconciler` (embeds `client.Client`, `Agent AgentCaller`, `Log`) | `Reconcile`, `SetupWithManager` | -| `handlers.go` | (methods on `AgenticRunReconciler`) | `handleAnalysis`, `handleRevision`, `handleExecution`, `handleVerification`, `handleEscalation`, `handleFailed`, `denyAgenticRun`, `conditionTime`, `hasMutationSuccess`, `isObservationAction` | +| `handlers.go` | (methods on `AgenticRunReconciler`) | `handleAnalysis`, `handleRevision`, `handleExecution`, `handleVerification`, `handleEscalation`, `handleFailed`, `denyAgenticRun`, `conditionTime`, `hasMutationSuccess`, `isObservationAction`, `analysisFailureMessage`, `executionFailureMessage` | | `helpers.go` | `revisionData`, `analysisQuery`, `executionQuery`, `verificationQuery`, `escalationData`; embedded templates via `//go:embed templates/*.tmpl` | `renderTemplate`, `failStep`, `statusPatch`, `hasSandboxClaims`, `isTerminal`, `setVerificationSkipped`, `getLatestAnalysisResult`, `selectedOption`, `trimNonSelectedOptions`, `resetExecutionAndVerification`, `maxAttempts`, `buildEscalationRequest`, `needsRevision`, `buildRevisionContext`, `buildAnalysisQuery`, `buildExecutionQuery`, `buildVerificationQuery`, `prettyJSON` | | `approval.go` | — | `getApprovalPolicy`, `getAgenticRunApproval`, `ensureAgenticRunApproval`, `isStageApproved`, `isStageDenied`, `getStageOverrideAgent`, `getStageOption` | | `resolve.go` | `resolvedStep`, `resolvedWorkflow` | `resolveAgenticRun`, `stepAgentName` | diff --git a/.ai/spec/what/run-lifecycle.md b/.ai/spec/what/run-lifecycle.md index e3ce675f..d28e3a72 100644 --- a/.ai/spec/what/run-lifecycle.md +++ b/.ai/spec/what/run-lifecycle.md @@ -50,6 +50,7 @@ Behavioral specification for the `AgenticRun` resource lifecycle. **Approval gat | `False` | `ImagePullFailed` | Pod stuck in ImagePullBackOff | 15. **Success**: `Verified=True` MUST yield `Completed` once rule 9 reaches the `Verified` branch, unless an earlier branch already returned `Escalated` or `Denied` per rules 9–10. 16. **Step failure**: Any of `Analyzed`, `Executed`, or `Verified` with status `False` and reasons that are not the dedicated retrying-execution reason MUST yield `Failed` when reached by the derivation order in rule 9 (unless superseded by `Escalated` / `Denied` per rules 9–10). +16a. **[OLS-3666] Failure condition message**: When the controller sets a step condition to `False` (reason `Failed`) because the agent returned `success: false`, the condition `message` MUST include context from the agent response rather than a generic string. The controller MUST use the first available source from this fallback chain: (1) the sandbox response `summary` field (which contains the error message for sandbox-level failures or the raw agent output when the output schema has no top-level `summary` property); (2) for analysis: the top-level or per-option `diagnosis.summary`; for execution: the first failed action's `description` and `error`, or the inline `verification.summary`; (3) a properly-cased generic fallback (`"Analysis agent reported failure"` / `"Execution agent reported failure"`). The message MUST use sentence casing. 17. **Escalation failure**: `Escalated` with status `False` MUST yield `Failed` once rule 9 evaluates the `Escalated` presence branch (non-`True`, non-`Unknown`). 18. **Result CR linkage**: Each analysis/execution/verification/escalation attempt SHOULD append a `status.steps.*.results[]` entry naming the corresponding result resource with an outcome matching agent success/failure for that attempt. **Exception:** when the execution agent reports `success=false` but all mutating actions succeeded (only inline verification checks failed), the controller MUST override the outcome to `Succeeded` and proceed to the verification step. Observation action types (`pre-check`, `post-check`, `verification`, `check`, `wait`) are not considered when determining mutation success. 19. **Observed generation**: Conditions SHOULD carry `observedGeneration` aligned with `metadata.generation` when the controller updates them for the current spec generation, except revision completion MAY pin the analyzed condition to the generation that triggered the revision, per existing reconciliation behavior. diff --git a/controller/agenticrun/agent.go b/controller/agenticrun/agent.go index 07ea3d89..0e8dda3a 100644 --- a/controller/agenticrun/agent.go +++ b/controller/agenticrun/agent.go @@ -11,6 +11,7 @@ import ( // backward-compatible with agents that don't emit the field). type AnalysisOutput struct { Success bool + Summary string ActionRequired *bool Options []agenticv1alpha1.RemediationOption Diagnosis *agenticv1alpha1.DiagnosisResult @@ -25,6 +26,7 @@ func (a *AnalysisOutput) IsActionRequired() bool { // ExecutionOutput holds the execution agent's output. type ExecutionOutput struct { Success bool + Summary string ActionsTaken []agenticv1alpha1.ExecutionAction Verification agenticv1alpha1.ExecutionVerification } diff --git a/controller/agenticrun/handlers.go b/controller/agenticrun/handlers.go index d7993b08..2723ad3d 100644 --- a/controller/agenticrun/handlers.go +++ b/controller/agenticrun/handlers.go @@ -104,7 +104,7 @@ func (r *AgenticRunReconciler) handleAnalysis( return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, err) } if !analysisResult.Success { - return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, fmt.Errorf("analysis agent reported failure")) + return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, fmt.Errorf("%s", analysisFailureMessage(analysisResult))) } base = run.DeepCopy() completedAt := metav1.Now() @@ -187,7 +187,7 @@ func (r *AgenticRunReconciler) handleRevision( return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, err) } if !analysisResult.Success { - return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, fmt.Errorf("analysis agent reported failure")) + return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, fmt.Errorf("%s", analysisFailureMessage(analysisResult))) } base = run.DeepCopy() @@ -335,7 +335,7 @@ func (r *AgenticRunReconciler) handleExecution( } if !execResult.Success { if !hasMutationSuccess(execResult.ActionsTaken) { - return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionExecuted, fmt.Errorf("execution agent reported failure")) + return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionExecuted, fmt.Errorf("%s", executionFailureMessage(execResult))) } log.Info("execution agent reported success=false but all mutations succeeded; deferring outcome to verification step") execResult.Success = true @@ -749,6 +749,45 @@ func (r *AgenticRunReconciler) setNoActionRequired(ctx context.Context, run *age return ctrl.Result{}, nil } +// analysisFailureMessage builds a descriptive failure message from the +// analysis output, preferring the sandbox summary when available. +func analysisFailureMessage(result *AnalysisOutput) string { + if result.Summary != "" { + return fmt.Sprintf("Analysis failed: %s", result.Summary) + } + if result.Diagnosis != nil && result.Diagnosis.Summary != "" { + return fmt.Sprintf("Analysis failed: %s", result.Diagnosis.Summary) + } + for _, opt := range result.Options { + if opt.Diagnosis.Summary != "" { + return fmt.Sprintf("Analysis failed: %s", opt.Diagnosis.Summary) + } + } + return "Analysis agent reported failure" +} + +// executionFailureMessage builds a descriptive failure message from the +// execution output, preferring the sandbox summary when available. +func executionFailureMessage(result *ExecutionOutput) string { + if result.Summary != "" { + return fmt.Sprintf("Execution failed: %s", result.Summary) + } + for _, action := range result.ActionsTaken { + if action.Outcome == agenticv1alpha1.ActionOutcomeFailed { + if action.Error != "" { + return fmt.Sprintf("Execution failed: %s — %s", action.Description, action.Error) + } + if action.Description != "" { + return fmt.Sprintf("Execution failed: %s", action.Description) + } + } + } + if result.Verification.Summary != "" { + return fmt.Sprintf("Execution failed: %s", result.Verification.Summary) + } + return "Execution agent reported failure" +} + func conditionTime(conditions []metav1.Condition, condType string) *metav1.Time { if c := meta.FindStatusCondition(conditions, condType); c != nil { return &c.LastTransitionTime diff --git a/controller/agenticrun/handlers_test.go b/controller/agenticrun/handlers_test.go index 2a311878..81873c6d 100644 --- a/controller/agenticrun/handlers_test.go +++ b/controller/agenticrun/handlers_test.go @@ -1526,3 +1526,122 @@ func TestConditionTime(t *testing.T) { t.Errorf("expected nil for missing condition, got %v", *got) } } + +func TestAnalysisFailureMessage(t *testing.T) { + tests := []struct { + name string + result *AnalysisOutput + want string + }{ + { + name: "summary takes priority", + result: &AnalysisOutput{ + Summary: "Unable to connect to cluster API", + Diagnosis: &agenticv1alpha1.DiagnosisResult{Summary: "should not appear"}, + }, + want: "Analysis failed: Unable to connect to cluster API", + }, + { + name: "falls back to top-level diagnosis", + result: &AnalysisOutput{ + Diagnosis: &agenticv1alpha1.DiagnosisResult{Summary: "OOMKilled due to memory limit of 256Mi"}, + }, + want: "Analysis failed: OOMKilled due to memory limit of 256Mi", + }, + { + name: "falls back to per-option diagnosis", + result: &AnalysisOutput{ + Options: []agenticv1alpha1.RemediationOption{ + {Diagnosis: agenticv1alpha1.DiagnosisResult{Summary: "CrashLoopBackOff caused by missing config"}}, + }, + }, + want: "Analysis failed: CrashLoopBackOff caused by missing config", + }, + { + name: "uses JSON summary when no top-level summary property", + result: &AnalysisOutput{ + Summary: `{"success": false, "options": []}`, + Diagnosis: &agenticv1alpha1.DiagnosisResult{Summary: "real diagnosis"}, + }, + want: `Analysis failed: {"success": false, "options": []}`, + }, + { + name: "no details available", + result: &AnalysisOutput{}, + want: "Analysis agent reported failure", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := analysisFailureMessage(tt.result); got != tt.want { + t.Errorf("analysisFailureMessage() = %q, want %q", got, tt.want) + } + }) + } +} + +func TestExecutionFailureMessage(t *testing.T) { + tests := []struct { + name string + result *ExecutionOutput + want string + }{ + { + name: "summary takes priority", + result: &ExecutionOutput{ + Summary: "Timed out waiting for pod readiness", + ActionsTaken: []agenticv1alpha1.ExecutionAction{ + {Description: "should not appear", Outcome: agenticv1alpha1.ActionOutcomeFailed, Error: "also ignored"}, + }, + }, + want: "Execution failed: Timed out waiting for pod readiness", + }, + { + name: "falls back to failed action with error", + result: &ExecutionOutput{ + ActionsTaken: []agenticv1alpha1.ExecutionAction{ + {Description: "Patched deployment/web", Outcome: agenticv1alpha1.ActionOutcomeFailed, Error: "forbidden: insufficient permissions"}, + }, + }, + want: "Execution failed: Patched deployment/web — forbidden: insufficient permissions", + }, + { + name: "falls back to failed action without error", + result: &ExecutionOutput{ + ActionsTaken: []agenticv1alpha1.ExecutionAction{ + {Description: "Scale deployment to 3 replicas", Outcome: agenticv1alpha1.ActionOutcomeFailed}, + }, + }, + want: "Execution failed: Scale deployment to 3 replicas", + }, + { + name: "uses JSON summary when no top-level summary property", + result: &ExecutionOutput{ + Summary: `{"success": false, "actionsTaken": []}`, + ActionsTaken: []agenticv1alpha1.ExecutionAction{ + {Description: "Patched deployment", Outcome: agenticv1alpha1.ActionOutcomeFailed}, + }, + }, + want: `Execution failed: {"success": false, "actionsTaken": []}`, + }, + { + name: "falls back to verification summary", + result: &ExecutionOutput{ + Verification: agenticv1alpha1.ExecutionVerification{Summary: "Pod still in CrashLoopBackOff"}, + }, + want: "Execution failed: Pod still in CrashLoopBackOff", + }, + { + name: "no details available", + result: &ExecutionOutput{}, + want: "Execution agent reported failure", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := executionFailureMessage(tt.result); got != tt.want { + t.Errorf("executionFailureMessage() = %q, want %q", got, tt.want) + } + }) + } +} diff --git a/controller/agenticrun/sandbox_agent.go b/controller/agenticrun/sandbox_agent.go index fa793998..72890f5f 100644 --- a/controller/agenticrun/sandbox_agent.go +++ b/controller/agenticrun/sandbox_agent.go @@ -31,6 +31,7 @@ const ( type analysisResponse struct { Success bool `json:"success"` + Summary string `json:"summary,omitempty"` ActionRequired *bool `json:"actionRequired,omitempty"` Diagnosis *agenticv1alpha1.DiagnosisResult `json:"diagnosis,omitempty"` Options []agenticv1alpha1.RemediationOption `json:"options"` @@ -38,6 +39,7 @@ type analysisResponse struct { type executionResponse struct { Success bool `json:"success"` + Summary string `json:"summary,omitempty"` ActionsTaken []agenticv1alpha1.ExecutionAction `json:"actionsTaken"` Verification *agenticv1alpha1.ExecutionVerification `json:"verification,omitempty"` } @@ -103,6 +105,7 @@ func (s *SandboxAgentCaller) Analyze(ctx context.Context, run *agenticv1alpha1.A return &AnalysisOutput{ Success: resp.Success, + Summary: resp.Summary, ActionRequired: &actionRequired, Options: resp.Options, Diagnosis: resp.Diagnosis, @@ -128,6 +131,7 @@ func (s *SandboxAgentCaller) Execute(ctx context.Context, run *agenticv1alpha1.A out := &ExecutionOutput{ Success: resp.Success, + Summary: resp.Summary, ActionsTaken: resp.ActionsTaken, } if resp.Verification != nil {