Repository navigation
Conversation
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a focused server-side bug fix that preserves normal shell-stream behavior while allowing repeated projection read failures to surface through the existing retryable error path. A targeted regression test covers the failed subscription and subsequent replay, with no schema, deployment, default, or static-analysis changes. Notes:
You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughShell projection reads now retry once and propagate persistent failures as ChangesShell projection error handling
Priority: ⬆️ High Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: High Suggested reviewers: Merge Risk: ⚪ Minimal · up to The shell subscription can retry from its last applied update after a projection-read failure. No outstanding issue identified here blocks merging after normal checks. Architecture SummaryArchitecture risk: 🟡 Medium · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
Reliability and maintainability
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Thanks for picking this up. The trace for #14046 is now posted in #14046 (comment), and it doesn't match the path this PR fixes.
With this PR applied, that occurrence would play out the same way. Dropping a shell update after two failed reads may still be worth fixing on its own, since it could produce the same symptom. Could you change |
Dismissing prior approval to re-evaluate fdb820e
|
Note Written by Hi! We are cleaning up open PRs, and this one appears to have been created with an older model ( |
Problem
A fast first turn can finish while the new-thread draft is waiting for its shell update. If the server's coalesced shell projection read fails twice,
subscribeShellcurrently drops that update. The client then has no reason to subscribe to the thread detail, so the draft can remain on “Thinking” until reload.Fix
Keep the immediate projection read retry. If it also fails, return a typed subscription error instead of dropping the update. The client retries from its last applied shell cursor and replays the missing thread update. A regression test covers the failed subscription and successful replay.
Related to #14046. The reported trace shows a separate client IndexedDB failure, so this server-side fix does not close that issue.
Validation
vp test run apps/server/src/server.test.ts -t 'subscribeShell (replays a shell update|retries a transient)'(2 passed)vp run --filter t3 typecheckvp lint apps/server/src/ws.ts apps/server/src/server.test.ts(passed; existing warnings in the test file)git diff --check upstream/main...HEADGenerated by GPT-6-Sol in Codex via T3 Code.
Summary by CodeRabbit