From 54f32261c918709de146849644cf181a5b06b70a Mon Sep 17 00:00:00 2001 From: li-xiu-qi Date: Tue, 20 Oct 2026 20:18:07 +0800 Subject: [PATCH] fix(step): include installer output in update failure messages When the installer script (install.ps1 / install.sh) exits non-zero during a TUI update, the failure message only contained the exit code, leaving users unable to see why the installer failed. Capture the installer's stdout/stderr in runCommand and append the tail to the failure message. - runCommand now pipes and captures child output instead of inheriting (inherited output is overwritten by the TUI's full-screen render) - formatInstallerFailure builds the message with the last 20 lines of installer output, noting how many earlier lines were omitted - tests cover the message formatting and an end-to-end installer failure --- .../src/step/local-update.test.ts | 52 ++++++++++++++++++- .../coding-agent/src/step/local-update.ts | 49 ++++++++++++++--- 2 files changed, 94 insertions(+), 7 deletions(-) diff --git a/packages/coding-agent/src/step/local-update.test.ts b/packages/coding-agent/src/step/local-update.test.ts index 2290838a..6df69064 100644 --- a/packages/coding-agent/src/step/local-update.test.ts +++ b/packages/coding-agent/src/step/local-update.test.ts @@ -2,7 +2,7 @@ import { mkdir, mkdtemp, rm, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { afterEach, beforeEach, expect, test } from "vitest"; -import { findArchiveRoot } from "./local-update.ts"; +import { findArchiveRoot, formatInstallerFailure, installLatestStepRelease } from "./local-update.ts"; let extractDir: string; @@ -46,3 +46,53 @@ test("ambiguous layouts fall back to the extract dir for recursive search", asyn expect(await findArchiveRoot(extractDir, "step")).toBe(extractDir); }); + +test("installer failure message stays bare when the installer produced no output", () => { + expect(formatInstallerFailure("install.ps1", 1, " \n \n ")).toBe("install.ps1 exited with code 1"); +}); + +test("installer failure message includes the captured installer output", () => { + const message = formatInstallerFailure("install.ps1", 1, "downloading release…\nfatal: checksum mismatch"); + + expect(message).toContain("install.ps1 exited with code 1"); + expect(message).toContain("downloading release…"); + expect(message).toContain("fatal: checksum mismatch"); +}); + +test("installer failure message keeps only the tail of long output", () => { + const lines = Array.from({ length: 50 }, (_, index) => `installer line ${index + 1}`); + const message = formatInstallerFailure("install.sh", 2, lines.join("\n")); + + expect(message).toContain("install.sh exited with code 2"); + expect(message).toContain("installer line 50"); + expect(message).toContain("installer line 31"); + expect(message).not.toContain("installer line 30\n"); + expect(message).toContain("last 20 of 50 lines"); +}); + +test("installLatestStepRelease surfaces installer output when the installer exits non-zero", async () => { + const isWindows = process.platform === "win32"; + const scriptName = isWindows ? "install.ps1" : "install.sh"; + const scriptBody = isWindows + ? 'Write-Output "installer-output-marker"\nexit 1' + : 'echo "installer-output-marker"\nexit 1'; + const originalFetch = globalThis.fetch; + globalThis.fetch = (async () => ({ + ok: true, + status: 200, + text: async () => scriptBody, + })) as unknown as typeof fetch; + try { + const result = await installLatestStepRelease({ + executablePath: path.join(extractDir, isWindows ? "step.exe" : "step"), + installDir: extractDir, + releaseBaseUrl: "https://example.invalid", + }); + + expect(result.ok).toBe(false); + expect(result.message).toContain(`${scriptName} exited with code 1`); + expect(result.message).toContain("installer-output-marker"); + } finally { + globalThis.fetch = originalFetch; + } +}); diff --git a/packages/coding-agent/src/step/local-update.ts b/packages/coding-agent/src/step/local-update.ts index da37f7b6..755effce 100644 --- a/packages/coding-agent/src/step/local-update.ts +++ b/packages/coding-agent/src/step/local-update.ts @@ -415,7 +415,7 @@ export function resolveStepUpdateInstallerSpec(platform: NodeJS.Platform = proce }; } -async function installLatestStepRelease(input: StepUpdateInstallInput): Promise { +export async function installLatestStepRelease(input: StepUpdateInstallInput): Promise { const tempRoot = await mkdtemp(path.join(os.tmpdir(), "stepcode-update-")); const installer = resolveStepUpdateInstallerSpec(); const scriptPath = path.join(tempRoot, installer.scriptName); @@ -427,14 +427,14 @@ async function installLatestStepRelease(input: StepUpdateInstallInput): Promise< if (!response.ok) return { ok: false, message: `failed to download ${installer.scriptName} (${response.status})` }; await writeFile(scriptPath, await response.text(), { encoding: "utf8", mode: 0o755 }); - const code = await runCommand(installer.command, installer.buildArgs(scriptPath, input.installDir), { + const { code, output } = await runCommand(installer.command, installer.buildArgs(scriptPath, input.installDir), { ...process.env, STEP_RELEASE_BASE_URL: input.releaseBaseUrl, STEP_VERSION: "latest", STEP_INSTALL_DIR: input.installDir, ...(input.agentDir ? { STEP_CODING_AGENT_DIR: input.agentDir } : {}), }); - if (code !== 0) return { ok: false, message: `${installer.scriptName} exited with code ${code}` }; + if (code !== 0) return { ok: false, message: formatInstallerFailure(installer.scriptName, code, output) }; return { ok: true, message: `Updated Step in ${formatHomeRelativePath(input.installDir)}; restarting.`, @@ -626,11 +626,48 @@ async function replaceInstallation( } } -function runCommand(command: string, args: string[], env: NodeJS.ProcessEnv): Promise { +const INSTALLER_OUTPUT_TAIL_LINES = 20; + +/** + * Build the failure message for a non-zero installer exit, appending the tail + * of the installer's captured output so the actual error is visible to the + * user instead of a bare exit code. + */ +export function formatInstallerFailure(scriptName: string, code: number, output: string): string { + const base = `${scriptName} exited with code ${code}`; + const trimmed = output.trim(); + if (!trimmed) return base; + const lines = trimmed.split(/\r?\n/u); + const omitted = Math.max(0, lines.length - INSTALLER_OUTPUT_TAIL_LINES); + const tail = lines.slice(-INSTALLER_OUTPUT_TAIL_LINES).join("\n"); + const header = + omitted > 0 + ? `installer output (last ${INSTALLER_OUTPUT_TAIL_LINES} of ${lines.length} lines):` + : "installer output:"; + return `${base}\n${header}\n${tail}`; +} + +function runCommand( + command: string, + args: string[], + env: NodeJS.ProcessEnv, +): Promise<{ code: number; output: string }> { return new Promise((resolve, reject) => { - const child = spawn(command, args, { env, stdio: "inherit" }); + // Capture instead of inherit: the TUI renders full-screen, so inherited + // child output is overwritten before the user can read it. The captured + // output is surfaced in the failure message instead. + const child = spawn(command, args, { env, stdio: ["ignore", "pipe", "pipe"] }); + const chunks: Buffer[] = []; + child.stdout?.on("data", (chunk: Buffer) => { + chunks.push(chunk); + }); + child.stderr?.on("data", (chunk: Buffer) => { + chunks.push(chunk); + }); child.once("error", reject); - child.once("exit", (code) => resolve(code ?? 1)); + child.once("exit", (code) => { + resolve({ code: code ?? 1, output: Buffer.concat(chunks).toString("utf8") }); + }); }); }