Skip to content

Keep restored terminal output and project scripts from typing into busy shells - #775

Merged
sak0a merged 3 commits into
mainfrom
fix/terminal-replay-and-idle-writes
Oct 23, 2026
Merged

sak0a merged 3 commits into
mainfrom
fix/terminal-replay-and-idle-writes

Conversation

@sak0a

@sak0a sak0a commented Oct 22, 2026 •

Copy link
Copy Markdown
Collaborator

What Changed

  • Server history sanitizer now also strips DECRQM/DECRPM (…$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.
  • New client replay gate (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.
  • The reply classifier lives once in @ryco/shared/terminalReports, shared by the client gate and the server.
  • terminal.write accepts requireIdle. Under the thread lock, the server refuses with TerminalBusyError (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.
  • Nodes advertise capabilities.terminalWriteRequireIdle. Clients read it from the node's own config, so hosted nodes that are not the primary count too.
  • Project scripts reuse the base terminal only with requireIdle and fall back to a new terminal when the node refuses. On nodes without the capability they always get a new terminal.
  • The setup script runner writes with requireIdle. The exit\n close fallback is sent only with requireIdle.
  • Closing a terminal that is running a command asks first. The drawer, workspace panel and close shortcut share one close helper.
  • An empty ps or PowerShell process table now counts as a failed read, including for activity polling.

Why

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, except apps/web service.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 passed
  • bun run --cwd packages/client-runtime test -- src/state/terminal: 37 passed
  • bun run --cwd packages/contracts test -- src/terminal.test.ts src/environment.test.ts: 40 passed
  • bun 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 passed
  • bun run --cwd apps/web test:browser -- src/components/ChatView.Workspace.browser.tsx -t "project script|closing a terminal|project scripts|asks before closing": 5 passed

Follow-ups

  • When several clients show one terminal, each live view answers live queries. Choose one reply owner per terminal.
  • terminal.open relaunches a live shell when cwd or env differ, which can stop a running command without asking.
  • A partly typed command at an idle prompt still gets the script command appended. A server check cannot see this.
  • The close confirmation trusts the client's running state. A server-verified close would need the UI to wait before removing the terminal.
  • A shell blocked in a builtin such as read looks idle to the process table.

🤖 Generated with Claude Code


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:L labels Oct 22, 2026
sak0a and others added 3 commits October 23, 2026 17:28
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
sak0a force-pushed the fix/terminal-replay-and-idle-writes branch from e7e9972 to 155f116 Compare October 23, 2026 15:28
@sak0a
sak0a merged commit e210b43 into main Oct 23, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:L vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant