fix(acq-kits): give verify scripts a "could not check" verdict state

[wz-gsa]

All seven kit verify scripts ended with the same two-state verdict:

[ "$fail" -eq 0 ]

That answers "did anything fail?" and cannot answer "did we actually check anything?". Two consequences, both measured rather than inferred.

1. A run where nothing executed reported success

Reproduced in isolation:

pass=0 fail=0  ->  "All checks passed."   exit 0

2. A skipped check was indistinguishable from a passing one

Across the seven scripts there are 29 skip/NOTE sites and 9 warn sites that increment no counter.

Worst case: git-ssh-sign skips its end-to-end signed-commit check — the kit's entire purpose — whenever no key is in the forwarded SSH agent, and still reported "All checks passed."

And openchamber/paseo do define skip() and warn(), but they're cosmetic printf with no counter:

skip() { printf '  \033[33mSKIP\033[0m %s\n' "$1"; }

So paseo can print 19 yellow SKIPs and still claim the offline gate passed.

This is the ninth instance of this defect class across this family of repos (ENV-1, SEC-6, DX-7/8/9, SEC-12/13/17, DEP-4 being the prior ones), which is why the fix is a shared contract rather than seven local patches.

The contract

integrations/isolation/acq-kits/verify-report.sh — five states, and a verdict that is a function of a coverage record rather than a failure counter:

Reporter	Meaning	Degrades verdict?
ok	ran and passed	—
bad	ran and failed	yes → 1
unver	could not run (missing tool, absent credential, no network)	yes → 3
skip	not applicable in this configuration	no
warn	advisory; a documented fallback was taken	no

exit 0   every check that ran passed, and at least one ran
exit 1   at least one check FAILED
exit 3   nothing failed, but the run is not a pass

3 is distinct from 1 so a caller can tell "we looked and found problems" from "we could not look." The unver/skip split is the load-bearing part — collapsing them is how a verify script ends up claiming coverage it does not have.

Staged on purpose: two kits here

The two smallest (git-ssh-sign, usai-provider), so the contract is reviewable in isolation before a bulk edit. Remaining: openchamber (641 lines, 3 gates), paseo (864, 3), pi-coding-agent (337, 3), agentic-coding-playbook (167), zscaler-ca-certificate (167) — listed in the test's UNCONVERTED set, which must reach empty.

Converted sites:

