Repository navigation
test(app): add verify hook revocation and race coverage - #97
Conversation
Work unit 3 child 8 (learning-promotion): revocation supersedes the appended proof; missing delegation and CAS version-conflict abort with zero writes. Completes task 2.5 with the full work-unit-3 gate.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a5be4137b7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - [x] 2.4 RED fake tests: candidate replay/collision/stale/concurrency; authority current/superseded/revoked. (W3A 11/11 candidate; W3B1 8/8 core + W3B2 10/10 coverage.) | ||
| - [ ] 2.6 RED+GREEN atomic verifyWork+proof owner (`app/src/worker/verify.ts`): one transaction = `completed→verified` win + proof append (verifier, fresh Delegation, policy); revocation supersede; no partial write. | ||
| - [ ] 2.5 `pnpm check` green. | ||
| - [x] 2.5 `pnpm check` green. (1582 passed / 6 skipped on the full work-unit-3 stack; 9 pre-existing warnings.) |
There was a problem hiding this comment.
When this slice lands, the new coverage and apply-progress section declare W3D2 and task 2.6 complete, but the canonical task immediately above remains unchecked. Any tooling or maintainer using tasks.md will therefore continue to treat work unit 3 as incomplete; check off task 2.6 together with this completed full-suite gate.
Useful? React with 👍 / 👎.
| updateIfVersion(_work: Work, _expectedVersion: number, _fencing?: FencingDirective) { | ||
| return Promise.resolve({ | ||
| ok: false as const, | ||
| reason: 'version-conflict' as const, | ||
| current: completedWork(), |
There was a problem hiding this comment.
In the version-conflict scenario, the fake returns completedWork(), which is indistinguishable from the snapshot read before the CAS, and it never installs a competing winner in its store. The test therefore still passes if the hook incorrectly returns the stale pre-race work instead of cas.current, and its final state assertion cannot verify that the losing attempt preserves the winner's mutation. Return and store an observably different competing version/state so this race coverage exercises both behaviors.
Useful? React with 👍 / 👎.
| Gate (task 2.5): `pnpm check` exit 0 — format ✓ typecheck ✓ build ✓ lint ✓ (9 pre-existing warnings) — test **1582 passed / 6 skipped**. | ||
| Deviations (Engram `sdd/learning-promotion/app-seam` + `sdd/learning-promotion/verify-hook`): seam deps = {events, skills, authority, trusted{principalId,actorId}} (design listed only {events,skills}; repository authority resolution requires the extra ports); verify-hook transition identity = (workId, 1) canonical (design left identity free); work-unit 3 split into EIGHT children (W3A/W3B1/W3B2/W3C1/W3C2/W3C3/W3D1/W3D2) because the named sub-boundaries "fakes" and "learning/" each also crossed 400 at apply — no size:exception. | ||
| Runtime harness: N/A — app-layer seam + hook over pure in-memory fakes; no PG/daemon/runtime boundary in this work unit. | ||
| Rollback: per child — drop the child's files + revert its checkbox/section; all files are new modules with no prior consumers. |
There was a problem hiding this comment.
The W3D1 rollback cannot safely “drop the child's files”: packages/app/src/worker/verify.ts already contained verifyEffect before W3D1 and is imported by worker.ts, so deleting that file would remove established worker verification behavior and break its consumers. Document a rollback that reverts only the learning-promotion hook additions and its new tests rather than claiming every affected file is a new, unused module.
Useful? React with 👍 / 👎.
Linked issue
Closes #89
PR type
type:bug)type:feature)type:docs)type:refactor)type:chore)type:breaking-change)Summary
worker-verify-work-coverage.test.ts: revocation-supersede (a revoked superseding proof makes the appended proof resolve revoked), delegation-missing abort with zero writes, and CAS version-conflict race (a losing CAS never appends a proof and never mutates the work).pnpm checkexit 0 — 1583 passed | 6 skipped, 0 new warnings.Changes
packages/app/test/worker-verify-work-coverage.test.tsopenspec/changes/learning-promotion/tasks.mdopenspec/changes/learning-promotion/apply-progress.mdVerification
pnpm exec vitest run packages/app/test/worker-verify-work-coverage.test.ts→ 3/3pnpm checkGREEN — 1583 passed | 6 skipped, 0 new warningsreview-8fbe320a9a60702c(medium, reliability lens) approved with focused+full-gate evidence, no findings; pre-pr gateallowContributor checklist
status:approved.type:*label is applied.type/lowercase-description.