feat: land the revision-preflight guard, fixed and wired into contract execution #134

Merged

abiba-bot merged 3 commits from fix/land-revision-preflight-guard-20260925 into master 2026-09-25 11:46:30 +00:00

Conversation 1 Commits 3 Files Changed 5

+751

abiba-bot commented 2026-09-25 11:04:45 +00:00

Owner

Copy Link

Copy Source

Closes backlog row land-revision-preflight-guard-20260925.

Why

A contract verdict is only meaningful if it came from the merged copy. The fleet was bitten three times on 2026-09-25: a clone parked on a merged feature branch while executing from a different clone; a script copied into the runner clone by hand; and 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.

The defect (fixed, and preserved as a fixture)

The draft resolved the master revision with:

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. Reproduced:

case	pre-fix draft	fixed guard
scripts/demo.sh (nested)	could not resolve, exit 0	resolves, exit 0 on match
script in no revision	exit 0	exit 1
mismatched contents	exit 1	exit 1
unresolvable ref	exit 0	exit 1

The draft verbatim is preserved at tests/fixtures/revision-preflight.prefix.sh (md5 7559c8290797e8d6be0d38a9fad94393), so the original content is in the PR and the tests can run against it.

The fixed guard

scripts/revision-preflight.sh <script-path> <clone-path>:

• 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 prints that freshness is assumed rather than hiding it.

Wiring into contract execution

scripts/contract-run.sh runs the guard before executing, against the clone it lives in:

CONTRACT_REVISION_PREFLIGHT	Behaviour
unset / enforce (default)	withhold the verdict, alert, exit 2
warn	log the mismatch and continue
off	skip

Verified live:

match:     ✅ revision-preflight: scripts/search-stack-check.py matches origin/master @ 9d64b0b
           ✅ VERDICT: PASS                                              EXIT=0
mismatch:  ❌ revision-preflight: MISMATCH — refusing to report from this copy
           🚫 VERDICT WITHHELD: executing copy does not match the merged revision  EXIT=2
=warn:     ⚠️  revision preflight failed — continuing because …=warn
           ✅ VERDICT: PASS                                              EXIT=0

Pinning documented

docs/contract-execution-pinning.md records the rule — every contract pins the clone contract-run.sh lives in, the deployed runner being /opt/contract-runner on CT 100 — with the per-contract table. It also records that daily-health-digest has no contract file at all, which is exactly 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, so the guard's fetch path is exercised. It runs the pre-fix draft against the same cases and demonstrates it passing a ghost script, so the tests provably bite.

passed: 15   failed: 0
All revision-preflight tests passed.

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.

Untracked file in firstmate's clone

Resolved. The untracked scripts/revision-preflight.sh there blocked that clone's fast-forward — confirmed empirically that git refuses even when the content is byte-identical (The following untracked working tree files would be overwritten by merge). It was removed after its content was durably preserved in this pushed branch (verified: git show origin/fix/...:tests/fixtures/revision-preflight.prefix.sh | md5sum = 7559c829…). That clone is now clean and fast-forwards.

No master push, no merge.

