d6376e5142f3cdc3d520a249a2bedc8dafa8f108
2
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
c5a6dcd42a |
fix(revision-preflight): default to warn, name the refusal class, bound the fetch
PR Pipeline — Authorize → Validate → Review → Merge / auth (pull_request) Successful in 10s
PR Pipeline — Authorize → Validate → Review → Merge / validate (pull_request) Successful in 12s
PR Pipeline — Authorize → Validate → Review → Merge / lint (pull_request) Failing after 6s
PR Pipeline — Authorize → Validate → Review → Merge / ai-review (pull_request) Successful in 5s
PR Pipeline — Authorize → Validate → Review → Merge / gate (pull_request) Skipped
Bounded correction round on PR #134 after a PASS-WITH-FINDINGS review whose
Finding 4 is High. The guard's purpose and its fail-closed fix stand; the
problem was that with 'enforce' as the default it gates EVERY scheduled
contract, and three legitimate states produce a refusal - a clone legitimately
ahead of origin/master mid-review, a detached HEAD, and an offline or failed
fetch - so any of them would turn the fleet's monitoring into withheld
verdicts. That risk outweighs the staleness the guard catches.
1. DEFAULT IS NOW 'warn'. 'enforce' remains available and documented. The
criteria for flipping the default later are written into the doc as a
decision with evidence - a sustained window (30 days / 200+ runs) with zero
mismatch:* and zero cannot-verify:* refusals, no fetch blips, and a pinned
clone demonstrably kept current - explicitly as its own change, not a silent
flip.
2. 'COULD NOT CHECK' IS NOW DISTINGUISHABLE FROM 'THIS COPY IS WRONG'. Every
non-zero exit prints a machine-readable REASON=<class> line:
cannot-verify:fetch-failed | cannot-verify:ref-unresolvable (exit 2)
mismatch:path-absent | mismatch:content
mismatch:detached-head | mismatch:clone-ahead (exit 1)
detached-head and clone-ahead are named separately because they are
legitimate states, far less alarming than a hand-edited file. clone-ahead
requires HEAD to be STRICTLY ahead; an uncommitted edit on a commit that IS
the ref is a plain content mismatch (my own first cut got this wrong and the
new test 7d caught it).
3. THE DEFAULT FETCH IS BOUNDED: --fetch-timeout, default 20s, 0 = unbounded,
and a missing 'timeout' binary is itself a cannot-verify rather than an
unbounded fetch inside a scheduled contract.
4. TEST COVERAGE ADDED for every new class: fetch failure, fetch timeout
(asserted to return promptly under a 1s bound), unresolvable ref, detached
HEAD, clone-ahead, genuine content mismatch, and the contract-run.sh default.
The pre-fix draft fixture comparisons are kept: 31 passed, 0 failed.
5. MERGE-TIME SEQUENCE documented: fast-forward /opt/contract-runner, confirm
clean, prove a contract runs and reports. Baseline recorded as of today -
firstmate has already fast-forwarded it to
|
||
|
|
574cb99d76 |
feat: land the revision-preflight guard, fixed and wired into contract execution
PR Pipeline — Authorize → Validate → Review → Merge / auth (pull_request) Successful in 6s
PR Pipeline — Authorize → Validate → Review → Merge / validate (pull_request) Successful in 4s
PR Pipeline — Authorize → Validate → Review → Merge / lint (pull_request) Failing after 6s
PR Pipeline — Authorize → Validate → Review → Merge / ai-review (pull_request) Successful in 2s
PR Pipeline — Authorize → Validate → Review → Merge / gate (pull_request) Skipped
A contract verdict is only meaningful if it came from the merged copy. The fleet has been bitten three times on 2026-09-25 (a clone parked on a merged feature branch while executing from another clone; a script copied into the runner clone by hand; a stale local origin/master making an ancestry check report unlanded work). The control for this existed as an untracked draft and protected nobody, because it was entirely fail-open. Defect in the draft, preserved verbatim as tests/fixtures/revision-preflight.prefix.sh: git -C "$CLONE" show "origin/master:$(basename "$SCRIPT")" basename drops the scripts/ prefix, so for any script under scripts/ it queried the repo root, failed, took the "warn but don't block" branch and exited 0 - passing a script that exists in no revision at all. Reproduced: pre-fix + scripts/demo.sh under scripts/ -> 'could not resolve', EXIT=0 pre-fix + a script in no revision -> EXIT=0 Fixed guard (scripts/revision-preflight.sh): * resolves the repo-relative path inside the clone, so scripts/ paths resolve; * FAILS CLOSED - a path absent from the ref, an unresolvable ref, or a failed fetch is a failure, never a warning; * fetches the remote by default, because a stale local ref would otherwise pass a stale script as current; --no-fetch states the assumption instead of hiding it. Wiring (scripts/contract-run.sh): before executing, the wrapper runs the guard against the clone it lives in. Default CONTRACT_REVISION_PREFLIGHT=enforce withholds the verdict, alerts and exits 2 on mismatch; =warn logs and continues; =off skips. Verified live: match -> contract proceeds and PASSes; mismatch -> 'VERDICT WITHHELD', exit 2; =warn -> continues. Pinning (docs/contract-execution-pinning.md): every contract pins the clone contract-run.sh lives in - the deployed runner being /opt/contract-runner on CT 100. Documented that daily-health-digest has no contract file at all, which is why its execution copy was silently operator-chosen. Tests: tests/test_revision_preflight.sh, 15 assertions over a throwaway clone with a real bare remote. It runs the pre-fix draft against the same cases and shows it passing a ghost script, so the tests provably bite. shellcheck: scripts/revision-preflight.sh and the new test are clean. The three findings remaining in contract-run.sh (SC2086 x2, SC2034) are pre-existing and byte-identical on master. |