From a3c0900d01705372b8f30fe9442bf86be911087f Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 27 May 2026 18:16:43 -0700 Subject: [PATCH 1/3] fix(host-service): stop gating preset launches on OSC 133;A marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit queueInitialCommand was holding the preset command behind shellReadyPromise, so any user whose shell never emits OSC 133;A (stale wrapper install, exec'd shell, .zshrc blocking on stdin, broken precmd hook) waited the full 15 s SHELL_READY_TIMEOUT_MS before their preset started. Write the initial command immediately: the PTY's stdin buffer holds bytes until the shell reads them, so the command runs as soon as rc files finish. Also drop the timeout from 15 s to 3 s — with initialCommand no longer gated, the only remaining job of the timeout is flushing partial OSC 133;A prefix bytes the scanner holds back from output, which doesn't need 15 s. Adds a reproduction test that spawns /bin/bash without the Superset wrapper on disk (the most common stale-install symptom). Before the fix the test took 15 257 ms; after, 142 ms. --- .../terminal/terminal.adoption.node-test.ts | 35 +++++++++++++++++++ .../host-service/src/terminal/terminal.ts | 22 +++++++----- 2 files changed, 48 insertions(+), 9 deletions(-) diff --git a/packages/host-service/src/terminal/terminal.adoption.node-test.ts b/packages/host-service/src/terminal/terminal.adoption.node-test.ts index 55bdb7d3a98..f59d6064e56 100644 --- a/packages/host-service/src/terminal/terminal.adoption.node-test.ts +++ b/packages/host-service/src/terminal/terminal.adoption.node-test.ts @@ -168,6 +168,41 @@ describe("createTerminalSessionInternal — host-service restart adoption", () = await disposeSessionAndWait(terminalId, db); }); + test("initialCommand runs promptly even when OSC 133;A never fires", async () => { + // Reproduces the v2 "preset send takes 15s" symptom: with a shell in + // SHELLS_WITH_READY_MARKER (bash/zsh/fish) and no Superset wrapper on + // disk, the marker never arrives and queueInitialCommand stalls on + // shellReadyPromise until SHELL_READY_TIMEOUT_MS (15s). + __setAccountShellForTesting("/bin/bash"); + try { + const terminalId = `e2e-no-marker-${randomUUID().slice(0, 8)}`; + const sentinelFile = path.join(TEST_HOME, `no-marker-${terminalId}`); + + const start = Date.now(); + const result = await createTerminalSessionInternal({ + terminalId, + workspaceId, + db, + listed: true, + initialCommand: `echo ok > ${sentinelFile}`, + }); + assert.ok(!("error" in result)); + if ("error" in result) return; + + await waitFor(() => fs.existsSync(sentinelFile), 3000); + const elapsed = Date.now() - start; + console.log(`[repro] initialCommand executed in ${elapsed}ms`); + assert.ok( + elapsed < 3000, + `expected initialCommand to run promptly, took ${elapsed}ms`, + ); + + await disposeSessionAndWait(terminalId, db); + } finally { + __setAccountShellForTesting("/bin/sh"); + } + }); + test("rejects reusing a live terminal id from another workspace", async () => { const terminalId = `e2e-cross-live-${randomUUID().slice(0, 8)}`; diff --git a/packages/host-service/src/terminal/terminal.ts b/packages/host-service/src/terminal/terminal.ts index 5ace3f1c0e6..a67f2f49609 100644 --- a/packages/host-service/src/terminal/terminal.ts +++ b/packages/host-service/src/terminal/terminal.ts @@ -208,11 +208,12 @@ type TerminalSocket = { // --------------------------------------------------------------------------- /** - * How long to wait for the shell-ready marker before unblocking writes. - * 15 s covers heavy setups like Nix-based devenv via direnv. On timeout - * buffered writes flush immediately (same behaviour as before this feature). + * How long to wait for the shell-ready marker before flushing the scanner's + * held bytes. initialCommand no longer waits on this — see queueInitialCommand + * — but the scanner still holds partial OSC 133;A prefixes back from output + * until either a real match arrives or this timeout fires. */ -const SHELL_READY_TIMEOUT_MS = 15_000; +const SHELL_READY_TIMEOUT_MS = 3_000; /** * Shell readiness lifecycle: @@ -793,11 +794,14 @@ function queueInitialCommand( const cmd = initialCommand.endsWith("\n") ? initialCommand : `${initialCommand}\n`; - session.shellReadyPromise.then(() => { - if (!session.exited) { - session.pty.write(cmd); - } - }); + // Write immediately rather than gating on OSC 133;A. The PTY's stdin + // buffer holds bytes until the shell reads them, so the preset command + // runs as soon as rc files finish — without a wrapper present (stale + // install, exec'd shell, broken precmd hook) the marker never arrives + // and gating turned every send into a SHELL_READY_TIMEOUT_MS stall. + if (!session.exited) { + session.pty.write(cmd); + } } interface DaemonCloseResult { From 13c7cb237869a112650b063b8381264c45b32574 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 27 May 2026 18:27:01 -0700 Subject: [PATCH 2/3] chore: trim stale comments around shell-ready timeout --- .../src/terminal/terminal.adoption.node-test.ts | 7 +++---- packages/host-service/src/terminal/terminal.ts | 15 +++------------ 2 files changed, 6 insertions(+), 16 deletions(-) diff --git a/packages/host-service/src/terminal/terminal.adoption.node-test.ts b/packages/host-service/src/terminal/terminal.adoption.node-test.ts index f59d6064e56..d680ad907fb 100644 --- a/packages/host-service/src/terminal/terminal.adoption.node-test.ts +++ b/packages/host-service/src/terminal/terminal.adoption.node-test.ts @@ -169,10 +169,9 @@ describe("createTerminalSessionInternal — host-service restart adoption", () = }); test("initialCommand runs promptly even when OSC 133;A never fires", async () => { - // Reproduces the v2 "preset send takes 15s" symptom: with a shell in - // SHELLS_WITH_READY_MARKER (bash/zsh/fish) and no Superset wrapper on - // disk, the marker never arrives and queueInitialCommand stalls on - // shellReadyPromise until SHELL_READY_TIMEOUT_MS (15s). + // Regression guard against reintroducing the SHELL_READY_TIMEOUT_MS + // stall: bash with no Superset wrapper on disk never emits OSC 133;A, + // but the preset command should still run as soon as the shell reads. __setAccountShellForTesting("/bin/bash"); try { const terminalId = `e2e-no-marker-${randomUUID().slice(0, 8)}`; diff --git a/packages/host-service/src/terminal/terminal.ts b/packages/host-service/src/terminal/terminal.ts index a67f2f49609..1af002b6310 100644 --- a/packages/host-service/src/terminal/terminal.ts +++ b/packages/host-service/src/terminal/terminal.ts @@ -207,12 +207,7 @@ type TerminalSocket = { // Scanner logic lives in @superset/shared/shell-ready-scanner. // --------------------------------------------------------------------------- -/** - * How long to wait for the shell-ready marker before flushing the scanner's - * held bytes. initialCommand no longer waits on this — see queueInitialCommand - * — but the scanner still holds partial OSC 133;A prefixes back from output - * until either a real match arrives or this timeout fires. - */ +/** Flush partial OSC 133;A prefix bytes the scanner is holding if a full marker never arrives. */ const SHELL_READY_TIMEOUT_MS = 3_000; /** @@ -794,11 +789,8 @@ function queueInitialCommand( const cmd = initialCommand.endsWith("\n") ? initialCommand : `${initialCommand}\n`; - // Write immediately rather than gating on OSC 133;A. The PTY's stdin - // buffer holds bytes until the shell reads them, so the preset command - // runs as soon as rc files finish — without a wrapper present (stale - // install, exec'd shell, broken precmd hook) the marker never arrives - // and gating turned every send into a SHELL_READY_TIMEOUT_MS stall. + // Don't gate on OSC 133;A: PTY stdin buffers until the shell reads it, + // and gating turned broken/missing markers into a guaranteed stall. if (!session.exited) { session.pty.write(cmd); } @@ -1005,7 +997,6 @@ interface CreateTerminalSessionOptions { themeType?: "dark" | "light"; db: HostDb; eventBus?: EventBus; - /** Command to run after the shell is ready. Queued behind shellReadyPromise. */ initialCommand?: string; cwd?: string; /** Hidden sessions are process-internal and should not appear in user pickers. */ From 5df3c3ebda8d97f38b8c01a8dfe95954a9cea078 Mon Sep 17 00:00:00 2001 From: Kiet Ho Date: Wed, 27 May 2026 19:01:17 -0700 Subject: [PATCH 3/3] review: address Greptile + cubic findings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - queueInitialCommand: drop redundant `if (!session.exited)` — outer guard at the top of the function already returns early when exited, and the write is now synchronous so there is no async gap to revisit. - adoption test: decouple the 3 s timing assertion from the waitFor budget it sat next to. waitFor now has 10 s of headroom; the timing assertion uses 5 s — still well under the pre-fix 15 s stall and gives CI room. --- .../src/terminal/terminal.adoption.node-test.ts | 6 ++++-- packages/host-service/src/terminal/terminal.ts | 4 +--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/host-service/src/terminal/terminal.adoption.node-test.ts b/packages/host-service/src/terminal/terminal.adoption.node-test.ts index d680ad907fb..301184ab3c5 100644 --- a/packages/host-service/src/terminal/terminal.adoption.node-test.ts +++ b/packages/host-service/src/terminal/terminal.adoption.node-test.ts @@ -188,11 +188,13 @@ describe("createTerminalSessionInternal — host-service restart adoption", () = assert.ok(!("error" in result)); if ("error" in result) return; - await waitFor(() => fs.existsSync(sentinelFile), 3000); + await waitFor(() => fs.existsSync(sentinelFile), 10_000); const elapsed = Date.now() - start; console.log(`[repro] initialCommand executed in ${elapsed}ms`); + // Pre-fix: SHELL_READY_TIMEOUT_MS forced this to 15 s. 5 s leaves + // generous headroom for CI overhead while still catching regression. assert.ok( - elapsed < 3000, + elapsed < 5000, `expected initialCommand to run promptly, took ${elapsed}ms`, ); diff --git a/packages/host-service/src/terminal/terminal.ts b/packages/host-service/src/terminal/terminal.ts index 1af002b6310..27bc8246190 100644 --- a/packages/host-service/src/terminal/terminal.ts +++ b/packages/host-service/src/terminal/terminal.ts @@ -791,9 +791,7 @@ function queueInitialCommand( : `${initialCommand}\n`; // Don't gate on OSC 133;A: PTY stdin buffers until the shell reads it, // and gating turned broken/missing markers into a guaranteed stall. - if (!session.exited) { - session.pty.write(cmd); - } + session.pty.write(cmd); } interface DaemonCloseResult {