Repository navigation
Keep restored terminal output and project scripts from typing into busy shells - #775
Merged
Merged
Conversation
When a terminal view restores output that is no longer live, xterm parses the terminal queries in it again and answers them. The answers go through onData into the live PTY and show up as junk such as ^[[?2026;2$y at the prompt after a reconnect, a reattach or reopening the drawer. Server: the retained-history sanitizer already dropped DSR/CPR, DA and OSC 10/11/12 colour queries. Port T3 Code's additions (pingdotgg/t3code #5319, 1cbd88aba0): DECRQM/DECRPM (...$p / ...$y, ANSI and private modes), XTVERSION (CSI > q), the kitty keyboard query/reply (CSI ? u) and the DECRQSS / XTGETTCAP DCS queries and replies, in 7-bit and 8-bit form. Setters that share final bytes (DECSTR, DECSCL, DECSCUSR, restore cursor, kitty push) and other DCS strings such as sixel stay. Sequences split across PTY chunks keep working through the pending control sequence, including a DCS split between ESC and the backslash of its terminator. Client, as defence in depth for query forms the sanitizer misses: a per-view replay gate in @ryco/client-runtime/state/terminal brackets the writes of a snapshot, a history delta and the output a kept view catches up on after reattaching. xterm parses asynchronously, so the gate is raised and lowered from write callbacks queued around the replayed bytes; replies to live output written before or after still reach the shell. While it is raised, onData that is shaped like a terminal report (DA, DSR/CPR, DECRPM, window and kitty reports, any OSC or DCS string) is dropped; typed input still goes out while a long history parses. The gate is kept with the cached xterm so a late reply after a detach is still caught. Output that arrived while an open was in flight is live and is still answered, so a shell waiting on a startup query (fish's DA1) gets its reply. Electing a single reply owner when several clients view one terminal is left for a follow-up. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Project scripts reused the base terminal when the client's cached running state said it was idle, then typed `<command>\r` unguarded. That state lags the server's subprocess poll, so the command could become input to vim, a dev server or a REPL started moments before. The server-side setup runner and the legacy close fallback (`exit\n`) typed into terminals the same way, and closing a terminal stopped a running command without asking. terminal.write now takes `requireIdle: true`. Under the thread lock, which every input to the thread also takes, the manager refuses with a typed TerminalBusyError and writes nothing when: - the shell is not running or its exit is queued (unverified), - input arrived within 500 ms, before a fresh process table could show the command it submitted (inputRaced), - the PTY's foreground is not the shell's idle name, e.g. `exec vim`, or a process-table read requested for this write finds a child (running), - that read fails (unverified); unlike polling, a failed read is never taken as idle. A shell with no input since it started runs only its startup files, as a fresh terminal does, so it is written without a read. Empty ps and PowerShell tables now count as failed reads for polling too. TerminalWriteInput is not strict, so older nodes would drop the key and write unguarded. Nodes advertise `capabilities.terminalWriteRequireIdle` and clients rely on it only when the live server config says so: - Project scripts send requireIdle when they reuse the base terminal and run in a new terminal when the node refuses. Without the capability they always use a new terminal. - The setup runner writes with requireIdle, since its terminal id is fixed per thread and script. - The snippet broker's `exit\n` close fallback is sent only with requireIdle, and skipped on nodes without it. - Closing a terminal whose shell runs a command asks first. The three close paths (drawer, workspace panel, close shortcut) now share one helper. Ports Synara 84daea248 (#1559, onlyIfIdle / assertSessionIdle) and the close confirmation from T3 Code 0508792c57 (pingdotgg/t3code#7592), limited to terminals with a running command. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
- Focus reports and query replies the emulator sends on its own no longer count as input for the requireIdle settle window or the no-input exemption, so a script run that moves focus right before its write is not refused as inputRaced. The timestamp now resets when a shell starts. - The terminal reply classifier moves to @ryco/shared/terminalReports and serves both the client replay gate and the server. - Project scripts and the close fallback read the terminalWriteRequireIdle capability through readNodeServerConfig, so a hosted node that is not the primary counts and the primary's projected config is only a fallback. Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
sak0a
force-pushed
the
fix/terminal-replay-and-idle-writes
branch
from
October 23, 2026 15:28
e7e9972 to
155f116
Compare
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What Changed
…$p/…$y), XTVERSION (CSI > q), the kitty keyboard query and reply (CSI ? u), and DECRQSS/XTGETTCAP DCS queries and replies (7-bit and 8-bit). Setters that share a final byte (DECSTR, DECSCL, DECSCUSR, restore-cursor, kitty push) and other DCS strings such as sixel stay in history.client-runtime/state/terminal/replayGate.ts). While a view replays a snapshot, a history delta or output buffered while it was detached, xterm's replies to queries in that output are dropped instead of being sent to the shell. Keystrokes typed during a replay still go through.@ryco/shared/terminalReports, shared by the client gate and the server.terminal.writeacceptsrequireIdle. Under the thread lock, the server refuses withTerminalBusyError(running,unverified,inputRaced) and writes nothing unless a fresh process-table read finds the shell idle. Focus reports and query replies do not count as input for this check.capabilities.terminalWriteRequireIdle. Clients read it from the node's own config, so hosted nodes that are not the primary count too.requireIdleand fall back to a new terminal when the node refuses. On nodes without the capability they always get a new terminal.requireIdle. Theexit\nclose fallback is sent only withrequireIdle.psor PowerShell process table now counts as a failed read, including for activity polling.Why
^[[?2026;2$yappeared at the prompt. Ports fix(server): strip replayable terminal queries from history pingdotgg/t3code#5319 (strip list) and the replay detach idea from feat(web): render terminals with libghostty-vt pingdotgg/t3code#4860.inputRaced.Testing
bun run check: pass (format, lint 0 errors, typecheck:fast)RYCO_TEST_MAX_WORKERS=4 bun run check:changed: format, lint, TS7 + Effect typecheck and all affected packages pass, exceptapps/webservice.hostedPrimary.test.ts, which timed out after 5 s under load. Rerun alone twice, it passed 4/4 both times; this PR does not touch that code.RYCO_TEST_MAX_WORKERS=4 bun run --cwd apps/server test -- src/terminal/Layers/Manager.test.ts: 73 passed. The new focus-report test fails when every write updates the settle timestamp.RYCO_TEST_MAX_WORKERS=4 bun run --cwd apps/server test -- src/terminal/Layers/Manager.test.ts src/project/Layers/ProjectSetupScriptRunner.test.ts src/environment/Layers/ServerEnvironment.test.ts src/agentControl/Mcp/hostedBoundaries.test.ts: 79 passed (before the review fixes)bun run --cwd packages/shared test -- src/terminalReports.test.ts: 4 passedbun run --cwd packages/client-runtime test -- src/state/terminal: 37 passedbun run --cwd packages/contracts test -- src/terminal.test.ts src/environment.test.ts: 40 passedbun run --cwd apps/web test -- src/terminalRequireIdle.test.ts: 3 passed. 2 fail with the old config lookup.bun run --cwd apps/web test:browser -- src/components/TerminalReplay.browser.tsx: 6 passedbun run --cwd apps/web test:browser -- src/components/ChatView.Workspace.browser.tsx -t "project script|closing a terminal|project scripts|asks before closing": 5 passedFollow-ups
terminal.openrelaunches a live shell when cwd or env differ, which can stop a running command without asking.readlooks idle to the process table.🤖 Generated with Claude Code
Need help on this PR? Tag
@codesmith-botwith what you need.