feat(terminal): close thread terminals when a thread settles - #4684
SunkenInTime wants to merge 1 commit into
Conversation
Settling a thread means the work is done, but any dev server it started keeps holding its port and CPU. This closes a settled thread's terminals, behind a server setting that defaults to off. Both settle paths are covered, and they need different mechanisms because only one of them is an event: - Explicit settle dispatches thread.settle, so the ws handler closes terminals right after the command is accepted. A settle the decider rejects (active session, pending approval, queued turn start) fails before that point, so blocked work never loses its terminals. - Auto-settle (inactivity window, merged/closed PR) emits nothing at all: effectiveSettled is a clock-derived predicate, and its inputs are a per-client setting and change-request state the server never persists. The client is therefore the only observer of that transition, so it reports it via terminal.close-settled. The report is advisory: the server re-derives canSettle against the thread shell and ignores any report for a thread still holding live work. canSettle and hasQueuedTurnStart move to @t3tools/shared so the server applies the same rules as the client instead of growing another copy of the settle predicates; client-runtime re-exports them, so import sites are unchanged. Terminals close without deleteHistory, so scrollback survives and the terminal can be reopened. Unsettling never restarts anything. Snooze is deliberately excluded — it means "not now", not "done". Co-Authored-By: Claude <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
|
Note 🤖 GPT-5.6 Sol responding on behalf of Theo We're closing this PR as we clean up the T3 Code backlog. Thank you for taking the time to put this together. Settling a thread is reversible and does not mean its terminal process has no useful state. Automatically closing terminals can kill shells, servers, or unsaved interactive work, so the cleanup benefit does not justify the data-loss risk. If you believe we closed this in error, please reopen the PR and leave a comment explaining what we missed. |
Settling a thread means the work is done, but any dev server it started keeps holding its port and CPU. This closes a settled thread's terminals, behind a new server setting
closeTerminalsOnThreadSettle, defaulting to off.Why two mechanisms
Only one of the two settle paths is an event, which shapes the whole design:
Explicit
✓ Settledispatchesthread.settle, sows.tscloses terminals right after the command is accepted. A settle the decider rejects (active session, pending approval, queued turn start) fails before that point, so blocked work never loses its terminals.Auto-settle (inactivity window, merged/closed PR) emits nothing.
effectiveSettledis a clock-derived predicate, and its inputs —sidebarAutoSettleAfterDays(a per-client setting) and change-request state (never persisted server-side; it lives only inVcsStatusBroadcaster's per-cwd memory while a client holds a subscription) — are both invisible to the server. The client is therefore the only possible observer of that transition.So the client reports it via a new
terminal.close-settledRPC. The report is advisory, never authoritative: the server re-derivescanSettleagainst the thread shell and ignores any report for a thread still holding live work. It also skips archived threads and threads explicitly pinnedactive. Idempotent, so several clients reporting the same thread is harmless.Shared predicate instead of a fourth copy
The settle rules were already duplicated across the server projector, the SQL projection pipeline, and the client reducer. Rather than hand-write a fourth,
canSettleandhasQueuedTurnStartmove to@t3tools/shared— the one package both sides already import.client-runtimere-exports them, so no import site changes.effectiveSettleditself stays client-side, since it genuinely depends on data the server lacks.Behavior notes
deleteHistory, so scrollback survives and the terminal can be reopened.activein the data model.settledTerminalCleanupcapability flag, so new clients against old servers send nothing rather than erroring.Testing
apps/web1595 passed;@t3tools/contracts203 passed; lint/fmt clean.Not done
threadListV2truncates the settled list tosettledLimitand returns onlyitems+hiddenSettledCount, so reporting off it would silently cover just the rendered rows. Correct wiring needs a return-shape change plus test updates. The server already accepts reports from any client.🤖 Generated with Claude Code
Note
Close thread terminals when a thread settles
closeTerminalsOnThreadSettleserver setting (defaultfalse) that triggers terminal closure when a thread settles, configurable via a new toggle in the General settings panel.thread.settledispatch, the server closes all thread terminals if the setting is enabled.terminal.close-settledRPC and a corresponding client-side effect in SidebarV2.tsx that reports auto-settled threads to the server; the server validates settle eligibility using sharedcanSettleguards before closing.canSettle,hasQueuedTurnStart, andQUEUED_TURN_START_GRACE_MSinto a new shared module at packages/shared/src/threadSettled.ts for consistent evaluation on both client and server.Macroscope summarized 9795ea3.