fix(server): only type scripts into a shell that is at its prompt by cleiter · Pull Request #2158 · getpaseo/paseo

cleiter · 2026-07-17T10:28:09Z

Linked issue

N/A - Discussed in Discord

Type of change

Bug fix
New feature (with prior issue + design alignment)
Refactor / code improvement
Docs

What does this PR do

paseo.json scripts are typed into an interactive PTY, and the check for
"is this shell ready for keystrokes" was: has it printed anything, or has 1.5s
passed. Neither is evidence. A shell that prints and then blocks on read
satisfies both, and gets a command typed into whatever is reading.

oh-my-zsh does exactly that on startup:

# what Paseo sent:
[oh-my-zsh] Would you like to update? [Y/n] PASEO_DEV_MANAGED_HOME=1 … ./scripts/dev-daemon.sh
# what actually ran:
$ ASEO_DEV_MANAGED_HOME=1 … ./scripts/dev-daemon.sh

The P answered the prompt; the rest replayed onto the command line. The daemon
came up with a silently wrong PASEO_DEV_MANAGED_HOME. It is luck that the
first character was P — a leading y would have answered "yes, update
oh-my-zsh" instead. Nothing errors; the script just runs wrong.

Four commits, each reviewable on its own:

1. Don't type until the shell's line editor owns stdin. The zsh integration
emits three nonce-tagged OSC 633 markers: R from zle-line-init (line editor
has the line — safe to type), C from preexec (a command took the foreground
— not safe), and readiness is gated on them. zle-line-init, not precmd:
precmd runs before prompt expansion and before any hook a later .zshrc adds,
so a read there still eats the command. At-prompt is tracked as current state,
cleared on C and on exit — which is also what makes reusing a script terminal
safe.

2. Let the shell say whether it can report readiness at all. A shell that
never reports is either blocked (must not type) or has no integration — zsh
< 5.3 has no add-zle-hook-widget, a .zshrc can clobber ZDOTDIR — where
never typing would break every script. Both look like silence. So the .zshenv
wrapper emits 633;I from its first lines, before any user rc file runs:
silence before it means no integration (fall back to the old heuristic, which
is what those shells have today); silence after it means blocked (wait, then
fail). It only announces when it can deliver (is-at-least 5.3).

3. Ask the worker, don't guess from a stale copy. Prompt state lives in the
terminal worker; the daemon holds a copy that lags one IPC hop, so "hasn't
announced" and "announce in flight" look identical there. Before falling back it
re-reads via getPromptState, and the command goes through sendInputIfAtPrompt,
which checks-and-writes in the same tick against the live session — so a
foreground command taking stdin between the check and the write can't get the
keystrokes.

4. Focus a script's terminal instantly, like a plain terminal. A plain
terminal's tab opens the instant its pty exists. A script's did not: the RPC
response was deferred until the command was typed (up to the readiness timeout
on a blocked shell), and the tab-focus rode on that response — so a blocked
start showed nothing, then an error telling you to answer a prompt you couldn't
see. The start now hands the terminal over the moment it is registered
(onTerminalReady) and runs the readiness wait + send detached: the tab focuses
immediately in every case, the connection's message loop no longer blocks, and a
blocked shell is no longer an error — its terminal is on screen showing the
prompt, and the still-running wait types the command once you answer it. Only a
failure before any terminal exists (bad config, already running) reports an
error.

When it can't type, it doesn't. On a readiness timeout the terminal stays
open; plain scripts keep their runtime entry so re-running reuses that same
shell. Rationale, the OSC protocol, and the worker/manager ordering rules are in
docs/terminal-readiness.md.

Known gaps, deliberately not closed here:

