fix(ci): guard each live Postgres contract invocation

[QinXi-ai]

The live CLI anchor test could be deleted, misspelled, or run without --ignored while the postgres-contract guard stayed green. The guard now derives live test targets from the Rust requirement marker and checks each CI/local invocation, its ignore flag, and any ignored-test name filter. Both local database paths now run the CLI contract alongside the store contract.

This follows the issue's proposed patch, corrects its #340 attribution, and fixes an additional parser gap: the quoted run_step label cannot satisfy checks on the command that actually executes. The guard also rejects disabled required-database settings and ambiguous duplicate target names. The local reproduction documentation includes both commands.

Validation on Windows/Git Bash:

• The guard passes before and after mutation testing.
• All 16 mutations were rejected, including the three issue examples, executable-argv changes with intact labels, absent/disabled database requirement settings, missing source targets, and missing Rust markers/ignore attributes.
• ShellCheck 0.11.0 (--severity=warning) passed for both changed shell scripts.
• python -X utf8 scripts/check_evidence_claims.py and git diff --check passed.

The live database tests were NOT_TESTED locally; this host cannot build the Unix-only workspace. No Rust test counts changed. Codex assisted with this change.

Closes #362.

The three original false greens were also reproduced against the unchanged d881d49 guard, with source files restored afterward. The 16-mutation matrix additionally deletes the CLI invocation from just one local path; the new guard rejects it.

Linux CI at commit 190a3ad: all applicable checks passed, including the full Rust job, live postgres-contract, docs-and-hygiene, scripts-lint and CodeQL. CI run: https://github.com/lacs-project/sysknife/actions/runs/34081898044.

[vladimirrott]

This is the right rewrite. Parsing each invocation instead of grepping the whole job is the correct reading of #362, and the diagnostics are the best part of it: the misspelled-filter case names the token, the file it searched, and what would have happened ("the step would run 0 tests and exit 0"). Most guards say less than that when they fire.

I reproduced your three regressions in a scratch clone at 190a3adf, measuring the exit code rather than reading the output, since a pipe to tail will hand you a 0 from tail and I nearly fooled myself with exactly that:

S0 pristine                                rc=0
M1 drop --ignored from the CLI step        rc=1
M2 misspell the filter by one character    rc=1
M3 delete the CLI step outright            rc=1

All three are green on main today. That is the case for this PR on its own.

One blocking item. The marker set can shrink without the guard noticing, and whichever file drops out loses its coverage silently:

M4 rename the marker in cli_smoke.rs       rc=0   passed: 1 live contract file(s) (postgres_store)
M5 rename the marker in postgres_store.rs  rc=0   passed: 1 live contract file(s) (cli_smoke)
M6 remove it from both                     rc=1   fails closed, correctly

M5 is the one that bites. The job still runs --test postgres_store, but the test no longer reads a variable CI sets, so its fail-closed check is inert and a missing database skips and exits 0. That is #313's shape returning for that file, and the old guard caught it, because it hardcoded postgres_store.rs and failed when the token left it.

The header states the guarantee outright: "renaming the env var or dropping #[ignore] fails here until the job catches up." M4 and M5 make that sentence false, and a guard documenting a guarantee it does not provide is what this repo fails CI over everywhere else.

The asymmetry is that marker -> step is checked and step -> marker is not: check_invocation returns quietly when the target has no marker file but the file exists. The other half closes it:

         if ! compgen -G "$repo_root/*/*/tests/$target.rs" >/dev/null; then
             report "$where targets --test $target, and no crates/*/tests/$target.rs or apps/*/tests/$target.rs exists to run"
+        else
+            report "$where targets --test $target, whose file no longer names $require_token; the contract set narrowed under the job"
         fi

I ran that before suggesting it, so it is tested rather than sketched:

S0 pristine + fix   rc=0   passed: 2 live contract file(s) (cli_smoke postgres_store)
M4 with fix         rc=1
M5 with fix         rc=1
S0 restored         rc=0

No false positives on the current tree. The nearby comment about a target having "no source of truth here to resolve a name against" then wants a small edit, since there would be one.

I also tried dropping a single #[ignore] from the store contract and the guard stayed green. That one is not a defect: you demand the attribute rather than a count, on purpose, and my mutation left the other ignored tests in place. Recording it so you know it was checked.

Nothing else blocks. One optional note: the >= 2 branch count in ci-local.sh reads as "both paths" only because there happen to be two today. A third entry path would satisfy it while skipping one. Worth a word in the comment rather than a change.

Your CHANGELOG entry is accurate and I would keep it exactly as written.

No new issue from me while #267, #239 and #238 are still open on your side; I would rather you have room than a queue. And since this diff is about proving a database contract actually ran: if you run Postgres anywhere yourself, sysknife audit verify checks a signed action chain against a live database and exits non-zero on a truncated anchor, read-only. It is the same property you are guarding here, from the operator's end.

[vladimirrott]

Approved at bacc07d39e4292b621f0f506970dc80b695f4463.

The bidirectional check is the part that matters, and you built it where the one-directional check lived rather than bolting a second pass beside it. Renaming the marker in either contract file now fails the guard by name, which is what the old shape could not see:

FAIL  /repo/scripts/ci-local.sh invocation 2 targets --test cli_smoke, whose file no longer names SYSKNIFE_REQUIRE_POSTGRES; the contract set narrowed under the job

