You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
fix(server): clearing a terminal that keeps printing no longer hangs - #31752
Clearing a terminal that keeps printing could hang until the output stopped. While it waited, it held the terminal thread lock, so every open, resize, attach and close for that thread's terminals waited too.
The path: the mobile Clear button calls terminal.clear, which runs TerminalManager.clear in apps/server/src/terminal/Manager.ts. That function called persistHistory, which queues an immediate history write and then calls persistWorker.drainKey. drainKey waits until the key is completely idle, and KeyedCoalescingWorker.test.ts tests that it waits for work queued during processing. A busy shell keeps calling queuePersist from drainProcessEvents. If output lands during the clear's write, the next round is a debounced write, output lands during its 250 ms sleep, and so on, so the key never goes idle and clear never returns. Examples: a build streaming logs, tail -f on a busy log, or a fast spinner.
Change
clear now queues the cleared history as an immediate write and does not drain the worker. It doesn't need to wait: everything that reads the history file flushes it first (open before readHistory, and closeSession through persistHistory after the process is stopped). Close and restart still drain, but they stop the process first, so their drain always finishes.
Scope and approval
This is a small fix for an obvious hang: one call site in clear changes, plus one test. drainKey keeps its tested semantics. #25192 (clear dropping a queued exit event) touches the lines next to this but not the persist call.
Verification
New test in apps/server/src/terminal/Manager.test.ts: "clears a terminal that keeps printing while its history is saved". Its file system wrapper makes the shell print during each history write.
Upstream main without the fix: the test hangs and fails with Error: Test timed out in 120000ms.
With the fix: vp test run src/terminal/Manager.test.ts passes all 76 tests.
vp lint and vp fmt --check on apps/server/src/terminal pass, effect-tsgo tsrs --noEmit in apps/server reports 0 errors, and the commit hook (fmt, lint, knip) passed.
Not checked: no run in a real mobile client. Also, if the server shuts down in the few milliseconds between a clear and its write, the cleared history can come back after restart. The same window already exists for debounced output writes.
Macroscope's review found this PR approvable — Terminal clearing no longer waits for continuous output to stop before notifying clients. History persistence remains queued, with existing flushes preserved when loading history or closing sessions.
You can add or adjust custom eligibility rules. Learn more.
No actionable comments were generated in the recent review.
Walkthrough
TerminalManager.clear now enqueues an immediate history-persistence request instead of calling persistHistory. A test covers clearing while history saves trigger new terminal output.
Changes: Terminal clear persistence
Layer / File(s)
Summary
Clear persistence and regression test apps/server/src/terminal/Manager.ts, apps/server/src/terminal/Manager.test.ts
clear enqueues an immediate history-persistence request. The test checks that clearing publishes a cleared event while saves trigger terminal output.
Clearing a terminal that keeps printing no longer hangs or blocks other terminal operations. The only remaining edge is a narrow shutdown window that already exists for other writes. Merge risk is minimal.
Pre-merge checks: 4
Check name
Status
Explanation
Title check
The title names the main change: clearing a terminal that keeps printing output no longer hangs. The raw summary confirms that TerminalManager.clear now queues an immediate history write instead of dr…
Description check
The description follows the repository template. It has Problem, Change, Scope, and Verification sections, and each section has substantive content. The Problem section explains the hang and its cause…
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Title check
Explanation
The title names the main change: clearing a terminal that keeps printing output no longer hangs. The raw summary confirms that TerminalManager.clear now queues an immediate history write instead of draining the persist worker, which is the fix behind that title. The title is specific and follows the conventional commit style used in the repository template.
Full details: Description check
Explanation
The description follows the repository template. It has Problem, Change, Scope, and Verification sections, and each section has substantive content. The Problem section explains the hang and its cause. The Change section explains why skipping the drain is safe. The Scope section says it is a small bug fix and does not link an issue, which the template permits for small obvious fixes. The Verification section gives a focused test, a before/after result, and lint and type-check results, and it states what was not checked. The Scope section does not link an issue or a maintainer request, so a reviewer may want that added. Otherwise the description is complete.
Finishing Touches
Generate unit tests (beta)
Create a new PR
Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Reviewing files that changed from the base of the PR and between b21f9bb and fc6d15d.
Files selected for processing: 2
apps/server/src/terminal/Manager.test.ts
apps/server/src/terminal/Manager.ts
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 1 remain after this review.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
size:XS0-9 changed lines (additions + deletions).vouch:unvouchedAuthor is not yet trusted in the VOUCHED list.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
Problem
Clearing a terminal that keeps printing could hang until the output stopped. While it waited, it held the terminal thread lock, so every open, resize, attach and close for that thread's terminals waited too.
The path: the mobile Clear button calls
terminal.clear, which runsTerminalManager.clearinapps/server/src/terminal/Manager.ts. That function calledpersistHistory, which queues an immediate history write and then callspersistWorker.drainKey.drainKeywaits until the key is completely idle, andKeyedCoalescingWorker.test.tstests that it waits for work queued during processing. A busy shell keeps callingqueuePersistfromdrainProcessEvents. If output lands during the clear's write, the next round is a debounced write, output lands during its 250 ms sleep, and so on, so the key never goes idle andclearnever returns. Examples: a build streaming logs,tail -fon a busy log, or a fast spinner.Change
clearnow queues the cleared history as an immediate write and does not drain the worker. It doesn't need to wait: everything that reads the history file flushes it first (openbeforereadHistory, andcloseSessionthroughpersistHistoryafter the process is stopped). Close and restart still drain, but they stop the process first, so their drain always finishes.Scope and approval
This is a small fix for an obvious hang: one call site in
clearchanges, plus one test.drainKeykeeps its tested semantics. #25192 (clear dropping a queued exit event) touches the lines next to this but not the persist call.Verification
New test in
apps/server/src/terminal/Manager.test.ts: "clears a terminal that keeps printing while its history is saved". Its file system wrapper makes the shell print during each history write.mainwithout the fix: the test hangs and fails withError: Test timed out in 120000ms.vp test run src/terminal/Manager.test.tspasses all 76 tests.vp lintandvp fmt --checkonapps/server/src/terminalpass,effect-tsgo tsrs --noEmitinapps/serverreports 0 errors, and the commit hook (fmt, lint, knip) passed.Not checked: no run in a real mobile client. Also, if the server shuts down in the few milliseconds between a clear and its write, the cleared history can come back after restart. The same window already exists for debounced output writes.
Claude Opus 5.5 (Claude Code desktop app)
🤖 Generated with Claude Code