Closes backlog row `land-revision-preflight-guard-20260925`. ## Why A contract verdict is only meaningful if it came from the merged copy. The fleet was bitten three times on 2026-09-25: a clone parked on a merged feature branch while executing from a different clone; a script copied into the runner clone by hand; and 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**. ## The defect (fixed, and preserved as a fixture) The draft resolved the master revision with: ```bash 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. Reproduced: | case | pre-fix draft | fixed guard | | --- | --- | --- | | `scripts/demo.sh` (nested) | `could not resolve`, **exit 0** | resolves, exit 0 on match | | script in no revision | **exit 0** | **exit 1** | | mismatched contents | exit 1 | exit 1 | | unresolvable ref | **exit 0** | **exit 1** | The draft verbatim is preserved at `tests/fixtures/revision-preflight.prefix.sh` (md5 `7559c8290797e8d6be0d38a9fad94393`), so the original content is in the PR and the tests can run against it. ## The fixed guard `scripts/revision-preflight.sh <script-path> <clone-path>`: - 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` prints that freshness is assumed rather than hiding it. ## Wiring into contract execution `scripts/contract-run.sh` runs the guard before executing, against the clone it lives in: | `CONTRACT_REVISION_PREFLIGHT` | Behaviour | | --- | --- | | unset / `enforce` (default) | withhold the verdict, alert, exit 2 | | `warn` | log the mismatch and continue | | `off` | skip | Verified live: ``` match: ✅ revision-preflight: scripts/search-stack-check.py matches origin/master @ 9d64b0b ✅ VERDICT: PASS EXIT=0 mismatch: ❌ revision-preflight: MISMATCH — refusing to report from this copy 🚫 VERDICT WITHHELD: executing copy does not match the merged revision EXIT=2 =warn: ⚠️ revision preflight failed — continuing because …=warn ✅ VERDICT: PASS EXIT=0 ``` ## Pinning documented `docs/contract-execution-pinning.md` records the rule — **every contract pins the clone `contract-run.sh` lives in**, the deployed runner being `/opt/contract-runner` on CT 100 — with the per-contract table. It also records that `daily-health-digest` has **no contract file at all**, which is exactly 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, so the guard's fetch path is exercised. It runs the pre-fix draft against the same cases and demonstrates it passing a ghost script, so the tests provably bite. ``` passed: 15 failed: 0 All revision-preflight tests passed. ``` `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. ## Untracked file in firstmate's clone Resolved. The untracked `scripts/revision-preflight.sh` there blocked that clone's fast-forward — confirmed empirically that git refuses even when the content is byte-identical (`The following untracked working tree files would be overwritten by merge`). It was removed **after** its content was durably preserved in this pushed branch (verified: `git show origin/fix/...:tests/fixtures/revision-preflight.prefix.sh | md5sum` = `7559c829…`). That clone is now clean and fast-forwards. No master push, no merge.

abiba-bot added 1 commit 2026-09-25 11:04:46 +00:00

feat: land the revision-preflight guard, fixed and wired into contract execution ... 574cb99d76

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.

abiba-bot referenced this issue from a commit 2026-09-25 11:29:17 +00:00

fix(revision-preflight): default to warn, name the refusal class, bound the fetch

abiba-bot added 1 commit 2026-09-25 11:29:20 +00:00

fix(revision-preflight): default to warn, name the refusal class, bound the fetch ... c5a6dcd42a

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 9faffe4 - with the note that an
   untracked file blocks a fast-forward even when byte-identical.

Live behaviour re-verified on the real runner path:
  default: REASON=mismatch:clone-ahead -> 'continuing because ...=warn' -> VERDICT: PASS, exit 0
  enforce: REASON=mismatch:clone-ahead -> 'VERDICT WITHHELD: mismatch:clone-ahead', exit 2

MANDATORY CHECKS (master went red once from a credential-SHAPED string, so
these are now run on every shippable branch):
  bash scripts/prose-lint.sh        -> LINT PASSED (18 warning(s))
  secret scan                       -> secret scan clean (tree; 34 allowlisted,
                                       24 inert value(s) ignored); No committed credentials
  shellcheck revision-preflight.sh  -> clean
  shellcheck test_revision_preflight.sh -> clean
  shellcheck contract-run.sh        -> SC2034 x1, SC2086 x2 - byte-identical on
                                       master, i.e. pre-existing, none introduced

tests/test_probe_drift.py::test_prose_lint_accepts_report_format_with_provenance
fails both before and after this branch (it runs prose-lint from a temp CWD and
cannot find its sibling secret-scan.sh). Pre-existing, unrelated, not fixed here.

abiba-bot added 1 commit 2026-09-25 11:29:51 +00:00

Merge remote-tracking branch 'origin/master' into fix/land-revision-preflight-guard-20260925 778424acd4

abiba-bot commented 2026-09-25 11:30:13 +00:00

Author

Owner

Copy Link

Copy Source

Bounded fix round — all five required items done

