Repository navigation
fix(host-service): classify worktree-gone terminal failures as NOT_FOUND - #6387
Conversation
📝 WalkthroughWalkthroughThe terminal error translator now maps errors that begin with ChangesTerminal error classification
Estimated code review effort: 1 (Trivial) | ~5 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/host-service/src/trpc/router/terminal/terminal.ts (1)
25-34: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd focused tests for the new classification.
This branch changes the
TRPCErrorcontract. Add tests for the exact prefix, the prefix followed by additional details, and similar messages that do not match. AssertNOT_FOUNDandcause.kind === "WORKTREE_GONE"for matching messages. Verify that existing mappings remain unchanged.🤖 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/trpc/router/terminal/terminal.ts` around lines 25 - 34, Add focused tests for the terminal error-classification logic covering the exact “Workspace worktree no longer exists” prefix, that prefix with additional details, and similar non-matching messages. Assert matching cases produce TRPCError code NOT_FOUND with cause.kind equal to WORKTREE_GONE, and confirm existing error mappings remain 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.
Nitpick comments:
In `@packages/host-service/src/trpc/router/terminal/terminal.ts`:
- Around line 25-34: Add focused tests for the terminal error-classification
logic covering the exact “Workspace worktree no longer exists” prefix, that
prefix with additional details, and similar non-matching messages. Assert
matching cases produce TRPCError code NOT_FOUND with cause.kind equal to
WORKTREE_GONE, and confirm existing error mappings remain unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: ab73d99b-b355-4170-818a-e65a535128fc
📒 Files selected for processing (1)
packages/host-service/src/trpc/router/terminal/terminal.ts
There was a problem hiding this comment.
No issues found across 1 file
You’re at about 99% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Problem
HOST-SERVICE-1N (14 events on 1.20.2): when a workspace's worktree directory is deleted while a terminal pane is still open, every
terminal.snapshotpoll throwsINTERNAL_SERVER_ERRORand the Sentry middleware reports it as a 500 — even though a worktree removed underneath an open terminal is a routine lifecycle condition, not an internal failure.Root cause
createTerminalSessionInternal(the adoption path undersnapshotSession/writeFramedInputToSession) returns the plain string errorWorkspace worktree no longer exists: <path>when the worktree directory is gone. The router'stoTerminalIoErrorhelper name-checks a few known messages ("belong" → FORBIDDEN; "not found"/"not active"/"exited" → NOT_FOUND), but this message matches none of them, so it falls through to theINTERNAL_SERVER_ERRORbranch and the dumb 500=report Sentry middleware files it on every poll.No typed error class crosses this boundary — the runtime returns
{ error: string }— so the classification has to match the stable message prefix.Fix
Add a branch to
toTerminalIoErrorthat recognizes theWorkspace worktree no longer existsprefix and returnsNOT_FOUNDwithcause: { kind: "WORKTREE_GONE" }. The message and all other behaviour are unchanged; genuine unexpected terminal I/O failures still fall through toINTERNAL_SERVER_ERRORand keep reporting.All terminal procedures that funnel errors through this helper (
send,snapshot) pick up the fix; there are no per-procedure copies.Verification
No existing tests cover this helper (
grep -rl "toTerminalIoError" **/*.test.ts→ no matches), so no test run applies.Refs HOST-SERVICE-1N
https://claude.ai/code/session_0c69890b-c39f-420c-b03a-8038231a89ad
Summary by cubic
Classifies "Workspace worktree no longer exists..." terminal errors as NOT_FOUND in
host-serviceinstead of INTERNAL_SERVER_ERROR, addressing HOST-SERVICE-1N and stopping false 500s/Sentry alerts. Old:terminal.snapshotandsendreturned 500; New: they return 404 with cause { kind: "WORKTREE_GONE" }; message unchanged.Review notes
toTerminalIoError; both procedures inherit it; other errors keep their codes.Migration
Written for commit a0e24d8. Summary will update on new commits.
Summary by CodeRabbit