maintainer screen 383 returns DO NOT EXECUTE, so I ran nothing on the host. Everything below ran in docker.io/library/debian:stable-slim under rootless podman with --network=none and the tree mounted from git archive bacc07d3.

Mutations

#	mutation	guard
M0	none	pass: 2 live contract file(s) (cli_smoke postgres_store)
M1	drop -- --include-ignored from the ci.yml store step	FAIL, naming the step
M2	replace the job's SYSKNIFE_REQUIRE_POSTGRES env key	FAIL: job is missing SYSKNIFE_REQUIRE_POSTGRES
M3	point the CLI filter at a name that does not exist	FAIL: would run 0 tests and exit 0
M4a	rename the marker in cli_smoke.rs to SYSKNIFE_NEEDS_PG	FAIL ×2, both ci-local.sh branches
M4b	same rename in postgres_store.rs	FAIL ×2
M6	delete the #[ignore] lines from postgres_store.rs	FAIL: has no #[ignore] test
M7	delete the CLI anchor step from ci.yml	FAIL: runs no step against --test cli_smoke
M9	delete the anchor invocations from both ci-local.sh branches	FAIL: runs --test cli_smoke in 0 branch(es)
M10	restore everything	pass

The command, verbatim:

$ bash /home/entropia/.local/state/sysknife-maint/pti.sh docker.io/library/debian:stable-slim p383 bash -c "$(cat /tmp/sk383proof2.sh)"
### M4a. rename the marker in cli_smoke.rs to a token that does NOT contain the old one
old token still present in cli_smoke.rs: 0
new token count: 2
FAIL  /repo/scripts/ci-local.sh invocation 2 targets --test cli_smoke, whose file no longer names SYSKNIFE_REQUIRE_POSTGRES; the contract set narrowed under the job
FAIL  /repo/scripts/ci-local.sh invocation 4 targets --test cli_smoke, whose file no longer names SYSKNIFE_REQUIRE_POSTGRES; the contract set narrowed under the job

3 postgres-contract guard failure(s).
exit=1

Two things I got wrong on the way, since they say something about the guard rather than about you.

My first M4 renamed the token to SYSKNIFE_REQUIRE_POSTGRES_RENAMED, and the guard read green. grep -F "$require_token" still matched the longer name as a substring, so my mutation never applied. That is the mutation-did-not-land trap, and it was mine.

Before that I ran the whole matrix in docker.io/library/bash:5 and every case printed the same line: no test file under crates/ or apps/ names SYSKNIFE_REQUIRE_POSTGRES. Alpine's busybox grep has no --include, so the marker derivation returned empty and the guard exited before reaching any mutation. Ten identical outputs is what a vacuous run looks like. Your || true comment about #347 is why the diagnostic printed at all instead of the script dying silently at the assignment.

Not blocking, worth a line each

bacc07d3 is a merge commit that carries the fix inline, so git log 190a3adf..bacc07d3 shows main's commits and the merge but not the change I asked for. The review diff is clean; a rebase would make the fix reviewable as one commit if you push again for another reason.

check_invocation runs against every run_step plus cargo test statement anywhere in ci-local.sh, not only the Postgres group. Nothing trips it today: I checked, and the only cargo test statements in that file are the four in run_postgres_contract_group, while every other Rust step is cargo nextest run, clippy, fmt, doc or audit. A future non-Postgres cargo test --test X step would fail the guard. Pre-existing, not yours.

The merge is blocked by my tooling, not by this PR

All eleven checks are green and nothing is pending. The gate refused:

$ maintainer-merge verify 383 bacc07d39e4292b621f0f506970dc80b695f4463 "tests/release/postgres-contract-guard.test.sh" "/audit_verify_exits_with_code_1_when_anchor_is_truncated/d"
maintainer-merge: no single suite in /home/entropia/.local/share/maintainer/profiles/sysknife/verify.d covers every changed path:
    CHANGELOG.md	docs
    docs/developer-guide.md	docs
    scripts/ci-local.sh	shell
    tests/release/postgres-contract-guard.test.sh	shell

  Suites that would cover the rest between them: docs shell.

A shell change plus the CHANGELOG entry the repo asks for spans two suites, so no receipt is earnable. Do not split the PR: the CHANGELOG entry belongs with the change. That part is mine to run, and I have the exact commands.

Next

#397 is the obvious follow-on for you: CHAIN_ROW_COLUMNS is declared twice and only a live Postgres run notices when the copies drift, the same shape as what you fixed here, one layer down. I am not offering it yet because you already hold #362 and #237; when both land I will bring it to you first.

You read CI logs and run Postgres for a living, going by this diff. If you ever want to point SysKnife at one of your own Ubuntu boxes, the read-only surface is the place to start (sysknife "show me disk usage and failed services" plans and previews without executing anything). No obligation, and it has nothing to do with this merging.

[vladimirrott]

Merged as f61e76f, closing #362.

You ran sixteen mutations and said which platform each one ran on, including the cases Git Bash could not reach. A mutation table is worth what its honesty about the gaps is worth, and yours is the one I trust most on this tracker.

Workspace at 5673d20:

$ cargo nextest run --workspace --locked
     Summary [   8.585s] 1851 tests run: 1851 passed, 6 skipped

#238 came your way as #384 merged, so you have that one if you want it. I recounted its arithmetic afterwards, because your own #384 is what invalidated it: 57 Debian-family actions run unchanged rather than 43, and 154 of 189 rather than 140. The body carries the recount and the two scripts behind it, so start from the body rather than from my offer.