bash / fish / custom shells keep the old heuristic and stay exposed. There
is no reliable shell-agnostic PTY readiness signal, so each shell needs its own
integration. The nonce env var is injected for every shell so a bash/fish hook
can adopt the same markers without a protocol change. Happy to follow up.
ZLE widgets that read stdin (fzf on Ctrl-R): preexec doesn't fire, so
at-prompt stays true while fzf owns stdin. zsh exposes no hook; the only sound
signal is the pty's foreground process group, which node-pty doesn't expose.
Fail-open, but it needs someone typing in a script terminal at the instant of
injection.
An announce whose hook then fails to register yields I with no R and fails
the script at the readiness timeout. Fail-closed, never corruption.

How did you verify it

Reproduced the original bug first, then confirmed the fix, using a real zsh
whose .zshrc prints and blocks on read -k 1 (standing in for oh-my-zsh's
update prompt), driving the real spawnWorkspaceScript through the real worker
terminal manager. The script under test writes $PASEO_PROBE_VALUE to a file,
so what the shell parsed is visible, not just what it printed.

Before (and still, on bash — the known gap above):

sent:      PASEO_PROBE_VALUE=intact echo probe-done
terminal:  [omz] Would you like to update? [Y/n] PASEO_PROBE_VALUE=intact echo probe-done
           $ ASEO_PROBE_VALUE=intact echo probe-done      ← the P was eaten

After, on zsh:

run 1 (startup blocked):  refused — nothing typed; terminal stays open at the prompt
answer "n", re-run:       reused the same terminal
                          PASEO_PROBE_VALUE = "intact"     ← command arrived whole

Also verified live in the desktop dev app: started the daemon script with the
omz prompt armed, saw its tab focus immediately showing the [Y/n] prompt,
answered it in the terminal, and the command ran on its own.

Automated — all green locally, plus npm run typecheck, npm run lint,
npm run format clean.

terminal.test.ts — real terminals: the marker sets at-prompt only when the
nonce matches (a foreign nonce is ignored), C clears it, and — the one that
pins the bug — a real zsh blocked on a startup read reports not at-prompt
while its output is already on screen.
worker-terminal-manager.test.ts — prompt state crosses the worker boundary;
re-registration can't roll live state back to the spawn-time snapshot; exit
forces at-prompt false; late markers from a dead shell are ignored.
worktree-bootstrap.test.ts — nothing is typed until at-prompt; a
never-announcing zsh falls back and still runs (the zsh-<5.3 case); an
announced-but-blocked shell fails instead of typing; a shell that loses its
prompt between the check and the write types nothing; an announce still in
flight to the parent does not trigger the fallback.
workspace-scripts-service.test.ts — the start responds with the terminal as
soon as it exists; a blocked shell still hands the terminal over with no
error (exactly one response, not a second error over it); only a
pre-terminal failure reports an error.

Two of those I checked the honest way — reverting the fix makes the test fail and
restoring it passes — so they pin the behavior, not the implementation: the
in-flight-announce fallback, and the single-response guarantee for a blocked
start.

Before:

Terminal recording of the live desktop repro:

Screen.Recording.2026-07-17.at.12.14.19.mov

Checklist

One focused change. Unrelated cleanups split out.
npm run typecheck passes
npm run lint passes
npm run format ran (Biome)
UI changes include screenshots or video for every affected platform
Tests added or updated where it made sense

Paseo runs paseo.json scripts by typing them into an interactive PTY. The readiness check resolved on any output or after 1.5s, which is wrong whenever shell startup prints and then blocks on input. oh-my-zsh's "Would you like to update? [Y/n]" satisfied it, so the command was typed into that prompt: read -k ate the first character and the script ran with ASEO_DEV_MANAGED_HOME=1, not PASEO_DEV_MANAGED_HOME=1. A swallowed "y" would have triggered an omz update instead. Gate injection on a real signal. The zsh integration now emits a nonce-tagged OSC 633;R marker from zle-line-init -- the moment the line editor, and nothing else, is reading stdin. precmd is too early: it runs before prompt expansion and before any precmd hook a later .zshrc adds. The hook registers lazily from precmd because the integration is sourced from .zshenv, where zle is not loaded yet, and uses add-zle-hook-widget so it composes with the user's own widget. at-prompt is tracked as current state, not history: it clears on 633;C and on exit, so reused script terminals wait instead of interleaving keystrokes into a running foreground command. On timeout the script fails rather than typing blind, and the terminal stays open holding the prompt that blocked it. Plain scripts keep their runtime entry so a re-run reuses that same shell -- answer the prompt, run again, and the command lands intact. Service scripts re-plan their port, so their terminal's env would be stale; their entry is dropped. bash/fish have no integration and keep the legacy heuristic. There is no shell-agnostic readiness signal, so each shell needs its own hook; the nonce env var is injected for every shell so one can adopt the marker without a protocol change. See docs/terminal-input-readiness.md. Verified live against real zsh through the worker terminal manager: a blocked startup no longer types, and answering it then re-running lands the full command. The same harness on bash still shows PASEO_ -> ASEO_, which is the gap above.

The readiness gate decided from the shell's filename: basename == "zsh" meant "a marker is coming". That conflates two states that look identical from outside -- a shell blocked on startup input, and a shell that will never report at all. zsh older than 5.3 has no add-zle-hook- widget, and a .zshrc can clobber ZDOTDIR or our hooks; those shells would now fail every script on a 15s timeout, where before they ran. The zsh integration even carried a comment promising the daemon "falls back to legacy behavior". Nothing implemented that: the shell had no way to say the hook was absent. So it says so. The .zshenv wrapper emits OSC 633;I;<nonce> from its first lines -- before the user's .zshenv, before .zshrc, before oh-my- zsh, so nothing user-configurable can delay it, and it only announces when it can deliver (is-at-least 5.3). Announce then silence means blocked: wait, then fail. Silence from the start means no integration: fall back to the old heuristic, which is what those shells already have. Also from review: - Nonce-tag 633;C. It is the marker that says "stop typing", so it must be as unforgeable as the one that says "start". A bare 633;C from foreign output (VS Code's own integration emits it) could strand a script. - Latch the zle hook registration only after it succeeds. Latching first turned one transient failure into a shell that never reports readiness again -- the shape of the resize latch bug in getpaseo#2059. - Key the zsh runtime dir by a hash of its contents. Every daemon writes that shared path, so an older one overwrote the newer integration and shells starting in that window lost their markers. - Drop the pre-registration prompt-state buffer. The worker subscribes before sending terminalCreated and IPC preserves order, so it was unreachable, and it leaked an entry per terminal that never registered. - Await killAndWait in the terminal test teardown. Killing without waiting raced rmSync of the shell's HOME (ENOTEMPTY). - Shorten the docs row so CLAUDE.md's table stops re-padding: 65 lines of formatting churn for one line of content. Verified live against real zsh: a blocked startup still refuses to type and keeps its terminal for the retry, and the command lands intact once the prompt is answered.

Codex review found two ways the readiness gate could still type into a shell that was not ready. Both have the same root: prompt state lives in the terminal worker, and the parent reasons about a copy of it that lags by one IPC hop. Across that gap, "has not announced yet" and "the announce is in flight" are indistinguishable, and a check-then-send pair can straddle a state change. So the parent stops guessing and asks: - getPromptState: before falling back to the legacy heuristic, re-read the state from the worker. Concluding "no integration" from a stale copy took the legacy path and retyped the original bug. The announce grace also goes 1.5s -> 5s: it only needs to cover zsh reaching the first line of our .zshenv, but being wrong types into a possibly-blocked shell, while being slow only costs latency for the rare shell with no integration at all. Scheduling on a loaded machine can exceed 1.5s. - sendInputIfAtPrompt: the worker checks and writes in the same tick against the live session. The readiness wait can only prove the shell WAS ready; a foreground command can take stdin before the write lands. A lost race now types nothing and fails with reason "raced", which keeps its terminal for the retry exactly like a timeout does. Legacy shells send unguarded. They never report a prompt, so guarding their write would mean never typing at all -- the tests caught that immediately. Two findings are documented rather than fixed, both in docs/terminal- readiness.md: preexec does not fire for readers launched by a ZLE widget (fzf on Ctrl-R), so atPrompt stays true while fzf owns stdin. zsh exposes no hook for it and the only sound signal, the pty foreground process group, is not reachable through node-pty. It is fail-open but needs someone typing in a script terminal at the instant of injection. And an announce whose hook later fails to register yields I with no R: fail-closed into the readiness timeout, never corruption. The in-flight-announce test fails with the re-read removed and passes with it, so it pins the fix rather than the implementation.

A plain terminal's tab opens the instant its pty exists, because the create RPC responds right away. Starting a paseo.json script did not: its response was deferred until the command had been typed into the shell, which the readiness gate holds back until the shell is at a prompt -- up to the 15s timeout when a shell is blocked on a startup prompt (an oh-my-zsh update ask). The tab-focus rode on that response, so a blocked start showed nothing for 15s and then surfaced an error telling the user to go answer a prompt they could not see. The terminal exists about 100ms into a script start too. So the start now hands it over the moment it is registered, through an onTerminalReady callback, and runs the readiness wait plus command send detached. The service responds from that callback, so a client focuses the tab immediately in every case -- and the connection's message loop no longer blocks for up to the readiness timeout, which it did before. A blocked shell is no longer an error. Its terminal is already on screen showing the prompt it is waiting for, and the still-running readiness wait types the command on its own once the prompt is answered. Only a failure before any terminal exists (bad config, a script already running) reports a start error; the pre-existing failure toast handles that unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

greptile-apps · 2026-07-17T10:36:02Z

Greptile Summary

This PR fixes a data-corruption bug where paseo.json scripts were injected into a PTY before the shell's line editor owned stdin, causing an interactive startup prompt (oh-my-zsh's update prompt, any read in .zshrc) to eat the first characters of the command. The fix adds a nonce-tagged OSC 633 readiness protocol to the zsh integration: the shell emits I;nonce from .zshenv (before user rc files), and R;nonce/C;nonce from zle-line-init/preexec hooks; the daemon waits for R;nonce before typing, and only falls back to the old heuristic for shells that never announce.

Shell-side changes: .zshenv emits 633;I;<nonce> early (before user rc files) gated on is-at-least 5.3; paseo-integration.zsh adds _paseo_zle_line_init (fires when the line editor takes the line) and nonce-tags C; in preexec to prevent foreign OSC 633 traffic from corrupting state.
Daemon-side changes: terminal.ts adds TerminalPromptState, parses nonce-tagged markers, and exposes fetchPromptState/sendInputIfAtPrompt as atomically-correct operations; worktree-bootstrap.ts replaces the old 1.5 s output-heuristic with waitForTerminalInputReadiness, falls back to legacy only after a 5 s announce grace confirmed via a worker round-trip, and introduces TerminalNotReadyError with a terminalStillOpen flag; workspace-scripts-service.ts fires onTerminalReady immediately so clients can focus the tab before the readiness wait completes.

Confidence Score: 5/5

Safe to merge. The fix is narrowly scoped to the PTY input-injection path, all changed code has matching tests, and the zsh integration falls back gracefully to the old heuristic for shells that never announce.

The implementation correctly handles every edge case discussed in the PR: in-flight IPC announces, write-time races, shell exit before prompt, and legacy non-integrated shells. The subscribe-before-check pattern in waitForTerminalInputReadiness closes the original TOCTOU gap. Tests cover the exact real-world scenario (zsh blocked on startup read) and the atomic sendInputIfAtPrompt operation. The only findings are two unnecessary as never casts in test code.

No files require special attention. The two worktree-bootstrap.test.ts tests with unnecessary as never casts are the only rough edges.

Important Files Changed

Filename	Overview
packages/server/src/terminal/terminal.ts	Core change: adds TerminalPromptState tracking, nonce-tagged OSC 633 marker parsing, and exposes getPromptState/fetchPromptState/sendInputIfAtPrompt on TerminalSession. Subscribe-before-check ordering in emitExit correctly prevents races. The content-hash keyed ZDOTDIR prevents version-clobber races between concurrent daemon versions.
packages/server/src/server/worktree-bootstrap.ts	Replaces the 1.5 s output heuristic with waitForTerminalInputReadiness; introduces TerminalNotReadyError with terminalStillOpen flag for retry-friendly UX; correctly worker-round-trips the announce check to avoid treating in-flight IPC as silence. The settle() guard prevents double-resolution across all racing timers/events.
packages/server/src/terminal/worker-terminal-manager.ts	Correctly keeps promptState out of the info object to prevent re-registration rollback; fetchPromptState round-trips to the worker and silently updates the local copy without notifying listeners (safe because the event was already in flight and will arrive before the response). Exit clears listeners and removes record so late events are ignored.
packages/server/src/terminal/terminal-worker-process.ts	handlePromptStateRequest correctly handles getPromptState and sendInputIfAtPrompt atomically in the worker process. The output flush before emitting prompt-state events preserves ordering for the parent.
packages/server/src/server/session/workspace-scripts/workspace-scripts-service.ts	Correctly decouples terminal hand-off (via onTerminalReady/respondOnce) from the readiness wait; a TerminalNotReadyError after respondOnce is logged at debug level and does not emit a second response. The failStart path only fires when no terminal was handed over yet.
packages/server/src/terminal/shell-integration/zsh/.zshenv	Emits 633;I;nonce before user rc files run, gated on is-at-least 5.3 and an interactive shell check. This correctly lets the daemon distinguish no integration from blocked waiting for input.
packages/server/src/terminal/shell-integration/zsh/paseo-integration.zsh	Correctly uses add-zle-hook-widget (composes, does not replace) for zle-line-init; latches _PASEO_ZLE_HOOK_REGISTERED only after successful registration to avoid permanently silencing a transient failure. The nonce on C; prevents foreign OSC 633 traffic from clearing atPrompt.
packages/server/src/server/worktree-bootstrap.test.ts	Comprehensive behavior tests for readiness, fallback, retry, and write-race scenarios; the StubTerminalManager is well-modelled. Two tests cast their options object as never to pass test-only fields that are declared optional in the interface — the cast is unnecessary and suppresses type-checking on the call site.
packages/server/src/terminal/terminal.test.ts	Real PTY tests correctly verify nonce matching, nonce rejection, at-prompt clearing, and the blocked-startup case that was the root cause. Switching cleanup from session.kill() to session.killAndWait() avoids ENOTEMPTY races on HOME removal.
packages/server/src/terminal/worker-terminal-manager.test.ts	New tests cover prompt-state mirroring, spawn-time seeding, re-registration rollback prevention, exit forcing off-prompt, and late-event ignoring after exit. All test through the session interface, not internal records.

_{Reviews (2): Last reviewed commit: "Quiet the log for a blocked script start..." | Re-trigger Greptile}

The instant-focus change emits the start response from onTerminalReady, not from spawnWorkspaceScript's resolved value. session.test.ts's mock resolved a terminal but never called onTerminalReady, so no response was emitted and the assertion saw an empty message list. The mock now hands the terminal over the way the real launcher does. Missed locally because I ran only the touched test files, not session.test.ts, which injects its own spawn mock. CI caught it.

Two P2s from review. A shell that stops on a startup prompt throws TerminalNotReadyError after its terminal was already handed over. That is a normal interaction, but both the service .catch and spawnWorkspaceScript's catch logged it at error level ("Failed to start/spawn workspace script") — so every script start under oh-my-zsh, the exact case this PR targets, would flood the daemon log with error entries describing the user answering a prompt. The service now returns without re-logging when the terminal was already handed over, and spawnWorkspaceScript logs a readiness stall at debug; only genuine failures stay at error. Also drop a dead optional chain in the worker's sendInputIfAtPrompt: once `sent` is true the session is non-null, so narrow it properly instead of `session?.send`.

greptile-apps · 2026-07-17T10:53:31Z

Want your agent to iterate on Greptile's feedback? Try greploops.

boudra · 2026-07-17T11:04:17Z

@paseo-bot reshape audit

paseo-bot · 2026-07-17T11:07:17Z

Reshape verdict

Verdict: mixed — reshape the start contract before merge
Smear score: 2/4
Baseline: merge-base 9f5f5fce → PR head 614a4526
Gross diff: +1,872 / -91 (net +1,781)
Shape-reviewed additions: +1,872 — earned ~1,800 / avoidable ~70
Neutral bulk: none
Main tension: spawnWorkspaceScript now has two completion points, but its API still models one promise plus an onTerminalReady escape hatch.

Line breakdown

Artifact	Added	Deleted	Net
Production	+786	-78	+708
Tests	+896	-13	+883
Docs/specs	+190	0	+190
Total	+1,872	-91	+1,781

Production shape

Intent	Added	Deleted
New readiness behavior	~620	—
Reshaped existing terminal/script flow	~116	~78
Avoidable callback coordination	~50	—

The low-level shape is good. Prompt ownership lives in the terminal, the worker performs the atomic check/write, and the fallback policy is centralized. The large test addition is mostly earned behavior coverage.

The bolt-on is the handoff path. onTerminalReady lets spawnWorkspaceScript announce an intermediate result outside its promise, so the service has to add responded, respondOnce, failStart, detached rejection handling, and mocks that manually invoke the callback. The function has become a two-phase operation while its return type still says it is one-phase.

One reshape

Return the two phases explicitly:

const spawned = await spawnWorkspaceScript(options); // terminal exists
respond(spawned.terminalId, null);
void spawned.commandStarted.catch(handlePostHandoffFailure);

spawnWorkspaceScript should reject before returning only when no terminal exists. Its returned completion promise should own readiness, command injection, and late cleanup. That removes the callback, the double-response guard, and callback-aware mocks while preserving the exact behavior.

After that, deletion is mechanical: remove the readiness implementation and its terminal/worker registrations. In the current shape, removing instant handoff requires tracing a callback through launcher, service, and mocks.

cleiter and others added 4 commits July 17, 2026 07:02

greptile-apps Bot reviewed Jul 17, 2026

View reviewed changes

Comment thread packages/server/src/server/session/workspace-scripts/workspace-scripts-service.ts

Comment thread packages/server/src/terminal/terminal-worker-process.ts Outdated

cleiter added 2 commits July 17, 2026 12:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(server): only type scripts into a shell that is at its prompt#2158

fix(server): only type scripts into a shell that is at its prompt#2158
cleiter wants to merge 6 commits into
getpaseo:mainfrom
cleiter:terminal-readiness

cleiter commented Jul 17, 2026

Uh oh!

greptile-apps Bot commented Jul 17, 2026 •

edited

Loading

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 17, 2026

Uh oh!

boudra commented Jul 17, 2026

Uh oh!

paseo-bot Bot commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

cleiter commented Jul 17, 2026

Linked issue

Type of change

What does this PR do

How did you verify it

Checklist

Uh oh!

greptile-apps Bot commented Jul 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Uh oh!

Uh oh!

Uh oh!

greptile-apps Bot commented Jul 17, 2026

Uh oh!

boudra commented Jul 17, 2026

Uh oh!

paseo-bot Bot commented Jul 17, 2026

Reshape verdict

Line breakdown

Production shape

One reshape

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

greptile-apps Bot commented Jul 17, 2026 •

edited

Loading