feat(#279): verify-full tier + type-widening scan (and the hollow tests that hid four bugs) - #375
Merged
Merged
Conversation
Partial implementation preserved after a provider 429 interrupted the develop step mid-dispatch. Adds invariant-scan and verify-full modules plus their smoke tests; extends lens, stepback-ci, work-status, and workflow-state events. Type-checked and linted, but NOT yet reviewed by the adversarial or six-pass lens gates — the cycle will resume from this base. Refs #279
test-invariant-scan.ts and test-verify-full.ts each defined `export async function run()` and never called it. `bun run <file>` therefore defined 23 test functions, executed none, printed nothing and exited 0 — and CI's only contract is `bun run "$t" || exit 1`, so both counted as passing while their subsystems had zero coverage. That is EPIC #328's thesis ("a gate that cannot fail is worse than no gate") reproduced inside the very issue meant to strengthen verification. Invoking them surfaced four defects that had been invisible. Two in the scanner (invariant-scan.ts): 1. Renamed files reported the PRE-rename path. The file-header regex was `/^(---| \+\+\+) (?:a\/)?(.+)$/` — a stray space before `\+\+\+` meant the `+++` line never matched at all, so only `---` ever set the path, and the `a/`-only strip would have left `b/` on it even if it had. Findings pointed the ARCHITECTURE lens at a file that no longer exists. The `+++` path is now authoritative, with `---` kept as the fallback for deletions where `+++` is /dev/null. 2. Every "removed X" class (readonly/const, assert, pub, mut) fired whenever the keyword appeared on a `-` line, without checking whether it was still present on the corresponding `+` line. So `- const x: string` / `+ const x: string | null` reported a removed `const` that is plainly still there — a false positive on nearly every touched const declaration in a TS diff. Since the scanner routes into the lens context, that is a noise pump. Findings are now gated on the token actually being absent from the hunk's added lines, hunk-scoped so an unrelated removal elsewhere in the file does not mask a genuine one. Two in the tests themselves: 3. The verify-full mock destructured a single `{cmd, cwd}` object while runVerifyFull calls `execFn(cmd, opts)` with two positional args, so every recorded call had `cmd: undefined`. The mock had never matched the implementation's calling convention. 4. `testRunVerifyFullEvidenceStderr` expected `failure` for a command that exits 0 with stderr output, and its mock did not even throw — it could never have passed. Exit code is the contract: promisify(exec) rejects on non-zero, and test runners routinely write warnings to stderr while succeeding. Now asserts success WITH stderr used as the evidence, which is what the implementation correctly does. Also relaxed two `result.ms > 0` assertions: an instantly-resolving mock legitimately elapses 0ms, so that was a timing assumption rather than a contract. Added testRunVerifyFullMeasuresElapsed with a delayed mock so the relaxed assertions cannot pass vacuously. testGenericWideningAny asserted `findings.length === 1` for `Result<any>`, but the seven pattern classes are independent detectors, not a partition — that line is both type erasure and generic widening. Pins the overlap explicitly instead, so it is intended rather than accidental. Verified the gate now fails: injecting a wrong `kind` into the scanner makes the suite exit 1; reverting restores exit 0. 62/62 offline smoke tests on this branch, tsc + biome clean. Refs #279
The branch shipped unit coverage for the scanner's classification and
the verify-full helpers, but nothing checked the two claims the issue
actually makes: that scanner findings REACH the lens, and that a failing
verify-full SUPPRESSES the ops dispatch. Those were unmet acceptance
criteria, and they are the part that can silently rot.
runLens resolves its diff through fetchAllMergedDiffs, which shells out
with raw execp and has no injection seam — so stubbing verifyExecFn does
nothing and the AC was, as written, untestable. The test therefore
builds a real repository with a local bare origin and commits the exact
vipune ea8c836 shape (`embedder: EmbeddingEngine` →
`Option<EmbeddingEngine>`), then asserts the mandate line, the finding
kind and the file path all appear in the context the lens actually
receives. Same reasoning as the always-worktree real-git test: mocks
prove we ask the right questions, not that the answers arrive.
Covers, per the issue's acceptance criteria:
- findings provably reach the lens context (framing line asserted)
- a widening is ROUTE-ONLY and does not block an approved review
- absent config emits verify-full-status: skipped VISIBLY
- green full suite emits success and the ops watch proceeds
- failing full suite emits failure, bumps ciRetryCount, and does NOT
dispatch ops that round, with evidence retained for the handoff
- both kill-switches restore pre-#279 behaviour exactly
Also covers the 2026-07-27 addendum, which matters now that #287-#289
have landed: under parallel groups repoRoot may sit on a different
group's branch by ci time, so verify-full must execute in the group's
worktree. Asserted directly — no verify-full invocation has cwd ===
repoRoot.
72/72 offline smoke tests, tsc + biome clean.
Refs #279
README env table: PI_ENSEMBLE_VERIFY_FULL, PI_ENSEMBLE_VERIFY_FULL_TIMEOUT_MS, PI_ENSEMBLE_WIDENING_SCAN. troubleshooting.md: two sections covering what each feature does, the three verify-full outcomes and why an absent file is visibly skipped rather than silent, why there is deliberately no derivation fallback, why the command runs in the worktree rather than the repo root, and why the "removed X" scan classes only fire when the token is genuinely gone. AGENTS.md §7: one sentence in the /work driver paragraph. Completes the last two acceptance criteria on the issue. Refs #279
The pull_request event did not dispatch a workflow run for this branch (zero check-runs on the head commit) — every other PR today triggered normally. An empty commit re-fires the event.
The pull_request event did not dispatch during the 2026-08-06 Actions outage (major outage from 15:22 UTC; zero check-runs on the head commit). Re-firing now that capacity is recovering.
Owner
Author
Merging on local-gate evidence — CI could not runGitHub Actions has been in a major outage since 15:22 UTC (incident ongoing; "workflow runs are still failing, and jobs may remain queued for an extended period"). No workflow has dispatched repo-wide since 13:46 UTC, and this branch has zero check-runs — not a failed run, no run at all. Two empty commits were pushed to re-fire the Every step
That is a superset of the CI job — same commands, same working directories. What remains unverified: an independent build on clean infrastructure. Local runs share this machine's toolchain and Worth re-stating for the record: this PR carries If CI comes back and reports a failure 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.
Closes #279.
Two independent gaps that ship together, plus the review the original WIP never got.
A — verify-full tier
.pi/verify-cmd-full(first non-empty non-comment line, verbatim, no derivation fallback) runs at the start ofci, before the CI watch, and reports through its ownverify-full-statusevent so fast and full outcomes are visible separately.skipped— visible, never silentsuccess, CI watch proceedsfailure,ciRetryCountbumped, ops dispatch skipped that roundThe separation is the point: in vipune the fast suite stayed green for ~2.5 months while the real-embedder tests sat behind
#[ignore], and nothing in the pipeline could express the difference.B — type-widening scan
Deterministic regex scan of the integrated diff for removed compiler-enforced invariants, routed into the lens context with an explicit mandate for the ARCHITECTURE lens. Route-only — it never fails a cycle.
The mechanism it exists to catch:
EmbeddingEngine→Option<EmbeddingEngine>removed an invariant, and a later change read theOptionas an invitation to write a mock-fallbackNonebranch in production.The part that took the time
The branch was WIP, explicitly unreviewed, and its two smoke tests were hollow — each defined
export async function run()and never called it.bun runprinted nothing and exited 0, so CI'sbun run "$t" || exit 1counted both as passing while the subsystems had zero coverage. EPIC #328's thesis, reproduced inside the issue meant to strengthen verification.Invoking them surfaced four defects, two of them real bugs in the shipped scanner:
Renamed files reported the pre-rename path. The header regex
/^(---| \+\+\+) (?:a\/)?(.+)$/has a stray space before\+\+\+, so the+++line never matched at all — only---ever set the path, and thea/-only strip would have leftb/on it regardless. Findings pointed the lens at a file that no longer exists.Every "removed X" class was a false-positive generator.
readonly/const/assert/pub/mutfired whenever the keyword appeared on a-line, with no check that it was gone from the+line — so- const x: string/+ const x: string | nullreported a removedconstthat is plainly still there. For a route-only signal that is a noise pump, and a signal that cries wolf gets ignored. Now hunk-scoped, and gated on the token actually being absent.The verify-full mock destructured
{cmd, cwd}whilerunVerifyFullcallsexecFn(cmd, opts)— every recorded call hadcmd: undefined. The mock had never matched the implementation.testRunVerifyFullEvidenceStderrexpectedfailurefor a command exiting 0 with stderr, and its mock did not throw — it could never have passed. Exit code is the contract; runners routinely write warnings to stderr while succeeding.Two acceptance criteria the WIP had skipped
Unit tests proved the scanner classifies and the helpers behave. Nothing proved the features were wired.
test-work-driver-279-integration.tscloses that:runLensresolves its diff throughfetchAllMergedDiffs, which uses rawexecpwith no injection seam — so stubbingverifyExecFndoes nothing and the AC was, as written, untestable. The test builds a real repo with a local bare origin and commits the exact vipune shape, then asserts the mandate line, the finding kind and the file path all appear in the context the lens actually receives. Same reasoning as the always-worktree real-git test: mocks prove we ask the right questions, not that the answers arrive.ciRetryCount, and retains evidence — end-to-end throughrunCi.citime, so verify-full must run in the group's worktree. Asserted directly — no invocation hascwd === repoRoot.Verification
Rebased onto current
main(10 commits, four overlapping files auto-merged; the new event kinds still satisfywork-status.ts's exhaustive switch, which tsc confirms).Full §1 gate: build,
tsc --noEmit,bun run check, 72/72 offline smoke tests.I confirmed the revived gates can fail: injecting a wrong
kindinto the scanner makes the suite exit 1; reverting restores exit 0.Docs: README env table, two
troubleshooting.mdsections, AGENTS.md §7 sentence.Not covered
The issue's live test plan (add
.pi/verify-cmd-fullon nessie, run/work N, confirm the PR comment shows both tiers). Everything here is offline — thorough on the call graph and, for the lens path, on real git, but no live cycle has run.