Skip to content

fix: report skipped gates as skipped, and honour ai.majorUpgrades.maxAttempts - #1433

Open
glennmichael123 wants to merge 1 commit into
mainfrom
fix/gate-neutral
Open

fix: report skipped gates as skipped, and honour ai.majorUpgrades.maxAttempts#1433
glennmichael123 wants to merge 1 commit into
mainfrom
fix/gate-neutral

Conversation

@glennmichael123

Copy link
Copy Markdown
Member

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, and docs/features/merge-gates.md says:

a check that could not run must never read as one that succeeded

But GateResult had only passed, and neutral() set it to true:

function neutral(name: string, summary: string): GateResult {
  return { name, mode: 'warning', passed: true, summary, detail: 'This check did not run.' }
}

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.

GateResult gains neutral. 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 concludes neutral — the "no checks configured" outcome, arrived at a different way. A skipped gate still never blocks.

maxAttempts ran exactly once

Declared 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.inlineComments being dropped — that touches the same lines as #1431's reviewPullRequest refactor 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

…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant