[Pyseph]

Problem

On a Windows host, typing exit (or Ctrl+D, or running a command that ends the shell) in a T3 terminal prints the shell's output at once, but the terminal only reports exited about 1.0 s later. In the drawer, "Process exited" and the tab auto-close wait that second; the onboarding install terminal and the mobile terminal screen leave the same second late.

node-pty's Windows backend cannot see the end of ConPTY's output pipe (ConPTY keeps it open until the pseudoconsole is closed, and node-pty has already dropped the handle when the exit callback runs). So after the shell exits it waits FLUSH_DATA_INTERVAL for trailing output and only then destroys the socket, whose close event emits exit. That interval is 1000 ms.

Measured through the real terminal Manager and NodePtyAdapter on Windows 11 (build 26200), Node 26.12.1, exit typed to the exited event, node-pty 1.2.0-beta.15 as T3 ships it:

shell	main	this PR
PowerShell 5.1	1,034 ms (min 1,024, n=10)	138 ms (min 123, n=10)
cmd	1,016 ms (min 1,007, n=10)	108 ms (min 106, n=10)

The remaining 100 to 140 ms is the shell itself exiting plus the 100 ms window.

Change

Only patches/node-pty@1.2.0-beta.15.patch changes (T3 already patches this file for watch mode and for the console-list helper), plus the regenerated patch hash in pnpm-lock.yaml and a unit test. In lib/windowsPtyAgent.js:

1. FLUSH_DATA_INTERVAL goes from 1000 to 100.
2. When the timer fires, the cleanup runs one setImmediate turn later, and only if no data event re-armed the timer in between.

Why 1000 ms exists: node-pty set 20 ms in 2018 (e4c294a) and raised it to 1000 ms in 2020 (e430d82e, "Host conout socket in a worker", the fix for the ConPTY close deadlock, microsoft/node-pty#375). The commit message says ConPTY "can send a final screen update after ClosePseudoConsole", and the conout worker added a second pipe hop. The value was never tuned or measured again.

What it protects is trailing output that conhost renders after the shell process has exited. Over 240 sessions per shell with the stock timer, data still arrived after the exit callback in 240/240 cmd sessions (median 12 ms, max 20 ms) and in 123/240 PowerShell sessions (median 2.8 ms, max 8 ms). 100 ms is 5x the longest tail seen.

Why lowering the constant alone is not safe: Node runs timers before the poll phase. If the main thread is blocked for longer than the interval right after the exit callback (T3 sees sporadic spawn() blocks of 85 to 329 ms and 100 to 200 ms terminal restores), the cleanup timer fires first and outSocket.destroy() discards output that is already sitting in the pipe. cmd prints its last line about 12 ms after the exit callback, so it is lost. With only the constant lowered, cmd's final line was lost in 20/20 runs once the stall reached the interval (stall 150 ms at interval 100; 60 ms at 50; 400 ms at 250). The extra setImmediate turn lets the poll phase read what is buffered first (every chunk re-arms the timer) before the decision, so a stall costs only time. It also removes the same failure mode from stock at a 1.1 s stall (cmd lost its last line in 18/20 runs on stock below).

Linux and macOS are unaffected: node-pty/lib/index.js loads windowsTerminal (and so windowsPtyAgent.js) only when process.platform === 'win32'. Nothing in apps/ or packages/ imports the agent file except tests.

The event-driven fix is upstream: microsoft/node-pty#990 and #991 (both open, unreviewed) close the pseudoconsole on natural exit so the pipe EOF ends the session about 60 ms after the shell exits. That needs a native conpty.cc change and new prebuilds, which T3 cannot take through patches/. If either lands, this patch should be dropped.

Scope and approval

I described this first in Ideas discussion #30872. There's no maintainer reply there yet, so I'm submitting it as a small, focused fix for an obvious defect: the terminal reports that the shell exited a second after it did, because of a fixed wait in node-pty's Windows agent. The change is two edits to lib/windowsPtyAgent.js inside the patch T3 already carries, plus its regenerated lockfile hash and a test in apps/server/src/terminal. No product source changes. It does change timing, as described below. If you'd rather settle it in the discussion first, I'm happy to close this.

Behaviour differences, stated plainly:

• exited is emitted about 0.9 s earlier on Windows for every natural shell exit.
• Output that conhost produces more than 100 ms (plus one loop turn) after the exit callback is no longer delivered. Not reproduced here (see below), but a pathologically slow or starved conhost could do it; before, the limit was 1000 ms.
• Windows builds before 24H2, ARM, PowerShell 7 and useConptyDll were not tested (useConptyDll skips this timer entirely).

Verification

Harness: the real apps/server/src/terminal/NodePtyAdapter.ts and Manager.ts on the installed, patched node-pty (vp i regenerated the lock hash; node_modules/node-pty/lib/windowsPtyAgent.js has the change), real PowerShell 5.1 and cmd, with a stock copy of the package (patch state of main) alternating with the patched one. A lost line means the unique last line was missing from the output; for big writes the exact count of payload characters is also checked. Stalls are injected with Atomics.wait on the main thread right after the exit callback.

Results (lost = unique last line, or any payload character, missing from the output; time = exit callback to exit/exited, median and min..max):

scenario (patched unless noted)	PowerShell 5.1	cmd
exit typed to exited through the Manager, stock	1,034 ms (1,024..1,040), n=10	1,016 ms (1,007..1,017), n=10
same, patched	138 ms (123..140), n=10	108 ms (106..111), n=10
final line then exit, adapter, stock	1,010 ms, 0/30 lost	1,022 ms, 0/30 lost
same, patched	111 ms (101..116), 0/60 lost	119 ms (103..121), 0/60 lost
1 MB / 10 MB / 50 MB then exit	0/30, 0/15, 0/5 lost	0/30, 0/15, 0/5 lost
output, 300 ms pause, last line, exit	0/20 lost	0/20 lost
main thread blocked 30 / 150 / 400 / 1,100 ms right after the exit callback, final line	0/20 each	0/20 each
same through the Manager (stalls 0 / 150 / 400 / 1,100 ms)	0/10 each	0/10 each
10 MB with a 400 ms stall	0/10 lost	0/10 lost
stock with 150 / 400 / 1,100 ms stalls	0/20 each	0/20, 0/20, 18/20 lost at 1,100 ms
40 busy loops at BelowNormal (CPU-starved conhost), final line	0/60 lost (stock 0/40)	0/60 lost (stock 0/40)
same, 10 MB / slow tail / Manager final line	see below	0/15, 0/20, 0/15 lost

Under the CPU-starved run the PowerShell Manager, 10 MB and slow-tail rows did not complete: PowerShell 5.1 itself took 5 to 14 s to start and run a command, so the harness timed out before sending exit (12/15 Manager runs, 2/4 big runs) and the run was stopped at its time limit. No completed run in that cell lost output, but it is not evidence for those rows.

Raw rows and harness: https://gist.github.com/Pyseph/4e3c3d1af2c7a495d9833b06bc133474 (m-*.jsonl holds the 768 matrix rows and load-*.jsonl the CPU-starved runs; hmgr.mjs and stress.mjs are the harness, run-matrix.sh and run-load.sh the drivers).

Unit tests (apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts, run on any OS because the agent file is plain JS; they replace the global timer functions with a tiny loop that places the poll phase between the timer and check phases): cleanup 100 ms after the exit and not before; more data restarts the window; buffered data read right after a stalled timer expiry keeps the socket open; cleanup after a stall when nothing was buffered; nothing runs with useConptyDll. All 5 pass on this branch; run against the unpatched agent file (main's patch) 4 of 5 fail. vp test run src/terminal in apps/server on Windows, Node 26.12.1: 5 files, 105 tests pass (this includes NodePtyAdapter, the terminal Manager and nodePtyConsoleList, the other test that loads the patched agent). vp fmt and vp lint on the new test are clean; typecheck was not run.

Not checked: a UI run (the lingering tab is read from ThreadTerminalDrawer.tsx, not observed; no screenshots), Windows before 24H2, ARM, PowerShell 7, the packaged Electron server, Defender-off, and real (non-injected) server stalls.

Claude Sonnet 5.5 and Claude Opus 5.5 in T3 Code (Claude Code harness).

[macroscopeapp]

Approvability

Verdict: Approved at 118f428

Macroscope's review found this PR approvable — Windows terminals report exit sooner by shortening the output-flush delay, while deferring cleanup to let buffered output arrive. The scoped change is limited to terminal shutdown timing and retains output-driven postponement of cleanup.

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

[coderabbitai]

Important

Review skipped

Review was skipped as selected files did not have any reviewable changes.

Files ignored due to path filters: 1

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: a6d7695
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: eb6b1aa1-fc18-4613-b46a-8548e3e7e4ee

Commits

Reviewing files that changed from the base of the PR and between 391d864 and de2c2ad.

Files ignored due to path filters: 1

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• Trigger review

No actionable comments were generated in the recent review.

Walkthrough

The Windows node-pty patch shortens the output flush interval from 1000 ms to 100 ms and defers cleanup by one event-loop turn. Tests simulate event-loop phases and verify output-socket cleanup timing.

Changes: Windows node-pty output cleanup

Layer / File(s)	Summary
Flush timeout and cleanup behavior patches/node-pty@1.2.0-beta.15.patch	The flush interval changes to 100 ms. Cleanup runs on the next immediate turn and skips stale timeouts when a later flush re-arms the timeout.
Flush timing and cleanup tests apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts	Tests simulate timers, polling, and immediates. They check that the socket remains open at 99 ms, is destroyed at 100 ms, and is not destroyed immediately when buffered output arrives after a stall. They also check cleanup after a stall without new output.

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

Change: Bug fix

Merge Risk: Minimal · previous review 391d8

This change makes Windows terminals report shell exit much faster, about 100 ms instead of about 1 second. No concrete defect was found. Late output arriving after the shortened window could be dropped, which is a stated tradeoff.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the primary change: Windows terminals report shell exit about 0.9 s sooner, backed by the PowerShell timing figures. It follows the conventional commit format. The title does not menti…
Description check		The description follows the template. It has Problem, Change, Scope, and Verification sections, and each is filled in with specific detail. Verification gives focused tests, measured results, and the …
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 primary change: Windows terminals report shell exit about 0.9 s sooner, backed by the PowerShell timing figures. It follows the conventional commit format. The title does not mention the test or the lockfile update, which is expected.

Full details: Description check

Explanation

The description follows the template. It has Problem, Change, Scope, and Verification sections, and each is filled in with specific detail. Verification gives focused tests, measured results, and the unchecked areas. It also names the agent model and harness. The Scope section explains the missing maintainer approval and links a discussion. The description does not repeat the template's conventional-commit title guidance, which is a minor point.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 3883ed5
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: 183ea478-2d00-4606-b517-6b8976122285

Commits

Reviewing files that changed from the base of the PR and between b06db36 and 391d864.

Files ignored due to path filters: 1

• pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml

Files selected for processing: 1

• apps/server/src/terminal/nodePtyFlushDataAndCleanUp.test.ts

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

[juliusmarminge]

Evidence

On the same Windows Server 2025 build 26100 machine with PowerShell 5.1 and Node 26.13.1, run 15 fresh terminals per commit through the real server's WebSocket RPC. Wait for the prompt, wait 300 ms, submit echo FINAL_LINE_<id>; exit, and measure receipt of final output and exited.

Before	After
3883ed544c	391d864e9a
Measurement output: 15 runs	Measurement output: 15 runs
Exit median 1,123.2 ms, range 1,098.4–1,252.3 ms	Exit median 211.8 ms, range 198.9–303.2 ms
Final-output median 109.4 ms; 15/15 final lines before exit; all exit codes zero	Final-output median 98.0 ms; 15/15 final lines before exit; all exit codes zero

The median improves by 911.4 ms (81.1%), exceeding the combined run spreads of 258.3 ms. Head's slowest run is faster than base's fastest run.

Measurement driver used for both commits.

Not shown:

• Exact Windows 11 build 26200 timings were not measured; the available machine runs Windows Server 2025 build 26100.
• cmd timing was not measured; the server selected PowerShell and the client has no shell selector.
• Visible terminal-tab closing was not observed; measurement ends at the client protocol exited event.
• Output preservation during a deliberately stalled server event loop was not exercised.

[juliusmarminge]

Evidence

On the same Windows Server 2025 host (10.0.26100), Node 26.13.1 and PowerShell 5.1, start each commit's real server with fresh isolated state. For 15 new terminals per side, wait for the prompt plus 300 ms, submit echo FINAL_LINE_<id>; exit, and measure final output and exited over the authenticated client WebSocket.

Before	After
All 15 base samples	All 15 head samples
a6d7695064	de2c2ad420
Exit median 1,126 ms, range 1,112–1,173 ms	Exit median 212 ms, range 200–232 ms
Final-line median 99 ms; installed flush interval 1,000 ms	Final-line median 89 ms; installed flush interval 100 ms

The median improves by 914 ms (81.2%), exceeding both run spreads (61 and 32 ms). All 30 runs delivered the complete final output line before exited and returned exit code 0.

Measurement driver

Not shown:

• The exact Windows 11 build 26200 / Node 26.12.1 figures were not measured; the available Windows Server 2025 / Node 26.13.1 host measured 212 ms on head.
• Cmd timing was not measured; the reproduced client path selected PowerShell and exposes no shell selector.
• Terminal drawer closing was not observed visually; measurements cover the real client-protocol exited event.
• Watch-mode helper handling and output preservation during a long event-loop stall were not exercised by this reproduction.