From a436ee8dcaef047ce72e8acf66401e9994f364d3 Mon Sep 17 00:00:00 2001 From: Bob Lee Date: Thu, 6 Aug 2026 21:29:53 -0700 Subject: [PATCH] fix(dispatch): stop PTY teardown from killing one-click CLI installs The SSH CLI installer launched its driver over a PTY exec channel. The driver only spawns a nohup body and exits, which it does about a millisecond later, and sshd tears the PTY down the moment it does. That teardown races the body: a body still inside bash's startup has not reached its own exit trap yet, so losing the race kills it silently. Nothing survives to explain it. The body writes no log and no exit file, the driver's cleanup trap removes the `.preparing` marker, and the next poll reaps the now-stale `.pid`. The controller reads an empty state, maps it to Failed on its very first poll, and the user gets "could not fully deploy BitFun" for an install that had already downloaded and verified the release. Launch over a plain exec channel instead. Without a controlling terminal there is no hangup to race, and the installer needs no TTY semantics anyway since it never uses sudo. Two diagnostics gaps kept this invisible and are fixed alongside it: - WebKit, which Tauri embeds on macOS, builds `Error.stack` from frames only. The logger preferred `stack` over the message, so the warning reached the log file as a bare source location with no reason. - The install poll's failure discarded the installer's own output, so even a populated remote log never reached the error. Verified against a real Ubuntu aarch64 target. Launching a detached process from a PTY channel is killed before it writes a line; over a plain channel it survives and the install completes (running=1 on the first poll, then marker=1 exit_code=0). --- .../src/remote_ssh/dispatch_ssh.rs | 18 ++++++--- .../dispatch/DispatchInstallDialog.tsx | 10 ++++- src/web-ui/src/shared/utils/logger.test.ts | 37 +++++++++++++++++++ src/web-ui/src/shared/utils/logger.ts | 21 +++++++++-- 4 files changed, 76 insertions(+), 10 deletions(-) diff --git a/src/crates/services/services-integrations/src/remote_ssh/dispatch_ssh.rs b/src/crates/services/services-integrations/src/remote_ssh/dispatch_ssh.rs index b1953e707a..40ddc703c9 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/dispatch_ssh.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/dispatch_ssh.rs @@ -846,19 +846,25 @@ async fn stage_and_launch_installer( ) .await?; - // The short-lived PTY driver only starts a nohup body and exits. Draining - // the channel in the background prevents a server-side channel leak while - // keeping the installer independent of the caller process. + // The short-lived driver only starts a nohup body and exits, and it must run + // without a PTY. sshd tears a PTY down as soon as the driver exits — about a + // millisecond after the hand-off — and that teardown races the body it just + // spawned. A body still inside bash's startup has not reached its own exit + // trap yet, so losing that race kills it silently: no log, no exit file, and + // a `.pid` the next poll then reaps as stale. The controller sees an empty + // state and reports the install as failed even though nothing went wrong. + // A plain exec channel has no controlling terminal, so the hand-off cannot be + // interrupted; the installer needs no TTY semantics either, since it never + // uses sudo. Draining the channel in the background prevents a server-side + // channel leak while keeping the installer independent of the caller process. let channel = match manager - .open_pty_exec_channel( + .open_exec_channel( connection_id, &format!( "bash {} {}", shell_quote_posix(script_path), shell_quote_posix(install_token) ), - 100, - 30, ) .await { diff --git a/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx b/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx index a777060fa5..a3c614c2ce 100644 --- a/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx +++ b/src/web-ui/src/features/dispatch/DispatchInstallDialog.tsx @@ -235,7 +235,15 @@ export const DispatchInstallDialog: React.FC = ({ cursor = poll.cursor; if (poll.status === 'succeeded') break; if (poll.status === 'failed') { - throw new Error('Target CLI installation failed'); + // Carry the installer's own tail into the error: without it the only + // record of the failure is a generic message, and the remote state + // that would explain it is cleaned up before anyone can look. + const detail = poll.output.trim(); + throw new Error( + detail + ? `Target CLI installation failed: ${detail}` + : 'Target CLI installation failed with no installer output', + ); } await new Promise(resolve => globalThis.setTimeout(resolve, 750)); } diff --git a/src/web-ui/src/shared/utils/logger.test.ts b/src/web-ui/src/shared/utils/logger.test.ts index e1acc21db7..4fcab680b3 100644 --- a/src/web-ui/src/shared/utils/logger.test.ts +++ b/src/web-ui/src/shared/utils/logger.test.ts @@ -18,6 +18,43 @@ async function importLoggerWithBootstrapLevel(level: unknown) { return import('./logger'); } +describe('error formatting', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + // WebKit — what Tauri embeds on macOS — builds `stack` from frames only, so a + // logged failure would otherwise arrive with a location and no reason. + it('keeps the message when the stack omits it', async () => { + const { createLogger } = await importLoggerWithBootstrapLevel('debug'); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = new Error('Target CLI installation failed'); + error.stack = '@tauri://localhost/assets/ChatPane.js:96:14836'; + + createLogger('DispatchInstallDialog').warn('Failed to prepare SSH dispatch target', { + connectionId: 'ssh-a', + error, + }); + + const line = String(warn.mock.calls.at(-1)?.[0] ?? ''); + expect(line).toContain('Target CLI installation failed'); + expect(line).toContain('ChatPane.js:96:14836'); + expect(line).toContain('"connectionId":"ssh-a"'); + }); + + it('does not repeat the message when the stack already carries it', async () => { + const { createLogger } = await importLoggerWithBootstrapLevel('debug'); + const warn = vi.spyOn(console, 'warn').mockImplementation(() => {}); + const error = new Error('boom'); + error.stack = 'Error: boom\n at somewhere'; + + createLogger('Ctx').warn('failed', error); + + const line = String(warn.mock.calls.at(-1)?.[0] ?? ''); + expect(line).toBe('[Ctx] failed Error: boom\n at somewhere'); + }); +}); + describe('logger bootstrap level', () => { afterEach(() => { delete globalThis.__BITFUN_BOOTSTRAP_LOG_LEVEL__; diff --git a/src/web-ui/src/shared/utils/logger.ts b/src/web-ui/src/shared/utils/logger.ts index 24649cff7e..c77693949f 100644 --- a/src/web-ui/src/shared/utils/logger.ts +++ b/src/web-ui/src/shared/utils/logger.ts @@ -49,6 +49,21 @@ export function areSensitiveDiagnosticsEnabled(): boolean { return includeSensitiveDiagnostics; } +/** + * Render an Error without losing its message. + * + * `stack` alone is not enough: WebKit — which is what Tauri embeds on macOS — + * builds a stack out of frames only, so `error.stack` for `new Error('boom')` is + * just `@app.js:1:2`. Logging that leaves a failure with a location and no + * reason, which is exactly the case a warning exists to explain. + */ +function formatError(error: Error): string { + const summary = `${error.name}: ${error.message}`; + const stack = error.stack; + if (!stack) return summary; + return stack.includes(error.message) ? stack : `${summary}\n${stack}`; +} + function formatConsoleArg(value: unknown): string { if (value === undefined) return 'undefined'; if (value === null) return 'null'; @@ -57,7 +72,7 @@ function formatConsoleArg(value: unknown): string { return String(value); } if (typeof value === 'symbol') return value.toString(); - if (value instanceof Error) return value.stack || `${value.name}: ${value.message}`; + if (value instanceof Error) return formatError(value); if (typeof value === 'object') { try { return JSON.stringify(value); @@ -229,7 +244,7 @@ export async function initLogger(): Promise { function formatData(data: unknown): string { if (data === undefined || data === null) return ''; if (data instanceof Error) { - return data.stack || data.message; + return formatError(data); } if (typeof data === 'object') { try { @@ -240,7 +255,7 @@ function formatData(data: unknown): string { for (const key of Object.keys(data as Record)) { const value = (data as Record)[key]; if (value instanceof Error) { - errors.push(value.stack || `${value.name}: ${value.message}`); + errors.push(formatError(value)); } else { regularData[key] = value; }