feat(loop): fail-closed gate preflight — never start work a broken gate can't accept (v0.33.0)#73
Merged
Merged
Conversation
…te can't accept (v0.33.0) The board loop's per-PR local gate (_run_local_gate) fails OPEN: a gate that can't run degrades to 'pass' so a flaky/misconfigured gate never blocks good work (CI is the real backstop). That's correct for the per-PR edge, but it means a coder will happily generate against a gate that CANNOT EXECUTE — e.g. the build tool isn't installed — burning generations on work no gate could ever accept. Add the fail-CLOSED complement: before dispatching ANY work, smoke-run local_gate_cmd on the CLEAN base checkout (the main repo — coders only touch worktrees, so it stays at base). - gate exits 0 → runnable; dispatch normally (cached for the run) - gate exits non-zero → base doesn't pass; HOLD all ready work - gate can't launch → broken env (missing tool); HOLD all ready work - gate times out → indeterminate; ALLOW (never wedge on a slow gate) Held features are flag_blocked with the reason so the stall is visible on the board, not silent; the preflight re-checks each cycle (throttled) and releases the holds the moment the gate goes green. Default on when a gate is configured (a healthy repo passes instantly — no behavior change); opt out with preflight:false. Surfaced dogfooding the roxy→protoContent delivery loop: a coder burned 5 gens producing test files against a 'pnpm -r build' gate whose pnpm wasn't installed. This makes that a one-line board hold with a clear reason instead. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
👀 Quinn is reviewing — verdict (PASS / WARN / FAIL) + findings to follow. |
![protoquinn[bot]](https://avatars.githubusercontent.com/in/3282126?s=60&v=4){kind=small}
There was a problem hiding this comment.
QA Audit — PR #73 | feat(loop): fail-closed gate preflight — never start work a broken gate can't accept (v0.33.0)
VERDICT: WARN (CI pending — test queued)
CI Status
- test: queued
Diff Review
- 3 files, +231/-1:
loop.py(preflight gate mechanism),pyproject.toml(0.32.0 → 0.33.0),tests/test_loop.py(7 new tests) - Design is sound: fail-closed preflight on clean base complements the existing fail-open per-PR gate. Timeout → allow (never wedge), missing tool → hold, non-zero exit on clean base → hold with board-visible
flag_blocked. Recovery re-checks throttled and releases holds automatically.preflight: falseopt-out available. Default-on whenlocal_gate_cmdis configured. - Clawpatch structural review: zero findings on changed files.
Observations
- LOW: CI
testis still queued — approve-on-green policy promotes this review to APPROVED automatically once every check settles terminal-green. No blocking issues identified in diff or structural review.
— Quinn, QA Engineer
|
Submitted COMMENT review on |
There was a problem hiding this comment.
QA Audit — PR #73 | feat(loop): fail-closed gate preflight — never start work a broken gate can't accept (v0.33.0)
VERDICT: WARN (CI pending — test queued)
CI Status
- test: queued
Incremental delta since prior review (13c8f43 → e184c3d)
- 4 files, +391/-3:
loop.pyadds_resolve_gate_cmd()(auto-discovery from package.json/Makefile/justfile),README.mddocuments theautosentinel andpreflightconfig,pyproject.tomlbumps to 0.33.0,tests/test_loop.pycarries forward 7 preflight tests - Clawpatch structural review: zero findings on changed files.
- No unresolved CodeRabbit threads. Design is clean: gate resolution favors a single repo-declared entrypoint (ci/check/verify script →
pnpm run <it>) whose own CI targets the same thing, so local == CI by construction. Falls back topnpm -r --if-present typecheck build testonly when nothing is declared.
Observations
- LOW: CI
testis still queued — the approve-on-green policy promotes this review to APPROVED automatically once every check settles terminal-green. No blocking issues identified.
— Quinn, QA Engineer
|
Submitted COMMENT review on #73. |
…repo Hard-coding one repo's check steps into the orchestrator (or the operator's dispatch) is a footgun: the repo's CI changes and the transcription silently goes stale (green-locally / red-in-CI), or the team is pointed at a different repo and the gate is just wrong. "auto" resolves the pre-PR gate from the checkout instead, favoring a single repo-DECLARED entrypoint (package.json ci/check/verify → `pnpm run <it>`; Makefile/justfile ci|check target → `make/just <it>`) so a repo whose OWN CI calls that same target keeps local == CI by construction. Falls back to the `pnpm -r --if-present typecheck build test` superset for an undeclared node repo, and to "" (gateless, fail-open, warns) when nothing is recognized. Resolved once at construction, so every downstream reader (coder_solve_test_cmd, _run_local_gate, _preflight, candidate preference) sees the concrete command with no further plumbing. Explicit command passes through; blank still = no gate (auto is opt-in, never inferred from blank). Composes with the fail-closed preflight: auto-resolve the gate, then smoke it on clean base before dispatch. 9 resolver unit tests; full suite 312 green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
mabry1985
force-pushed
the
feat/gate-preflight
branch
from
e184c3d to
732ccdb
Compare
There was a problem hiding this comment.
QA Audit — PR #73 | feat(loop): fail-closed gate preflight — never start work a broken gate can't accept (v0.33.0)
VERDICT: WARN (CI pending — test in_progress)
CI Status
- test: in_progress
Incremental delta since prior review (e184c3d → 732ccdb)
- 4 files, +387/-3 (down 4 lines from prior):
loop.py—_resolve_gate_cmd()auto-discovery + preflight mechanism,README.mddocs forautosentinel +preflightconfig,pyproject.toml→ 0.33.0,tests/test_loop.py— 7 preflight tests. - Clawpatch structural review: zero findings on changed files.
- No unresolved CodeRabbit threads.
Observations
- LOW: CI
testis still in_progress — the approve-on-green policy promotes this review to APPROVED automatically once every check settles terminal-green. No blocking issues identified on the current head.
— Quinn, QA Engineer
|
Submitted COMMENT review on #73. |
…anifest to 0.33.0 Two CI-only failures on this branch: - _release_preflight_holds() built the store (get_store → needs the `br` beads CLI) even with zero holds to release, so a PASSING preflight called it, and on a host without `br` the error was swallowed by _preflight's outer except and masqueraded as a gate failure (_preflight_state = "gate command could not run: beads CLI 'br' not on PATH"). It passed locally only because the dev host has `br`. Short-circuit before building the store: a clean preflight (the common path) never touches it. - protoagent.plugin.yaml still said 0.32.0 while pyproject moved to 0.33.0 (test_manifest_and_pyproject_versions_agree). Bump to 0.33.0. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
QA Audit — PR #73 | feat(loop): fail-closed gate preflight — never start work a broken gate can't accept (v0.33.0)
VERDICT: WARN (CI pending — test queued)
CI Status
- test: queued
Incremental delta since prior review (732ccdb → d55d00b)
- 5 files, +393/-4 (+1 file, +6 lines from prior): added
protoagent.plugin.yaml; minor adjustments toloop.pyon top of the_resolve_gate_cmd()+ preflight mechanism reviewed previously. - Clawpatch structural review on
d55d00b: zero findings on changed files. - No unresolved CodeRabbit threads.
Observations
- LOW: CI
testis queued — the approve-on-green policy promotes this review to APPROVED automatically once every check settles terminal-green. No blocking issues identified on the current head.
— Quinn, QA Engineer
|
Submitted COMMENT review on #73. |
test_preflight_recovery_releases_holds pre-set a failed _preflight_state but left _last_preflight=0.0, so whether the throttled recheck fired depended on time.monotonic()'s absolute value — large on a dev host (passed), small in a fresh CI container (< the 60s throttle → recheck skipped → stale state, flaky fail). Anchor _last_preflight well in the past so the recheck always fires. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
✅ CI went terminal-green with no blockers on the prior review — promoting it to APPROVED per the approve-on-green policy (#748). |
There was a problem hiding this comment.
QA Audit — PR #73 | feat(loop): fail-closed gate preflight — never start work a broken gate can't accept (v0.33.0)
VERDICT: WARN (CI pending — test in_progress)
CI Status
- test: in_progress
Incremental re-review (d55d00b → 98858a5)
- +4 net lines on top of the prior-reviewed head. Clawpatch structural review on
98858a5: zero findings on changed files (18 mapped, 0 reviewed — no features touched by the diff that weren't already covered). - No unresolved CodeRabbit threads.
- Prior review findings carry forward — no new blocking issues on this head.
Observations
- LOW: CI
testis still settling. The approve-on-green policy promotes this review to APPROVED automatically once every check completes terminal-green. No blocking issues identified.
— Quinn, QA Engineer
|
Submitted COMMENT review on |
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.
Why
The board loop's per-PR local gate (
_run_local_gate) intentionally fails open — a gate that can't run degrades to "pass" so a flaky/misconfigured gate never blocks good work (CI is the backstop). Correct for the per-PR edge, but it means a coder happily generates against a gate that cannot execute (e.g. the build tool isn't installed), burning generations on work no gate could ever accept.Surfaced dogfooding the roxy→protoContent delivery loop: a coder burned 5 generations producing test files against a
pnpm -r buildgate whosepnpmwasn't installed in the image.What
The fail-closed complement. Before dispatching ANY work, smoke-run
local_gate_cmdon the clean base checkout (the main repo — coders only touch worktrees, so it stays at base):flag_blockedwith the reason, so the stall is visible on the board — not a silent stop.preflight: false.Tests
7 new tests (pass/non-zero/launch-error/timeout classification,
_spawn_readyhold, recovery release). Full suite: 303 passed (test_packaging.pyskipped — pre-existingtomllib/py3.10 env issue, unrelated).🤖 Generated with Claude Code