OLS-3666: Analysis and Execution phases show more specific error message when failing. - #415
OLS-3666: Analysis and Execution phases show more specific error message when failing.#415raptorsun wants to merge 1 commit into
Conversation
openshift-ci-robot
added
the
jira/valid-reference
|
@raptorsun: This pull request references OLS-3666 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the bug to target the "5.0.0" version, but no target version was set. DetailsIn response to this: Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the openshift-eng/jira-lifecycle-plugin repository. |
|
Warning Review limit reached
Next review available in: 55 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe change parses optional analysis and execution summaries, propagates them through agent outputs, and uses them to generate contextual failure messages. Tests verify message precedence and generic fallbacks. ChangesAgentic run failure context
Sequence Diagram(s)sequenceDiagram
participant SandboxResponse
participant AgentOutputs
participant FailureHandlers
participant FailureMessageHelpers
SandboxResponse->>AgentOutputs: parse and propagate optional summaries
FailureHandlers->>FailureMessageHelpers: request contextual failure message
FailureMessageHelpers->>AgentOutputs: inspect summaries and failure details
FailureMessageHelpers-->>FailureHandlers: return selected or generic message
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with 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.
Inline comments:
In `@controller/agenticrun/handlers.go`:
- Around line 775-782: Update the failed-action formatting in the
result.ActionsTaken loop so that when action.Description is empty but
action.Error is present, it returns the error alone without the empty
description prefix. Preserve the existing combined message when both fields are
populated and the description-only behavior when no error is available.
In `@controller/agenticrun/sandbox_agent.go`:
- Around line 34-42: Add top-level summary properties to AnalysisOutputSchema
and ExecutionOutputSchema, and ensure MinimalAnalysisOutputSchema retains its
option-item summary field. Align each schema with the top-level resp.Summary
decoding so AnalysisOutput.Summary and ExecutionOutput.Summary are populated
without removing nested option summaries.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7d19ed15-8619-4bf3-9b1d-477eabd2957e
📒 Files selected for processing (4)
controller/agenticrun/agent.gocontroller/agenticrun/handlers.gocontroller/agenticrun/handlers_test.gocontroller/agenticrun/sandbox_agent.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
|
@raptorsun Do we not need this for Verification Agent ? |
|
Primary path can dump full LLM JSON into condition messages because schemas omit top-level |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with 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.
Inline comments:
In @.ai/spec/what/run-lifecycle.md:
- Line 53: Update rule 16a in the run-lifecycle specification to explicitly
target the top-level status.conditions field and retain reason "Failed",
distinguishing it from status.steps.*.conditions and the AgentFailed step-result
reason defined in rule 14a. Do not alter the failure-message fallback
requirements.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a8d1e4c9-2914-4ef5-9519-4c91c77508ae
📒 Files selected for processing (4)
.ai/spec/how/reconciler.md.ai/spec/what/run-lifecycle.mdcontroller/agenticrun/handlers.gocontroller/agenticrun/handlers_test.go
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- controller/agenticrun/handlers_test.go
- controller/agenticrun/handlers.go
…age when failing. Surface the sandbox response summary in failure condition messages instead of hardcoded generic strings. Add Summary field to analysisResponse/executionResponse structs so the operator reads the summary the sandbox already returns. Build descriptive messages via analysisFailureMessage/executionFailureMessage with a fallback chain: summary, diagnosis/action details, properly-cased generic. Also fixes incorrect all-lowercase casing. Add spec rule 16a documenting the failure message fallback chain. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verification phase has already the summary field used in error message.
good catch, spec has been update, too :) |
|
@raptorsun: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
Summary
When an agent phase (analysis or execution) fails, the operator now surfaces a specific error message instead of the generic
"execution agent reported failure".summaryfield from the sandbox response (already present but ignored for analysis/execution)"Analysis agent reported failure")Before:
execution agent reported failureAfter:
Execution failed: Agent timed out after 5000msChanges
controller/agenticrun/sandbox_agent.go— addedSummaryfield toanalysisResponseandexecutionResponsestructs; propagated to output structscontroller/agenticrun/agent.go— addedSummaryfield toAnalysisOutputandExecutionOutputcontroller/agenticrun/handlers.go— addedanalysisFailureMessage()andexecutionFailureMessage()helpers with fallback chain; replaced three hardcoded error stringscontroller/agenticrun/handlers_test.go— unit tests for both failure message functionsOperator-only change — no schema changes, no sandbox modifications, no spec update.
Test plan
make testpasses (all unit tests)analysisFailureMessageandexecutionFailureMessagecover: summary present, diagnosis fallback, action error fallback, no-details fallbackmake test-e2ewith mock agent configured to returnsuccess: falseJira: OLS-3666