Repository navigation
feat(host-service,mobile): host sessions sync v1 — canonical sessions over tRPC snapshots + /sessions/sync, mobile rewired onto the sync client - #5630
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughChangesThe PR adds a canonical host-session protocol, ACP-backed runtime, projection reducer, WebSocket synchronization hub, client-side Zustand replication, tRPC endpoints, host wiring, extensive unit/integration/stress coverage, and updated rollout documentation. Canonical host sessions
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Ready to review this PR? Stage has broken it down into 21 individual chapters for you: 
Chapters generated by Stage for commit 357175a on Jul 13, 2026 4:10pm UTC. |
Greptile SummaryThis PR adds the first canonical host sessions sync surface. The main changes are:
Confidence Score: 4/5The live sync subscribe path can miss events while reporting that clients are caught up.
packages/host-service/src/runtime/sessions/sync-hub.ts and packages/host-service/src/runtime/sessions/canonical-sessions.ts
|
| Filename | Overview |
|---|---|
| packages/host-service/src/runtime/sessions/sync-hub.ts | Adds the sessions WebSocket hub, but live subscriptions are registered after replay/snapshot delivery, leaving event-loss windows. |
| packages/host-service/src/runtime/sessions/canonical-sessions.ts | Adds the canonical ACP-backed sessions runtime, with an event-window edge case for anchors before the first event. |
| packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts | Adds the framework-free sync client and local store orchestration for session retention, history seeding, reconnects, and tool resolution. |
| packages/host-service/src/runtime/sessions/sync-route.ts | Adds the /sessions/sync route adapter, but it does not enforce the expected WebSocket subprotocol. |
| packages/host-service/src/trpc/router/sessions/sessions.ts | Adds the schema-bound sessions.* tRPC router over the canonical sessions runtime. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant Client
participant Hub as SessionsSyncHub
participant Runtime as CanonicalSessionsRuntime
Client->>Hub: hello
Hub-->>Client: hello_ack
Client->>Hub: subscribe host/session
Hub->>Runtime: snapshot or replay from cursor
Runtime-->>Hub: events or snapshot + head
Hub-->>Client: subscribed
Hub-->>Client: snapshot or replay
Hub-->>Client: caught_up
Hub->>Hub: register live subscription
Runtime-->>Hub: later live events
Hub-->>Client: event frames
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant Client
participant Hub as SessionsSyncHub
participant Runtime as CanonicalSessionsRuntime
Client->>Hub: hello
Hub-->>Client: hello_ack
Client->>Hub: subscribe host/session
Hub->>Runtime: snapshot or replay from cursor
Runtime-->>Hub: events or snapshot + head
Hub-->>Client: subscribed
Hub-->>Client: snapshot or replay
Hub-->>Client: caught_up
Hub->>Hub: register live subscription
Runtime-->>Hub: later live events
Hub-->>Client: event frames
Prompt To Fix All With AI
Fix the following 4 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 4
packages/host-service/src/runtime/sessions/sync-hub.ts:525-530
**Session Subscribe Drops Events**
When a session event is emitted after `sessionReplay()` captures `replay.head` but before this subscription is added, `fanOutSessionEvent()` has no subscription to deliver to. The client still receives `caught_up` through the old cursor, so that event is missed until a full resync.
### Issue 2 of 4
packages/host-service/src/runtime/sessions/sync-hub.ts:596-601
**Host Subscribe Drops Changes**
When a host change happens after `head` is captured but before this subscription is registered, the change is not in the replay or snapshot and cannot be fanned out. Clients can be told they are caught up while missing a session upsert, removal, or permission transition.
### Issue 3 of 4
packages/host-service/src/runtime/sessions/canonical-sessions.ts:488-493
**Early Time Anchor Skips Events**
When `getEvents` is anchored at a time before the first event, `anchorIndex` stays `-1`, so this range calculation returns an empty page with `hasMoreAfter` instead of the first available events. Clients asking for context around an early timestamp cannot page into the existing log from that response.
### Issue 4 of 4
packages/host-service/src/runtime/sessions/sync-route.ts:36-38
**Subprotocol Gate Is Missing**
The route admits the socket without checking that the negotiated protocol is `superset.sessions.sync`. A client or proxy using the wrong subprotocol can still open the sessions endpoint and only fail later at the JSON handshake, which makes protocol misconfiguration look like a live sessions connection.
Reviews (1): Last reviewed commit: "fix(host-service): harden sync-hub again..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| connection.subscriptions.set(packet.subscriptionId, { | ||
| subscriptionId: packet.subscriptionId, | ||
| stream: "session", | ||
| sessionId, | ||
| lastCursor: replay.head, | ||
| }); |
There was a problem hiding this comment.
When a session event is emitted after sessionReplay() captures replay.head but before this subscription is added, fanOutSessionEvent() has no subscription to deliver to. The client still receives caught_up through the old cursor, so that event is missed until a full resync.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/runtime/sessions/sync-hub.ts
Line: 525-530
Comment:
**Session Subscribe Drops Events**
When a session event is emitted after `sessionReplay()` captures `replay.head` but before this subscription is added, `fanOutSessionEvent()` has no subscription to deliver to. The client still receives `caught_up` through the old cursor, so that event is missed until a full resync.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Not reproducible: the window you describe doesn't exist. subscribeSession runs synchronously from the sessionReplay() head capture through subscribed → snapshot/replay → caught_up → subscriptions.set(...) — there is no await in that block (the only async gap, warmSession, happens before the head is captured, so anything folded during warm-up is included in the replay). A runtime fold can only run on a later event-loop turn, at which point the subscription is registered and fanOutSessionEvent delivers it; the padded-cursor <= lastCursor check dedupes the overlap. This is pinned by the randomized churn stress suite (gapless-suffix oracle per subscription, sync-hub.stress.test.ts) and was independently re-verified this week by a cross-provider review that specifically targeted subscribe atomicity (verdict: sound).
| connection.subscriptions.set(packet.subscriptionId, { | ||
| subscriptionId: packet.subscriptionId, | ||
| stream: "host", | ||
| sessionId: null, | ||
| lastCursor: head, | ||
| }); |
There was a problem hiding this comment.
When a host change happens after head is captured but before this subscription is registered, the change is not in the replay or snapshot and cannot be fanned out. Clients can be told they are caught up while missing a session upsert, removal, or permission transition.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/runtime/sessions/sync-hub.ts
Line: 596-601
Comment:
**Host Subscribe Drops Changes**
When a host change happens after `head` is captured but before this subscription is registered, the change is not in the replay or snapshot and cannot be fanned out. Clients can be told they are caught up while missing a session upsert, removal, or permission transition.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Same answer as the session-stream twin of this comment: subscribeHost is synchronous from the head capture through registration — no await anywhere in the function — so a fanOutHostChange cannot interleave; it either lands before the capture (included in ring replay/snapshot) or after registration (delivered live, deduped by the padded-cursor check). Covered by the same stress oracle and the independent atomicity review.
| const start = Math.max(0, anchorIndex - input.before); | ||
| const endExclusive = Math.max( | ||
| start, | ||
| Math.min(filtered.length, anchorIndex + input.after + 1), | ||
| ); | ||
| const items = filtered.slice(start, endExclusive); |
There was a problem hiding this comment.
When getEvents is anchored at a time before the first event, anchorIndex stays -1, so this range calculation returns an empty page with hasMoreAfter instead of the first available events. Clients asking for context around an early timestamp cannot page into the existing log from that response.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/runtime/sessions/canonical-sessions.ts
Line: 488-493
Comment:
**Early Time Anchor Skips Events**
When `getEvents` is anchored at a time before the first event, `anchorIndex` stays `-1`, so this range calculation returns an empty page with `hasMoreAfter` instead of the first available events. Clients asking for context around an early timestamp cannot page into the existing log from that response.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Deliberate semantics, documented at the -1 assignment: a time anchor earlier than every event means "anchored just before the first event". before-context is then correctly empty, and with any after > 0 the window returns the earliest events — so callers asking for context around an early timestamp do page into the log from that very response (start=0, endExclusive=after). The empty page only occurs for after: 0, where "no events at or before T, hasMoreAfter: true" is the accurate answer to the question asked, and the recovery is the same query with after > 0. No current client issues time anchors with after: 0.
There was a problem hiding this comment.
You're right — I traced through the math incorrectly. With anchorIndex = -1:
start = Math.max(0, -1 - before)→0endExclusive = Math.max(0, Math.min(length, -1 + after + 1))→Math.min(length, after)
So after > 0 correctly yields the earliest events from filtered[0..after), and the after: 0 empty result is semantically accurate. The comment at the -1 assignment makes the intent clear. My original concern was wrong — withdrawing it.
| onOpen: (_event, ws) => { | ||
| connection = hub.connect(ws as SyncSocket); | ||
| }, |
There was a problem hiding this comment.
The route admits the socket without checking that the negotiated protocol is superset.sessions.sync. A client or proxy using the wrong subprotocol can still open the sessions endpoint and only fail later at the JSON handshake, which makes protocol misconfiguration look like a live sessions connection.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/runtime/sessions/sync-route.ts
Line: 36-38
Comment:
**Subprotocol Gate Is Missing**
The route admits the socket without checking that the negotiated protocol is `superset.sessions.sync`. A client or proxy using the wrong subprotocol can still open the sessions endpoint and only fail later at the JSON handshake, which makes protocol misconfiguration look like a live sessions connection.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Partly stale premise: the client offers superset.sessions.sync and the ws server (via @hono/node-ws, no handleProtocols configured) echoes the first offered subprotocol, so RFC-compliant clients (browser/React Native) negotiate successfully today — the route is not silently protocol-less. What's true is that a wrong subprotocol is also echoed and only dies at the hello gate (UNSUPPORTED_PROTOCOL_VERSION / INVALID_PACKET → close 1008), which is the designed authoritative gate. A strict upgrade-time check needs handleProtocols on the shared WebSocketServer that @hono/node-ws constructs internally and doesn't expose per-route; not worth forking the adapter for a fail-slightly-faster on a misconfigured proxy. Deferring — tracked with the transport hardening items in plans/host-sessions-rollout.md.
There was a problem hiding this comment.
Fair correction — the original comment overstated the failure mode. Since @hono/node-ws echoes the first offered subprotocol, RFC-compliant clients do negotiate superset.sessions.sync successfully today, so "silently protocol-less" was inaccurate. And the hello gate (close 1008 on UNSUPPORTED_PROTOCOL_VERSION/INVALID_PACKET) is a well-defined authoritative failure path, not a silent one.
The architectural constraint — handleProtocols being on the shared internal WebSocketServer that @hono/node-ws doesn't expose per-route — makes a strict upgrade-time gate genuinely impractical without forking the adapter, which isn't a reasonable ask for fail-slightly-faster behavior on a misconfigured proxy. Tracking it in the rollout doc is the right call.
![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
🧹 Nitpick comments (4)
packages/host-service-sync/src/package-boundaries.test.ts (1)
22-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winBoundary regexes miss dynamic and side-effect imports.
All patterns anchor on
\bfrom, soimport("node:crypto"),require("react"), and side-effect imports (import "expo") would slip past this guard while still pulling in a prohibited dependency. Consider matching the module specifier independently of thefromkeyword.♻️ Sketch: match specifiers regardless of import form
const prohibited = [ - /\bfrom\s+["']react(?:\/|["'])/, - /\bfrom\s+["']react-native(?:\/|["'])/, - /\bfrom\s+["']expo(?:\/|["'])/, - /\bfrom\s+["']node:/, - /\bfrom\s+["']`@superset`\/host-service(?:\/|["'])/, - /\bfrom\s+["']`@agentclientprotocol`\//, + /["']react(?:\/|["'])/, + /["']react-native(?:\/|["'])/, + /["']expo(?:\/|["'])/, + /["']node:/, + /["']`@superset`\/host-service(?:\/|["'])/, + /["']`@agentclientprotocol`\//, ];Note: broadening to bare specifiers can match strings/comments; if that risk matters, prefer parsing
import/require/import(...)forms explicitly.🤖 Prompt for 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. In `@packages/host-service-sync/src/package-boundaries.test.ts` around lines 22 - 38, Update the prohibited patterns used by the package-boundary scan in the test so they detect prohibited module specifiers across static imports, side-effect imports, require calls, and dynamic import() expressions, not only `from` imports. Preserve the existing package targets and violation reporting while matching these forms explicitly enough to avoid unrelated strings or comments where practical.packages/host-service/src/runtime/sessions/translate-acp.ts (1)
704-715: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTurn-tracking caps reuse unrelated constants.
mintedTurnIdsis bounded byMAX_TRACKED_TOOL_CALLSandturnIdsByRequestbyMAX_TRACKED_SUBAGENT_THREADS. Functionally these bound memory fine, but the semantic mismatch is confusing. Consider a dedicatedMAX_TRACKED_TURNSfor clarity.🤖 Prompt for 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. In `@packages/host-service/src/runtime/sessions/translate-acp.ts` around lines 704 - 715, Introduce a dedicated MAX_TRACKED_TURNS constant and use it for both mintedTurnIds and turnIdsByRequest size limits in the turn-tracking logic. Replace the unrelated MAX_TRACKED_TOOL_CALLS and MAX_TRACKED_SUBAGENT_THREADS references while preserving the existing eviction behavior.packages/host-service-sync/src/protocol/primitives.ts (1)
11-51: 🚀 Performance & Scalability | 🔵 TrivialVerify upstream size caps exist for
jsonValueSchema.Per-level bounds are generous (16MB strings, 10k array items, 10k object keys, 64 levels deep), and this schema validates
toolCall.input/outputand settingsconfigurationmaps flowing through the WS sync hub and tRPC mutations. Cost of validating a maximal conforming payload could be significant if there's no hard byte-size cap enforced before Zod parsing at the transport layer (not visible in this file set). Worth confirmingsync-hub.ts/routes.tsenforce a message-size ceiling ahead of this schema.🤖 Prompt for 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. In `@packages/host-service-sync/src/protocol/primitives.ts` around lines 11 - 51, Verify the transport entry points that validate jsonValueSchema, especially sync-hub.ts and routes.ts, enforce a hard message or request byte-size ceiling before Zod parsing. If no upstream cap exists, add one at the earliest WS and tRPC boundaries while preserving the existing jsonValueSchema validation for payloads within that limit.packages/host-service-sync/src/protocol/sync.ts (1)
26-26: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the deprecated, redundant
.safe()refinement.In Zod 4,
z.number().int()already restricts to safe integers, and.safe()is deprecated (it now just aliases.int()and is slated for removal in the next major). Chaining.int().safe()is redundant today and a future-break risk.♻️ Suggested cleanup
- version: z.number().int().safe().positive(), + version: z.number().int().positive(),- maxSubscriptions: z.number().int().safe().positive(), - maxFrameBytes: z.number().int().safe().positive(), + maxSubscriptions: z.number().int().positive(), + maxFrameBytes: z.number().int().positive(),Also applies to: 87-88
🤖 Prompt for 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. In `@packages/host-service-sync/src/protocol/sync.ts` at line 26, Remove the redundant deprecated .safe() refinement from the visible version schema and the additionally affected schema occurrences, leaving z.number().int().positive() and all other validation behavior unchanged.
🤖 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 `@packages/host-service-sync/src/protocol/api.ts`:
- Around line 353-423: Update the eventsWindowSchema superRefine validation to
reject any window whose range.truncatedBefore is true while range.hasMoreBefore
is also true. Add a cross-field issue on the range metadata, preserving all
existing event identity, boundary, and empty-window checks.
---
Nitpick comments:
In `@packages/host-service-sync/src/package-boundaries.test.ts`:
- Around line 22-38: Update the prohibited patterns used by the package-boundary
scan in the test so they detect prohibited module specifiers across static
imports, side-effect imports, require calls, and dynamic import() expressions,
not only `from` imports. Preserve the existing package targets and violation
reporting while matching these forms explicitly enough to avoid unrelated
strings or comments where practical.
In `@packages/host-service-sync/src/protocol/primitives.ts`:
- Around line 11-51: Verify the transport entry points that validate
jsonValueSchema, especially sync-hub.ts and routes.ts, enforce a hard message or
request byte-size ceiling before Zod parsing. If no upstream cap exists, add one
at the earliest WS and tRPC boundaries while preserving the existing
jsonValueSchema validation for payloads within that limit.
In `@packages/host-service-sync/src/protocol/sync.ts`:
- Line 26: Remove the redundant deprecated .safe() refinement from the visible
version schema and the additionally affected schema occurrences, leaving
z.number().int().positive() and all other validation behavior unchanged.
In `@packages/host-service/src/runtime/sessions/translate-acp.ts`:
- Around line 704-715: Introduce a dedicated MAX_TRACKED_TURNS constant and use
it for both mintedTurnIds and turnIdsByRequest size limits in the turn-tracking
logic. Replace the unrelated MAX_TRACKED_TOOL_CALLS and
MAX_TRACKED_SUBAGENT_THREADS references while preserving the existing eviction
behavior.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: b5d3ad9a-dc57-4523-89e9-6b0eeacc1dc2
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (57)
apps/mobile/plans/20260709-1438-unified-home-workspaces-sessions.mdapps/mobile/plans/mobile-chat-runtime.mdpackages/host-service-sync/package.jsonpackages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.test.tspackages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.tspackages/host-service-sync/src/client/createSessionsSyncClient/index.tspackages/host-service-sync/src/client/createSessionsSyncClient/state.tspackages/host-service-sync/src/client/index.tspackages/host-service-sync/src/client/types.tspackages/host-service-sync/src/index.tspackages/host-service-sync/src/package-boundaries.test.tspackages/host-service-sync/src/projection/fixtures/attention-and-settings.jsonpackages/host-service-sync/src/projection/fixtures/turn-lifecycle.jsonpackages/host-service-sync/src/projection/index.tspackages/host-service-sync/src/projection/projection.tspackages/host-service-sync/src/projection/reduceProjection.test.tspackages/host-service-sync/src/projection/reduceProjection.tspackages/host-service-sync/src/protocol/api.tspackages/host-service-sync/src/protocol/entities.tspackages/host-service-sync/src/protocol/events.tspackages/host-service-sync/src/protocol/index.tspackages/host-service-sync/src/protocol/primitives.tspackages/host-service-sync/src/protocol/protocol.test.tspackages/host-service-sync/src/protocol/routes.tspackages/host-service-sync/src/protocol/sync.tspackages/host-service-sync/tsconfig.jsonpackages/host-service/docs/acp-sessions.mdpackages/host-service/package.jsonpackages/host-service/src/app.tspackages/host-service/src/runtime/sessions/canonical-sessions.test.tspackages/host-service/src/runtime/sessions/canonical-sessions.tspackages/host-service/src/runtime/sessions/index.tspackages/host-service/src/runtime/sessions/sync-hub.stress.test.tspackages/host-service/src/runtime/sessions/sync-hub.test.tspackages/host-service/src/runtime/sessions/sync-hub.tspackages/host-service/src/runtime/sessions/sync-route.tspackages/host-service/src/runtime/sessions/testing/fake-acp-port.tspackages/host-service/src/runtime/sessions/testing/sync-harness.tspackages/host-service/src/runtime/sessions/translate-acp.test.tspackages/host-service/src/runtime/sessions/translate-acp.tspackages/host-service/src/trpc/router/router.tspackages/host-service/src/trpc/router/sessions/index.tspackages/host-service/src/trpc/router/sessions/sessions.tspackages/host-service/src/types.tspackages/host-service/test/helpers/createTestHost.tspackages/host-service/test/helpers/gym.tspackages/host-service/test/integration/acp-user-story.integration.test.tspackages/host-service/test/integration/sessions-sync-client.integration.test.tsplans/acp-session-follow-ups.mdplans/cross-client-session-tab-sync.mdplans/done/20260710-session-harness-acp.mdplans/host-integration-test.mdplans/host-service-chat-architecture.mdplans/host-sessions-protocol.mdplans/host-sessions-rollout.mdplans/session-harness-claude-agent-sdk.mdplans/v2-chat-greenfield-architecture.md
💤 Files with no reviewable changes (4)
- plans/cross-client-session-tab-sync.md
- plans/session-harness-claude-agent-sdk.md
- plans/acp-session-follow-ups.md
- plans/host-integration-test.md
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
8 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/runtime/sessions/sync-hub.ts">
<violation number="1" location="packages/host-service/src/runtime/sessions/sync-hub.ts:764">
P1: Large valid host snapshots or event payloads make the bundled client close the socket as `frame_size_limit`, so hosts with enough sessions cannot complete initial sync. Bound/chunk outbound snapshots and events to the advertised `maxFrameBytes` before sending.</violation>
</file>
<file name="packages/host-service/src/runtime/sessions/canonical-sessions.ts">
<violation number="1" location="packages/host-service/src/runtime/sessions/canonical-sessions.ts:1081">
P2: Malformed or unsupported ACP frames disappear from the canonical stream with no observable failure because `foldError` is never consumed. Log or surface the fold failure while preserving journal fan-out isolation.
(Based on your team's feedback about handling async failures observably.) [FEEDBACK_USED]</violation>
<violation number="2" location="packages/host-service/src/runtime/sessions/canonical-sessions.ts:1094">
P2: When `reduceProjection` throws for one event in the fold loop, the error is recorded but processing continues for subsequent events in the same batch — all operating against the now-stale projection that never incorporated the failed event. This means events accumulate in the log (`tracked.events`) while the projection silently drifts from the ground truth. The next batch of events then folds against the stale projection, compounding the error. Callers reading the composed session state (e.g., `getSession`, `hostSnapshotData`) see a projection that diverges from what replaying the event log from scratch would produce.
Consider exiting the batch on projection failure so the drift is bounded to one event, or skipping just the failing event instead of continuing with the full remainder.</violation>
<violation number="3" location="packages/host-service/src/runtime/sessions/canonical-sessions.ts:1292">
P2: A batch with a later unsupported option can apply earlier settings before rejecting, despite the no-partial-application contract. Validate all requested option IDs/capabilities before issuing any mutation, or make the batch atomic.</violation>
</file>
<file name="packages/host-service-sync/src/protocol/api.ts">
<violation number="1" location="packages/host-service-sync/src/protocol/api.ts:114">
P3: The cross-field validation logic for session snapshots is duplicated across three `superRefine` blocks: `sessionSnapshotSchema` in api.ts, `hostSnapshotPacketSchema` in sync.ts, and `sessionSnapshotPacketSchema` in sync.ts. Each independently validates thread-parent consistency, permission/tool-call session-ID matching, claim-to-tool-call references, state invariants on claimed tools, duplicate detection, and orphan-claim detection — roughly 100 lines of structurally identical logic each.
If a new client tool call state is introduced or an invariant changes, all three blocks need coordinated updates. Consider extracting the shared validation into a helper function (e.g., `validateSnapshotIntegrity(sessionId, threads, pendingPermissions, clientToolCalls, toolCallClaims, context)`). The per-schema `superRefine` would then call the helper with context, keeping the integrity rules defined once.</violation>
</file>
<file name="packages/host-service-sync/src/protocol/primitives.ts">
<violation number="1" location="packages/host-service-sync/src/protocol/primitives.ts:46">
P1: A valid extension payload can consume unbounded practical memory/CPU despite these per-item limits: one array may hold 10,000 16-MiB strings, with the same fan-out recursively. Add a cumulative payload-size/node budget so hostile sync or tRPC inputs are rejected before validation allocates or traverses enormous structures.</violation>
</file>
<file name="packages/host-service/src/runtime/sessions/sync-route.ts">
<violation number="1" location="packages/host-service/src/runtime/sessions/sync-route.ts:43">
P3: Non-ASCII frames can bypass the advertised byte cap because `String()` produces UTF-16 text and the hub compares `.length`. Measure encoded UTF-8 bytes (or preserve the Buffer through intake) before parsing so `maxFrameBytes` limits actual WebSocket payload size.</violation>
</file>
<file name="packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts">
<violation number="1" location="packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts:764">
P2: `seedPromises` not cleared when the socket disconnects or the client disconnects, leaving stale in-flight seed-history HTTP promises that can apply events to the wrong session state after reconnection.
When a session is retained and has no cursor (`latestCursor === null`), `seedAndSubscribe` kicks off a `getEvents` HTTP call and stores the promise in the `seedPromises` map. If the socket drops while that promise is in flight, a new socket connects, and `hello_ack` triggers `seedAndSubscribe` for the same session, the `seedPromises.has(sessionId)` guard causes an early return — the new connection never sends a subscribe for that session. When the **old** seed promise eventually resolves, its `applyWindow` call writes events fetched against the old connection into the current store, which may already contain newer events from the new connection.
Clear `seedPromises` in both the `onclose` handler and the `disconnect()` method (similar to how `sentSessionSubscriptions` is already cleared there).</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| const childSchema = boundedJsonValueSchema; | ||
| boundedJsonValueSchema = z.union([ | ||
| jsonPrimitiveSchema, | ||
| z.array(childSchema).max(MAX_JSON_COLLECTION_ITEMS), |
There was a problem hiding this comment.
P1: A valid extension payload can consume unbounded practical memory/CPU despite these per-item limits: one array may hold 10,000 16-MiB strings, with the same fan-out recursively. Add a cumulative payload-size/node budget so hostile sync or tRPC inputs are rejected before validation allocates or traverses enormous structures.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service-sync/src/protocol/primitives.ts, line 46:
<comment>A valid extension payload can consume unbounded practical memory/CPU despite these per-item limits: one array may hold 10,000 16-MiB strings, with the same fan-out recursively. Add a cumulative payload-size/node budget so hostile sync or tRPC inputs are rejected before validation allocates or traverses enormous structures.</comment>
<file context>
@@ -0,0 +1,133 @@
+ const childSchema = boundedJsonValueSchema;
+ boundedJsonValueSchema = z.union([
+ jsonPrimitiveSchema,
+ z.array(childSchema).max(MAX_JSON_COLLECTION_ITEMS),
+ jsonObjectSchema(childSchema),
+ ]);
</file context>
There was a problem hiding this comment.
Partially mitigated, remainder deferred deliberately. The sync path is already bounded by maxFrameBytes (1MiB) before any Zod traversal, and e174f02 caps adapter-derived rawInput/rawOutput at 256KB at the normalization boundary — the two ingress points where hostile/degenerate payloads actually enter this surface. A cumulative node budget inside jsonValueSchema itself is the wrong layer for the remaining tRPC-body case: that wants a transport-level body limit host-wide (all routers share the risk), tracked with the strict-runtime-contracts backlog in plans/host-sessions-rollout.md.
| return false; | ||
| } | ||
| try { | ||
| socket.send(JSON.stringify(packet)); |
There was a problem hiding this comment.
P1: Large valid host snapshots or event payloads make the bundled client close the socket as frame_size_limit, so hosts with enough sessions cannot complete initial sync. Bound/chunk outbound snapshots and events to the advertised maxFrameBytes before sending.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/runtime/sessions/sync-hub.ts, line 764:
<comment>Large valid host snapshots or event payloads make the bundled client close the socket as `frame_size_limit`, so hosts with enough sessions cannot complete initial sync. Bound/chunk outbound snapshots and events to the advertised `maxFrameBytes` before sending.</comment>
<file context>
@@ -0,0 +1,819 @@
+ return false;
+ }
+ try {
+ socket.send(JSON.stringify(packet));
+ } catch {
+ // A synchronous send throw (torn socket, ws internals) must not leave
</file context>
There was a problem hiding this comment.
Valid at scale; consciously deferred. Today's host_snapshot scope is unarchived+unclosed sessions only (~2-4KB per entity ⇒ hundreds of sessions before the 1MiB inbound cap), per-event size is now bounded by the 256KB rawInput/rawOutput cap (e174f02), and a client that does hit the cap fails loudly (frame_size_limit close), not silently. Real fix is snapshot pagination in the wire protocol, which belongs with the durable-history/pagination slice tracked in plans/host-sessions-rollout.md rather than a bolt-on here.
| // The protocol is JSON text frames; String() folds the rare | ||
| // Buffer delivery into the same parse path, where non-JSON is | ||
| // answered with INVALID_PACKET. | ||
| void connection?.handleMessage(String(event.data)); |
There was a problem hiding this comment.
P3: Non-ASCII frames can bypass the advertised byte cap because String() produces UTF-16 text and the hub compares .length. Measure encoded UTF-8 bytes (or preserve the Buffer through intake) before parsing so maxFrameBytes limits actual WebSocket payload size.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/runtime/sessions/sync-route.ts, line 43:
<comment>Non-ASCII frames can bypass the advertised byte cap because `String()` produces UTF-16 text and the hub compares `.length`. Measure encoded UTF-8 bytes (or preserve the Buffer through intake) before parsing so `maxFrameBytes` limits actual WebSocket payload size.</comment>
<file context>
@@ -0,0 +1,56 @@
+ // The protocol is JSON text frames; String() folds the rare
+ // Buffer delivery into the same parse path, where non-JSON is
+ // answered with INVALID_PACKET.
+ void connection?.handleMessage(String(event.data));
+ },
+ onClose: () => {
</file context>
| clientToolCalls: z.array(toolCallSchema).max(10_000), | ||
| toolCallClaims: z.array(toolCallClaimSchema).max(10_000), | ||
| }) | ||
| .superRefine((snapshot, context) => { |
There was a problem hiding this comment.
P3: The cross-field validation logic for session snapshots is duplicated across three superRefine blocks: sessionSnapshotSchema in api.ts, hostSnapshotPacketSchema in sync.ts, and sessionSnapshotPacketSchema in sync.ts. Each independently validates thread-parent consistency, permission/tool-call session-ID matching, claim-to-tool-call references, state invariants on claimed tools, duplicate detection, and orphan-claim detection — roughly 100 lines of structurally identical logic each.
If a new client tool call state is introduced or an invariant changes, all three blocks need coordinated updates. Consider extracting the shared validation into a helper function (e.g., validateSnapshotIntegrity(sessionId, threads, pendingPermissions, clientToolCalls, toolCallClaims, context)). The per-schema superRefine would then call the helper with context, keeping the integrity rules defined once.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service-sync/src/protocol/api.ts, line 114:
<comment>The cross-field validation logic for session snapshots is duplicated across three `superRefine` blocks: `sessionSnapshotSchema` in api.ts, `hostSnapshotPacketSchema` in sync.ts, and `sessionSnapshotPacketSchema` in sync.ts. Each independently validates thread-parent consistency, permission/tool-call session-ID matching, claim-to-tool-call references, state invariants on claimed tools, duplicate detection, and orphan-claim detection — roughly 100 lines of structurally identical logic each.
If a new client tool call state is introduced or an invariant changes, all three blocks need coordinated updates. Consider extracting the shared validation into a helper function (e.g., `validateSnapshotIntegrity(sessionId, threads, pendingPermissions, clientToolCalls, toolCallClaims, context)`). The per-schema `superRefine` would then call the helper with context, keeping the integrity rules defined once.</comment>
<file context>
@@ -0,0 +1,569 @@
+ clientToolCalls: z.array(toolCallSchema).max(10_000),
+ toolCallClaims: z.array(toolCallClaimSchema).max(10_000),
+ })
+ .superRefine((snapshot, context) => {
+ const sessionId = snapshot.session.id;
+ const threadsById = new Map<string, (typeof snapshot.threads)[number]>();
</file context>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/host-service/test/integration/sessions-user-story.integration.test.ts (1)
848-861: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: parametrize the store lenses by
sessionIdto avoid re-implementing them for session B.
events()/agentText()are hardcoded tosessionA, so session B's stream reading and text extraction are re-implemented inline here. Extracting asessionEvents(sessionId)lens (and lettingevents()/agentText()delegate to it) would let session B reuse the same folding logic.🤖 Prompt for 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. In `@packages/host-service/test/integration/sessions-user-story.integration.test.ts` around lines 848 - 861, The session B assertions duplicate stream event and text extraction logic because the existing events()/agentText() lenses are fixed to sessionA. Add a sessionId-parameterized sessionEvents(sessionId) lens, update events() and agentText() to delegate to it while preserving their current sessionA behavior, and replace the inline streamB/textB folding with the shared lenses for createdB.session.id.
🤖 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.
Nitpick comments:
In
`@packages/host-service/test/integration/sessions-user-story.integration.test.ts`:
- Around line 848-861: The session B assertions duplicate stream event and text
extraction logic because the existing events()/agentText() lenses are fixed to
sessionA. Add a sessionId-parameterized sessionEvents(sessionId) lens, update
events() and agentText() to delegate to it while preserving their current
sessionA behavior, and replace the inline streamB/textB folding with the shared
lenses for createdB.session.id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a352e4fa-a562-41d4-93e0-0a1244357238
📒 Files selected for processing (3)
packages/host-service-sync/src/protocol/api.tspackages/host-service-sync/src/protocol/protocol.test.tspackages/host-service/test/integration/sessions-user-story.integration.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/host-service-sync/src/protocol/protocol.test.ts
- packages/host-service-sync/src/protocol/api.ts
There was a problem hiding this comment.
1 issue found across 3 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/test/integration/sessions-user-story.integration.test.ts">
<violation number="1" location="packages/host-service/test/integration/sessions-user-story.integration.test.ts:809">
P2: The `overlapWatch` background IIFE in the step-17 test polls until `secondDone` is set, but if any setup code throws before that line, the IIFE runs forever and can produce an unhandled rejection during `afterAll` cleanup. Consider wrapping the IIFE body in try/catch, or using an `AbortController` with a deadline so the loop always terminates regardless of the success/failure path.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
|
|
||
| let overlapped = false; | ||
| let secondDone = false; | ||
| const overlapWatch = (async () => { |
There was a problem hiding this comment.
P2: The overlapWatch background IIFE in the step-17 test polls until secondDone is set, but if any setup code throws before that line, the IIFE runs forever and can produce an unhandled rejection during afterAll cleanup. Consider wrapping the IIFE body in try/catch, or using an AbortController with a deadline so the loop always terminates regardless of the success/failure path.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/test/integration/sessions-user-story.integration.test.ts, line 809:
<comment>The `overlapWatch` background IIFE in the step-17 test polls until `secondDone` is set, but if any setup code throws before that line, the IIFE runs forever and can produce an unhandled rejection during `afterAll` cleanup. Consider wrapping the IIFE body in try/catch, or using an `AbortController` with a deadline so the loop always terminates regardless of the success/failure path.</comment>
<file context>
@@ -0,0 +1,901 @@
+
+ let overlapped = false;
+ let secondDone = false;
+ const overlapWatch = (async () => {
+ while (!secondDone) {
+ if (sessionEntity().runState === "running") overlapped = true;
</file context>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.test.ts (1)
1214-1247: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winPrefer Bun's fake timers over a real
Bun.sleep(5)wait.This test relies on a real wall-clock
Bun.sleep(5)to let thereconnectDelayMs: 1retry timer fire, which is inherently flaky under CI load (razor-thin margin between a 1ms scheduled delay and a 5ms real wait). Bun's test runner addedjest.useFakeTimers()/advanceTimersByTime()support in v1.3.4, so a real-time-free alternative should be available given the project's declared@types/bun@1.3.14.♻️ Suggested refactor using fake timers
- client.connect(); - expect(logs).toContainEqual({ - event: "sessions_sync.connect_failed", - reason: "socket_factory_threw", - }); - expect(client.store.getState().connection.error).toEqual({ - code: "SOCKET_FACTORY_FAILED", - retryable: true, - }); - expect(client.store.getState().connection.status).toBe("reconnecting"); - - // The retry loop still runs — and succeeds once the factory recovers. - await Bun.sleep(5); - expect(sockets).toHaveLength(1); + jest.useFakeTimers(); + try { + client.connect(); + expect(logs).toContainEqual({ + event: "sessions_sync.connect_failed", + reason: "socket_factory_threw", + }); + expect(client.store.getState().connection.error).toEqual({ + code: "SOCKET_FACTORY_FAILED", + retryable: true, + }); + expect(client.store.getState().connection.status).toBe("reconnecting"); + + // The retry loop still runs — and succeeds once the factory recovers. + jest.advanceTimersByTime(1); + expect(sockets).toHaveLength(1); + } finally { + jest.useRealTimers(); + }Please verify the reconnect scheduling actually uses
setTimeout/clearTimeout(per the graph context oncreateSessionsSyncClient) soadvanceTimersByTimedeterministically triggers it before adopting this change.Based on learnings: "test code should avoid real wall-clock timers... do not recommend changing... unless a safe alternative that doesn't use real time is available" — Bun 1.3.4+ fake timers appear to be that safe alternative here.
🤖 Prompt for 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. In `@packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.test.ts` around lines 1214 - 1247, Replace the real-time Bun.sleep wait in the “socket factory failures surface a log event and a connection error” test with Bun fake timers, confirming the reconnect path in createSessionsSyncClient schedules retries through setTimeout. Enable fake timers before connecting, advance them by the configured reconnectDelayMs, assert the recovered socket, and restore real timers during cleanup.Source: Learnings
🤖 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.
Nitpick comments:
In
`@packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.test.ts`:
- Around line 1214-1247: Replace the real-time Bun.sleep wait in the “socket
factory failures surface a log event and a connection error” test with Bun fake
timers, confirming the reconnect path in createSessionsSyncClient schedules
retries through setTimeout. Enable fake timers before connecting, advance them
by the configured reconnectDelayMs, assert the recovered socket, and restore
real timers during cleanup.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6aac6f08-1e70-4a9b-9f68-30b0b5583286
📒 Files selected for processing (5)
packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.test.tspackages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.tspackages/host-service-sync/src/client/createSessionsSyncClient/state.tspackages/host-service-sync/src/client/types.tsplans/host-sessions-rollout.md
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/host-service-sync/src/client/types.ts
- packages/host-service-sync/src/client/createSessionsSyncClient/state.ts
- packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts
There was a problem hiding this comment.
2 issues found across 5 files (changes from recent commits).
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts">
<violation number="1" location="packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts:609">
P2: A delayed `unsubscribed` acknowledgement after `session_removed` recreates an idle stream for the deleted session. Include unsubscribe acknowledgements in the stale-session filter so this race is dropped like other post-unsubscribe packets.</violation>
<violation number="2" location="packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts:735">
P1: A stale `syncUrl` rejection can mark a newer live connection as reconnecting and leave its status incorrect. Gate failure handling on the captured connection epoch (and stopped state), as `attachSocket` already does.</violation>
</file>
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
| } | ||
| void value.then( | ||
| (url) => attachSocket(url, epoch), | ||
| () => failSyncUrl(), |
There was a problem hiding this comment.
P1: A stale syncUrl rejection can mark a newer live connection as reconnecting and leave its status incorrect. Gate failure handling on the captured connection epoch (and stopped state), as attachSocket already does.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts, line 735:
<comment>A stale `syncUrl` rejection can mark a newer live connection as reconnecting and leave its status incorrect. Gate failure handling on the captured connection epoch (and stopped state), as `attachSocket` already does.</comment>
<file context>
@@ -675,7 +732,7 @@ export function createSessionsSyncClient(
void value.then(
(url) => attachSocket(url, epoch),
- () => scheduleReconnect(),
+ () => failSyncUrl(),
);
}
</file context>
| () => failSyncUrl(), | |
| () => { | |
| if (stopped || epoch !== connectionEpoch) return; | |
| failSyncUrl(); | |
| }, |
| !sentSessionSubscriptions.has(packet.sessionId) | ||
| ? packet.sessionId | ||
| : null; | ||
| case "subscribed": |
There was a problem hiding this comment.
P2: A delayed unsubscribed acknowledgement after session_removed recreates an idle stream for the deleted session. Include unsubscribe acknowledgements in the stale-session filter so this race is dropped like other post-unsubscribe packets.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.ts, line 609:
<comment>A delayed `unsubscribed` acknowledgement after `session_removed` recreates an idle stream for the deleted session. Include unsubscribe acknowledgements in the stale-session filter so this race is dropped like other post-unsubscribe packets.</comment>
<file context>
@@ -585,6 +588,37 @@ export function createSessionsSyncClient(
+ !sentSessionSubscriptions.has(packet.sessionId)
+ ? packet.sessionId
+ : null;
+ case "subscribed":
+ case "caught_up":
+ case "reset":
</file context>
| case "subscribed": | |
| case "subscribed": | |
| case "unsubscribed": |
db7eb6a to
2fdbde0
Compare
bra1nDump
changed the title
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/host-service-sync/src/protocol/sync.ts (1)
17-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDrop the redundant
.safe()calls
.safe()is deprecated in Zod 4 and duplicates.int(), soz.number().int().positive()is enough here; the same applies to themaxSubscriptionsandmaxFrameBytesschemas.♻️ Optional simplification
export const toolResolverDescriptorSchema = z.object({ name: z.string().min(1).max(256), - version: z.number().int().safe().positive(), + version: z.number().int().positive(), });🤖 Prompt for 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. In `@packages/host-service-sync/src/protocol/sync.ts` around lines 17 - 20, Remove the redundant deprecated safe() calls from toolResolverDescriptorSchema and the maxSubscriptions and maxFrameBytes schema definitions, leaving the integer-positive validation as z.number().int().positive().
🤖 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 `@plans/done/20260709-1438-unified-home-workspaces-sessions.md`:
- Line 47: Update the stale reference in the observation about canonical live
status to point from plans/host-sessions-rollout.md to
plans/host-sessions-sync.md, leaving the surrounding explanation unchanged.
In `@plans/host-sessions-sync.md`:
- Line 25: The socket packet budget in the Budgets section is inconsistent with
the enumerated client and server packet types; update it from 12 to 13 to match
the packet lists under the client-side and server-side socket packet sections.
---
Nitpick comments:
In `@packages/host-service-sync/src/protocol/sync.ts`:
- Around line 17-20: Remove the redundant deprecated safe() calls from
toolResolverDescriptorSchema and the maxSubscriptions and maxFrameBytes schema
definitions, leaving the integer-positive validation as
z.number().int().positive().
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e417ad98-9bc0-4c29-a5dc-9e4526521dcd
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
apps/mobile/lib/host-service/client.tsapps/mobile/lib/trpc/host-chat-types.tsapps/mobile/plans/acp-vs-claude-sdk.htmlapps/mobile/plans/mobile-chat-runtime.mdpackages/host-service-sync/package.jsonpackages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.test.tspackages/host-service-sync/src/client/createSessionsSyncClient/createSessionsSyncClient.tspackages/host-service-sync/src/client/createSessionsSyncClient/index.tspackages/host-service-sync/src/client/createSessionsSyncClient/state.tspackages/host-service-sync/src/client/index.tspackages/host-service-sync/src/client/types.tspackages/host-service-sync/src/index.tspackages/host-service-sync/src/package-boundaries.test.tspackages/host-service-sync/src/projection/fixtures/cards-and-settings.jsonpackages/host-service-sync/src/projection/fixtures/turn-lifecycle.jsonpackages/host-service-sync/src/projection/index.tspackages/host-service-sync/src/projection/projection.tspackages/host-service-sync/src/projection/reduceProjection.test.tspackages/host-service-sync/src/projection/reduceProjection.tspackages/host-service-sync/src/protocol/api.tspackages/host-service-sync/src/protocol/entities.tspackages/host-service-sync/src/protocol/events.tspackages/host-service-sync/src/protocol/index.tspackages/host-service-sync/src/protocol/primitives.tspackages/host-service-sync/src/protocol/protocol.test.tspackages/host-service-sync/src/protocol/routes.tspackages/host-service-sync/src/protocol/sync.tspackages/host-service-sync/tsconfig.jsonpackages/host-service/docs/acp-sessions.mdpackages/host-service/docs/interim-router-types.mdpackages/host-service/package.jsonpackages/host-service/src/app.tspackages/host-service/src/db/schema.tspackages/host-service/src/runtime/acp-sessions/stream.tspackages/host-service/src/runtime/sessions/canonical-sessions.test.tspackages/host-service/src/runtime/sessions/canonical-sessions.tspackages/host-service/src/runtime/sessions/index.tspackages/host-service/src/runtime/sessions/sync-hub.stress.test.tspackages/host-service/src/runtime/sessions/sync-hub.test.tspackages/host-service/src/runtime/sessions/sync-hub.tspackages/host-service/src/runtime/sessions/sync-route.tspackages/host-service/src/runtime/sessions/testing/fake-acp-port.tspackages/host-service/src/runtime/sessions/testing/sync-harness.tspackages/host-service/src/runtime/sessions/translate-acp.test.tspackages/host-service/src/runtime/sessions/translate-acp.tspackages/host-service/src/trpc/index.tspackages/host-service/src/trpc/router/acp-sessions/acp-sessions.tspackages/host-service/src/trpc/router/router.tspackages/host-service/src/trpc/router/sessions/index.tspackages/host-service/src/trpc/router/sessions/sessions.tspackages/host-service/src/types.tspackages/host-service/test/helpers/createTestHost.tspackages/host-service/test/helpers/gym.tspackages/host-service/test/integration/acp-user-story.integration.test.tspackages/host-service/test/integration/sessions-sync-client.integration.test.tspackages/host-service/test/integration/sessions-user-story.integration.test.tspackages/session-protocol/src/acp.tsplans/acp-session-follow-ups.mdplans/chat-mastra-rebuild-execplan.mdplans/chat-service-at-linking.mdplans/cross-client-session-tab-sync.mdplans/done/20260709-1438-unified-home-workspaces-sessions.mdplans/done/20260710-session-harness-acp.mdplans/host-integration-test.mdplans/host-service-chat-architecture.mdplans/host-sessions-sync.mdplans/session-harness-claude-agent-sdk.mdplans/v1-to-v2-fast-migration.mdplans/v2-chat-greenfield-architecture.md
💤 Files with no reviewable changes (10)
- packages/host-service/docs/interim-router-types.md
- plans/cross-client-session-tab-sync.md
- plans/host-service-chat-architecture.md
- plans/chat-service-at-linking.md
- plans/host-integration-test.md
- plans/chat-mastra-rebuild-execplan.md
- plans/session-harness-claude-agent-sdk.md
- plans/acp-session-follow-ups.md
- packages/host-service/docs/acp-sessions.md
- apps/mobile/plans/mobile-chat-runtime.md
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/host-service-sync/tsconfig.json
- packages/host-service-sync/src/client/createSessionsSyncClient/index.ts
- packages/host-service/src/trpc/router/sessions/index.ts
- packages/host-service-sync/src/package-boundaries.test.ts
- packages/host-service-sync/src/protocol/index.ts
- packages/host-service-sync/src/protocol/routes.ts
- packages/host-service-sync/src/client/index.ts
- packages/host-service-sync/package.json
- packages/host-service-sync/src/index.ts
- packages/host-service-sync/src/projection/index.ts
- packages/host-service/package.json
- packages/host-service/test/helpers/createTestHost.ts
- packages/host-service-sync/src/projection/fixtures/turn-lifecycle.json
- packages/host-service/src/types.ts
- packages/host-service/src/runtime/sessions/sync-route.ts
- packages/host-service-sync/src/projection/projection.ts
- packages/host-service/src/runtime/sessions/testing/sync-harness.ts
- packages/host-service/src/app.ts
- packages/host-service-sync/src/protocol/primitives.ts
- packages/host-service/src/runtime/sessions/sync-hub.ts
- packages/host-service/src/runtime/sessions/testing/fake-acp-port.ts
- packages/host-service/src/runtime/sessions/translate-acp.test.ts
- packages/host-service/src/runtime/sessions/sync-hub.test.ts
- packages/host-service/test/integration/acp-user-story.integration.test.ts
| replicas: they render state the host owns and submit intents. The protocol | ||
| between them is deliberately small: | ||
|
|
||
| **Budgets (hard):** 9 tRPC procedures · 12 socket packet types · 2 cursor |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Socket packet budget likely off by one.
Line 25 states "12 socket packet types", but the enumerated packets total 13: client-side hello, subscribe, unsubscribe, toolResolversChanged, ping (5) plus server-side helloAck, subscribed, event, caughtUp, unsubscribed, reset, error, pong (8). Since this doc is the "single source of truth," reconcile the budget with the packet lists in Lines 174-179.
🤖 Prompt for 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.
In `@plans/host-sessions-sync.md` at line 25, The socket packet budget in the
Budgets section is inconsistent with the enumerated client and server packet
types; update it from 12 to 13 to match the packet lists under the client-side
and server-side socket packet sections.
2fdbde0 to
0a24557
Compare
a057c20 to
a234df3
Compare
bra1nDump
changed the title
0cd49eb to
9cc7465
Compare
… over tRPC snapshots + /sessions/sync, mobile rewired onto the sync client
9cc7465 to
357175a
Compare
|
Picked this up and carried it to a merge-ready state in #5940 — your branch history is preserved there, rebased-via-merge onto main (the drizzle 0010 collision regenerated as 0012, FinishReviewSheet ported off the deleted ACP APIs), plus fixes from a four-track deep review (unbounded session log → real 5k ring, orphan-sweep data-loss guard, client cold-path retries + heartbeat, timeline fold determinism, mobile permission-card/failed-send fixes). Open follow-ups are listed in #5940's description. Closing this one in favor of it is your/team's call. |
Host sessions sync v1 — the canonical session plane (tRPC snapshots + /sessions/sync WS) with the mobile app fully rewired onto it, the legacy Mastra/ACP chat planes deleted from the phone, settings catalogs + pickers, and the ChatGPT-class chat scroll discipline with an on-device regression lab. Plan below, verbatim, including the shipped follow-ups.
Host Sessions Sync
Status: v1 — implemented on
feat/sessions-v1-simplified-api, in teamreview. This is the single source of truth for the host sessions surface:
the protocol spec (three review layers), rollout state, and the E2E user
story the suites assert against. It supersedes the previous wider protocol
draft and the separate rollout doc; everything cut was implemented and
tested first, so the cuts are deliberate removals of speculation, not
unbuilt wishes.
The spec is structured as three review layers. Layer 1 is the pitch — read
it alone to judge the approach. Layer 2 is the design — state ownership, the
exact surface, recovery, testing. Layer 3 is one realistic end-to-end
scenario with exact payloads. Rollout and the user story close the doc.
Layer 1 — Principles and approach
One host process per machine runs coding-agent sessions through vendor
harnesses (Claude Code today, more later). Phones and desktops are thin
replicas: they render state the host owns and submit intents. The protocol
between them is deliberately small:
Budgets (hard): 9 tRPC procedures · 12 socket packet types · 2 cursor
kinds · 1 automatic recovery path + 1 human hammer.
projection. The host folds vendor output into canonical events; host
and client run the same pure reducer over them. No second state
machine to drift.
carries an idempotent
requestIdand returns a receipt; the outcomearrives later as events whose
causationIdechoes the request. Retries,double-taps, and races between devices are absorbed by receipts —
first write wins, losers get a clean stale error.
Snapshots (session list, session state) are tRPC query responses. The
WebSocket carries individual events, acks, and control frames — nothing
that grows with session size.
them verbatim, never parse or compare. A cursor from a dead log
generation fails loudly (
reset) instead of silently misaligning.reseton a streamtriggers exactly what first-connect does: tRPC snapshot → subscribe from
its head. There is no separate resync state machine. When the host
itself is wedged, the existing desktop "restart host" button is the
answer — sessions re-hydrate from durable state; we add no intermediate
per-session recovery procedures.
Message/ContentBlock/ToolCall/Threadshapes — not AI-SDK or ACP types — so the same wireserves the ACP adapter today and a direct Claude SDK adapter next
without client changes.
Trajectory: one host control plane, terminals stay separate
This is host-level sync, not session-level sync — sessions are just the
first domain riding it.
/sessions/syncisthe host's control plane: small, ordered, cursor-replayable events for
everything the host owns. Today that is session/permission lifecycle;
workspace lifecycle (created/deleted/renamed), host status, and agent
availability join as new host-stream event types plus matching tRPC
snapshot procedures — never as new sockets or a second sync machine.
throughput/data plane — raw PTY bytes, latency-sensitive, tolerant of
loss on reconnect — already served by the dedicated
/terminal/*path.Control and data planes never share a socket: a busy terminal must not
head-of-line-block a permission card, and PTY bytes must never pay the
event-log/cursor/replay overhead built for control events.
already host-scoped (one replica per host: connection, host stream,
sessionsById, per-session streams). New domains land as sibling slicesfolded from the same host stream by the same reducer pattern — one
socket, one cursor discipline, one store per host.
Comparison at a glance
Why the host contract is product-owned
ACP and AI SDK solve valuable but narrower problems than this contract.
ACP standardizes communication between an editor/client and an agent. AI SDK
UI standardizes how one chat consumes and renders a model response stream.
Superset must synchronize an entire host across multiple clients: session
directory and metadata, nested threads, permissions, client tools, workflows,
host status, workspace lifecycle, and future host-owned domains.
Using ACP or AI SDK as the public host wire would therefore not remove a
Superset protocol. Superset-specific semantics would move into
_meta, customdata parts, extension methods, or a second side channel. The result would have
two nested envelopes, two versioning surfaces, and more intermediate state,
while the product contract would still be ours in practice.
The product-owned boundary gives us:
subscribe/unsubscribesemantics for many streams over onehost connection;
wins;
Thread, workflow, subagent, and future side-chat semantics evenwhen the active harness does not expose them;
domain;
and other host capabilities without pretending they are chat messages;
state machine wrapped inside a second product state machine.
The transport split follows the data:
/sessions/synccarries small ordered control events and replay;/terminal/*remains a separate throughput-oriented data plane for raw PTYbytes.
This is not a rejection of OSS. Upstream implementations are inputs, not the
source of product truth. We can vendor AI Elements, reuse AI SDK interaction
patterns, and align canonical shapes with ACP where the semantics match.
Upstream types terminate at adapters rather than leaking into the public
contract.
ACP remains useful at the vendor boundary today because the Claude and Codex
adapters reduce integration work and are already proven. Moving an adapter
in-process or driving a vendor SDK directly is a reversible optimization when
adapter lag, lost semantic fidelity, process complexity, or debugging cost
justifies it. SDK and adapter bumps run through the same real-model gym,
deterministic protocol suites, and Maestro regression story before shipping.
Testing approach
permission cards, always-allow, multi-select questions, subagent,
workflow, parallel sessions) replayed over every execution surface:
(1) the shipping ACP surface, (2) this protocol with a real model,
(3) the direct SDK adapter, (4) a phone via Maestro. Same story, four
wires — divergence is a bug by definition.
(real JSON-RPC over stdio, no model) drives restart/crash/eviction
scenarios in milliseconds inside
bun test.drop/reconnect/eviction storm; an oracle asserts byte-parity between what
the host holds and what every client converged to.
(internal + external AI reviewers + second-opinion codex runs) becomes a
Zod
superRefinethe host enforces at runtime on inputs and outputs —the contract is executable, not prose.
Layer 2 — Design
Where real state lives
~/.claude/...JSONL)session/load), under a fresh log generationThe invariant that keeps recovery simple: every layer is rebuildable from
the layers to its right. Nothing a client holds is ever authoritative;
nothing in host RAM is unrecoverable.
Control flow (high level)
tRPC surface — 9 procedures
All wire identifiers (packet types, event types, enum values) are
camelCase. Every mutation input carries
requestId; receipts areadmission-acks.
list→ host snapshot{} → { sessions: Session[], pendingPermissions: PermissionRequest[], openClientToolCalls: ToolCall[], head: Cursor | null }— all non-archived sessions, no paging. Ungated capability probe: gated-off hosts return empty arrays,head: null.get→ session snapshot{sessionId} → { session, threads[], activeTurns[], pendingPermissions[], openToolCalls[], recentEvents[], hasOlderEvents, head }—recentEventsis a ≤ 50 tail ending exactly athead;hasOlderEventstells the client whethergetEventspaging has anything to fetchcreate{requestId, workspaceId, agentId, title, settings} → {session, mainThread}update{requestId, sessionId, title?, archived?, settings?}— one patch mutation for title/archive/model/mode/effort (absent = unchanged)getEvents{sessionId, threadId?, beforeCursor?, limit ≤ 100} → EventsWindow— backwards-only scrollback; no cursor = newest windowsubmitTurn{requestId, sessionId, threadId, content: ContentBlock[]} → {status: "accepted", turnId}cancelTurn{requestId, sessionId, turnId} → receiptresolvePermission{requestId, sessionId, permissionId, outcome}— first write winsresolveToolCall{requestId, sessionId, toolCallId, outcome}— client tools (ask_user); no claims: the card renders on every device, first resolve wins, others drain via eventsEventsWindow:{ items ≤ 100 ascending, range: { oldest, newest, hasMoreBefore, truncatedBefore }, head }. Boundaries are full{eventId, cursor, occurredAt}triples, schema-verified against the actualfirst/last items. Pages come from the in-memory log (messages are
deliberately not persisted on the host);
truncatedBefore: true= olderhistory existed but the source journal evicted it before the host observed
it.
Sync socket — 12 packet types, all small
client →
hello(protocolVersion, clientInstanceId, toolResolvers),subscribe(subscriptionId, stream, after: Cursor — required),unsubscribe,toolResolversChanged,pingserver →
helloAck(hostId, connectionId),subscribed,event,caughtUp,unsubscribed,reset(code, recovery),error,pongThere are no snapshot packets.
subscribe.afteralways comes from atRPC response (
list/get/getEventsall returnhead). Subscribing withan unservable cursor yields
reset, and the client re-runs the cold path.The hot-path frame is
event: ~250–600 bytes typical. The only boundedexception: tool-call
rawInput/rawOutputup to the 256 KB cap (oversizevalues are dropped to
nullat translation, never truncated mid-JSON).Events:
sessionUpsert,sessionRemoved,permissionAvailable,permissionResolved,clientToolCallAvailable,clientToolCallResolved. All are idempotent upserts/removals keyed byentity id — replay overlap after a tRPC snapshot is harmless by
construction.
threadCreated/Updated,turnStarted/Completed/Failed/Cancelled,messageStarted/Delta/Completed,toolCallStarted/Updated,permissionRequested/Resolved,planUpdated,settingsUpdated,error. Envelope:{id, sessionId, threadId, cursor, occurredAt, causationId, payload}; dedup byid.Cursors and incarnations — 2 kinds
h<boot-id>-<serial>c<generation>-<serial>per session; same value in live stream, snapshothead,getEventsboundariesAn incarnation is one lifetime of a log's owner. Host cursors embed the
boot id, so a restarted host deterministically rejects stale cursors
(
CURSOR_INVALID). Session cursors embed the log generation, mintedfresh on every tracking: the canonical log is rebuilt from vendor replay
each time (content is deliberately not persisted host-side), so a cursor
from a previous tracking — usually a dead host process — fails the same
deterministic way (
CURSOR_INVALID→ snapshot rebuild) instead ofsilently aliasing a serial in the new log. The bare zero cursor
c000000000000stays generation-less: it is the head of a never-trackedsession's snapshot and means "from the very beginning". Session-list
paging cursors were deleted along with list pagination.
Recovery model — two paths, one guard
(client slept), host restart, session resurrection, upstream adapter
resync.
resetnames one stream; only it re-fetches. Handler = thefirst-connect code path, literally.
the canonical session meta (titles, archive/close state), and the vendor
transcripts survive; sessions come back
offline; the first livemutation resurrects (spawn adapter →
session/load→ vendor replays itsown disk → canonical log rebuilt under a fresh generation). Old session
cursors reset
CURSOR_INVALIDinto the snapshot cold path. Reads neverresurrect — listing 50 dead sessions spawns 0 children.
Guard: reset circuit breaker — N resets on one stream within a window →
stream status
error, stop auto-resubscribing, surface to UI. A reset loopis by definition a host bug; clients must stop, not spin.
Read-consumption reality (retention design note)
Humans re-read user messages, attachments, and each turn's final assistant
response. Old turns' tool calls and deltas are write-only in practice.
Consequences: client eviction drops old-turn tool events/deltas first,
keeping a meaningful "skeleton"; tool payloads are already separable
(nullable, capped); a future condensed/skeleton
getEventsmode is anadditive parameter. Not in v1 — plain backwards paging at 100
events/page is enough.
Decision (2026-07-12): the host SQLite persists no messages or events —
only top-level session metadata (title/archive/close, in
session_meta) plus the existing registry. Conversation content hasexactly one durable home, the vendor's own transcript, resumed via the
native session id. Delete semantics follow: metadata rows whose session
left the registry are swept at startup; archive/close never delete
anything — sessions reopen.
What was cut from the previous draft (and why)
claimToolCall, claims,claimedstate,claimId)askUserpops on all devices by design; first resolve wins; receipts already make the loser's submit a clean stale errorSession.attention[]pendingPermissions/openClientToolCallsin the host snapshot are the primitiveshostSnapshot,sessionSnapshot)list/getoutputs; snapshots are big and belong on tRPCsearchcreateSideChat+sideChatthread kindgetThread,listThreadslistpaginationgetEventsanchors (time/event) + forward pagingupdateSettingsTest evidence and work needed
Already proven on THIS surface (all automated,
bun test, green onfeat/sessions-v1-simplified-api+feat/sessions-durable-store):sessions-sync-client.integration.test.ts,11 tests, real host + real WebSocket, fake adapter): cold connect via
tRPC snapshots, streaming turns with causation receipts, permission
cards, offline replay across a mid-stream disconnect, a fresh client
converging from nothing, archiving, scrollback paging until the store
equals the host's full journal, session switching under load, and a
host reboot over the same host DB — canonical titles survive via the
metadata table, every cursor (host boot id, session log generation) is
rejected with exactly one
reset(CURSOR_INVALID, refetchSnapshot)perstream, the cold path re-seeds each replica from the rebuilt log, no
duplicated event ids, streaming resumes. The reboot test also pins the
causationId-loss trade-off of the no-messages-in-SQLite decision.
canonical-sessions.durable.test.ts):title/archive/close overrides survive host reboots (archived rows stay
out of the host snapshot scope), createSession titles persist, rebuilt
logs mint a new generation whose foreign cursors reject
deterministically while the zero cursor stays servable, and the startup
orphan sweep drops metadata for sessions the registry no longer knows.
error(
RESET_LOOP) and stops auto-resubscribe; cleared on reconnect.collision; send-failure drop; dispose idempotence; randomized stress
storm with a byte-parity oracle (required-
aftergrammar).resolved cards, mid-flight fetch races, never-subscribed packet drops,
reconnect backoff, socket loss downgrades
live→idle(data kept).head= zerocursor, empty tail), live paths resurrect (fake adapter); backwards
paging with
hasMoreBefore/truncatedBefore; mergedupdatereceipts./sessions/syncon this exact surface (tRPC-seeded client, claimlessquestion cards, merged
updatefor the acceptEdits switch, final parityoracle that pages the whole journal backwards and deep-equals it against
the client fold; zero drops, zero resets).
Work needed (details in the Rollout section below):
session-protocoldata plane); Maestro phone replay (execution 4).Layer 3 — One realistic scenario, exact payloads
Phone opens the app, drives a turn with a permission card, loses the
network mid-stream, recovers, then the host is restarted and the session
resumes. (This is the shape of the automated scenario test; the gym E2E
runs the same surface with a real model.)
1 — Cold connect. Phone calls
sessions.list:Phone opens the socket and subscribes to the host stream from
head; usertaps the session, phone calls
sessions.get("ses_01")(snapshot above) andsubscribes the session stream from its
head:{ "type": "hello", "protocolVersion": 1, "requestId": "req_h1", "clientInstanceId": "cli_phone", "clientVersion": "1.15.0", "toolResolvers": [ { "toolName": "ui.ask_user", "version": 1 } ] } { "type": "helloAck", "requestId": "req_h1", "hostId": "org_7", "connectionId": "conn_a" } { "type": "subscribe", "requestId": "req_s1", "subscriptionId": "sub_host", "stream": { "type": "host" }, "after": "h1734-000000000381" } { "type": "subscribed", "subscriptionId": "sub_host", "cursor": "h1734-000000000381" } { "type": "caughtUp", "subscriptionId": "sub_host", "cursor": "h1734-000000000381" } { "type": "subscribe", "requestId": "req_s2", "subscriptionId": "sub_ses1", "stream": { "type": "session", "sessionId": "ses_01" }, "after": "c000000000512" } { "type": "subscribed", "subscriptionId": "sub_ses1", "cursor": "c000000000512" } { "type": "caughtUp", "subscriptionId": "sub_ses1", "cursor": "c000000000512" }2 — A turn with a permission card. User sends a message:
Events stream on
sub_ses1(envelope shown once, then payloads only):{ "type": "event", "subscriptionId": "sub_ses1", "stream": "session", "cursor": "c000000000513", "event": { "id": "evt_513", "sessionId": "ses_01", "threadId": "thr_main", "cursor": "c000000000513", "occurredAt": 1783532410000, "causationId": "req_t9", "payload": { "type": "turnStarted", "turn": { "id": "trn_10", "sessionId": "ses_01", "threadId": "thr_main", "status": "accepted", "originatingClientInstanceId": "cli_phone", "createdAt": 1783532410000, "updatedAt": 1783532410000 } } } } // … subsequent payloads: { "type": "messageStarted", "message": { "id": "msg_u1", "role": "user", "content": [ … ], … } } { "type": "messageStarted", "message": { "id": "msg_a1", "role": "assistant", "content": [], … } } { "type": "messageDelta", "messageId": "msg_a1", "content": { "type": "text", "text": "Running the test" } } { "type": "toolCallStarted", "toolCall": { "id": "tc_bash1", "tool": { "name": "bash", "version": 1 }, "title": "bun test relay.test.ts", "input": { "command": "bun test relay.test.ts" }, "resolver": { "type": "host" }, "state": "awaitingPermission", "parentToolCallId": null, "turnId": "trn_10", … } } { "type": "permissionRequested", "permission": { "id": "perm_1", "toolCallId": "tc_bash1", "multiSelect": false, "options": [ { "id": "allow_once", "name": "Allow once", "kind": "allowOnce" }, { "id": "allow_always", "name": "Always allow", "kind": "allowAlways" }, { "id": "reject_once", "name": "Reject", "kind": "rejectOnce" } ], "requestedAt": 1783532412000 } }The card shows on the phone AND the desktop (no claims). Phone answers
first:
{ "type": "permissionResolved", "permissionId": "perm_1", "outcome": { "type": "selected", "optionIds": ["allow_once"] } } { "type": "toolCallUpdated", "toolCallId": "tc_bash1", "update": { "state": "running", "updatedAt": 1783532413000 } } { "type": "toolCallUpdated", "toolCallId": "tc_bash1", "update": { "state": "succeeded", "output": { "exitCode": 0 }, "updatedAt": … } } { "type": "messageDelta", "messageId": "msg_a1", "content": { "type": "text", "text": " — passes now." } } { "type": "messageCompleted", "messageId": "msg_a1" } { "type": "turnCompleted", "turnId": "trn_10", "stopReason": "endTurn" }3 — Mid-stream disconnect. Phone loses network after
c…525; the agentkeeps streaming on the host. Phone reconnects (exponential backoff), replays
the gap, converges:
{ "type": "subscribe", "requestId": "req_s3", "subscriptionId": "sub_ses1b", "stream": { "type": "session", "sessionId": "ses_01" }, "after": "c000000000525" } { "type": "subscribed", "subscriptionId": "sub_ses1b", "cursor": "c000000000525" } // events c…526 … c…541 replay here; dedup by event.id absorbs any overlap { "type": "caughtUp", "subscriptionId": "sub_ses1b", "cursor": "c000000000541" }4 — Long sleep → reset → cold path. Phone returns after hours; the ring
(5k events) evicted its cursor:
{ "type": "subscribe", "requestId": "req_s4", "subscriptionId": "sub_ses1c", "stream": { "type": "session", "sessionId": "ses_01" }, "after": "c000000000525" } { "type": "reset", "subscriptionId": "sub_ses1c", "sessionId": "ses_01", "code": "CURSOR_INVALID", "recovery": "refetchSnapshot" } // phone re-runs the cold path: tRPC get → subscribe(after: new head). One stream, // one refetch; every other stream on the socket is untouched.If the host answered every such subscribe with another
reset(a host bug),the circuit breaker trips after N attempts: the stream goes
error, the UIshows a retry affordance, nothing spins.
5 — Host reboot. User restarts the host (desktop button). The socket
closes; the phone's store keeps rendering its replica. On reconnect,
sessions.liststill showsses_01— the SQLite registry survived, andits canonical title/archive state survived in
session_meta— withrunState: "offline". The phone's cursors all belong to dead incarnations:the host-stream cursor
h1734-…fails parsing under booth1802, and thesession cursor's log generation died with the old process (the canonical
log is rebuilt from the vendor's
session/loadreplay, never from hostdisk) →
reset(CURSOR_INVALID)→ cold path per stream. User types a newmessage →
submitTurn→ the host spawns a fresh adapter child, issuessession/loadwith the stored native session id, the vendor replays itsown on-disk transcript, the canonical log is rebuilt (new generation, new
event ids, fresh cursors), and events flow again. A turn that was
mid-flight when the host died is terminalized (
turnFailed), not silentlyresumed — the user resubmits.
Known, deliberate limitation (pinned by the reboot test): the rebuilt
canonical log loses
causationIdattribution. Request→event linkage isarmed in the serving process; the vendor journal doesn't record it, so
every rebuilt event carries
causationId: null. Receipts and UI stillwork (they key on entities, not causation) — the trade for keeping
messages out of host SQLite entirely.
Open questions for review
recentEventstail size (trade first-paint completeness vsresponse size over the relay) — shipped default: 50.
(
maxStreamResetsPerWindow/streamResetWindowMs, overridable perclient).
Session-cursor incarnation tag ships with the durable store (Bump electron from 37.8.0 to 39.0.0 #10), orResolved: shipped as the log generation tagblock v1 on it?
(
c<generation>-<serial>), minted per tracking — no persistenceneeded, since the log itself is never persisted.
reboots, scrollback beyond the vendor replay window)? Decided against
for now — no messages in host SQLite; revisit alongside the direct SDK
adapter if its native store changes the calculus.
Rollout
Where this stands (2026-07-12):
Done (on
feat/sessions-v1-simplified-api, all suites green plus areal-model E2E): the protocol package, the
sessions.*router and/sessions/synchub, the shared projection reducer, the framework-freeclient store with reset circuit breaker, the 11-test realistic-scenario
suite (disconnect, scrollback parity, session switching, host reboot), and
the gym story 10/10 with a real Opus instance.
Done (on
feat/sessions-durable-store, stacked): rollout item 1,rescoped by decision (2026-07-12) to durable session metadata, not a
durable event log — messages/events are deliberately kept OUT of host
SQLite; the vendor transcript stays the only content store, resumed via
the native session id. Shipped: the
session_metametadata table(title/archive/close overrides survive host restarts, write-through,
loaded at construction, orphan-swept at startup) and generation-tagged
session cursors (
c<generation>-<serial>, minted per tracking) so arebooted host deterministically rejects stale session cursors with
CURSOR_INVALID→ cold path, exactly like host cursors. Deferred withthe event log: causationId parity across reboots and scrollback beyond
the vendor replay window (both pinned as for-now limitations in the
reboot test).
Direction: ACP and the Mastra chat stack are both being retired from
this path. The host serves this surface; the ACP bridge behind it gets
replaced by a direct Claude SDK adapter with no client-visible change, then
removed. Mastra chat stays in the tree only so existing desktop users keep
working — frozen, no new features, and never used by mobile.
Next, in order:
Durable history store— done as durable session metadata +cursor generations, see above.
React bindings and the mobile rewire— done (2026-07-12). Shipped:packages/host-service-react(provider + zustand-backed hooks +useSessionTimelinewith incremental fold), the framework-freetranscript fold at
host-service-sync/timeline(canonical events →message/tool-call/plan items, thought-splitting, subagent nesting,
pending-permission cards), and
packages/host-client'ssessionsmodule (command plane +
syncApi/syncUrlfacades). Mobile's chatscreens keep their shipped UI but ride
sessions.*+/sessions/syncover the relay (verified live end-to-end against a dev host through the
relay tunnel — the relay WS proxy doesn't negotiate subprotocols, so
the mobile socket factory requests none). DELETED with the rewire: the
acpSessions.*router, the per-session/acp-sessions/:id/streamroute,
host-client/acp, session-protocol'sclient/reactsubpaths,and their surface tests.
packages/session-protocolitself shrinks tothe ACP-bridge contract (acp/api/envelope/fold/state) still imported by
the AcpSessionManager + canonical translator — it goes with item 3, not
before.
Follow-up shipped (2026-07-13): settings catalogs, pickers, and the
full mobile plane kill. The session entity gained
settingOptions—harness-reported catalogs (
{id, name, kind: model|mode|effort|other, currentValue, options[{value,name,description}]}) composed live fromthe adapter's config options (grouped selects flattened, mode
synthesized from
currentModewhen not mirrored, booleans skipped as av1 reduction). Catalogs ride the composed session (snapshots +
session_upsert), never the event log; the translator's settingschange-key includes the catalogs so a catalog-only rebuild still moves
the log and re-pushes the entity. The mobile composer's mode/model/
effort chips are back (generic
OptionPickeroversettingOptions,writes through
sessions.update), verified on-sim: chip → native sheet→ live model switch reflected in the chip. The NewChatWidget's
new-workspace flow moved off the legacy
workspaces.createagentssugar onto
workspaces.create(no agents) +sessions.create+submitTurn(image attachments ride the first turn as canonical imageblocks), with the widget's model pick passed as adapter aliases (the
shared "claude" preset: fable/opus/sonnet/haiku —
SUPERSET_CHAT_MODELSMastra ids are gone from mobile). And the old planes are now fully
deleted from the phone: the Mastra chat thread screen
(
chat/[sessionId]UI +useChatThread+ cloudchat_sessionsElectric collection + the
HostChatClientchat facade) is gone, thecanonical thread now owns the
chat/[sessionId]route (thechat/acp/path segment is dead), and the session-row menu's rename/archive go
through
sessions.update. Verified end-to-end on-sim: widget → newworkspace → canonical thread → real agent reply, with the picked model
active.
Follow-up shipped (2026-07-13): chat scroll discipline + thread
polish. The
Conversationcomponent now implements the ChatGPT-classscroll contract — the list moves only on user intent: drag-begin kills
following instantly; re-pinning requires a real gesture resting at the
true end (programmatic scrolls also emit momentum-end and must never
re-pin); send anchors the message to the top with whitespace for the
reply (bottom-relative target + corrective settle legs, exact even when
sent from deep in history); streaming, content re-measure dips, the
permission stack appearing/resolving, and the keyboard hiding never
shift an unpinned reader — trailing scroll range is pre-banked as a
native
contentInset.bottomsized against the largest viewport seenplus dip slack (insets fire no layout events, which kills the
spacer-view feedback loop), and every scroll target derives from the
native content height, never LegendList's estimates
(
initialScrollAtEnd/maintainScrollAtEnd/scrollToEndare banned —they chase estimated totals into blank space). Invariants are documented
in
apps/mobile/components/ai-elements/AGENTS.mdand enforced by ahidden regression lab (
superset://debug/conversation-lab+.maestro/flows/conversation-lab.yml) whose five scripted scenarios(pin-follow, no-move-unpinned with dips, overlay resolve, send anchor,
send anchor from history) self-assert PASS/FAIL on-device. Thread
polish riding along: tool rows title with the model-written command
description(raw command stays in the detail sheet), GitHub-style+added/−removed line badges on file-edit tool rows (input-derived,
tested), the running tool title breathes with a seam-free sine pulse,
the working indicator is a caret-sized boids flock (six dots milling in
an elliptical pen, speed-fade alpha, UI-thread
useFrameCallbackportof the web caret), and the glass header title widens to 70% of the
screen before truncating. Cold thread load paints the FULL last page in
one frame:
sessions.getnow resurrects on read so the snapshot alwayscarries the recent tail + real head (the old zero-head fallback made the
hub stream the entire replayed transcript event-by-event — the thread
visibly built up from the first message), the thread holds a spinner
until that one-shot page lands, and the list mounts invisible until the
page is measured and parked at the bottom (quiet-gate + hard cap) —
older history stays
getEventsscrollback territory.Direct Claude SDK adapter behind the same surface (prototype on the
claude-sdk-session-harnessbranch, PR feat(host-service): prototype direct Claude SDK sessions #5582): run both adaptersthrough the same real-model story, switch, then remove the ACP bridge
(AcpSessionManager, translate-acp, and the rest of
packages/session-protocol).Maestro phone replay of the story (execution 4) as the final gate.
Gates:
bun run lintand typecheck exit 0; the story passes on everylane it applies to; teardown leaves no process, socket, DB handle, or temp
directory; the documented memory caps are measured with a transcript beyond
those caps.
E2E user story (the validation spine)
One realistic session, in user terms. It must pass with a real Claude
instance on each surface it migrates across: (1) the shipping ACP surface —
acp-user-story.integration.test.ts; (2)sessions.*+/sessions/sync—sessions-user-story.integration.test.ts; both pass 10/10 with real Opustoday; then (3) the direct SDK adapter, and finally (4) replayed from a real
phone via Maestro.
The gym project
The story runs in a disposable "gym" repo the harness provisions per run
(
packages/host-service/test/helpers/gym.ts):notes.txt— known data (fixture_id,values=13,29,expected_sum=42)that the subagent and workflow steps read, so results are assertable.
.claude/workflows/acp-e2e-dummy.js— the saved five-agent workflow(inspect → analyze → audit ×2 → verify) over
notes.txt..claude/skills/gym-check/SKILL.md— a trivial skill that readsnotes.txtand prints a sentinel.scripts/ok.shandREADME.md— a runnable script and an editable file,so two different tools (Bash vs file edit) can each trigger a
permission card.
Run the story with the model pinned to
opus: the scripted prompts dependon precise instruction-following.
The first message
Sent verbatim as the session's initial prompt (the story tests quote this
block):
The steps
opus,permission mode
default(never bypass).connected devices see the same stream live.
returns to idle, nothing left pending.
continueresumes the same conversation and reachesTOUR_DONE.reject); the session reads "needs attention", not "stuck".
RUN_OK; a second tap gets "already answered", never a doubleexecution.
continue— the edit applieswith no card; every tool call still lands in the transcript.
the turn shows "waiting for you", not running.
exactly once; the agent replies
ANSWERS blue.notes.txt— its own partial-fidelitysubagentthread (never flattened), result flows back asSUB_OK 42.background, and completes verified (
WORKFLOW_VERIFIED).between streams, cards, or attention.
resyncs transparently via the cold path.
transcript; a new message resumes the same native conversation.
the first answer wins everywhere. Final gate: Maestro replays steps
2–19 from the phone against a real host.
https://claude.ai/code/session_016DKKd4kLa3Xd7kGsYfxsyP