fix: report skipped gates as skipped, and honour ai.majorUpgrades.maxAttempts - #1433
Open
glennmichael123 wants to merge 1 commit into
Open
fix: report skipped gates as skipped, and honour ai.majorUpgrades.maxAttempts#1433glennmichael123 wants to merge 1 commit into
glennmichael123 wants to merge 1 commit into
Conversation
…Attempts Two more settings and states that existed on paper. The assertion gates return `neutral()` when they cannot run — no AI configured, nothing to check against, the assessment failed — and the docs say a neutral result is "never a pass". But `GateResult` had only `passed`, and `neutral()` set it to `true`. So a gate that could not run rendered with the same green tick as one that ran and found nothing, and a check run made entirely of skipped gates was titled "All checks passed" and concluded `success`. `GateResult` gains `neutral`; a skipped gate renders as skipped in the check run and the terminal, is counted apart in the title, and a check run of nothing but skipped gates concludes `neutral` — the "no checks configured" outcome, arrived at a different way. A skipped gate still never blocks. `ai.majorUpgrades.maxAttempts` was declared, documented as "maximum agent attempts per upgrade", and the agent ran exactly once whatever it said. It now does what it says: a run that stopped short — out of tool calls, out of time, errored — is handed its own output and asked to continue from the workspace as it now stands, up to the limit. Starting over would redo the work that did land. Unset means one attempt, as before, and the outcome records how many it took. The runner is injectable so the loop can be tested without a model. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UWouahBJT3msK7V2VDYY6T
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
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.
Two more settings and states that existed on paper.
A gate that could not run rendered as a pass
The assertion gates return
neutral()when they can't run, anddocs/features/merge-gates.mdsays:But
GateResulthad onlypassed, andneutral()set it totrue:So a skipped gate got the same ✅ as one that ran and found nothing, and a check run made entirely of skipped gates was titled "All checks passed" and concluded
success.GateResultgainsneutral. A skipped gate renders ⏭️ in the check run and the terminal, is counted apart in the title ("1 check(s) passed, 1 did not run"), and a run of nothing but skipped gates concludesneutral— the "no checks configured" outcome, arrived at a different way. A skipped gate still never blocks.maxAttemptsran exactly onceDeclared on
ai.majorUpgrades, documented as "maximum agent attempts per upgrade", never read. Now: a run that stops short —max_tool_calls,timeout,error— is handed its own output and told to continue from the workspace as it stands, up to the limit. Starting over would redo the work that did land.Unset means one attempt, as before. The outcome records how many it took. The runner is injectable, so the loop is tested without a model.
Not in this PR
ReviewSubmissionResult.inlineCommentsbeing dropped — that touches the same lines as #1431'sreviewPullRequestrefactor and would conflict, so it follows once #1431 lands.Tests
2153 → 2164.
Lint ✓ · typecheck ✓ ·
check:docs✓ · 0 fail🤖 Generated with Claude Code
https://claude.ai/code/session_01UWouahBJT3msK7V2VDYY6T