Repository navigation
fix(host-service): keep v2 preset commands alive during shell init - #5177
saddlepaddle wants to merge 1 commit into
Conversation
v2 workspace terminals lost preset commands (e.g. `claude`) when the shell was still initializing — the same bug PR #2469 fixed for the v1 terminal stack, which the host-service stack never got. Root cause: the shell emits DA/DSR queries during startup; the renderer's xterm answers them, and those replies arrive as PTY stdin. With nothing gating them, `?62;4;9;22c`-style responses land on the command line ahead of the queued preset command and displace it. Mirror v1's two-part fix in the host-service stack: - Drop `\x1b`-prefixed stdin (query responses) while `shellReadyState` is `pending`, in both input paths (writeInputToSession + the "input" socket handler). - Answer the shell's startup queries server-side via the per-session headless xterm (modeTracker.onData), gated on `pending`, so dropping the renderer's duplicate doesn't leave the queries unanswered. This is the v2 analogue of v1's headless emulator and avoids the init stall that earlier marker-gating attempts hit (#3478 / #4963). The preset command itself is still written immediately — readiness only gates query responses, not the command, so a broken/missing marker can't stall the launch.
|
Ready to review this PR? Stage has broken it down into 2 individual chapters for you:
Chapters generated by Stage for commit 57f2811 on Jun 7, 2026 8:35pm UTC. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughTerminal startup queries are now handled server-side during shell initialization. A new ChangesTerminal startup query handling
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR ports the v1 preset-command-preservation fix to the v2 host-service terminal stack, addressing the same race where terminal capability query responses (DA/DSR) from the renderer arrive as PTY stdin and displace the queued preset command before the shell is ready.
Confidence Score: 4/5Safe to merge — the fix is well-scoped, correctly mirrors v1's two-part mechanism, and the gate logic is sound on the single-threaded JS event loop. The core approach is solid: modeTracker.feed triggers xterm's DA/DSR auto-replies synchronously via writeSync, the onData callback correctly checks shellReadyState at invocation time, and resolveShellReady flushes held bytes only after transitioning away from pending so the guard stays consistent. Both input paths are covered. Minor concerns are the single-subscriber onData design and the escape-prefix filter muting more than just query responses. Both changed files are worth a second look: terminal-mode-tracker.ts for the onData subscriber design, and terminal.ts for the breadth of the escape-sequence filter and how it interacts with the held-bytes flush in resolveShellReady.
|
| Filename | Overview |
|---|---|
| packages/host-service/src/terminal/terminal-mode-tracker.ts | Adds onData to ModeTracker to surface xterm's auto-replies (DA/DSR) to callers; single-subscriber design with no disposable return value |
| packages/host-service/src/terminal/terminal.ts | Wires modeTracker.onData to write DA/DSR replies to the PTY during pending, and drops all ESC-prefixed input in both paths while pending; logic is correct and mirrors v1 |
Sequence Diagram
sequenceDiagram
participant Shell as Shell PTY
participant Host as Host Service
participant Headless as Headless xterm
participant Renderer as Renderer
Shell->>Host: PTY output containing DA query
Host->>Headless: modeTracker.feed(bytes)
Headless-->>Host: onData fires with DA response
Host->>Shell: pty.write DA response server-side
Renderer->>Host: WS input with ESC-prefixed DA response
Note over Host: Dropped while shellReadyState=pending
Shell->>Host: PTY output with OSC 133A ready marker
Host->>Host: resolveShellReady transitions to ready
Renderer->>Host: WS input with arrow key
Host->>Shell: pty.write passes through normally
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/host-service/src/terminal/terminal-mode-tracker.ts:35
The `onData` method returns `void` and only stores a single callback — calling it a second time silently drops the first registration with no way to unsubscribe. This diverges from xterm's own `IDisposable` convention (which every `term.onXxx` follows) and means callers can't safely clean up the listener on session dispose. If `onData` is ever wired a second time (e.g. in a test helper or a future adoption code path), the prior callback disappears without a warning.
```suggestion
onData(callback: (data: string) => void): IDisposable;
```
### Issue 2 of 2
packages/host-service/src/terminal/terminal.ts:1416-1426
**Escape filter is broader than DA/DSR alone**
The `\x1b` prefix check drops *all* escape sequences during `pending` — arrow keys, function keys, Alt+key combos, `Esc`, etc. — not just terminal query responses. The comments in both input paths say "drop the renderer's query responses (DA/DSR, `\x1b`-prefixed)" but the actual effect is wider: any user who attaches to a slow-init workspace and immediately presses an arrow key will silently lose that keystroke. The behaviour deliberately mirrors v1, but the comment is understated; worth noting here (and in `writeInputToSession`) that all escape input is muted during init, not only query responses.
Reviews (1): Last reviewed commit: "fix(host-service): keep v2 preset comman..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| * renderer. Mirrors the v1 headless emulator (apps/desktop/src/main/ | ||
| * lib/terminal-host). | ||
| */ | ||
| onData(callback: (data: string) => void): void; |
There was a problem hiding this comment.
onData method returns void and only stores a single callback — calling it a second time silently drops the first registration with no way to unsubscribe. This diverges from xterm's own IDisposable convention (which every term.onXxx follows) and means callers can't safely clean up the listener on session dispose. If onData is ever wired a second time (e.g. in a test helper or a future adoption code path), the prior callback disappears without a warning.
| onData(callback: (data: string) => void): void; | |
| onData(callback: (data: string) => void): IDisposable; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal/terminal-mode-tracker.ts
Line: 35
Comment:
The `onData` method returns `void` and only stores a single callback — calling it a second time silently drops the first registration with no way to unsubscribe. This diverges from xterm's own `IDisposable` convention (which every `term.onXxx` follows) and means callers can't safely clean up the listener on session dispose. If `onData` is ever wired a second time (e.g. in a test helper or a future adoption code path), the prior callback disappears without a warning.
```suggestion
onData(callback: (data: string) => void): IDisposable;
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| if (message.type === "input") { | ||
| // See writeInputToSession: drop the renderer's query | ||
| // responses while the shell is still initializing. | ||
| if ( | ||
| session.shellReadyState === "pending" && | ||
| message.data.startsWith("\x1b") | ||
| ) { | ||
| return; | ||
| } | ||
| session.pty.write(message.data); | ||
| return; |
There was a problem hiding this comment.
The \x1b prefix check drops all escape sequences during pending — arrow keys, function keys, Alt+key combos, Esc, etc. — not just terminal query responses. The comments in both input paths say "drop the renderer's query responses (DA/DSR, \x1b-prefixed)" but the actual effect is wider: any user who attaches to a slow-init workspace and immediately presses an arrow key will silently lose that keystroke. The behaviour deliberately mirrors v1, but the comment is understated; worth noting here (and in writeInputToSession) that all escape input is muted during init, not only query responses.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/host-service/src/terminal/terminal.ts
Line: 1416-1426
Comment:
**Escape filter is broader than DA/DSR alone**
The `\x1b` prefix check drops *all* escape sequences during `pending` — arrow keys, function keys, Alt+key combos, `Esc`, etc. — not just terminal query responses. The comments in both input paths say "drop the renderer's query responses (DA/DSR, `\x1b`-prefixed)" but the actual effect is wider: any user who attaches to a slow-init workspace and immediately presses an arrow key will silently lose that keystroke. The behaviour deliberately mirrors v1, but the comment is understated; worth noting here (and in `writeInputToSession`) that all escape input is muted during init, not only query responses.
How can I resolve this? If you propose a fix, please make it concise.![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/host-service/src/terminal/terminal.ts">
<violation number="1" location="packages/host-service/src/terminal/terminal.ts:427">
P2: ESC-prefix filtering is too broad during pending init and can drop legitimate user keystrokes (e.g. arrows/Alt), not just DA/DSR query replies.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| // is initializing. The host answers those queries itself (see | ||
| // modeTracker.onData); forwarding the renderer's duplicate here would | ||
| // inject it as typed text and displace the queued preset command. | ||
| if (session.shellReadyState === "pending" && data.startsWith("\x1b")) { |
There was a problem hiding this comment.
P2: ESC-prefix filtering is too broad during pending init and can drop legitimate user keystrokes (e.g. arrows/Alt), not just DA/DSR query replies.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/host-service/src/terminal/terminal.ts, line 427:
<comment>ESC-prefix filtering is too broad during pending init and can drop legitimate user keystrokes (e.g. arrows/Alt), not just DA/DSR query replies.</comment>
<file context>
@@ -420,6 +420,14 @@ export function writeInputToSession({
+ // is initializing. The host answers those queries itself (see
+ // modeTracker.onData); forwarding the renderer's duplicate here would
+ // inject it as typed text and displace the queued preset command.
+ if (session.shellReadyState === "pending" && data.startsWith("\x1b")) {
+ return { success: true };
+ }
</file context>
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
Problem
In v2 workspaces, launching a terminal preset (e.g.
claude) loses the command when the shell is still initializing (direnv / nvm / oh-my-zsh / powerlevel10k). This is the same class of bug #2469 fixed for the v1 terminal stack (apps/desktop/src/main/terminal-host) — but v2 workspaces run an entirely separate terminal stack inpackages/host-service, which never received the fix.Reported in Slack ("command doesn't live… thought we fixed this… v2 needs this I guess").
Root cause
During startup the shell emits terminal capability queries (DA
\x1b[c, DSR/CPR\x1b[6n). The renderer's xterm answers them, and those replies arrive back as PTY stdin. The host-service stack forwarded all stdin unconditionally, so responses like?62;4;9;22clanded on the command line ahead of the queued preset command and displaced it — the command never ran.v1 already solved this. v2 was missing both halves of the solution:
\x1b-prefixed stdin while the shell ispending(terminal-host/session.ts).v2 had the
shellReadyStatemachine and the OSC 133;A scanner wired for output, but never gated input, and its per-session headless xterm (modeTracker) only tracked modes — it never answered queries.Fix
Mirror v1's two-part mechanism in the host-service stack:
\x1b-prefixed stdin is dropped whileshellReadyState === "pending", in both input paths (writeInputToSessionand the WebSocket"input"handler).onDataon the existing per-session headless xterm (modeTracker) and, whilepending, write its DA/DSR replies back to the PTY. Gated onpendingso the renderer stays the sole responder once the shell is interactive. This is the v2 analogue of v1's headless emulator.The preset command itself is still written immediately (
queueInitialCommandunchanged) — readiness only gates query responses, not the command. A broken/missing marker can't stall the launch, which is the failure mode earlier marker-gating attempts kept hitting (#3478 / #4963).Files changed
packages/host-service/src/terminal/terminal-mode-tracker.tsonDatatoModeTracker— surface the headless xterm's auto-replies to terminal queriespackages/host-service/src/terminal/terminal.tspending; drop\x1b-prefixed stdin duringpendingin both input pathsVerification
\x1b[?1;2cand CPR →\x1b[1;1RviaonDatawhen fed viawriteSync(the pathmodeTracker.feeduses).bun test src/terminal— 69 pass / 0 fail.typecheckandlintclean.Test plan
.envrc+ direnv, or oh-my-zsh/powerlevel10k) and launch a preset — command renders immediately and runs once the prompt appears, with no?62;...cgarbage on the line.Summary by cubic
Keep v2 terminal preset commands alive during slow shell init by filtering escape-sequence responses and answering startup queries server-side in
packages/host-service. Preset commands now run reliably once the prompt is ready, without?62;...cnoise.\x1b-prefixed stdin whileshellReadyState === "pending"in both input paths (writeInputToSession, WebSocket"input").onDatatoModeTrackerand, while pending, write its DA/DSR replies to the PTY so the host answers queries during init.Written for commit 57f2811. Summary will update on new commits.
Summary by CodeRabbit
New Features
Bug Fixes