feat(github): report the gates behind the one that blocked an apply - #1310
feat(github): report the gates behind the one that blocked an apply#1310aparajon wants to merge 2 commits into
Conversation
An apply runs an ordered ladder of gates and the first one that blocks answers the command. An operator who clears it can land straight on the next, so a single apply becomes a sequence of edit-retry-read rounds, each one learning about a gate the previous comment had no reason to mention. Every apply gate rejection now also reports the gates behind it: the approval, the PR checks, the prior environments, and the database lock, each named with what is not satisfied. When everything behind the blocking gate is ready the checklist collapses to one line saying so, which is both the common case and the useful one — it tells the operator their retry gets through (UX-4). The report is read-only and decides nothing. Each probe takes one reading, without the retry waits the enforcing gates use, and a probe that cannot reach a verdict reports unknown rather than ready, so uncertainty is never rendered as clear. No gate's verdict, ordering, or fail-closed behavior changes: the gate that blocked is still the command's answer, and the ladder is byte-for-byte the ladder it was. apply gates before after ───────────────────────────────────────────────────────── review approval → ⛔ comment ⛔ comment PR checks (not reached) + reported: not passing prior environments (not reached) + reported: ready database lock (not reached) + reported: ready Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
The new checklist rendering/probing has a couple of verified cases where it can present inaccurate “ready vs unknown/in-progress” UI, which directly impacts operator decision-making.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds an advisory “remaining gates” checklist to apply-gate rejection comments so operators can see what will block the next retry (and what won’t) without iterating through the gate ladder one rejection at a time. This fits into SchemaBot’s GitHub PR UX by improving the operator-facing guidance while keeping gate ordering and fail-closed decisions unchanged.
Changes:
- Introduces a new template helper to append a preflight checklist to existing rejection comment bodies.
- Adds an
applyPreflightprober that reads downstream gate state (single-shot, read-only, unknown-never-ready) and threads it through the gate helpers. - Expands unit/integration coverage and updates the UX invariant registry to include the new reporting rule.
File summaries
| File | Description |
|---|---|
| pkg/webhook/templates/preflight.go | New checklist rendering helpers appended into existing rejection comment bodies. |
| pkg/webhook/templates/preflight_test.go | Unit tests for checklist rendering behavior and support-offer placement. |
| pkg/webhook/review_gate.go | Appends downstream-gate checklist to review-gate merit-block comment. |
| pkg/webhook/review_gate_test.go | Updates call sites for the new preflight parameter. |
| pkg/webhook/check_prior_env.go | Threads preflight through prior-environment gating and appends checklist on merit blocks. |
| pkg/webhook/check_prior_env_test.go | Updates call sites for the new preflight parameter. |
| pkg/webhook/apply_preflight.go | New read-only prober that reports statuses for gates behind the blocking gate. |
| pkg/webhook/apply_preflight_test.go | Tests end-to-end comment output for preflight checklist scenarios (ready/blocked/unknown/no-prober). |
| pkg/webhook/apply_integration_test.go | Updates call sites for the new preflight parameter in E2E apply enforcement coverage. |
| pkg/webhook/apply_handlers.go | Constructs and passes preflight through the apply/apply-confirm gate ladder. |
| pkg/webhook/apply_gating.go | Appends downstream-gate checklist to checks-gate merit-block comments. |
| pkg/webhook/apply_gating_test.go | Updates call sites for the new preflight parameter. |
| docs/invariants.md | Extends UX-4 to require gate rejections to report downstream gates (advisory, unknown-never-ready). |
Review details
Suppressed comments (1)
pkg/webhook/apply_preflight.go:204
- For cross-deployment prior-environment checks, the probe also collapses all non-success states into "not passing". In particular, a Check Run with status
in_progress/queuedshould be reported as still running, not failing, to keep the checklist actionable.
if checkResult.Status == checkStatusCompleted && checkResult.Conclusion == checkConclusionSuccess {
return templates.PreflightReady, ""
}
return templates.PreflightBlocked, priorEnv + " is not passing"
- Files reviewed: 13/13 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…or failed The prior-environment row collapsed every non-success reading into "is not passing", which reads as a failure even when the environment is mid-flight. It now splits the way the gate itself splits: still running, failed, or has pending changes. The status cell rendered an unrecognized status as ready through its default branch. Ready is now explicit and the default is uncertainty, so the checklist's fail-closed rule holds for any status value. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An apply runs an ordered ladder of gates, and the first one that blocks answers the command. An operator who clears it can land straight on the next, so one apply becomes a sequence of edit-retry-read rounds — each round learning about a gate the previous comment had no reason to mention.
Every apply gate rejection now also reports the gates behind it. When something else is in the way it is named; when nothing is, the checklist collapses to one line saying so, which is both the common case and the more useful one — it tells the operator their retry gets through (UX-4).
Before — the ladder is discovered one rejection at a time:
After — the first rejection carries the whole remaining list:
The report decides nothing. Every probe is read-only — no lock taken, no comment posted, no stored state written — and takes one reading rather than the retry waits the enforcing gates use, because an operator reading a rejection is not served by a probe that re-polls a check for several seconds. A probe that cannot reach a verdict reports unknown, never ready, so uncertainty is never rendered as clear.
No gate's verdict, ordering, or fail-closed behavior changes. The gate that blocked is still the command's answer; the checklist is appended to the comment that gate already posts, and a call site that passes no prober renders exactly the comment it rendered before. Gate evaluation-failure comments (the fail-closed paths that return an error rather than a block) carry no checklist — the command is retryable there, not answered.
Most of the diff is one new parameter threaded through three gate helpers and the test call sites that pass it.
Invariants: extends UX-4's enforcement — the registry entry gains the rule that a gate rejection reports the gates behind it, with the advisory and unknown-never-ready constraints stated, and an
Enforced:line naming the probes. The merge-gating (MG) and authorization (AZ) entries the ladder enforces are untouched: enforcement code is unchanged, and the probes read the same sources the gates do without deciding anything.Review gate blocks, and other gates are also in the way
Review Required
Database:
orders| Environment:productionRequested by @Someone at 2026-09-05 19:55:24 UTC
Schema changes require approval from an authorized reviewer before applying.
Operators of
orders:Next steps
schemabot apply -e productionagainRemaining before this apply can run
Read when this comment was posted, so clear them together and run the apply command once.
Review gate blocks, and it is the only thing left
Review Required
Database:
orders| Environment:productionRequested by @Someone at 2026-09-05 19:55:24 UTC
Schema changes require approval from an authorized reviewer before applying.
Operators of
orders:Next steps
schemabot apply -e productionagainRemaining before this apply can run
Nothing else blocks it. Clear the item above and run the apply command again.
This pull request was written by Claude Code (Claude Opus 5).