Branch head: 778424acd4481bee73d6f18c52cf65a8e9dfb5b0 (includes a merge of origin/master @ 9faffe4, so this branch also carries the lint fix from #135).

Fix-round diff (574cb99..c5a6dcd):

 docs/contract-execution-pinning.md |  95 ++++++++++++++++++++++--
 scripts/contract-run.sh            |  60 +++++++++------
 scripts/revision-preflight.sh      | 145 +++++++++++++++++++++++++++++--------
 tests/test_revision_preflight.sh   | 108 ++++++++++++++++++++++++++-
 4 files changed, 341 insertions(+), 67 deletions(-)

1. Default is now warn

CONTRACT_REVISION_PREFLIGHT unset → warn: the refusal is logged with its class and the contract still reports. enforce remains available and documented.

The flip criteria are written into docs/contract-execution-pinning.md as a decision with evidence, explicitly to be made as its own change rather than a silent defaults flip: a sustained window (30 days or 200+ contract runs) with zero mismatch:* and zero cannot-verify:* refusals; no cannot-verify:fetch-failed from ordinary network blips; and the pinned runner clone demonstrably kept current.

2. "Could not check" is now distinguishable from "this copy is wrong"

Every non-zero exit prints a machine-readable line first:

REASON=cannot-verify:fetch-failed
REASON=cannot-verify:ref-unresolvable
REASON=mismatch:path-absent
REASON=mismatch:content
REASON=mismatch:detached-head
REASON=mismatch:clone-ahead

Exit 2 = could not check, exit 1 = copy is wrong. detached-head and clone-ahead are named separately because they are legitimate states, far less alarming than a hand-edited file — and clone-ahead requires HEAD to be strictly ahead, so an uncommitted edit on a commit that is the ref is a plain content mismatch. (My first cut got that wrong; new test 7d caught it.)

3. The fetch is bounded

--fetch-timeout (default 20s, 0 = unbounded). A missing timeout binary is itself cannot-verify:fetch-failed rather than an unbounded fetch inside a scheduled contract. Test 5c asserts it returns promptly under a 1s bound.

4. Test coverage added

New classes covered: fetch failure, fetch timeout, unresolvable ref, detached HEAD, clone-ahead, genuine content mismatch, and the contract-run.sh default. The pre-fix draft fixture comparisons are kept.

passed: 31   failed: 0
All revision-preflight tests passed.

5. Merge-time sequence documented

docs/contract-execution-pinning.md now carries the sequence — fast-forward /opt/contract-runner, confirm clean, prove a contract runs and reports normally — with the baseline recorded as of today: firstmate has already fast-forwarded it to 9faffe4, and the note that an untracked file blocks a fast-forward even when byte-identical.

Live proof on the real runner path

default: REASON=mismatch:clone-ahead
         ⚠️  revision preflight: mismatch:clone-ahead — continuing because CONTRACT_REVISION_PREFLIGHT=warn
         ✅ VERDICT: PASS                                    EXIT=0
enforce: REASON=mismatch:clone-ahead
         🚫 VERDICT WITHHELD: mismatch:clone-ahead            EXIT=2

(The clone-ahead classification is correct here — this branch is genuinely ahead of origin/master mid-review, which is exactly the case the reviewer flagged.)

Mandatory checks (raw)

── secret scan (tree): 95 files ──
✅ secret scan clean (tree; 34 allowlisted exception(s), 24 inert value(s) ignored)
  ✅ No committed credentials
✅ LINT PASSED (18 warning(s))

shellcheck scripts/revision-preflight.sh       -> clean
shellcheck tests/test_revision_preflight.sh    -> clean
shellcheck scripts/contract-run.sh             -> SC2034 x1, SC2086 x2
   (byte-identical counts on master: pre-existing, none introduced)

python3 -m pytest tests/ -q                    -> 1 failed, 69 passed

The one failure is tests/test_probe_drift.py::test_prose_lint_accepts_report_format_with_provenance, which fails both before and after this branch (it runs prose-lint from a temp CWD and cannot find its sibling secret-scan.sh). Pre-existing and unrelated; flagged, not fixed here.

No master push, no merge.

## Bounded fix round — all five required items done **Branch head:** `778424acd4481bee73d6f18c52cf65a8e9dfb5b0` (includes a merge of `origin/master` @ `9faffe4`, so this branch also carries the lint fix from #135). Fix-round diff (`574cb99..c5a6dcd`): ``` docs/contract-execution-pinning.md | 95 ++++++++++++++++++++++-- scripts/contract-run.sh | 60 +++++++++------ scripts/revision-preflight.sh | 145 +++++++++++++++++++++++++++++-------- tests/test_revision_preflight.sh | 108 ++++++++++++++++++++++++++- 4 files changed, 341 insertions(+), 67 deletions(-) ``` ### 1. Default is now `warn` `CONTRACT_REVISION_PREFLIGHT` unset → `warn`: the refusal is logged with its class and the contract **still reports**. `enforce` remains available and documented. The flip criteria are written into `docs/contract-execution-pinning.md` as a decision with evidence, explicitly to be made as its own change rather than a silent defaults flip: a sustained window (**30 days or 200+ contract runs**) with **zero** `mismatch:*` and **zero** `cannot-verify:*` refusals; no `cannot-verify:fetch-failed` from ordinary network blips; and the pinned runner clone demonstrably kept current. ### 2. "Could not check" is now distinguishable from "this copy is wrong" Every non-zero exit prints a machine-readable line first: ``` REASON=cannot-verify:fetch-failed REASON=cannot-verify:ref-unresolvable REASON=mismatch:path-absent REASON=mismatch:content REASON=mismatch:detached-head REASON=mismatch:clone-ahead ``` Exit `2` = could not check, exit `1` = copy is wrong. `detached-head` and `clone-ahead` are named separately because they are *legitimate* states, far less alarming than a hand-edited file — and `clone-ahead` requires HEAD to be **strictly** ahead, so an uncommitted edit on a commit that *is* the ref is a plain `content` mismatch. (My first cut got that wrong; new test 7d caught it.) ### 3. The fetch is bounded `--fetch-timeout` (default **20s**, `0` = unbounded). A missing `timeout` binary is itself `cannot-verify:fetch-failed` rather than an unbounded fetch inside a scheduled contract. Test 5c asserts it returns promptly under a 1s bound. ### 4. Test coverage added New classes covered: fetch failure, fetch timeout, unresolvable ref, detached HEAD, clone-ahead, genuine content mismatch, and the `contract-run.sh` default. The pre-fix draft fixture comparisons are kept. ``` passed: 31 failed: 0 All revision-preflight tests passed. ``` ### 5. Merge-time sequence documented `docs/contract-execution-pinning.md` now carries the sequence — fast-forward `/opt/contract-runner`, confirm clean, prove a contract runs and reports normally — with the **baseline recorded as of today: firstmate has already fast-forwarded it to `9faffe4`**, and the note that an untracked file blocks a fast-forward even when byte-identical. ### Live proof on the real runner path ``` default: REASON=mismatch:clone-ahead ⚠️ revision preflight: mismatch:clone-ahead — continuing because CONTRACT_REVISION_PREFLIGHT=warn ✅ VERDICT: PASS EXIT=0 enforce: REASON=mismatch:clone-ahead 🚫 VERDICT WITHHELD: mismatch:clone-ahead EXIT=2 ``` (The `clone-ahead` classification is correct here — this branch is genuinely ahead of `origin/master` mid-review, which is exactly the case the reviewer flagged.) ### Mandatory checks (raw) ``` ── secret scan (tree): 95 files ── ✅ secret scan clean (tree; 34 allowlisted exception(s), 24 inert value(s) ignored) ✅ No committed credentials ✅ LINT PASSED (18 warning(s)) shellcheck scripts/revision-preflight.sh -> clean shellcheck tests/test_revision_preflight.sh -> clean shellcheck scripts/contract-run.sh -> SC2034 x1, SC2086 x2 (byte-identical counts on master: pre-existing, none introduced) python3 -m pytest tests/ -q -> 1 failed, 69 passed ``` The one failure is `tests/test_probe_drift.py::test_prose_lint_accepts_report_format_with_provenance`, which fails both before and after this branch (it runs `prose-lint` from a temp CWD and cannot find its sibling `secret-scan.sh`). Pre-existing and unrelated; flagged, not fixed here. No master push, no merge.

abiba-bot merged commit 552c776c0c into master 2026-09-25 11:46:30 +00:00

abiba-bot deleted branch fix/land-revision-preflight-guard-20260925 2026-09-25 11:46:31 +00:00

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

No items

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

abiba-bot jerome (Jerome Kwame Tabiri) kagentz-bot kwame (Kwame Tabiri) mumuni-bot (Mumuni 🦅 (Syslog Falcon)) tanko-bot theo

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: SyslogSolution/prose-contracts#134