diff --git a/packages/cli/src/commands/dev.ts b/packages/cli/src/commands/dev.ts index 080d8acd0..d1568a0bd 100644 --- a/packages/cli/src/commands/dev.ts +++ b/packages/cli/src/commands/dev.ts @@ -9,7 +9,7 @@ import { resolveConfigFilePathOption } from "../lib/config-file-path.js"; import { devEnvStatePath, ensureLocalDashboardSecret, readDevEnvState, recordLocalDashboardProcess } from "../lib/dev-env-state.js"; import { CliError } from "../lib/errors.js"; import { cliVersion } from "../lib/own-package.js"; -import { maybeReexecToLatest } from "../lib/self-update.js"; +import { maybeReexecToLatest, REEXEC_MARKER_ENV } from "../lib/self-update.js"; type ChildCommand = { command: string, @@ -693,8 +693,6 @@ const childEnv = { ...process.env }; delete childEnv.HEXCLAVE_DEV_APP_COMMAND_PARENT_PID; delete childEnv.HEXCLAVE_DEV_APP_COMMAND; delete childEnv.HEXCLAVE_DEV_APP_COMMAND_ARGS_JSON; -// Internal to the npx auto-update handshake; never meant for the user's command. -delete childEnv.HEXCLAVE_CLI_REEXEC_MARKER; const child = spawn(command, args, { env: childEnv, @@ -751,14 +749,20 @@ child.on("error", (error) => { `; function runChildProcess(command: ChildCommand, env: NodeJS.ProcessEnv): Promise { + // The npx auto-update handshake marker is internal to the parent<->child + // re-exec; never leak it into the user's command. Scrub it here (rather than in + // the wrapper script) so it uses the exported constant and covers both the + // Windows-direct and POSIX-wrapper spawn paths. + const childEnv = { ...env }; + delete childEnv[REEXEC_MARKER_ENV]; return new Promise((resolvePromise, reject) => { const child = process.platform === "win32" - ? spawn(command.command, command.args, { stdio: "inherit", env }) + ? spawn(command.command, command.args, { stdio: "inherit", env: childEnv }) : spawn(process.execPath, ["-e", APP_COMMAND_WRAPPER_SCRIPT], { detached: true, stdio: "inherit", env: { - ...env, + ...childEnv, [APP_COMMAND_WRAPPER_PARENT_PID_ENV_VAR]: String(process.pid), [APP_COMMAND_WRAPPER_COMMAND_ENV_VAR]: command.command, [APP_COMMAND_WRAPPER_ARGS_ENV_VAR]: JSON.stringify(command.args), diff --git a/packages/cli/src/lib/self-update.test.ts b/packages/cli/src/lib/self-update.test.ts index 69faae175..a01e2f6af 100644 --- a/packages/cli/src/lib/self-update.test.ts +++ b/packages/cli/src/lib/self-update.test.ts @@ -336,4 +336,24 @@ describe("maybeReexecToLatest fallback wiring", () => { await promise; expect(exitSpy).toHaveBeenCalledWith(1); }); + + it("propagates a nonzero, non-NaN code when killed by a signal missing from os.constants", async () => { + const exitSpy = vi.spyOn(process, "exit").mockImplementation(((() => { + throw new Error("__exit__"); + }) as never)); + const child = mockNpxChild({ writeMarker: false }); + + const promise = maybeReexecToLatest({ forwardArgs: ["dev"] }); + // SIGSTKFLT is absent from os.constants.signals on macOS (present on Linux). + // Either way the abort must surface as a real nonzero code — never NaN (which + // process.exit coerces to 0, masking the abort as success). + child.emit("close", null, "SIGSTKFLT"); + + await promise; + expect(exitSpy).toHaveBeenCalledTimes(1); + const code = exitSpy.mock.calls[0][0]; + expect(typeof code).toBe("number"); + expect(Number.isNaN(code as number)).toBe(false); + expect(code as number).toBeGreaterThan(0); + }); }); diff --git a/packages/cli/src/lib/self-update.ts b/packages/cli/src/lib/self-update.ts index f9e33f8ad..86392ce10 100644 --- a/packages/cli/src/lib/self-update.ts +++ b/packages/cli/src/lib/self-update.ts @@ -138,11 +138,16 @@ function runReexec(invocation: NpxInvocation, markerFile: string | null): Promis child.on("close", (code, signal) => { cleanup(); if (signal != null) { - // Killed by a signal we forwarded (e.g. Ctrl-C). Report it with the - // conventional 128 + signal-number exit code so the caller can both - // recognize the abort and propagate a sensible code. - const signalNumber = osConstants.signals[signal]; - resolvePromise({ exited: true, code: 128 + signalNumber, signal }); + // Killed by a signal (a Ctrl-C we forwarded, or an external kill). Report + // the conventional 128 + signal-number exit code so the caller can both + // recognize the abort and propagate a sensible code. `os.constants.signals` + // doesn't list every NodeJS.Signals name on every platform, so the lookup + // can be undefined at runtime (the type says otherwise) — `128 + undefined` + // is NaN and `process.exit(NaN)` coerces to 0, masking the abort as + // success. Fall back to a generic nonzero code in that case. + const signalNumber = osConstants.signals[signal] as number | undefined; + const code = signalNumber != null ? 128 + signalNumber : 1; + resolvePromise({ exited: true, code, signal }); return; } resolvePromise({ exited: true, code: code ?? 1, signal: null });