fix(capture-recovery): type attempt counters as AttemptNumber, brand captureId at ingress - #36
Open
obvious-autobuild[bot] wants to merge 1 commit into
Open
obvious-autobuild[bot] wants to merge 1 commit into
obvious-autobuild[bot] wants to merge 1 commit into
Conversation
…captureId at ingress PR #15's recovery schemas referenced domain's `ExtractionAttempt` (the attempt RECORD, per contract v0.3 adaptation A5) where the numeric counter was intended — the record type that v0.3 renamed AWAY from that name. The fixtures, reducer arithmetic (attempt + 1), comparisons, and expectations all treat attempt as a number, so the wrong field type failed three gates on master: typecheck (TS2322/TS2365 across state/replay/tests), runtime test (fail-closed decodeStoredState rejected numeric attempts — "Expected object"), and build. - state.ts: CaptureRecoveryState.attempt and DiscardReceipt.attempt → AttemptNumber - events.ts: SubmitRejected.attempt and ExtractionResultArrived.attempt → AttemptNumber - reducer.ts: createCapture brands captureId at ingress via Schema.decodeSync(CaptureId) (canonical identifier space, fail-closed on empty) so the branded state field is honestly constructed - test: brand the two captureId expectations the same way No behavior change: every fixture, reducer branch, and expectation already used numeric semantics; this aligns the schemas with the code's actual contract and unblocks the shared typecheck/test/build gates for all PRs. Co-authored-by: Gilbert Polanco <gilbertpolanco42@gmail.com>
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
2 participants
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.
Why
PR #15 (
9ea7e73) merged with the recovery schemas referencing domain'sExtractionAttempt— which contract v0.3 adaptation A5 renamed to name the attempt record — where a numeric counter was intended (the canonical counter type isAttemptNumber, itself introduced by A5's rename note inpackages/domain/src/extraction.ts). The mismatch fails three gates onmasterfor every PR in the repo:pnpm turbo run typecheck(10/11),test(7/8, runtime: fail-closeddecodeStoredStaterejects numeric attempts — "Expected object" at fixture eventsteps[2].attempt), andbuild(5/6).Acceptance criteria
pnpm turbo run typecheckgreen (11/11) on the exact head.pnpm turbo run testgreen (8/8) — includes capture-recovery's 20 bun tests / 107 assertions.pnpm turbo run buildgreen (6/6).bun test ./securitygreen (29/0); month-history journeys 28/28; evaluation harness 6/6.state.attempt + 1,event.attempt < state.attempt), timeline entries, and test expectations already used numeric semantics everywhere — the schemas now match the code's actual contract.What changed
state.ts—CaptureRecoveryState.attemptandDiscardReceipt.attempttyped asAttemptNumber(wasExtractionAttempt).events.ts—SubmitRejected.attemptandExtractionResultArrived.attempttyped asAttemptNumber.reducer.ts—createCapturebrandscaptureIdat ingress viaSchema.decodeSync(CaptureId)so the branded state field is honestly constructed (fail-closed on empty; fixtures/tests unchanged at call sites).test/recovery.test.ts— the twocaptureIdlist expectations use the same brand helper.Tradeoffs
Alternatives rejected: loosening the state schema (
captureId: Schema.String) would weaken the canonical identifier alignment the package's own comment pins ("Same identifier space as the canonical extraction envelope"); leaving the repair to ride in PR #33 would put an unrelated package inside a reviewed catch-up PR's diff. Kept: schema-only alignment with the already-shipped v0.3 rename; zero runtime behavior change.Verification
All run at this branch's head
48e4b81(parent9ea7e73): turbo typecheck 11/11, test 8/8, build 6/6 (uncached); capture-recovery bun test 20 pass / 0 fail; security 29/0; journeys 28/28; harness 6/6.Human author: Gilbert Polanco (gilbertpolanco42@gmail.com)
🔗 Obvious Project · 🧵 Obvious Thread