Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/v1alpha1/agenticrun_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,15 @@ const (
AgenticRunPhaseEscalating AgenticRunPhase = "Escalating"
AgenticRunPhaseEscalated AgenticRunPhase = "Escalated"
AgenticRunPhaseEmergencyStopped AgenticRunPhase = "EmergencyStopped"
AgenticRunPhaseNoActionRequired AgenticRunPhase = "NoActionRequired"
)

// Condition reasons used by DerivePhase for state transitions.
// SYNC: must match derivePhaseFromConditions in lightspeed-agentic-console/src/models/agenticrun.ts
const (
ReasonRetryingExecution = "RetryingExecution"
ReasonRetriesExhausted = "RetriesExhausted"
ReasonNoActionRequired = "NoActionRequired"
)

// DerivePhase computes the display phase from conditions. Conditions are
Expand Down Expand Up @@ -112,6 +114,9 @@ func DerivePhase(conditions []metav1.Condition) AgenticRunPhase {
if c := get(AgenticRunConditionAnalyzed); c != nil {
switch c.Status {
case metav1.ConditionTrue:
if c.Reason == ReasonNoActionRequired {
return AgenticRunPhaseNoActionRequired
}
return AgenticRunPhaseProposed
case metav1.ConditionUnknown:
return AgenticRunPhaseAnalyzing
Expand Down
34 changes: 33 additions & 1 deletion api/v1alpha1/analysisresult_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,26 @@ import (
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
)

// ActionRequiredValue is a string enum indicating whether the analysis
// determined that remediation action is required. Uses string values
// "True"/"False" instead of a boolean to satisfy the kube-api-linter
// nobools rule.
// +kubebuilder:validation:Enum=True;False
type ActionRequiredValue string

const (
ActionRequiredTrue ActionRequiredValue = "True"
ActionRequiredFalse ActionRequiredValue = "False"
)

// ActionRequiredFromBool converts a Go bool to ActionRequiredValue.
func ActionRequiredFromBool(b bool) ActionRequiredValue {
if b {
return ActionRequiredTrue
}
return ActionRequiredFalse
}

// AnalysisResultStatus is the status of an AnalysisResult.
//
// +kubebuilder:validation:MinProperties=1
Expand All @@ -34,10 +54,22 @@ type AnalysisResultStatus struct {
// +kubebuilder:validation:MaxItems=8
Conditions []metav1.Condition `json:"conditions,omitempty" patchStrategy:"merge" patchMergeKey:"type" protobuf:"bytes,1,rep,name=conditions"`

// actionRequired indicates whether the analysis determined that
// remediation action is needed. When "False", the run transitions
// to NoActionRequired (terminal) instead of Proposed.
// +optional
ActionRequired ActionRequiredValue `json:"actionRequired,omitempty"`

// diagnosis is the top-level root cause analysis, populated when
// actionRequired is "False" (no remediation options are generated).
// When options are present, each option carries its own diagnosis.
// +optional
Diagnosis DiagnosisResult `json:"diagnosis,omitzero"`
Comment on lines +63 to +67

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## .ai/spec/README.md\n'
cat -n .ai/spec/README.md

printf '\n## Relevant files\n'
for f in api/v1alpha1/analysisresult_types.go controller/agenticrun/sandbox_agent.go controller/agenticrun/results.go; do
  echo
  echo "### $f"
  sed -n '1,220p' "$f" | cat -n
done

Repository: openshift/lightspeed-agentic-operator

Length of output: 26635


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## Search for diagnosis / actionRequired validation\n'
rg -n "Diagnosis|actionRequired|NoActionRequired|AnalysisOutput|IsActionRequired|missing diagnosis" api controller .ai -g '!**/vendor/**'

printf '\n## AnalysisOutput definition\n'
file=$(rg -n "type AnalysisOutput struct" -l .)
sed -n '1,220p' "$file" | cat -n

printf '\n## Related validation / schema in api/v1alpha1\n'
rg -n "kubebuilder:validation|XValidation|MinProperties|Required|DiagnosisResult" api/v1alpha1 -g '*.go'

Repository: openshift/lightspeed-agentic-operator

Length of output: 22973


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## controller/agenticrun/agent.go\n'
sed -n '1,120p' controller/agenticrun/agent.go | cat -n

printf '\n## controller/agenticrun/handlers.go (no-action path)\n'
sed -n '100,240p' controller/agenticrun/handlers.go | cat -n
printf '\n## controller/agenticrun/handlers.go (setNoActionRequired)\n'
sed -n '730,780p' controller/agenticrun/handlers.go | cat -n

printf '\n## controller/agenticrun/schemas.go (analysis schema contract)\n'
sed -n '1,210p' controller/agenticrun/schemas.go | cat -n

Repository: openshift/lightspeed-agentic-operator

Length of output: 29507


Reject no-action results without a diagnosis. controller/agenticrun/sandbox_agent.go only logs the missing diagnosis, and controller/agenticrun/results.go still persists an AnalysisResult with an empty status.diagnosis. Fail the analysis before creating the CR so NoActionRequired always carries an explanation.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@api/v1alpha1/analysisresult_types.go` around lines 63 - 67, Update the
analysis result validation flow in sandbox_agent.go and results.go to reject
NoActionRequired results when status.diagnosis is empty, rather than only
logging the condition or persisting the CR. Ensure validation fails before
AnalysisResult creation and that every persisted NoActionRequired result carries
a diagnosis.


// options contains the remediation options returned by the analysis agent.
// +optional
// +listType=atomic
// +kubebuilder:validation:MinItems=1
// +kubebuilder:validation:MinItems=0
// +kubebuilder:validation:MaxItems=10
Options []RemediationOption `json:"options,omitempty"`
Comment thread
coderabbitai[bot] marked this conversation as resolved.

Expand Down
23 changes: 23 additions & 0 deletions api/v1alpha1/derive_phase_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,29 @@ func TestDerivePhase(t *testing.T) {
},
want: AgenticRunPhaseProposed,
},
{
name: "no action required",
conditions: []metav1.Condition{
cond(AgenticRunConditionAnalyzed, metav1.ConditionTrue, ReasonNoActionRequired),
},
want: AgenticRunPhaseNoActionRequired,
},
{
name: "no action required overridden by denied",
conditions: []metav1.Condition{
cond(AgenticRunConditionAnalyzed, metav1.ConditionTrue, ReasonNoActionRequired),
cond(AgenticRunConditionDenied, metav1.ConditionTrue, "UserDenied"),
},
want: AgenticRunPhaseDenied,
},
{
name: "no action required overridden by emergency stopped",
conditions: []metav1.Condition{
cond(AgenticRunConditionAnalyzed, metav1.ConditionTrue, ReasonNoActionRequired),
cond(AgenticRunConditionEmergencyStopped, metav1.ConditionTrue, "SystemSuspended"),
},
want: AgenticRunPhaseEmergencyStopped,
},
}

for _, tt := range tests {
Expand Down
47 changes: 46 additions & 1 deletion config/crd/bases/agentic.openshift.io_analysisresults.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,15 @@ spec:
description: status contains result data and conditions.
minProperties: 1
properties:
actionRequired:
description: |-
actionRequired indicates whether the analysis determined that
remediation action is needed. When "False", the run transitions
to NoActionRequired (terminal) instead of Proposed.
enum:
- "True"
- "False"
type: string
conditions:
description: conditions track the lifecycle of this result.
items:
Expand Down Expand Up @@ -130,6 +139,42 @@ spec:
x-kubernetes-list-map-keys:
- type
x-kubernetes-list-type: map
diagnosis:
description: |-
diagnosis is the top-level root cause analysis, populated when
actionRequired is "False" (no remediation options are generated).
When options are present, each option carries its own diagnosis.
properties:
confidence:
description: |-
confidence is the agent's self-assessed confidence in its diagnosis.
Higher confidence generally correlates with clearer symptoms and
more deterministic root causes.
enum:
- Low
- Medium
- High
type: string
rootCause:
description: |-
rootCause is a concise Markdown-formatted description of the identified
root cause (e.g., "OOMKilled due to memory limit of 256Mi").
Maximum 1024 characters.
maxLength: 1024
minLength: 1
type: string
summary:
description: |-
summary is a Markdown-formatted diagnosis summary explaining the
problem, its symptoms, and the agent's findings. Maximum 8192 characters.
maxLength: 8192
minLength: 1
type: string
required:
- confidence
- rootCause
- summary
type: object
failureReason:
description: failureReason is populated when the step failed due to
a system error.
Expand Down Expand Up @@ -586,7 +631,7 @@ spec:
- message: diagnosis is required when remediationPlan is present
rule: '!has(self.remediationPlan) || has(self.diagnosis)'
maxItems: 10
minItems: 1
minItems: 0
type: array
x-kubernetes-list-type: atomic
sandbox:
Expand Down
18 changes: 15 additions & 3 deletions controller/agenticrun/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,19 @@ import (
)

// AnalysisOutput holds the analysis agent's output.
// ActionRequired is a pointer: nil means "action required" (safe default,
// backward-compatible with agents that don't emit the field).
type AnalysisOutput struct {
Success bool
Options []agenticv1alpha1.RemediationOption
Success bool
ActionRequired *bool
Options []agenticv1alpha1.RemediationOption
Diagnosis *agenticv1alpha1.DiagnosisResult
}

// IsActionRequired returns true when ActionRequired is nil (safe default)
// or explicitly true.
func (a *AnalysisOutput) IsActionRequired() bool {
return a.ActionRequired == nil || *a.ActionRequired
}

// ExecutionOutput holds the execution agent's output.
Expand Down Expand Up @@ -54,8 +64,10 @@ type AgentCaller interface {
type StubAgentCaller struct{}

func (s *StubAgentCaller) Analyze(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string) (*AnalysisOutput, error) {
actionRequired := true
return &AnalysisOutput{
Success: true,
Success: true,
ActionRequired: &actionRequired,
Options: []agenticv1alpha1.RemediationOption{{
Title: "Stub remediation",
Diagnosis: agenticv1alpha1.DiagnosisResult{
Expand Down
26 changes: 26 additions & 0 deletions controller/agenticrun/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ func (r *AgenticRunReconciler) handleAnalysis(
r.Audit.EmitAnalysisCompleted(spanCtx, run, analysisCR)
}
run.Status.Steps.Analysis.Results = append(run.Status.Steps.Analysis.Results, agenticv1alpha1.StepResultRef{Name: crName, Outcome: agenticv1alpha1.ActionOutcomeFromBool(analysisResult.Success)})

if !analysisResult.IsActionRequired() {
return r.setNoActionRequired(ctx, run, base, ErrUpdateAfterAnalysis, "Analysis determined no remediation action is required")
}

meta.SetStatusCondition(&run.Status.Conditions, metav1.Condition{
Type: agenticv1alpha1.AgenticRunConditionAnalyzed,
Status: metav1.ConditionTrue,
Expand Down Expand Up @@ -202,6 +207,11 @@ func (r *AgenticRunReconciler) handleRevision(
r.Audit.EmitAnalysisCompleted(spanCtx, run, analysisCR)
}
run.Status.Steps.Analysis.Results = append(run.Status.Steps.Analysis.Results, agenticv1alpha1.StepResultRef{Name: crName, Outcome: agenticv1alpha1.ActionOutcomeFromBool(analysisResult.Success)})

if !analysisResult.IsActionRequired() {
return r.setNoActionRequired(ctx, run, base, ErrUpdateAfterRevision, "Revision analysis determined no remediation action is required")
}

meta.SetStatusCondition(&run.Status.Conditions, metav1.Condition{
Type: agenticv1alpha1.AgenticRunConditionAnalyzed,
Status: metav1.ConditionTrue,
Expand Down Expand Up @@ -726,6 +736,22 @@ func (r *AgenticRunReconciler) handleEscalation(
return ctrl.Result{}, nil
}

func (r *AgenticRunReconciler) setNoActionRequired(ctx context.Context, run *agenticv1alpha1.AgenticRun, base *agenticv1alpha1.AgenticRun, errSentinel, message string) (ctrl.Result, error) {
log := logf.FromContext(ctx)
meta.SetStatusCondition(&run.Status.Conditions, metav1.Condition{
Type: agenticv1alpha1.AgenticRunConditionAnalyzed,
Status: metav1.ConditionTrue,
Reason: reasonNoActionRequired,
Message: message,
ObservedGeneration: run.Generation,
})
if err := r.statusPatch(ctx, run, base); err != nil {
return ctrl.Result{}, fmt.Errorf("%s: %w", errSentinel, err)
}
log.Info("no action required", "message", message)
return ctrl.Result{}, nil
}

func conditionTime(conditions []metav1.Condition, condType string) *metav1.Time {
if c := meta.FindStatusCondition(conditions, condType); c != nil {
return &c.LastTransitionTime
Expand Down
13 changes: 9 additions & 4 deletions controller/agenticrun/handlers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"k8s.io/apimachinery/pkg/api/meta"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"
"k8s.io/utils/ptr"
"sigs.k8s.io/controller-runtime/pkg/client"
"sigs.k8s.io/controller-runtime/pkg/client/fake"

Expand Down Expand Up @@ -684,7 +685,8 @@ func TestReconcile_RevisionWithFeedback(t *testing.T) {
func TestReconcile_ExecutionRBACCreatedOnApproval(t *testing.T) {
agent := newTestAgentCaller()
agent.analyzeResult = &AnalysisOutput{
Success: true,
Success: true,
ActionRequired: ptr.To(true),
Options: []agenticv1alpha1.RemediationOption{{
Title: "Increase memory",
Diagnosis: agenticv1alpha1.DiagnosisResult{
Expand Down Expand Up @@ -769,7 +771,8 @@ func TestReconcile_ExecutionRBACCreatedOnApproval(t *testing.T) {
func TestReconcile_ExecutionRBACCleanedOnFailure(t *testing.T) {
agent := newTestAgentCaller()
agent.analyzeResult = &AnalysisOutput{
Success: true,
Success: true,
ActionRequired: ptr.To(true),
Options: []agenticv1alpha1.RemediationOption{{
Title: "Fix it",
Diagnosis: agenticv1alpha1.DiagnosisResult{
Expand Down Expand Up @@ -1082,7 +1085,8 @@ func TestReconcile_ExecutionSelectsOption(t *testing.T) {

agent := newTestAgentCaller()
agent.analyzeResult = &AnalysisOutput{
Success: true,
Success: true,
ActionRequired: ptr.To(true),
Options: []agenticv1alpha1.RemediationOption{
{Title: "Option A", Diagnosis: agenticv1alpha1.DiagnosisResult{Summary: "diag-A"}},
{Title: "Option B", Diagnosis: agenticv1alpha1.DiagnosisResult{Summary: "diag-B"}},
Expand Down Expand Up @@ -1159,7 +1163,8 @@ func TestReconcile_TrimOptionsOnExecution(t *testing.T) {

agent := newTestAgentCaller()
agent.analyzeResult = &AnalysisOutput{
Success: true,
Success: true,
ActionRequired: ptr.To(true),
Options: []agenticv1alpha1.RemediationOption{
{Title: "Option A", Diagnosis: agenticv1alpha1.DiagnosisResult{Summary: "diag-A"}},
{Title: "Option B", Diagnosis: agenticv1alpha1.DiagnosisResult{Summary: "diag-B"}},
Expand Down
5 changes: 3 additions & 2 deletions controller/agenticrun/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ const (
reasonRetryingExecution = agenticv1alpha1.ReasonRetryingExecution
reasonRetriesExhausted = agenticv1alpha1.ReasonRetriesExhausted
reasonSystemSuspended = "SystemSuspended"
reasonNoActionRequired = agenticv1alpha1.ReasonNoActionRequired

LogKeyName = "name"
LogKeyStep = "step"
Expand Down Expand Up @@ -150,7 +151,7 @@ func terminalReason(run *agenticv1alpha1.AgenticRun) string {
if c.Status == metav1.ConditionFalse && c.Reason == reasonFailed {
return c.Message
}
if c.Status == metav1.ConditionTrue && (c.Reason == reasonUserDenied || c.Reason == reasonSystemSuspended) {
if c.Status == metav1.ConditionTrue && (c.Reason == reasonUserDenied || c.Reason == reasonSystemSuspended || c.Reason == reasonNoActionRequired) {
return c.Message
}
}
Expand All @@ -159,7 +160,7 @@ func terminalReason(run *agenticv1alpha1.AgenticRun) string {

func isTerminal(phase agenticv1alpha1.AgenticRunPhase) bool {
switch phase {
case agenticv1alpha1.AgenticRunPhaseCompleted, agenticv1alpha1.AgenticRunPhaseFailed, agenticv1alpha1.AgenticRunPhaseDenied, agenticv1alpha1.AgenticRunPhaseEscalated, agenticv1alpha1.AgenticRunPhaseEmergencyStopped:
case agenticv1alpha1.AgenticRunPhaseCompleted, agenticv1alpha1.AgenticRunPhaseFailed, agenticv1alpha1.AgenticRunPhaseDenied, agenticv1alpha1.AgenticRunPhaseEscalated, agenticv1alpha1.AgenticRunPhaseEmergencyStopped, agenticv1alpha1.AgenticRunPhaseNoActionRequired:
return true
}
return false
Expand Down
23 changes: 22 additions & 1 deletion controller/agenticrun/reconciler.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,21 @@ func (r *AgenticRunReconciler) Reconcile(ctx context.Context, req ctrl.Request)

// --- Terminal phases (before suspension guard so audit cleanup always runs) ---
switch phase {
case agenticv1alpha1.AgenticRunPhaseNoActionRequired:
if !needsRevision(&run) {
if hasSandboxClaims(&run) {
if err := r.Agent.ReleaseSandboxes(ctx, &run); err != nil {
log.Error(err, "sandbox cleanup failed at terminal phase")
}
}
if r.Audit != nil {
r.Audit.EndApprovalWait(&run, nil)
r.Audit.EmitAgenticRunTerminal(ctx, &run, string(phase), terminalReason(&run))
r.Audit.EndLifecycleSpan(&run)
}
return ctrl.Result{}, nil
}

Comment on lines +88 to +102

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Retry when terminal sandbox cleanup fails.

ReleaseSandboxes errors are only logged, after which the reconciler emits terminal audit events and returns success. A transient failure can therefore leave sandbox claims orphaned with no guaranteed retry. Return a wrapped error (using the repository’s error-constant convention) or explicitly requeue before marking the run terminal.

As per path instructions, Go code must never ignore error returns.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@controller/agenticrun/reconciler.go` around lines 88 - 102, Update the
terminal cleanup branch in the AgenticRun reconciler so an error from
ReleaseSandboxes is propagated using the repository’s wrapped error-constant
convention (or explicitly requeued) instead of only being logged. Ensure
terminal audit events and successful completion occur only after cleanup
succeeds, and do not ignore the ReleaseSandboxes error.

Source: Path instructions

case agenticv1alpha1.AgenticRunPhaseCompleted,
agenticv1alpha1.AgenticRunPhaseDenied,
agenticv1alpha1.AgenticRunPhaseEscalated,
Expand Down Expand Up @@ -135,7 +150,7 @@ func (r *AgenticRunReconciler) Reconcile(ctx context.Context, req ctrl.Request)
// Recover lifecycle trace context for in-progress runs after operator restart (§5).
// Uses RecoverLifecycleContext (not EnsureLifecycleSpan) to avoid exporting a duplicate span.
// Also restarts the approval wait span if the run is waiting for execution approval.
if r.Audit != nil && !isTerminal(phase) {
if r.Audit != nil && (!isTerminal(phase) || (phase == agenticv1alpha1.AgenticRunPhaseNoActionRequired && needsRevision(&run))) {
r.Audit.RecoverLifecycleContext(ctx, &run)
if phase == agenticv1alpha1.AgenticRunPhaseProposed {
r.Audit.StartApprovalWait(ctx, &run)
Expand Down Expand Up @@ -197,6 +212,12 @@ func (r *AgenticRunReconciler) Reconcile(ctx context.Context, req ctrl.Request)
}
return r.handleEscalation(ctx, &run, resolved, approval, policy)

case agenticv1alpha1.AgenticRunPhaseNoActionRequired:
if needsRevision(&run) {
return r.handleRevision(ctx, &run, resolved, approval, policy)
}
return ctrl.Result{}, nil

default:
log.V(1).Info("unhandled phase, no-op", LogKeyPhase, phase)
return ctrl.Result{}, nil
Expand Down
6 changes: 6 additions & 0 deletions controller/agenticrun/results.go
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,10 @@ func (r *AgenticRunReconciler) createAnalysisResult(

if result != nil {
cr.Status.Options = result.Options
cr.Status.ActionRequired = agenticv1alpha1.ActionRequiredFromBool(result.IsActionRequired())
if result.Diagnosis != nil {
cr.Status.Diagnosis = *result.Diagnosis
}
}

snapshot := cr.DeepCopy()
Expand Down Expand Up @@ -294,6 +298,8 @@ func copyResultStatus(dst, src client.Object) {
case *agenticv1alpha1.AnalysisResult:
if s, ok := src.(*agenticv1alpha1.AnalysisResult); ok {
d.Status.Options = s.Status.Options
d.Status.ActionRequired = s.Status.ActionRequired
d.Status.Diagnosis = s.Status.Diagnosis
d.Status.FailureReason = s.Status.FailureReason
d.Status.Sandbox = s.Status.Sandbox
}
Expand Down
Loading