Skip to content

fix(protocol): reject incomplete replay after ACK deletion - #43

Merged
austinchennn merged 1 commit into
EmergentExit:mainfrom
wangyeyu1129:fix/protocol-replay-after-ack
Oct 17, 2026
Merged

austinchennn merged 1 commit into
EmergentExit:mainfrom
wangyeyu1129:fix/protocol-replay-after-ack

Conversation

@wangyeyu1129

@wangyeyu1129 wangyeyu1129 commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

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 returns undefined because 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

  • One private, monotonic removed-history boundary covers both ACK deletion and capacity eviction. ACK advances it to the highest entry actually removed, never an arbitrary incoming upTo.
  • Requests behind removed history fail; requests at the boundary replay the complete ordered suffix; current cursors return []; ahead cursors remain unavailable.
  • Duplicate/stale ACKs cannot lower the boundary. Ahead ACKs clear only existing messages and grant no credit for future sends. Transients do not consume sequence/cache space or advance this boundary. Independent instances retain separate state.
  • Server regression reproduces ACK deletion through public Hub/Connection/model APIs: cursor 0 fails with nonrecoverable, bound/sequenced RESUME_FAILED and session closure; cursor 1 successfully replays seq 2. The test uses bounded observation and closes/drains its fake connections/clock.
  • Python README no longer describes the TS replay-availability discrepancy as outstanding. No Python behavior changed.

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 undefined into 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.

  • Before production changes, 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 build and npm 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 pack and npm run test:consumer: passed (13 production tarballs, 15 entry points, bundler and NodeNext type resolution).
  • Additional repository-external temporary installation of the actual protocol tarball: incomplete replay rejected, equality cursor succeeds, ahead ACK creates no future credit; temporary project removed afterwards.
  • 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

  • Review the interpretation that §5.4 completeness includes acknowledged-but-deleted history when a peer later supplies an older cursor. ACK deletion is permitted; accepting an incomplete requested suffix is not. Python's current helper already follows this distinction.
  • §4.2 prohibits a receiver ACK beyond received data while also defining sender handling for an ACK at/beyond its last sequence. This patch preserves existing sender tolerance and records only actual removed entries; no new peer-error policy is introduced.
  • §5.4 lists ready before resend/failure. The server regression therefore permits ready followed by an explicit failure and closure; it does not invent a requirement to preflight before ready. The error is seq 3 because the session has been bound (§4.1). No specific close code is mandated for this replay-loss path by the test.
  • These are deterministic unit/server adapter regressions, not a new real-WebSocket recovery conformance scenario. Full bidirectional replay/lost-result recovery and fullSuite certification remain incomplete.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants