fix(gate): an outage of ours must not block a customer's merge #5580
ccantynzAI Reviewcommented 2d ago
AI Triage
(no summary)
Priority: medium Risk area: mixed
Suggested labels: (no label suggestions) Suggested reviewers: (no reviewer suggestions)
Suggestions only — nothing has been applied. The PR author stays in control.
gluecron[bot]🤖 botAI Reviewcommented 2d ago
AI review unavailable
The platform's AI balance is exhausted, so AI generation is temporarily unavailable. Nothing was lost. You can queue this as a repair for the internal agent from the repository's Health page, or try again once the balance is restored. The PR is otherwise unchanged.
Cross-repo impact
See what breaks downstream if this PR merges.
⮌ Merged
This pull request was merged into main.
c comment · e edit title · m merge · a approve · r request changes · ? shortcuts
Three places where an AI outage — or a dead worker — was charged to the customer. One sentence covers all of them: "we could not look" is not "we looked and it was bad", and it isn't "we looked and it was fine" either.
1. "AI Review" as a required check blocked every merge, forever
The gate already fails open — it records the check
skippedwith "not blocking the merge". ButpassingCheckNames()re-read that persisted row and admitted onlypassed/repaired, so a repo that made AI Review a required check was hard-blocked the moment the Anthropic balance hit zero, with no override, on both the merge button and the API. The required-checks UI suggests that exact string.Fixed with a discriminator, not a blanket rule.
gate_runs.detailshas existed since migration 0001 and been NULL for every row ever written; it now carries{skipReason}. A skipped row satisfies a required check only when it's AI Review skipped asai_unavailable.Every exclusion is load-bearing:
CIskippedSecurity scanskippedGateTestskippedAI Review/not_configuredAI Review/ai_unavailableA new status value was rejected deliberately: eight status enumerators would have silently mis-bucketed it, and
ai-loop.tswould have started treating it as not green — the opposite of the intent.2. Two functions read the same comment and disagreed
aiCommentLooksApproved()returnedfalsefor "AI review unavailable" whileai-review.tsreturned not-blocking — so an outage silently stalled every auto-merge underrequireAiApproval. It now uses the same tri-state expression as the manual merge button and the gated chain.Two quieter bugs go with it: the old path used
.some(), so a stale approving comment outvoted a newer blocking one, and it returnedfalsewhen there was no review comment at all.3.
abandonedwasn't classified as infrastructurereapStuckRunswrites it for a run whose worker died holding it.classifyRun()returned"other", so a dead container counted against the repo insuccessRateExcludingInfra— the one statistic the module exists to protect — and the merge gate told the author "CI failed (abandoned)". Separating platform failure from your failure is a stated product promise; this was a regression against it.Also
The required-checks UI hint suggested
Secret ScanandType Check. The gate emitsSecret scan(lowercase s) and no type check at all — so anyone who followed the hint gave themselves a required check that could never pass. Now lists only names the gate actually emits.Tests
Pinned name by name and reason by reason, including that NULL details on every historical row grants nothing and that malformed JSON isn't a licence to pass.
Verified by mutation: removing the gate-name scope credits
Security scan,GateTestandMerge checkon an AI outage — and fails the suite. 76 pass across the eight suites touching these files.