diff --git a/api/v1alpha1/agenticrun_types.go b/api/v1alpha1/agenticrun_types.go index 311c4d83..2b54381b 100644 --- a/api/v1alpha1/agenticrun_types.go +++ b/api/v1alpha1/agenticrun_types.go @@ -269,10 +269,22 @@ type AgenticRunStep struct { // for this step. Use this when different steps need different skills. // +optional Tools ToolsSpec `json:"tools,omitzero"` + + // timeoutMinutes sets the timeout for this step's sandbox agent call. + // This controls only the agent call duration; pod startup always uses + // a fixed five-minute ceiling (defaultSandboxTimeout). Increase this + // for long-running tools (e.g., IntelliAide RCA takes 10-30 minutes). + // Defaults to 5 minutes when omitted. + // + // Mutable: can be adjusted at any time; the value is read when the step starts. + // +optional + // +kubebuilder:validation:Minimum=1 + // +kubebuilder:validation:Maximum=60 + TimeoutMinutes int32 `json:"timeoutMinutes,omitempty"` } func (s AgenticRunStep) IsZero() bool { - return s.Agent == "" && s.Tools.IsZero() + return s.Agent == "" && s.Tools.IsZero() && s.TimeoutMinutes == 0 } // AgenticRunSpec defines the desired state of AgenticRun. diff --git a/config/crd/bases/agentic.openshift.io_agenticruns.yaml b/config/crd/bases/agentic.openshift.io_agenticruns.yaml index 17e1fd83..3b7a7b4b 100644 --- a/config/crd/bases/agentic.openshift.io_agenticruns.yaml +++ b/config/crd/bases/agentic.openshift.io_agenticruns.yaml @@ -83,6 +83,19 @@ spec: - message: 'must be a valid DNS subdomain: lowercase alphanumeric characters, hyphens, and dots' rule: '!format.dns1123Subdomain().validate(self).hasValue()' + timeoutMinutes: + description: |- + timeoutMinutes sets the timeout for this step's sandbox agent call. + This controls only the agent call duration; pod startup always uses + a fixed five-minute ceiling (defaultSandboxTimeout). Increase this + for long-running tools (e.g., IntelliAide RCA takes 10-30 minutes). + Defaults to 5 minutes when omitted. + + Mutable: can be adjusted at any time; the value is read when the step starts. + format: int32 + maximum: 60 + minimum: 1 + type: integer tools: description: |- tools provides per-step tools that replace the shared spec.tools @@ -500,6 +513,19 @@ spec: - message: 'must be a valid DNS subdomain: lowercase alphanumeric characters, hyphens, and dots' rule: '!format.dns1123Subdomain().validate(self).hasValue()' + timeoutMinutes: + description: |- + timeoutMinutes sets the timeout for this step's sandbox agent call. + This controls only the agent call duration; pod startup always uses + a fixed five-minute ceiling (defaultSandboxTimeout). Increase this + for long-running tools (e.g., IntelliAide RCA takes 10-30 minutes). + Defaults to 5 minutes when omitted. + + Mutable: can be adjusted at any time; the value is read when the step starts. + format: int32 + maximum: 60 + minimum: 1 + type: integer tools: description: |- tools provides per-step tools that replace the shared spec.tools @@ -1294,6 +1320,19 @@ spec: - message: 'must be a valid DNS subdomain: lowercase alphanumeric characters, hyphens, and dots' rule: '!format.dns1123Subdomain().validate(self).hasValue()' + timeoutMinutes: + description: |- + timeoutMinutes sets the timeout for this step's sandbox agent call. + This controls only the agent call duration; pod startup always uses + a fixed five-minute ceiling (defaultSandboxTimeout). Increase this + for long-running tools (e.g., IntelliAide RCA takes 10-30 minutes). + Defaults to 5 minutes when omitted. + + Mutable: can be adjusted at any time; the value is read when the step starts. + format: int32 + maximum: 60 + minimum: 1 + type: integer tools: description: |- tools provides per-step tools that replace the shared spec.tools diff --git a/controller/agenticrun/agent.go b/controller/agenticrun/agent.go index 8a28e7eb..bf6f32ab 100644 --- a/controller/agenticrun/agent.go +++ b/controller/agenticrun/agent.go @@ -2,6 +2,7 @@ package agenticrun import ( "context" + "time" agenticv1alpha1 "github.com/openshift/lightspeed-agentic-operator/api/v1alpha1" ) @@ -53,10 +54,10 @@ type EscalationOutput struct { // HTTP implementations POST to /v1/agent/run — a step-agnostic // endpoint where all workflow context is in the request payload. type AgentCaller interface { - Analyze(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string) (*AnalysisOutput, error) - Execute(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, serviceAccount string) (*ExecutionOutput, error) - Verify(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, exec *ExecutionOutput, serviceAccount string) (*VerificationOutput, error) - Escalate(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string) (*EscalationOutput, error) + Analyze(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string, timeout time.Duration) (*AnalysisOutput, error) + Execute(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, serviceAccount string, timeout time.Duration) (*ExecutionOutput, error) + Verify(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, exec *ExecutionOutput, serviceAccount string, timeout time.Duration) (*VerificationOutput, error) + Escalate(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string, timeout time.Duration) (*EscalationOutput, error) ReleaseSandboxes(ctx context.Context, run *agenticv1alpha1.AgenticRun) error } @@ -64,7 +65,7 @@ type AgentCaller interface { // implementation (sandbox + HTTP) when the agent infrastructure is ready. type StubAgentCaller struct{} -func (s *StubAgentCaller) Analyze(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string) (*AnalysisOutput, error) { +func (s *StubAgentCaller) Analyze(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string, _ time.Duration) (*AnalysisOutput, error) { actionRequired := true return &AnalysisOutput{ Success: true, @@ -84,7 +85,7 @@ func (s *StubAgentCaller) Analyze(_ context.Context, _ *agenticv1alpha1.AgenticR }, nil } -func (s *StubAgentCaller) Execute(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ string) (*ExecutionOutput, error) { +func (s *StubAgentCaller) Execute(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ string, _ time.Duration) (*ExecutionOutput, error) { return &ExecutionOutput{ Success: true, ActionsTaken: []agenticv1alpha1.ExecutionAction{{ @@ -95,7 +96,7 @@ func (s *StubAgentCaller) Execute(_ context.Context, _ *agenticv1alpha1.AgenticR }, nil } -func (s *StubAgentCaller) Escalate(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string) (*EscalationOutput, error) { +func (s *StubAgentCaller) Escalate(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string, _ time.Duration) (*EscalationOutput, error) { return &EscalationOutput{ Success: true, Summary: "Stub escalation summary", @@ -107,7 +108,7 @@ func (s *StubAgentCaller) ReleaseSandboxes(_ context.Context, _ *agenticv1alpha1 return nil } -func (s *StubAgentCaller) Verify(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ *ExecutionOutput, _ string) (*VerificationOutput, error) { +func (s *StubAgentCaller) Verify(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ *ExecutionOutput, _ string, _ time.Duration) (*VerificationOutput, error) { return &VerificationOutput{ Success: true, Checks: []agenticv1alpha1.VerifyCheck{{ diff --git a/controller/agenticrun/handlers.go b/controller/agenticrun/handlers.go index 5b4f501e..6994e0b5 100644 --- a/controller/agenticrun/handlers.go +++ b/controller/agenticrun/handlers.go @@ -99,7 +99,7 @@ func (r *AgenticRunReconciler) handleAnalysis( r.Audit.EmitAgenticRunReceived(spanCtx, run) } - analysisResult, err := r.Agent.Analyze(spanCtx, run, resolved.Analysis, run.Spec.Request, defaultSandboxSA) + analysisResult, err := r.Agent.Analyze(spanCtx, run, resolved.Analysis, run.Spec.Request, defaultSandboxSA, stepTimeout(resolved.Analysis)) if err != nil { return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, err) } @@ -182,7 +182,7 @@ func (r *AgenticRunReconciler) handleRevision( revisionSuffix := buildRevisionContext(run) requestWithRevision := run.Spec.Request + "\n\n" + revisionSuffix - analysisResult, err := r.Agent.Analyze(spanCtx, run, resolved.Analysis, requestWithRevision, defaultSandboxSA) + analysisResult, err := r.Agent.Analyze(spanCtx, run, resolved.Analysis, requestWithRevision, defaultSandboxSA, stepTimeout(resolved.Analysis)) if err != nil { return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionAnalyzed, err) } @@ -329,7 +329,7 @@ func (r *AgenticRunReconciler) handleExecution( } } - execResult, err := r.Agent.Execute(spanCtx, run, *resolved.Execution, selectedOption, execSA) + execResult, err := r.Agent.Execute(spanCtx, run, *resolved.Execution, selectedOption, execSA, stepTimeout(*resolved.Execution)) if err != nil { return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionExecuted, err) } @@ -468,7 +468,7 @@ func (r *AgenticRunReconciler) handleVerification( } } - verifyResult, err := r.Agent.Verify(spanCtx, run, *resolved.Verification, selectedOption, execOutput, defaultSandboxSA) + verifyResult, err := r.Agent.Verify(spanCtx, run, *resolved.Verification, selectedOption, execOutput, defaultSandboxSA, stepTimeout(*resolved.Verification)) if err != nil { return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionVerified, err) } @@ -669,7 +669,7 @@ func (r *AgenticRunReconciler) handleEscalation( if err := r.Get(ctx, types.NamespacedName{Name: agent.Spec.LLMProvider.Name}, &llm); err != nil { return r.failStep(ctx, run, agenticv1alpha1.AgenticRunConditionEscalated, fmt.Errorf("%s %q: %w", ErrGetEscalationLLMProvider, agent.Spec.LLMProvider.Name, err)) } - step = resolvedStep{Agent: &agent, LLM: &llm, Tools: step.Tools} + step = resolvedStep{Agent: &agent, LLM: &llm, Tools: step.Tools, TimeoutMinutes: step.TimeoutMinutes} } base := run.DeepCopy() @@ -694,7 +694,7 @@ func (r *AgenticRunReconciler) handleEscalation( } escalationText := buildEscalationRequest(run) - escalationResult, err := r.Agent.Escalate(spanCtx, run, step, escalationText, defaultSandboxSA) + escalationResult, err := r.Agent.Escalate(spanCtx, run, step, escalationText, defaultSandboxSA, stepTimeout(step)) if err != nil { return r.failStep(spanCtx, run, agenticv1alpha1.AgenticRunConditionEscalated, err) } diff --git a/controller/agenticrun/reconciler_test.go b/controller/agenticrun/reconciler_test.go index 44067410..930892cd 100644 --- a/controller/agenticrun/reconciler_test.go +++ b/controller/agenticrun/reconciler_test.go @@ -34,36 +34,42 @@ type testAgentCaller struct { executeResult *ExecutionOutput verifyResult *VerificationOutput escalateResult *EscalationOutput + + // lastAnalyzeTimeout records the timeout passed to the most recent + // Analyze call, so tests can assert that timeoutMinutes resolved from + // the AgenticRun spec is actually forwarded through the reconciler. + lastAnalyzeTimeout time.Duration } func newTestAgentCaller() *testAgentCaller { stub := &StubAgentCaller{} - a, _ := stub.Analyze(context.Background(), nil, resolvedStep{}, "", "") - e, _ := stub.Execute(context.Background(), nil, resolvedStep{}, nil, "") - v, _ := stub.Verify(context.Background(), nil, resolvedStep{}, nil, nil, "") - esc, _ := stub.Escalate(context.Background(), nil, resolvedStep{}, "", "") + a, _ := stub.Analyze(context.Background(), nil, resolvedStep{}, "", "", 0) + e, _ := stub.Execute(context.Background(), nil, resolvedStep{}, nil, "", 0) + v, _ := stub.Verify(context.Background(), nil, resolvedStep{}, nil, nil, "", 0) + esc, _ := stub.Escalate(context.Background(), nil, resolvedStep{}, "", "", 0) return &testAgentCaller{analyzeResult: a, executeResult: e, verifyResult: v, escalateResult: esc} } -func (ta *testAgentCaller) Analyze(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string) (*AnalysisOutput, error) { +func (ta *testAgentCaller) Analyze(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string, timeout time.Duration) (*AnalysisOutput, error) { + ta.lastAnalyzeTimeout = timeout if ta.analyzeErr != nil { return nil, ta.analyzeErr } return ta.analyzeResult, nil } -func (ta *testAgentCaller) Execute(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ string) (*ExecutionOutput, error) { +func (ta *testAgentCaller) Execute(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ string, _ time.Duration) (*ExecutionOutput, error) { if ta.executeErr != nil { return nil, ta.executeErr } return ta.executeResult, nil } -func (ta *testAgentCaller) Verify(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ *ExecutionOutput, _ string) (*VerificationOutput, error) { +func (ta *testAgentCaller) Verify(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ *agenticv1alpha1.RemediationOption, _ *ExecutionOutput, _ string, _ time.Duration) (*VerificationOutput, error) { if ta.verifyErr != nil { return nil, ta.verifyErr } return ta.verifyResult, nil } -func (ta *testAgentCaller) Escalate(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string) (*EscalationOutput, error) { +func (ta *testAgentCaller) Escalate(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ resolvedStep, _ string, _ string, _ time.Duration) (*EscalationOutput, error) { if ta.escalateErr != nil { return nil, ta.escalateErr } @@ -966,3 +972,40 @@ func TestHandleRBACCleanup_InvalidAnnotation(t *testing.T) { t.Error("expected Requeue (cleanup succeeded, annotation reset to 0)") } } + +// TestReconcile_PropagatesStepTimeout verifies that timeoutMinutes set on an +// AgenticRunStep is resolved and forwarded to the AgentCaller as time.Duration. +func TestReconcile_PropagatesStepTimeout(t *testing.T) { + const wantMinutes int32 = 30 + + scheme := testScheme() + run := &agenticv1alpha1.AgenticRun{ + ObjectMeta: metav1.ObjectMeta{Name: "timeout-check", Namespace: "default"}, + Spec: agenticv1alpha1.AgenticRunSpec{ + Request: "Pod crashing", + Tools: testTools(), + Analysis: agenticv1alpha1.AgenticRunStep{ + Agent: "default", + TimeoutMinutes: wantMinutes, + }, + Execution: agenticv1alpha1.AgenticRunStep{Agent: "default"}, + Verification: agenticv1alpha1.AgenticRunStep{Agent: "default"}, + }, + } + + objs := append([]client.Object{run}, defaultObjects()...) + fc := fake.NewClientBuilder().WithScheme(scheme).WithObjects(objs...). + WithStatusSubresource(run, &agenticv1alpha1.AnalysisResult{}, &agenticv1alpha1.ExecutionResult{}, &agenticv1alpha1.VerificationResult{}, &agenticv1alpha1.EscalationResult{}).Build() + + caller := newTestAgentCaller() + r := &AgenticRunReconciler{Client: fc, Agent: caller, Namespace: "default"} + + if _, err := reconcileOnce(r, "timeout-check"); err != nil { + t.Fatalf("reconcile: %v", err) + } + + want := time.Duration(wantMinutes) * time.Minute + if caller.lastAnalyzeTimeout != want { + t.Errorf("Analyze timeout = %v, want %v", caller.lastAnalyzeTimeout, want) + } +} diff --git a/controller/agenticrun/resolve.go b/controller/agenticrun/resolve.go index 23dcb24e..c86a68be 100644 --- a/controller/agenticrun/resolve.go +++ b/controller/agenticrun/resolve.go @@ -19,9 +19,10 @@ const ( ) type resolvedStep struct { - Agent *agenticv1alpha1.Agent - LLM *agenticv1alpha1.LLMProvider - Tools *agenticv1alpha1.ToolsSpec + Agent *agenticv1alpha1.Agent + LLM *agenticv1alpha1.LLMProvider + Tools *agenticv1alpha1.ToolsSpec + TimeoutMinutes int32 } type resolvedWorkflow struct { @@ -80,14 +81,14 @@ func resolveAgenticRun(ctx context.Context, c client.Client, run *agenticv1alpha if err != nil { return nil, fmt.Errorf("%s: %w", ErrResolveAnalysisStep, err) } - resolved.Analysis = resolvedStep{Agent: agent, LLM: llm, Tools: toolsForStep(run.Spec.Analysis)} + resolved.Analysis = resolvedStep{Agent: agent, LLM: llm, Tools: toolsForStep(run.Spec.Analysis), TimeoutMinutes: run.Spec.Analysis.TimeoutMinutes} if !run.Spec.Execution.IsZero() { agent, llm, err := resolveAgent(effectiveAgent(agenticv1alpha1.SandboxStepExecution, run.Spec.Execution)) if err != nil { return nil, fmt.Errorf("%s: %w", ErrResolveExecutionStep, err) } - resolved.Execution = &resolvedStep{Agent: agent, LLM: llm, Tools: toolsForStep(run.Spec.Execution)} + resolved.Execution = &resolvedStep{Agent: agent, LLM: llm, Tools: toolsForStep(run.Spec.Execution), TimeoutMinutes: run.Spec.Execution.TimeoutMinutes} } if !run.Spec.Verification.IsZero() { @@ -95,7 +96,7 @@ func resolveAgenticRun(ctx context.Context, c client.Client, run *agenticv1alpha if err != nil { return nil, fmt.Errorf("%s: %w", ErrResolveVerificationStep, err) } - resolved.Verification = &resolvedStep{Agent: agent, LLM: llm, Tools: toolsForStep(run.Spec.Verification)} + resolved.Verification = &resolvedStep{Agent: agent, LLM: llm, Tools: toolsForStep(run.Spec.Verification), TimeoutMinutes: run.Spec.Verification.TimeoutMinutes} } return resolved, nil diff --git a/controller/agenticrun/sandbox_agent.go b/controller/agenticrun/sandbox_agent.go index 4892207a..f74d1cd1 100644 --- a/controller/agenticrun/sandbox_agent.go +++ b/controller/agenticrun/sandbox_agent.go @@ -17,10 +17,6 @@ import ( const ( defaultSandboxTimeout = 5 * time.Minute - analysisStepTimeout = 10 * time.Minute - executionStepTimeout = 10 * time.Minute - verificationStepTimeout = 30 * time.Minute - ErrAnalysisAgentCall = "analysis agent call" ErrParseAnalysisResponse = "parse analysis response" ErrExecutionAgentCall = "execution agent call" @@ -70,13 +66,23 @@ type SandboxAgentCaller struct { Audit AuditLogger } +// stepTimeout returns the effective timeout for a single step's sandbox operation. +// Reads the step's timeoutMinutes when set; falls back to defaultSandboxTimeout. +// This is the single place where timeout policy is decided. +func stepTimeout(step resolvedStep) time.Duration { + if step.TimeoutMinutes > 0 { + return time.Duration(step.TimeoutMinutes) * time.Minute + } + return defaultSandboxTimeout +} + func stepString(step agenticv1alpha1.SandboxStep) string { return strings.ToLower(string(step)) } -func (s *SandboxAgentCaller) Analyze(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string) (*AnalysisOutput, error) { +func (s *SandboxAgentCaller) Analyze(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string, timeout time.Duration) (*AnalysisOutput, error) { query := buildAnalysisQuery(requestText, run) - raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepAnalysis), step, query, buildAgentContext(run), serviceAccount) + raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepAnalysis), step, query, buildAgentContext(run), serviceAccount, timeout) if err != nil { return nil, fmt.Errorf("%s: %w", ErrAnalysisAgentCall, err) } @@ -114,14 +120,14 @@ func (s *SandboxAgentCaller) Analyze(ctx context.Context, run *agenticv1alpha1.A }, nil } -func (s *SandboxAgentCaller) Execute(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, serviceAccount string) (*ExecutionOutput, error) { +func (s *SandboxAgentCaller) Execute(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, serviceAccount string, timeout time.Duration) (*ExecutionOutput, error) { agentCtx := buildAgentContext(run) if option != nil { agentCtx.ApprovedOption = option } query := buildExecutionQuery(option) - raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepExecution), step, query, agentCtx, serviceAccount) + raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepExecution), step, query, agentCtx, serviceAccount, timeout) if err != nil { return nil, fmt.Errorf("%s: %w", ErrExecutionAgentCall, err) } @@ -138,7 +144,7 @@ func (s *SandboxAgentCaller) Execute(ctx context.Context, run *agenticv1alpha1.A }, nil } -func (s *SandboxAgentCaller) Verify(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, exec *ExecutionOutput, serviceAccount string) (*VerificationOutput, error) { +func (s *SandboxAgentCaller) Verify(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, option *agenticv1alpha1.RemediationOption, exec *ExecutionOutput, serviceAccount string, timeout time.Duration) (*VerificationOutput, error) { agentCtx := buildAgentContext(run) if option != nil { agentCtx.ApprovedOption = option @@ -146,7 +152,7 @@ func (s *SandboxAgentCaller) Verify(ctx context.Context, run *agenticv1alpha1.Ag agentCtx.ExecutionResult = executionOutputToAgentResult(exec) query := buildVerificationQuery(option, exec) - raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepVerification), step, query, agentCtx, serviceAccount) + raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepVerification), step, query, agentCtx, serviceAccount, timeout) if err != nil { return nil, fmt.Errorf("%s: %w", ErrVerificationAgentCall, err) } @@ -163,9 +169,9 @@ func (s *SandboxAgentCaller) Verify(ctx context.Context, run *agenticv1alpha1.Ag }, nil } -func (s *SandboxAgentCaller) Escalate(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string) (*EscalationOutput, error) { +func (s *SandboxAgentCaller) Escalate(ctx context.Context, run *agenticv1alpha1.AgenticRun, step resolvedStep, requestText string, serviceAccount string, timeout time.Duration) (*EscalationOutput, error) { agentCtx := buildAgentContext(run) - raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepEscalation), step, requestText, agentCtx, serviceAccount) + raw, err := s.callWithSandbox(ctx, run, stepString(agenticv1alpha1.SandboxStepEscalation), step, requestText, agentCtx, serviceAccount, timeout) if err != nil { return nil, fmt.Errorf("%s: %w", ErrEscalationAgentCall, err) } @@ -186,19 +192,6 @@ func (s *SandboxAgentCaller) Escalate(ctx context.Context, run *agenticv1alpha1. }, nil } -func stepTimeout(step string) time.Duration { - switch step { - case "analysis", "escalation": - return analysisStepTimeout - case "execution": - return executionStepTimeout - case "verification": - return verificationStepTimeout - default: - return analysisStepTimeout - } -} - func (s *SandboxAgentCaller) callWithSandbox( ctx context.Context, run *agenticv1alpha1.AgenticRun, @@ -207,9 +200,12 @@ func (s *SandboxAgentCaller) callWithSandbox( query string, agentCtx *agentContext, serviceAccount string, + timeout time.Duration, ) (json.RawMessage, error) { - agentTimeout := stepTimeout(stepName) - podDeadline := agentTimeout + defaultSandboxTimeout + if timeout <= 0 { + timeout = defaultSandboxTimeout + } + podDeadline := timeout + defaultSandboxTimeout name, err := s.Sandbox.Create(ctx, run, stepName, step.Agent, step.LLM, step.Tools, serviceAccount, podDeadline) if err != nil { @@ -218,6 +214,10 @@ func (s *SandboxAgentCaller) callWithSandbox( s.patchSandboxInfo(ctx, run, stepName, name) + // Pod startup is an infrastructure concern unrelated to the agent's work + // budget. Using a fixed ceiling here ensures that the full user-configured + // timeout is available for the agent call itself, and avoids the effective + // wall-clock time being 2x the configured value. endpoint, err := s.Sandbox.WaitReady(ctx, name, defaultSandboxTimeout) if err != nil { return nil, fmt.Errorf("%s: %w", ErrWaitForSandbox, err) @@ -236,8 +236,8 @@ func (s *SandboxAgentCaller) callWithSandbox( s.Audit.InjectTraceContext(ctx, run, headers) } - client := s.ClientFactory(agentURL, podDeadline) - resp, err := client.Run(ctx, "", query, schema, agentCtx, headers, agentTimeout) + client := s.ClientFactory(agentURL, timeout) + resp, err := client.Run(ctx, "", query, schema, agentCtx, headers, timeout) if err != nil { return nil, err } diff --git a/controller/agenticrun/sandbox_agent_test.go b/controller/agenticrun/sandbox_agent_test.go index 28c64f5b..6ac3f1a6 100644 --- a/controller/agenticrun/sandbox_agent_test.go +++ b/controller/agenticrun/sandbox_agent_test.go @@ -19,20 +19,22 @@ import ( // --- Hand-written mocks --- type mockSandboxProvider struct { - claimName string - claimErr error - endpoint string - readyErr error - releaseErr error - claimCalls int - releaseCalls int + claimName string + claimErr error + endpoint string + readyErr error + releaseErr error + claimCalls int + releaseCalls int + lastWaitReadyTimeout time.Duration } func (m *mockSandboxProvider) Create(_ context.Context, _ *agenticv1alpha1.AgenticRun, _ string, _ *agenticv1alpha1.Agent, _ *agenticv1alpha1.LLMProvider, _ *agenticv1alpha1.ToolsSpec, _ string, _ time.Duration) (string, error) { m.claimCalls++ return m.claimName, m.claimErr } -func (m *mockSandboxProvider) WaitReady(_ context.Context, _ string, _ time.Duration) (string, error) { +func (m *mockSandboxProvider) WaitReady(_ context.Context, _ string, d time.Duration) (string, error) { + m.lastWaitReadyTimeout = d return m.endpoint, m.readyErr } func (m *mockSandboxProvider) Release(_ context.Context, _ string) error { @@ -104,7 +106,7 @@ func TestSandboxAgentCaller_Analyze_HappyPath(t *testing.T) { } caller := newTestSandboxAgentCaller(sandbox, httpClient) - result, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "Pod crashing", defaultSandboxSA) + result, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "Pod crashing", defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -126,7 +128,7 @@ func TestSandboxAgentCaller_Execute_HappyPath(t *testing.T) { caller := newTestSandboxAgentCaller(sandbox, httpClient) option := &agenticv1alpha1.RemediationOption{Title: "Fix it"} - result, err := caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), option, defaultSandboxSA) + result, err := caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), option, defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -147,7 +149,7 @@ func TestSandboxAgentCaller_Verify_HappyPath(t *testing.T) { } caller := newTestSandboxAgentCaller(sandbox, httpClient) - result, err := caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, nil, defaultSandboxSA) + result, err := caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, nil, defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -169,7 +171,7 @@ func TestSandboxAgentCaller_ClaimError(t *testing.T) { httpClient := &mockHTTPClient{} caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "test", defaultSandboxSA) + _, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "test", defaultSandboxSA, defaultSandboxTimeout) if err == nil { t.Fatal("expected error") } @@ -186,7 +188,7 @@ func TestSandboxAgentCaller_WaitReadyError(t *testing.T) { httpClient := &mockHTTPClient{} caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, err := caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, defaultSandboxSA) + _, err := caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, defaultSandboxSA, defaultSandboxTimeout) if err == nil { t.Fatal("expected error") } @@ -200,7 +202,7 @@ func TestSandboxAgentCaller_HTTPError(t *testing.T) { httpClient := &mockHTTPClient{err: fmt.Errorf("connection refused")} caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, err := caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, nil, defaultSandboxSA) + _, err := caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, nil, defaultSandboxSA, defaultSandboxTimeout) if err == nil { t.Fatal("expected error") } @@ -216,7 +218,7 @@ func TestSandboxAgentCaller_ParseError(t *testing.T) { } caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "test", defaultSandboxSA) + _, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "test", defaultSandboxSA, defaultSandboxTimeout) if err == nil { t.Fatal("expected parse error") } @@ -232,7 +234,7 @@ func TestSandboxAgentCaller_SandboxNotReleasedAfterCall(t *testing.T) { } caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, _ = caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "test", defaultSandboxSA) + _, _ = caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "test", defaultSandboxSA, defaultSandboxTimeout) if sandbox.claimCalls != 1 { t.Errorf("Claim calls = %d, want 1", sandbox.claimCalls) @@ -273,7 +275,7 @@ func TestSandboxAgentCaller_ContextPropagation(t *testing.T) { }, } - _, _ = caller.Analyze(context.Background(), run, testSandboxStep(), "test", defaultSandboxSA) + _, _ = caller.Analyze(context.Background(), run, testSandboxStep(), "test", defaultSandboxSA, defaultSandboxTimeout) if httpClient.lastCtx == nil { t.Fatal("expected context to be set") @@ -305,7 +307,7 @@ func TestSandboxAgentCaller_VerifyPassesExecutionResult(t *testing.T) { }, } - _, _ = caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), option, exec, defaultSandboxSA) + _, _ = caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), option, exec, defaultSandboxSA, defaultSandboxTimeout) if httpClient.lastCtx == nil { t.Fatal("expected context to be set") @@ -334,7 +336,7 @@ func TestSandboxAgentCaller_VerifyNilExecLeavesExecutionResultNil(t *testing.T) } caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, _ = caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, nil, defaultSandboxSA) + _, _ = caller.Verify(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, nil, defaultSandboxSA, defaultSandboxTimeout) if httpClient.lastCtx == nil { t.Fatal("expected context to be set") @@ -352,7 +354,7 @@ func TestSandboxAgentCaller_ExecutePassesApprovedOption(t *testing.T) { caller := newTestSandboxAgentCaller(sandbox, httpClient) option := &agenticv1alpha1.RemediationOption{Title: "Scale up replicas"} - _, _ = caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), option, defaultSandboxSA) + _, _ = caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), option, defaultSandboxSA, defaultSandboxTimeout) if httpClient.lastCtx == nil || httpClient.lastCtx.ApprovedOption == nil { t.Fatal("expected approved option in context") @@ -393,7 +395,7 @@ func TestSandboxAgentCaller_Analyze_EmptyTopLevelDiagnosis(t *testing.T) { } caller := newTestSandboxAgentCaller(sandbox, httpClient) - result, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "Too many connections", defaultSandboxSA) + result, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "Too many connections", defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -421,7 +423,7 @@ func TestSandboxAgentCaller_AnalysisQueryFraming(t *testing.T) { } caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, _ = caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "Pod crashing with OOMKilled", defaultSandboxSA) + _, _ = caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "Pod crashing with OOMKilled", defaultSandboxSA, defaultSandboxTimeout) if !strings.Contains(httpClient.lastQuery, "analysis agent") { t.Error("analysis query should contain role framing") @@ -450,7 +452,7 @@ func TestSandboxAgentCaller_ExecutionQueryFraming(t *testing.T) { } run := testSandboxAgenticRun() run.Spec.Request = "Pod crashing with OOMKilled" - _, _ = caller.Execute(context.Background(), run, testSandboxStep(), option, defaultSandboxSA) + _, _ = caller.Execute(context.Background(), run, testSandboxStep(), option, defaultSandboxSA, defaultSandboxTimeout) if !strings.Contains(httpClient.lastQuery, "execution agent") { t.Error("execution query should contain role framing") @@ -482,7 +484,7 @@ func TestSandboxAgentCaller_VerificationQueryFraming(t *testing.T) { } run := testSandboxAgenticRun() run.Spec.Request = "Pod crashing with OOMKilled" - _, _ = caller.Verify(context.Background(), run, testSandboxStep(), option, exec, defaultSandboxSA) + _, _ = caller.Verify(context.Background(), run, testSandboxStep(), option, exec, defaultSandboxSA, defaultSandboxTimeout) if !strings.Contains(httpClient.lastQuery, "verification agent") { t.Error("verification query should contain role framing") @@ -511,7 +513,7 @@ func TestSandboxAgentCaller_ExecutionQueryNilOption(t *testing.T) { } caller := newTestSandboxAgentCaller(sandbox, httpClient) - _, _ = caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, defaultSandboxSA) + _, _ = caller.Execute(context.Background(), testSandboxAgenticRun(), testSandboxStep(), nil, defaultSandboxSA, defaultSandboxTimeout) if !strings.Contains(httpClient.lastQuery, "execution agent") { t.Error("execution query should still contain role framing with nil option") @@ -534,7 +536,7 @@ func TestSandboxAgentCaller_Analyze_PatchesSandboxInfo(t *testing.T) { run := testSandboxAgenticRun() caller := newTestSandboxAgentCallerWithAgenticRun(sandbox, httpClient, run) - _, err := caller.Analyze(context.Background(), run, testSandboxStep(), "Pod crashing", defaultSandboxSA) + _, err := caller.Analyze(context.Background(), run, testSandboxStep(), "Pod crashing", defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -563,7 +565,7 @@ func TestSandboxAgentCaller_Execute_PatchesSandboxInfo(t *testing.T) { run := testSandboxAgenticRun() caller := newTestSandboxAgentCallerWithAgenticRun(sandbox, httpClient, run) - _, err := caller.Execute(context.Background(), run, testSandboxStep(), nil, defaultSandboxSA) + _, err := caller.Execute(context.Background(), run, testSandboxStep(), nil, defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -589,7 +591,7 @@ func TestSandboxAgentCaller_Verify_PatchesSandboxInfo(t *testing.T) { run := testSandboxAgenticRun() caller := newTestSandboxAgentCallerWithAgenticRun(sandbox, httpClient, run) - _, err := caller.Verify(context.Background(), run, testSandboxStep(), nil, nil, defaultSandboxSA) + _, err := caller.Verify(context.Background(), run, testSandboxStep(), nil, nil, defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("unexpected error: %v", err) } @@ -617,7 +619,7 @@ func TestSandboxAgentCaller_SandboxInfoPatch_DoesNotBlockOnError(t *testing.T) { caller := newTestSandboxAgentCaller(sandbox, httpClient) run := testSandboxAgenticRun() - _, err := caller.Analyze(context.Background(), run, testSandboxStep(), "test", defaultSandboxSA) + _, err := caller.Analyze(context.Background(), run, testSandboxStep(), "test", defaultSandboxSA, defaultSandboxTimeout) if err != nil { t.Fatalf("analysis should succeed even when sandbox info patch fails: %v", err) } @@ -743,3 +745,61 @@ func (m *trackingMockSandbox) Release(_ context.Context, claimName string) error } return nil } + +func TestSandboxAgentCaller_TimeoutPropagation(t *testing.T) { + const customTimeout = 20 * time.Minute + + sandbox := &mockSandboxProvider{ + claimName: "ls-analysis-test", + endpoint: "http://sandbox:8080", + } + httpClient := &mockHTTPClient{ + response: &agentRunResponse{ + Response: json.RawMessage(`{"success": true, "options": []}`), + }, + } + + var lastFactoryTimeout time.Duration + fc := fake.NewClientBuilder().WithScheme(testScheme()).Build() + if err := fc.Create(context.Background(), fakeBaseTemplate()); err != nil { + t.Fatalf("setup: %v", err) + } + caller := &SandboxAgentCaller{ + Sandbox: sandbox, + K8sClient: fc, + ClientFactory: func(_ string, d time.Duration) AgentHTTPClientInterface { + lastFactoryTimeout = d + return httpClient + }, + Namespace: "test-ns", + } + + _, err := caller.Analyze(context.Background(), testSandboxAgenticRun(), testSandboxStep(), "test", defaultSandboxSA, customTimeout) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // Pod startup always uses the fixed ceiling, not the step-level timeout. + if sandbox.lastWaitReadyTimeout != defaultSandboxTimeout { + t.Errorf("WaitReady timeout = %v, want defaultSandboxTimeout (%v)", sandbox.lastWaitReadyTimeout, defaultSandboxTimeout) + } + // The agent's work budget is the full configured timeout. + if lastFactoryTimeout != customTimeout { + t.Errorf("ClientFactory timeout = %v, want %v", lastFactoryTimeout, customTimeout) + } +} + +func TestStepTimeout_ReturnsDefault(t *testing.T) { + step := resolvedStep{TimeoutMinutes: 0} + if got := stepTimeout(step); got != defaultSandboxTimeout { + t.Errorf("stepTimeout() = %v, want %v", got, defaultSandboxTimeout) + } +} + +func TestStepTimeout_ReturnsCustom(t *testing.T) { + step := resolvedStep{TimeoutMinutes: 30} + want := 30 * time.Minute + if got := stepTimeout(step); got != want { + t.Errorf("stepTimeout() = %v, want %v", got, want) + } +}