Repository navigation
feat(app): add atomic learning promotion verify hook - #96
Conversation
Work unit 3 child 7 (learning-promotion): ONE transaction owns the completed-to-verified CAS win and the append-only verification proof (distinct verifier, fresh active delegation, policy reference); every failure rolls both writes back - no half-verified work, no orphan proof.
![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: abf60f1d1f
ℹ️ 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".
| const now = deps.now?.() ?? Date.now(); | ||
| if (freshDelegation.validFrom > now || freshDelegation.validUntil <= now) | ||
| throw new VerifyWorkAbortError('delegation-unavailable'); | ||
| const cas = await work.updateIfVersion({ ...current, state: 'verified' }, current.version); |
There was a problem hiding this comment.
When the delegation is active and in-window but its authorityScope.actions omits learning.promote or its scope belongs to another subject, this CAS still permanently transitions the Work to verified and appends a proof. InMemoryPromotionAuthorityRepository.resolve subsequently rejects that proof on the action/scope checks, leaving terminal Work with unusable promotion authority and no legal verification retry. Validate the delegation action and canonical promotion scope before performing the CAS.
Useful? React with 👍 / 👎.
| if (deps.verifier === current.proposer) | ||
| throw new VerifyWorkAbortError('verifier-not-distinct', current); |
There was a problem hiding this comment.
When the principal that executed and completed the Work invokes this hook as verifier but differs from the proposer, this guard passes and permits self-verification. The existing worker SoD contract makes executor/verifier an absolute distinct pair; checking only current.proposer does not enforce that invariant. Carry the completed Work's executor identity, such as the receipt actor or trusted execution context, into this transaction and reject an equal verifier before the CAS.
Useful? React with 👍 / 👎.
| scope: promotionScopeFor(input.companyId, input.subject), | ||
| policyRef: input.policyRef, |
There was a problem hiding this comment.
A structurally typed but semantically invalid subject can make this function reject instead of returning its advertised VerifyWorkResult; for example, an unpaired surrogate in skillId makes promotionScopeFor throw here, while a non-positive skillVersion reaches appendProof and throws there. These failures occur after the Work CAS, relying on rollback while escaping the typed failure contract entirely. Validate the subject and policy reference before any read/write and return an invalid-input result.
Useful? React with 👍 / 👎.
Linked issue
Closes #89
PR type
type:bug)type:feature)type:docs)type:refactor)type:chore)type:breaking-change)Summary
verifyWorkWithProofAtomicallyinpackages/app/src/worker/verify.ts: ONE transaction owns BOTH writes — thecompleted → verifiedCAS win AND the append-only verification proof (verifier actor, delegation principal, fresh active Delegation, policy reference, canonical scope). Failures abort + roll BOTH back and return as typed VALUES (work-not-found,invalid-work-state,verifier-not-distinct,version-conflict,delegation-unavailable,proof-conflict) — finalize twin pattern, zero partial writes.verifier-not-distinct, rejected before any state transition or proof append).completed, no proof row after conflict abort). Full gatepnpm checkexit 0 — 1580 passed | 6 skipped, 0 new warnings.Changes
packages/app/src/worker/verify.tspackages/app/test/worker-verify-work.test.tsVerification
pnpm exec vitest run packages/app/test/worker-verify-work.test.ts→ 4/4pnpm checkGREEN — 1580 passed | 6 skipped, 0 new warningsreview-58d7deee4fb6f229(medium, reliability lens) — one bounded correction transaction for CRITICAL R3-001 (23 ≤ 197 budget), targeted validator + correction evidence captured, final stateapproved; pre-pr gateallowContributor checklist
status:approved.type:*label is applied.type/lowercase-description.