What happened
PR #894 merged and deployed to production with local-evidence failing. gh pr merge --squash accepted it because that check is not branch-protection-required.
I am the cause: I treated Athena's GREEN review as the gate and never looked at CI. But the reason it was possible is a repo setting, and the same hole is open for everyone.
The failure was real, not flaky
terminal REST and MCP run parity failed
only in RIGHT (mcp): ['situation_digest']
scripts/project-routine-lifecycle-collect.mjs:576 asserts REST and MCP return identical run shapes. #894 deliberately makes routine_run_get return situation_digest to the run's assigned agent, and the collector reads with assignedToken — so the invariant genuinely changed and nothing but this check noticed.
It ran for ~2 minutes, failed cleanly, and reported the exact diff. It did its job. Nobody read it.
Why review could not have caught it
Athena's gate on #894 was thorough — she re-ran my tests, reproduced my mutations, and found a separate real defect. Both of us wrote in review that "REST and the dashboard read publicRoutineRun and are untouched." That statement is true. Neither of us asked whether anything asserts they must match.
That is the class of defect local-evidence exists to catch: cross-surface contract drift, invisible in a diff and invisible to unit tests, because the assertion lives in a conformance collector three directories away. A careful human reviewer and a machine check were each holding half the picture. Making the machine half optional means the halves only combine when someone remembers to look.
Ask
Make local-evidence a required status check on main.
Counter-argument worth stating: it is the slowest job (~2 min) and it has a genuinely flaky mode — the Playwright chromium binary goes missing at an absolute cache path, which failed at the merge-base too and is unrelated to any diff. Making a flaky check required buys blocked merges and a habit of overriding, which is worse than not requiring it.
So the honest sequencing is:
- Fix or quarantine the chromium-cache flake so the job is trustworthy.
- Then make it required.
Until step 2, the mitigation is procedural and weak: read CI before merging, every time, and do not treat a human gate as a substitute. I did not do that tonight.
Fixed forward, not reverted
The parity assertions now state the actual contract instead of blanket equality (#898): MCP-for-the-assignee is REST plus exactly one field, that field is a well-formed 64-hex digest, REST carrying it is its own named failure, and the digest must survive a Worker restart — an invariant the old equality check could not express. Two collector fixtures predating #894 were updated.
Related: #894 (the change), #898 (the fix), #897.
What happened
PR #894 merged and deployed to production with
local-evidencefailing.gh pr merge --squashaccepted it because that check is not branch-protection-required.I am the cause: I treated Athena's GREEN review as the gate and never looked at CI. But the reason it was possible is a repo setting, and the same hole is open for everyone.
The failure was real, not flaky
scripts/project-routine-lifecycle-collect.mjs:576asserts REST and MCP return identical run shapes. #894 deliberately makesroutine_run_getreturnsituation_digestto the run's assigned agent, and the collector reads withassignedToken— so the invariant genuinely changed and nothing but this check noticed.It ran for ~2 minutes, failed cleanly, and reported the exact diff. It did its job. Nobody read it.
Why review could not have caught it
Athena's gate on #894 was thorough — she re-ran my tests, reproduced my mutations, and found a separate real defect. Both of us wrote in review that "REST and the dashboard read
publicRoutineRunand are untouched." That statement is true. Neither of us asked whether anything asserts they must match.That is the class of defect
local-evidenceexists to catch: cross-surface contract drift, invisible in a diff and invisible to unit tests, because the assertion lives in a conformance collector three directories away. A careful human reviewer and a machine check were each holding half the picture. Making the machine half optional means the halves only combine when someone remembers to look.Ask
Make
local-evidencea required status check onmain.Counter-argument worth stating: it is the slowest job (~2 min) and it has a genuinely flaky mode — the Playwright chromium binary goes missing at an absolute cache path, which failed at the merge-base too and is unrelated to any diff. Making a flaky check required buys blocked merges and a habit of overriding, which is worse than not requiring it.
So the honest sequencing is:
Until step 2, the mitigation is procedural and weak: read CI before merging, every time, and do not treat a human gate as a substitute. I did not do that tonight.
Fixed forward, not reverted
The parity assertions now state the actual contract instead of blanket equality (#898): MCP-for-the-assignee is REST plus exactly one field, that field is a well-formed 64-hex digest, REST carrying it is its own named failure, and the digest must survive a Worker restart — an invariant the old equality check could not express. Two collector fixtures predating #894 were updated.
Related: #894 (the change), #898 (the fix), #897.