• git-ssh-sign — no host SSH key → unver; no forwarded key → unver (the kit's only end-to-end assertion).
• usai-provider — no USAi secret → unver; API probe empty → unver. That last one matters: in_sbx() swallows the exit status, so an empty result means either "no curl in the guest" or "sbx exec failed" — indistinguishable, and never a pass.

Enforcement, because nothing in CI runs these scripts

They need a live sandbox CI cannot create, and this repo has no shellcheck hook — so 2,400 lines of shell are currently linted by nothing. scripts/tests/test_verify_contract.py (12 tests) reads them as text and asserts:

• the library keeps both degrading branches — deleting either silently restores the two-state verdict for every caller at once;
• converted kits source it, call verify_reset, and don't shadow its reporters with no-op locals (which would pass every other test while behaving exactly as before);
• no bare echo "SKIP" remains in a converted kit;
• UNCONVERTED describes reality in both directions — a kit converted without being delisted fails, and so does a listed kit that reverted.

Every helper asserts its input set is non-empty first, so the suite cannot pass vacuously over a glob that stopped matching. That would be the same defect wearing a different hat.

A near-miss worth recording

The library was first written to acq-kits/lib/. .gitignore:13 carries the stock Python build-artifact block, whose lib/ rule silently excluded it:

$ git check-ignore -v integrations/isolation/acq-kits/lib/verify-report.sh
.gitignore:13:lib/	integrations/isolation/acq-kits/lib/verify-report.sh

Committing would have shipped two kits sourcing a file git never tracked — every verify run dying on "No such file or directory." Caught because git status listed the two modified kits and the new test but not the library. It now sits at the acq-kits root beside validate-kits.py (the existing precedent for shared assets there), and the reason is recorded in the test file so nobody re-adds a lib/.

Verification

all five verdict states exercised directly:
  nothing ran -> exit 3     clean -> exit 0      a failure -> exit 1
  an unver    -> exit 3     skip-only -> exit 0

degraded path of the TESTS: reverting usai-provider to origin/main
  -> FAILS "does not source lib/verify-report.sh"

pytest scripts/tests/                -> 413 passed (12 new)
validate_repo.py --root .            -> exit 0
validate-kits.py --root . --strict   -> all kits OK
scan_unsafe_shell.py .               -> 0 errors, 0 warnings
shellcheck -s sh verify-report.sh    -> exit 0
ruff check / ruff format --check     -> clean
markdownlint                         -> 0 issues
git diff --check                     -> clean

shellcheck on the converted kits:
  git-ssh-sign    10 findings before / 10 after
  usai-provider    7 findings before /  7 after    (pre-existing SC2015/SC2016
                                                    info-level; I added none)

I exercised the degraded paths of both the library and the tests, because a guard that only passes when things are fine is indistinguishable from a guard that always passes.

Not in this PR, deliberately

General shell linting. These scripts have never been shellcheck'd, and adding a hook would surface unrelated findings across 2,400 lines — that belongs in its own change. This one closes the verdict-honesty gap only.

acq-kits/README.md gains a short section documenting what each exit code means, since the exit contract is now something a caller can rely on.

---

AI-assisted (OpenCode), prepared on operator instruction. Human review and merge required.

[wz-gsa]

CI is green now (7/7). Rebased onto main (1.10.0) and fixed a real defect the failure exposed — in my own test, not the library.

What CI caught

oci-engine (#437) landed on main while this was in review — an eighth verify script that didn't exist when I branched. The test derived its converted set by subtraction:

{k: p for k, p in _verify_scripts().items() if k not in UNCONVERTED}

So a kit in neither set was treated as converted by default, and four contract tests failed against it with "oci-engine/scripts/verify does not source lib/verify-report.sh" — true, but misleading. The real problem was that a kit appeared nobody had classified.

I'd guarded both directions I thought of — a kit converted but still listed, and a listed kit that reverted — and missed the one that actually happened. The default was the bug: an unclassified kit silently inherited a contract it doesn't implement, which is the same shape as the defect this whole change exists to fix.

The fix

Two explicit sets, CONVERTED and UNCONVERTED, plus test_every_kit_is_classified asserting every verify script appears in exactly one. Proved it by simulating a ninth unclassified kit:

AssertionError: verify script(s) in no classification set: ['zz-phantom'].
Add each to CONVERTED (if it sources verify-report.sh) or to UNCONVERTED
(if it is still on the old two-state verdict). A kit is never converted by default.

Corrected counts

Recounted on the rebased tree rather than carried forward from the original description:

• Eight verify scripts, not seven.
• 35 uncounted skip/NOTE sites, not 29. (9 warn sites, unchanged.)
• oci-engine has 303 lines, 2 gates, and 2 [ "$fail" -eq 0 ] verdicts — it has the primary defect too, and is listed UNCONVERTED for the staged bulk conversion.

Stale numbers in the library header and test docstring are updated. UNCONVERTED is now agentic-coding-playbook, oci-engine, openchamber, paseo, pi-coding-agent, zscaler-ca-certificate — it must reach empty.

Verification on the rebased tree

pytest scripts/tests/                -> 420 passed (13 contract tests)
unclassified-kit probe               -> FAILS with the naming assertion
validate_repo.py --root .            -> exit 0
validate-kits.py --root . --strict   -> all kits OK
scan_unsafe_shell.py .               -> 0 errors, 0 warnings
shellcheck -s sh verify-report.sh    -> exit 0
ruff check / ruff format --check     -> clean
gh pr checks                         -> 7/7 pass

One note, not a repo issue

I initially couldn't read the CI log at all: gh run view --log failed with x509: certificate signed by unknown authority. Worth recording in case anyone else hits it, because it looks like a CI outage and isn't.

Go's crypto/x509.SystemCertPool() omits macOS admin-installed roots — on my host macOS trusts 177 roots (158 Apple + 19 admin-added, including the corporate proxy CA) while Go sees 121, and the proxy root is not among them. So any Go tool fails against a proxy-intercepted host while curl to that same host verifies fine. GitHub's API is on a bypass list, which is why gh api works and only the Actions-log redirect to *.blob.core.windows.net fails.

Reading the log required pointing SSL_CERT_FILE at a bundle including the admin roots. No change needed here — just not a trust-store problem, and not something to work around by disabling verification.

---

AI-assisted (OpenCode), on operator instruction. Human review and merge required.

[mogul]

Technically this is fine, but the level of bot-rumination in the comments distracts from understanding what it does, and memorializes context that won't matter going forward.

So I'm grudgingly requesting changes, even though that means another review round-trip... Sorry.

If you respond with improvements that only land in comments, then I approve your next push in advance, so feel free to check the box that skips the "require review" gate and merge it then!

[mogul]

The usual critique: Code comments should not memorialize bugs fixed, but rather describe the code as it is.

[mogul]

The extra commentary in the final sentence here is distracting. The section is documenting what the exit code means, not why the verify script is designed the way it is.

[mogul]

Same complaint; describing "what was" is not relevant for the person or agent trying to read and understand the code. This is the kind of information that belongs in the commit, not in the code.

[mogul]

Same superfluous design note in the last sentence here. It reads as self-flagellation for previous sins ("what was") rather than documentation of "what is".

[mogul]

Irrelevant and distracting information that belongs in the commit, not the code.

[mogul]

The description of "what was", and what is no longer true, is not relevant, and an obstacle to understanding. This section should just focus on the positive "why" aspects:

This helps us distinguish "did anything fail" from "did we actually check anything?"

[wz-gsa]

Addressed in `4f26eb4`. Two changes:

1. Comment-only trims (the 6 flagged blocks) — `git-ssh-sign/scripts/verify`, `usai-provider/scripts/verify`, `README.md`, `verify-report.sh`'s module header, and `test_verify_contract.py`'s module docstring + 2 inline comments. All narration of the old two-state verdict's history removed; each now just describes the current contract.

2. One allowlist data entry, not comment-only: `opencode` added to `UNCONVERTED` in `test_verify_contract.py`. This isn't from your review — `main` moved (opencode kit landed via feat(acq-kits): install OpenCode v2 via opencode kit #466) between your review and now, and `test_every_kit_is_classified` correctly caught it as unclassified (reproduced locally first: `AssertionError: verify script(s) in no classification set: ['opencode']`). opencode's verify script is still on the old two-state verdict, so `UNCONVERTED` is the correct bucket.

Flagging #2 explicitly because your standing offer was for comment-only fixes — this PR also has that one allowlist line, so I didn't want to assume it's covered and self-merge on a stretched reading. Your call whether that still qualifies or you'd rather take another look.

CI is 7/7 green. `python3 scripts/validate_repo.py`: all validators pass.

[mogul]

One approval-blocking documentation issue remains.

integrations/isolation/acq-kits/README.md:87 says “Verify scripts share one verdict contract,” then presents exit codes 0, 1, and 3 as the meaning of scripts/verify. But scripts/tests/test_verify_contract.py explicitly classifies only git-ssh-sign and usai-provider as CONVERTED; seven kits remain UNCONVERTED and retain the old two-state verdict. A caller can therefore rely on this documented contract for a listed kit and receive an incorrect result.

Please scope this section to converted kits and make the staged migration status clear, or defer the general exit-code documentation until every kit uses the shared reporter. The implementation and contract tests otherwise look sound.

[wz-gsa]

Addressed in 2dc0af4. You were right, and the defect was worse than a wording problem: the section asserted one contract while the suite classified only two of nine kits as implementing it, so a caller could read the table, run an unconverted kit, and treat its 0 as coverage evidence.

What changed

integrations/isolation/acq-kits/README.md now states the staged split explicitly and scopes the table to the kits that implement it:

• Converted — git-ssh-sign, usai-provider. The exit-code table and the five reporters apply to these "and only these".
• Unconverted — the other seven. Stated plainly: nothing above applies, a non-zero exit does not distinguish a failed check from an unmet precondition, and a 0 does not establish that every check ran.

On that last point I went looking for the concrete mechanism rather than asserting it generically. These scripts report an unmet precondition with a bare message that increments no counter, then exit 0 on the strength of whatever did run — for example zscaler-ca-certificate/scripts/verify:130 (NOTE: could not read fingerprint … skipping match) and agentic-coding-playbook/scripts/verify:64 (NOTE: no 'github' secret), and the local no-op skip() definitions in opencode, openchamber, paseo, and pi-coding-agent. All of those paths reach [ "$fail" -eq 0 ] and exit 0. The README now says to read such a 0 as "nothing that ran failed", never as full coverage.

The split is now enforced, not just described

Prose that must track a migration will drift, so scripts/tests/test_verify_contract.py gained test_readme_classification_matches_this_suite, which parses both lists out of the README paragraph and compares them to CONVERTED / UNCONVERTED. Measured, each drift direction fails:

converted-list over-claim inside the paragraph   -> FAIL
unconverted kit removed from the README only     -> FAIL

Three details worth flagging, because a guard like this is easy to write so that it passes vacuously:

1. It scans the whole paragraph, not the first sentence. An earlier draft stopped at the first . , which meant a later sentence in the same paragraph claiming extra kits were converted would have passed.
2. Kit names are intersected against the set of real scripts/verify paths, so backticked prose in the paragraph (exit codes, filenames) cannot be mistaken for a kit.
3. The unconverted lookup treats an absent section as the empty set. The README says the carve-out disappears once the list empties, and an earlier draft would have failed at exactly that moment — blaming the README for being correct. I simulated the end state (section removed, UNCONVERTED emptied, all nine kits converted) and the test passes.

The converted lookup still hard-fails on a missing section, since that one is not supposed to disappear.

Verification

scripts/tests/ (excluding the unrelated pre-existing
  test_oci_engine_vfs_fallback.py path failures)   424 passed
test_verify_contract.py                             23 passed
validate_repo.py                                   OK
validate-kits.py --strict                          all 10 valid
ruff check / ruff format                           clean

The five test_oci_engine_vfs_fallback.py failures are pre-existing on main and unrelated: the test targets files/home/oci-engine-config/oci-engine-vfs-fallback.sh while the script lives at files/home/oci-engine-vfs-fallback.sh. This PR touches neither.

No implementation or contract-test behavior changed — only the documentation scope plus the new guard.

AI-assisted (OpenCode). Advisory; requires your review.