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 b1953e707..40ddc703c 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 a777060fa..a3c614c2c 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 e1acc21db..4fcab680b 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 24649cff7..c77693949 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; }