fix(client-runtime): threads no longer stay on "Syncing messages..." after a failed resume

[shivamhwp]

Problem

A saved thread can remain stuck on “Syncing messages…” when replaying events from its saved cursor fails. Relaunching asks for the same failed replay again.

Change

Retry a failed resumed subscription once using a full thread snapshot. Keep snapshot failures terminal so recovery cannot loop, and use the new snapshot cursor for later sessions. Report a replay failure only if recovery gives up, avoiding an error banner during successful snapshot recovery.

Scope and approval

This is a maintainer-requested takeover of the focused thread-sync bug fix. The shared client runtime serves web, desktop, and mobile. Refs #19563. Based on the original work by @shivamhwp.

The original report’s precise failing event is unconfirmed. #15937 already handles unknown event types on the server; this change adds client recovery for other replay defects.

Verification

49 focused sync tests, the client-runtime typecheck, and scoped lint/format checks pass. The two regression cases verify snapshot recovery, later cursor-based resumption, and terminal snapshot failure. All observed recovery states are checked for the absence of a transient error. Reintroducing transient error publication or retrying every defect fails the corresponding test.

The actual web app was tested against the same temporary one-shot replay failure on baseline 07d38b8d and rebuilt head e4c9b469. The baseline requested cursor 18 and remained at “Syncing messages…”. The head requested cursor 18, recovered through a full snapshot, then successfully resumed from cursor 18 on a later reconnect. No error banner was observed during recovery. The temporary server probe was removed afterward; no provider turn or database write was needed.

This was a synthetic replay failure, not a reproduction of the original Interrupted / ClientAbort report. Native clients were not run; they share the changed client runtime. The terminal double-failure path was verified by focused tests, not the recording.

Before — cached thread remains syncing

After — snapshot recovery clears syncing

Recovery recording

Taken over with Claude Opus 5.5 (tests and review), Claude Fable 5.1 (implementation), and GPT-6.1 Sol (review) through T3 Code; coordinated with Codex.

[macroscopeapp]

Approvability

Verdict: Approved at e4c9b46

Macroscope's review found this PR approvable — This is a narrowly scoped client-runtime bug fix that recovers cached threads through one full snapshot after a failed replay, while keeping unrecoverable snapshot failures terminal. Regression tests cover recovery, error suppression, cursor advancement, and the double-failure case without changing schemas, defaults, or static-analysis settings.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Path: .coderabbit.config.ts
• Review profile: CHILL
• Plan: Advanced
• Run ID: c7e07d7a-923a-4945-9fff-c00dbd1d8c34

📥 Commits

Reviewing files that changed from the base of the PR and between 4aceeef and e4c9b46.

📒 Files selected for processing: 2

• packages/client-runtime/src/state/threads-sync.test.ts
• packages/client-runtime/src/state/threads.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

📝 Walkthrough

The thread subscription tracks snapshot and resume attempts. A defect during resumed replay triggers a full-snapshot retry. A defect during the snapshot attempt reports a synchronization error and does not trigger another subscription.

Changes: Thread synchronization

Layer / File(s)	Summary
Subscription retry behavior packages/client-runtime/src/state/threads.ts	Subscription attempts track whether they load a snapshot or resume from existing thread data. A defect during resumed replay forces a full-snapshot retry. A snapshot-attempt failure does not use this retry path.
Defect recovery tests packages/client-runtime/src/state/threads-sync.test.ts	The test harness distinguishes stream defects from typed errors and records each subscription’s afterSequence. Tests cover snapshot recovery after a resumed replay defect, resumption from the reloaded snapshot sequence, and no further subscription after a snapshot defect.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to e4c9b

Resumed thread sync defects now fall back once to a full snapshot, and snapshot failures stay terminal. No actionable merge-blocking risk was found, and the tests cover both the recovery and the terminal path.

Security architecture risk: 🔵 Low · up to e4c9b

The recovery remains scoped to the same thread and environment and uses the existing subscription method. A local defect in the fallback snapshot terminates rather than repeatedly retrying. No introduced security issue was established, although interruption during recovery and transport-reported defects have limited direct verification.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — A defect-inducing stream item can now cause an additional snapshot request for the affected thread-state instance. The inspected path does not select another thread or environment or add write authority; its direct effect is synchronization and local cached state.

Trust Boundaries and Controls

• observed — The fallback passes through the existing session-selected RPC subscription. Authorization failures remain excluded from automatic expected-failure retries in that layer. This is client-side control evidence, not verification of server-side authorization.

Resilience and Maintainability Implications

• inferred — The fallback catch surrounds the complete dynamic subscription rather than one session branch, and interrupt-only causes are not retried. This supports sequential replacement and scoped cancellation rather than intentionally concurrent recovery subscriptions. Runtime interleavings during fallback remain unverified.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Linked Issues check		Issue #19563 requires a cached thread to reach live instead of remaining in Syncing messages..., including after relaunch. The change recovers a resumed replay defect and verifies the new snapshot…	Handle the interruption or ClientAbort termination that can leave cached thread state non-live, or establish with code evidence that the observed termination is represented by the handled defect path. Add an automated test that starts wit…

