fix(worker): bind governed evidence and terminal to WorkOrder - #53
fix(worker): bind governed evidence and terminal to WorkOrder#53thegeorgepu wants to merge 8 commits into
Conversation
Closes the CCM-v0 Wave-1 gap that PR #49 named in its own body: on the GOVERNED (contract-envelope) path the economics summary reached disk and never reached the ledger. task.mjs builds a contract envelope with `ref: undefined` and carries the identity in the contract triple, so uploadEvidence's `isLedgerRef(ref)` gate was false and no evidence was POSTed at all. The work order id IS the ledger row, and real ids are `bk_`-shaped, which `isLedgerRef` already admits. `resolveEvidenceRef` (evidence.mjs) resolves it: contract work order id when it is a ledger ref, else the envelope ref, else null. The GATE IS NOT WIDENED — task.mjs's WORK_ORDER_ID is broader than LEDGER_REF, so a `wo-`-shaped id still falls through and posts nothing, exactly as today. This can only turn "posted nowhere" into "posted under the work order", never "posted under the wrong row". Behaviour change to state plainly: governed attempts now create evidence rows that did not exist before. They are NEW rows, not moved ones. The evidence job_ref also drives the economics projection's known-work-order test and the worker-principal visibility filter in vinci-gpu-control #288, so those surfaces will start showing governed attempts. Also registers vinci/worker/test/*.test.mjs in vinci/test/run.sh. Nothing in CI ran that directory, so the economics unit tests shipped in #49 have never executed there — an inert guard, the same shape run.sh already records for the byok tests. Zero matches is a failure, not a pass. Controls: the resolver's mutation (drop the contract branch) fails both unit tests and the integration test at "exactly one evidence POST"; the non-ledger work order id is a negative control in the same run; the integration case asserts the POST's job_ref equals the summary's work_order_id, which is what makes #288's binding check pass by construction rather than by luck. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…; state the precondition Independent review (Opus, fresh agent) — two findings taken: F4: the resolver fell through to the envelope ref when the contract id was present but not ledger-shaped. The economics summary takes contract-first UNCONDITIONALLY, so that could file a bundle under a row the summary does not name, which the ledger refuses as binding:work_order_mismatch. Unreachable today (a contract envelope has no ref), but the failure direction must be refusal, never a plausible wrong row. A present non-ledger contract id now returns null. F1: the comment asserted "real ids are bk_-shaped" with no corroboration, and that was the load-bearing premise. Checked: work order ids are caller-supplied, not minted in one shape; every order registry in vinci-gpu-control uses wo- ids while live backlog rows are bk_. So for a wo- order this change does nothing at all. The comment now states that as a precondition instead of an assumption, and says plainly what to read the change as. F3: adds the newly-reachable failure surface. A governed attempt now POSTs where it never did, so a non-2xx downgrades COMPLETED to UNVERIFIED — the only coverage was prose. The integration block now drives a governed attempt with evidencePostStatus 500 and asserts UNVERIFIED plus evidence_error, in the terminal post rather than the claim post. Controls: removing the refusal fails both new unit tests; removing the contract branch still fails the integration positive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Independent review (Opus, fresh agent): GO-WITH-FIXES. Four findings; three taken in 3a72e43, one rejected on evidence. Taken.
Rejected, with evidence. F2 said the server drops F5 (the Suites after the fixes: 48 worker unit tests, 8 worker integration files, all green. |
Found by projects-7f reviewing #53: worker-fixture.mjs filtered `evidence.refs`, a key the worker never sends to /v1/evidence — a dead branch, which made `rejectedPosts.length === 0` a check that could not fail. The fake therefore accepted ANY job_ref, including ones the real server refuses. That matters more than a dead assertion. vinci-gpu-control's POST /v1/evidence 422s a job_ref outside ("job_", "exp_", "bk_"), and WorkGraph-issued work order ids are `wo-{graph}-{node}` — so a test could file evidence under a governed work order id here and pass while production refused it. The integration test for #53 could not have caught the namespace collision now filed as vinci-gpu-control#295. The fake now applies the server's rule to `job_ref`. Control: bk_ccm7 -> 200 wo-g1-n1 -> 422 (matches the real server) All 8 worker integration files still pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🔴 Upstream finding: this PR is inert by construction, not "inert until the issuer changes". Filed as getsimpledirect/vinci-gpu-control#295. Measured, not inferred: Every WorkGraph-issued work order id is I wrote both halves of that collision — the protocol keys the economics identity on What still stands in this PR, and why I think it is worth keeping open rather than closing:
Also fixed here (thanks to projects-7f): Correction to a claim made in review, checked with a positive control: |
…e POST Precision defect in my own comments, caught by projects-7f's reviewer. I wrote that the ledger 'refuses binding:work_order_mismatch', which reads as the POST being rejected. It is not: app.py records ECONOMICS_REFUSED and still stores the evidence row, by a design rule I wrote myself — economics never blocks evidence. So a misfiled bundle would PERSIST with a refusal event beside it, which is a stronger reason to refuse at the worker than the one I gave. Also narrows the integration comment: agreement by construction keeps the pair joinable; it is not the ledger rejecting bad pairs on our behalf. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ef rule projects-7f found the residual in my own fidelity fix: the filter dropped undefined/null BEFORE testing, so a POST with job_ref absent produced an empty invalidRefs and was accepted. The real server has no such escape hatch — it requires job_ref, sha256, uri, kind and produced_at to each be a non-blank string BEFORE it checks the prefix. Absent is invalid, not nothing-to-check. Same class as the wo- hole, one level narrower. Closing the class rather than the instance: the fake now enforces both of the server's rules in the server's order. Controls: valid bk_ -> 200 wo- ref -> 422 absent job_ref -> 422 null job_ref -> 422 blank job_ref -> 422 missing sha256 -> 422 All 8 worker integration files pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Head is 16eab17. Two commits past the reviewed
All 8 worker integration files pass. Note for anyone re-issuing a sha-pinned verdict over this range: unlike the previous range, this change refuses strictly more, so it could in principle fail a test that previously passed. It did not, but that is a measurement rather than a property. Review status: GO from projects-7f, re-issued at Merge is not mine. The open question is not "is #53 correct" — it is — but "should a correct fix land on a path that cannot execute", which is vinci-gpu-control#295 and George's call. |
…hat makes it true Patch authored by projects-7f (reviewer of this PR, deliberately not pushed by them). Verified before applying rather than taken on trust — and the verification found the table had already decayed. Their measurement, taken at 2b457b8/3a72e43c, said widening BOTH gates fails the integration control. At this head it SURVIVED. Cause: my own fixture fidelity fix. Once the fake bus enforced the server's job_ref rule, a widened pair stopped producing an extra ACCEPTED post — which the count assertion caught — and started producing a REFUSED one, which no assertion looked at. The stricter fake silently removed a control while looking like an improvement. Fixed by asserting `f.rejectedPosts` is empty in the governed block: the bus refusing anything is now a failure, which also gives the previously-dead "worker must not send refs rejected by the bus" idea a live meaning. Re-measured at this head: widen RESOLVER gate only -> integration SURVIVES; unit tests FAIL widen POST gate only -> integration SURVIVES; unit tests PASS (masked) widen BOTH gates -> integration FAILS Both notes land in the repo rather than on the bus, and the unit-test note records that the third row depends on the new assertion, so removing it cannot quietly revert the table. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Use the registry-validated WorkOrder as the single governed evidence identity, carry it through the durable evidence POST and completed terminal, and keep prose refs on the legacy namespace. Add server-boundary, retry, failure, idempotency, and mutation coverage.\n\nCloses #54
Follow-up to #49, closing the governed-evidence gap it identified and folding in #54's terminal/evidence mismatch.
Contract
This implements option 1 from getsimpledirect/vinci-gpu-control#295:
wo-WorkOrder id may be used as the evidence reference./v1/evidenceremains the only evidence ingress.job_/exp_/bk_namespace and cannot useref: wo-...to bypass contract validation.What changed
The worker resolves one evidence reference immediately after a digest handoff has passed registry fetch, record validation, digest recomputation, and order/spec binding. That exact value is then used for both:
job_ref), andrefs).This fixes #54: a governed COMPLETED attempt no longer creates a durable evidence row while ending with an unreferenced status message. Failed, blocked, or unverified attempts remain status terminals and do not advertise a successful finding ref.
The bus and upload boundaries admit a bounded
wo-syntax in addition to the legacy ledger refs. The test server independently requires the WorkOrder to exist in the registry and be bound to the configured program, so the positive path cannot pass merely because both client-side checks were widened.The existing worker-unit directory remains registered in
vinci/test/run.sh, including a zero-tests guard.Controls and coverage
job_ref, economicswork_order_id, and completed terminalrefs[0]are the exact samewo-WorkOrder id.task/2attempt label.wo-text is trusted, the POST gate is narrowed back to legacy refs, the terminal loses the resolved ref, or a backlog id is substituted.Local validation:
npm run check: passed in fullThe full offline harness also ran with live model smoke disabled. Every Worker group passed. Four unrelated host-startup/UI groups timed out under the aggregate Node 26 run; the Crew timeout moved between scenarios on an isolated rerun, while all preceding assertions passed. No code from those areas is changed here; natural CI is the authoritative clean-environment result.
Natural CI on candidate
94f4015fpassed all five jobs: build/check/test, repository check, upstream suite, macOS sandbox, and offline Node 24.Scope
The branch was merged once with current
main(4c22def339c77fdd1be5843f3229cac793506d78) before its candidate push. It does not touch any path changed by draft PR #59.Closes #54.
This update is AI-generated by Codex.