Repository navigation
fix(protocol): reject incomplete replay after ACK deletion - #43
Merged
austinchennn merged 1 commit intoOct 17, 2026
Merged
Conversation
This was referenced Oct 17, 2026
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
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.
Description
Reject incomplete TypeScript Outbox replays after cumulative ACK deletion. Previously, sending seq 1 and 2, processing ACK 1, then requesting
resendFrom(0)returned only seq 2 as a successful replay. The helper now returnsundefinedbecause it can no longer supply the entire requested range (§5.4). A cursor equal to the removed boundary still receives the complete remaining suffix.Issue association
Refs #17. Background: #18's Python reliability foundation already treats ACK deletion as unavailable replay history. This is a production prerequisite fix for future recovery conformance work, not a new black-box profile or completion of either issue.
Based on upstream main
269194d(merged #41). #42 remains open/unmerged at delivery; this change is independent, branches from main and contains none of its heartbeat changes.Behaviours Completed
upTo.[]; ahead cursors remain unavailable.RESUME_FAILEDand session closure; cursor 1 successfully replays seq 2. The test uses bounded observation and closes/drains its fake connections/clock.Files Changed
packages/protocol/src/sequence.ts: rename the private eviction-only boundary and update it during ACK removal (three net production lines).packages/protocol/test/protocol.test.ts: correct the prior incomplete-replay expectation and add four independent trajectory tests covering ACK boundaries, future credit, eviction/transient interleaving and instance isolation.packages/server/test/server.test.ts: two parameterized public-API resume regressions (unavailable cursor and valid equality boundary).python/README.md: describe the now-shared replay-availability rule.Brief Explanation
ACK is permission to discard received messages (§4.2), whereas the later resume cursor determines the entire suffix requested (§5.4). Once either ACK deletion or bounded eviction removes part of that suffix, returning the remaining entries would hide data loss. Recording the last actual removed sequence preserves completeness without retaining acknowledged payloads, adding dependencies or changing the public API. Production client and server already translate
undefinedinto resume failure; their lifecycle code is unchanged.The earlier unit assertion accepting
[seq 2]for cursor 0 after ACK 1 contradicted the complete-replay requirement, so it now expects failure and separately checks cursor 1 succeeds. No normative file or shared vector changed. Inbox/gap semantics, sessionless errors, uint32 exhaustion, broad parameter validation and snapshot ownership are outside this diff.Testing
Local environment: macOS 26.6.2 arm64; Node 22.23.3; Vitest 3.2.4. Commands ran with Node 22 first on PATH.
vitest run packages/protocol/test/protocol.test.ts packages/server/test/server.test.ts: 6 failed / 59 passed against original main code. Five protocol failures expose incomplete replay; the server cursor-0 case fails because the old session remains open rather than rejecting missing history. The cursor-1 positive control passes. After the fix: 65 passed.npm test: 465 passed across 26 files, including all client/server tests (459 existing + 6 added test instances).npm run typecheck: passed, including conformance types.npm run check:spec: passed (23 schemas, 1 capability, 156 control vectors, 9 audio vectors).npm run buildandnpm run check:budgets: passed, including zero-dependency/import-boundary checks; protocol 2.16 KB min+gzip / 5 KB.npm run test:e2e: 7 passed.npm run test:workers: 2 passed.npm run packandnpm run test:consumer: passed (13 production tarballs, 15 entry points, bundler and NodeNext type resolution).npm run test:conformance: 49 passed, all profiles already merged on main.npm run test:conformance:consumer: passed against the installed standalone CLI outside the repository. Pending feat(conformance): verify server heartbeat and idle retention #42's heartbeat tests are deliberately not included.git diff --check: passed. No standalone lint/format script is configured. Python tests/build were not rerun because only its explanatory README changed. Publish dry-run was not run; no package was published. Hosted CI results are separate from these local results.Unsure About
Implementation Order and Acceptance — #17
Issues #17 and #18 were re-read before delivery. The contributor has read-only upstream access, so neither author's issue body was modified. Checked subitems below mean locally implemented and verified, PR pending merge, not delivery on upstream main. Maintainer-copyable progress:
#42's heartbeat substep remains pending in its own PR; its state and checklist were not changed here. No existing supervisor/contributor issue wording or checkmarks were rewritten.