Passed checks: 3 passed

Check name	Status	Explanation
Out of Scope Changes check		The changes stay in the client thread subscription and its focused tests. Snapshot fallback, cursor advancement, terminal snapshot failure, and suppressed intermediate errors directly support issue #1…
Title check		The title clearly identifies the client-runtime fix and the user-visible failure mode: threads no longer remain on "Syncing messages..." after a failed resume.
Description check		The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the failure, the recovery behavior, linked issue context, focused test results, manua…

Full details: Out of Scope Changes check

Explanation

The changes stay in the client thread subscription and its focused tests. Snapshot fallback, cursor advancement, terminal snapshot failure, and suppressed intermediate errors directly support issue #19563. No unrelated behavior is demonstrated.

Full details: Description check

Explanation

The description includes all required sections: Problem, Change, Scope and approval, and Verification. It explains the failure, the recovery behavior, linked issue context, focused test results, manual verification, limitations, screenshots, and recording.

Full details: Linked Issues check

Explanation

Issue #19563 requires a cached thread to reach live instead of remaining in Syncing messages..., including after relaunch. The change recovers a resumed replay defect and verifies the new snapshot cursor. However, threads.ts handles only die causes and RpcClientDefect causes. For an interruption-only termination, Cause.hasInterruptsOnly(cause) returns the failure without a snapshot retry. The linked issue identifies an interrupted subscribeThread with ClientAbort as the observed failure. The added tests do not cover that path.

Resolution

Handle the interruption or ClientAbort termination that can leave cached thread state non-live, or establish with code evidence that the observed termination is represented by the handled defect path. Add an automated test that starts with cached data, exercises that termination, verifies snapshot recovery reaches live, and verifies later resumption uses the new cursor.

• Fix all pre-merge checks with AI

✨ Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[kvnloo]

Thanks for the focused recovery path and the Android before/after test; the cached-resume defect case is well covered.

One question about the linked report's failure mode: #19563 records subscribeThread ending with Interrupted / ClientAbort, whereas subscribeDynamicMapped only invokes onDefect for Die or RpcClientDefect (packages/client-runtime/src/rpc/client.ts). This PR's fallback flag is set by onDefect, and the new positive test injects Effect.die. Is the interrupted/aborted subscription in the report expected to pass through that same callback? If not, a focused interruption/reconnect reproduction would help distinguish the verified replay-defect fix from the reported stuck-hydration case before treating #19563 as fully closed.

Not requesting a broader redesign—just checking the causal link between the observed trace and this tested recovery. Credit to @hichish for the original trace and @shivamhwp for the focused fix and mobile evidence.

[juliusmarminge]

Taking over this one. I’ll reduce the tests to the useful coverage, update the implementation against current main, and verify it before publishing the changes.

[juliusmarminge]

@kvnloo An interruption-only client cause does not invoke onDefect and still passes through without retry. We have not established that the original report reaches the replay-defect path.

The ClientAbort trace alone does not establish the original cause. A client-side replay decode defect can terminate the subscription and cause the server to observe that abort. The original reporter’s precise failing event remains unconfirmed.

#15937 already addresses unknown event types server-side. This takeover is scoped as client-side recovery for other replay defects: a failed cursor-based replay retries once from a full snapshot; a snapshot defect reports a terminal error. Ordinary client interruption is passed through, and the PR references #19563 rather than claiming its exact cause was reproduced.

At rebuilt head e4c9b469, 49 focused sync tests pass. The two retained regression cases cover recovery without an intermediate error and terminal fallback failure. Reintroducing the intermediate error or retrying every defect fails the corresponding case.

The actual web app was also checked with the same one-shot synthetic replay failure on baseline 07d38b8d and rebuilt head: baseline stays syncing, while the head retries a full snapshot and successfully resumes at the saved cursor on reconnect. Recovery recording. This verifies that recovery path, not the exact cause of the original report; native clients were not run.

Dismissing prior approval to re-evaluate e4c9b46

[juliusmarminge]

The linked-issue warning identifies an unresolved part of #19563, not a claim this PR makes. The body uses “Refs,” explicitly says the original Interrupted/ClientAbort cause is unconfirmed, and labels the live proof as an injected replay defect. It does not claim to close that issue.

The retry branch deliberately passes interruption-only causes through. Retrying cancellation would also restart intentional subscription teardown. Failed resumed replay instead requests a full snapshot; a failed snapshot remains terminal. The two focused regressions and the recorded web transport probe cover that narrower behavior, including the subsequent cursor-based resume and absence of a transient error.

I am declining the request to add ClientAbort recovery to this PR without a demonstrated causal path. The original issue remains open for that investigation; expanding the retry predicate based only on its server-side symptom could introduce unwanted subscriptions during teardown.