feat: seq-based incremental resume + send idempotency (protocol P0, 2/2) - #103
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThis PR adds sequence-based scrollback resume and send idempotency across the protocol, daemon, and web client. Protocol contracts gain resume, clientMsgId, seq, mode, resumeKey, and maxSeq fields. The daemon tracks replay sequences and duplicate sends. The web app stores resume cursors and sends clientMsgId on prompt submits. ChangesIncremental resume & send idempotency
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant WebClient
participant SessionManager
participant Session
participant ScrollbackBuffer
WebClient->>SessionManager: session.attach(resume)
SessionManager->>Session: attach(client, resume)
Session->>ScrollbackBuffer: readChunkedSince(sinceSeq) or full snapshot
Session-->>WebClient: scrollback.replay(mode, resumeKey, maxSeq)
WebClient->>SessionManager: session.send(clientMsgId)
SessionManager->>Session: markClientMsgSeen(clientMsgId)
Session-->>WebClient: response.ok(duplicate=true) or dispatch send
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
saucam
force-pushed
the
feat/protocol-handshake-validation
branch
from
July 5, 2026 08:05
05ec861 to
f32f7f8
Compare
saucam
force-pushed
the
feat/protocol-seq-resume-idempotency
branch
from
July 5, 2026 08:05
1141bf0 to
7119962
Compare
Protocol P0 hardening, part 2 of 2 (all additive — no version bump): Incremental resume (`replay.resume`): every reconnect previously re-transferred the ENTIRE scrollback (chunked since #84, but still full) — the dominant reconnect cost on flaky/mobile links. Now: - ScrollbackBuffer owns a monotonic mutation counter; every push / update / touch advances it. Entries remember their last-mutation seq. - Outbound frames carry the cursor: messages via scrollback.push stamping, streaming deltas via an O(1) touch() in #broadcastRaw (no re-serialization on the per-token hot path). - session.attach accepts resume {key, sinceSeq}; on a resumeKey match the daemon replays only entries mutated after the cursor (mode "incremental", append/upsert client-side) — including older messages grown by deltas or tool-state transitions. Any mismatch (restart rebuilt the buffer) falls back to the authoritative snapshot; caught-up resumes get an empty ack. Replay frames now carry mode/resumeKey/maxSeq (additive). - Web client tracks per-session cursors (new state/resume.ts), resumes on re-attach, resets cursors on daemon-key change, clears on destroy. - Replaces the never-wired timestamp readSince() with seq-based readChunkedSince() (shared partition logic with readChunked). Send idempotency (`send.idempotency`): a send whose ack is lost to a socket drop is ambiguous — a retry could run (and bill) the same prompt twice. session.send now takes an optional clientMsgId; the daemon acks duplicates ({duplicate: true}) instead of dispatching a second turn (bounded FIFO window of 256 ids/session). Web mints one UUID per composer submit. Daemon advertises replay.resume + send.idempotency on auth.ok. Tests: scrollback seq/touch/readChunkedSince units; session-level resume matrix (snapshot meta, tail-only resume, wrong-key fallback, caught-up ack, live-turn cursor advance incl. delta stamping); idempotency (first/dup/FIFO eviction + manager-guard semantics: two deliveries -> one turn); web cursor unit tests (key-change reset, monotonic raise, per-session isolation). Full suite 842 pass; web 171 pass; build green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
saucam
force-pushed
the
feat/protocol-seq-resume-idempotency
branch
from
July 5, 2026 08:39
7119962 to
bc1c6cb
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #103 +/- ##
==========================================
+ Coverage 76.34% 76.38% +0.04%
==========================================
Files 70 70
Lines 11539 11597 +58
==========================================
+ Hits 8809 8858 +49
- Misses 2730 2739 +9
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/daemon/scrollback.ts`:
- Around line 133-138: The scrollback cursor update in touch() advances the
stored entry seq but leaves the buffered SessionMessage stale, so replay paths
can emit an outdated per-message seq. Update touch() in the Scrollback class to
keep the replayed message object in sync by stamping the message’s seq whenever
entry.seq is incremented, ensuring readChunked*() replays the updated sequence
value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b87d37e4-3f81-4558-b102-d44fd6432082
📒 Files selected for processing (15)
packages/protocol/src/schemas.tspackages/protocol/src/types.tssrc/daemon/scrollback.tssrc/daemon/server.tssrc/daemon/session-manager.tssrc/daemon/session.tssrc/tests/scrollback.test.tssrc/tests/session-integration.test.tsweb/src/App.tsxweb/src/components/prompt/PromptBox.tsxweb/src/protocol/types.tsweb/src/state/connection.tsweb/src/state/resume.test.tsweb/src/state/resume.tsweb/src/state/sessions.ts
Address CodeRabbit review on #103: touch() advanced the entry cursor but left the buffered SessionMessage's wire seq at its push-time value, so a later replay could emit a stale per-message seq. Clients that follow the documented contract (cursor = max of frame maxSeq + live seqs) were safe — stale-low can never over-advance — but self-consistency (entry.seq === msg.seq) removes the footgun for clients deriving cursors from per-message seqs alone. Same stamping added to updateMessage(), where it is fully byte-accounted; in touch() the ~15-byte one-time drift is deliberate to keep re-serialization off the per-token hot path (documented inline). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 5, 2026
Merged
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
1 participant
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.
Incremental resume (
replay.resume) — cheap reconnectsEvery reconnect previously re-transferred the entire scrollback (chunked since #84, but still full) — the dominant reconnect cost on flaky/mobile links, and the top finding of the audit.
push, streaming deltas via an O(1)touch()in the broadcast path (no re-serialization per token).session.attachacceptsresume {key, sinceSeq}: on aresumeKeymatch the daemon replays only entries mutated after the cursor (mode: "incremental"— client appends/upserts, never resets), including older messages grown by deltas or tool-state transitions since the cursor. Key mismatch (daemon restart rebuilt the buffer) → authoritative snapshot. Caught-up → empty ack frame. Replay frames now carrymode/resumeKey/maxSeq.state/resume.ts), resumes on re-attach, resets the cursor when the daemon's key changes, clears on session destroy. Safety property documented + tested: the cursor may lag reality (safe — resends get deduped by upsert) but can never lead it.readSince()with seq-basedreadChunkedSince()(shared partition logic withreadChunked).Send idempotency (
send.idempotency) — the duplicate-turn guardA send whose ack is lost to a socket drop is ambiguous; a retry could run and bill the same prompt twice.
session.sendnow takes an optionalclientMsgId(minted once per user action): the daemon acks duplicates ({duplicate: true}) instead of dispatching a second turn, over a bounded FIFO window (256 ids/session). The web composer mints one UUID per submit.Daemon now advertises
replay.chunked+replay.resume+send.idempotencyonauth.ok.Tests (new)
touchsemantics, upsert/update advancing entries past a cursor,readChunkedSinceordering + byte-budget partition.Verification
Full daemon suite 842 pass · web 171 pass · typecheck (root + package) clean · biome clean · build green.
Note for review
ScrollbackReplayMsg.seq(chunk index, from #84) andSessionMessage.seq(session cursor) are different domains — called out in the type docs on both fields.🤖 Generated with Claude Code
Summary by CodeRabbit