From 378b05f9f9fb719fea80f4edb1af58077f2b3bf1 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Sat, 7 Nov 2026 12:16:50 -0500 Subject: [PATCH 1/2] fix(native): stop a restart on the first Ctrl-C at any phase A native `hack restart` installed no SIGINT or SIGTERM handler until its new foreground start did. A first Ctrl-C during preflight, the cleanup of the old run or the early start was therefore either dropped (a SIGINT inherited as ignored, as for a background job, which is how the restart reached ready and needed a second Ctrl-C) or ended the frontend mid-restart with its restart lock and intent left behind. The restart now handles stop requests from its first step until the new start installs its own handlers, then hands them off: - during preflight it ends with nothing stopped; - during cleanup the owned stop runs to completion, the old frontend's finalizers are awaited and the new run is not started; - before the hand-off the start is abandoned before any hook or service; - after the hand-off the start stops its run as an interrupted `up`. Every cancel prints the same lines as Ctrl-C after `up` (one "Stopping..." line, then the confirmed-stop line), removes the restart intent so a plain `hack up` resumes the retained data, and exits 130 (143 for SIGTERM). Refs HACK-1953 Co-Authored-By: Claude Opus 5.5 --- src/backends/native-project-restart.ts | 339 ++++++++++++++---- src/backends/native-project-start.ts | 8 + src/commands/project.ts | 22 +- .../fixtures/native-restart-signal-worker.ts | 96 +++++ tests/native-project-restart.test.ts | 302 ++++++++++++++++ 5 files changed, 699 insertions(+), 68 deletions(-) create mode 100644 tests/fixtures/native-restart-signal-worker.ts diff --git a/src/backends/native-project-restart.ts b/src/backends/native-project-restart.ts index 0a408e5d0..745a658a1 100644 --- a/src/backends/native-project-restart.ts +++ b/src/backends/native-project-restart.ts @@ -12,12 +12,59 @@ import { loadNativeRestartIntent, type NativeProjectRun, type NativeProjectRunScope, + type NativeRestartIntent, removeNativeRestartIntent, saveNativeRestartIntent, withNativeRestartLock, } from "./native-project-run.ts"; +/** + * Stop requests of a native restart until its new foreground `up` installs its + * own (see `nativeForegroundStopSignals`). Without these, a first Ctrl-C during + * preflight, cleanup or the early start was either ignored (a SIGINT inherited + * as ignored, as in a background job) or ended the frontend mid-restart. + */ +export function nativeRestartStopSignals() { + const controller = new AbortController(); + let interruptedExit = 130; + const interrupt = () => controller.abort(); + const terminate = () => { + if (!controller.signal.aborted) { + interruptedExit = 143; + } + controller.abort(); + }; + process.on("SIGINT", interrupt); + process.on("SIGTERM", terminate); + let disposed = false; + return { + signal: controller.signal, + interruptedExit: () => interruptedExit, + dispose: () => { + if (!disposed) { + disposed = true; + process.off("SIGINT", interrupt); + process.off("SIGTERM", terminate); + } + }, + }; +} +export type NativeRestartStopSignals = ReturnType< + typeof nativeRestartStopSignals +>; + +/** Progress lines of a canceled restart, matching an interrupted foreground `up`. */ +export type NativeRestartStopProgress = + | { readonly phase: "stopping" } + | { readonly phase: "stopped"; readonly summary: string }; + +const STOPPED = + "Native project stopped; owned cleanup confirmed. Named volumes and the stopped run mapping are kept for the next hack up."; +const CANCELED_BEFORE_CLEANUP = + "Native restart canceled before cleanup; nothing was stopped."; + const DEFAULTS = { + signals: nativeRestartStopSignals, load: loadNativeProjectRun, pending: loadNativeRestartIntent, save: saveNativeRestartIntent, @@ -42,7 +89,24 @@ export async function restartNativeProject(opts: { readonly start: (options: { run: NativeProjectRun; onReady: () => Promise; + /** Aborted by a stop request that arrived before the start took over. */ + signal: AbortSignal; + /** + * Called once the start's own stop handling is installed; later stop + * requests belong to it, so the restart's handlers are removed. + */ + onStopSignals: () => void; + /** + * The start reports that its own stop handling received a stop request. A + * start that then returns stopped its new run with confirmed cleanup. + */ + onStopRequested: () => void; }) => Promise; + /** + * Stop lines for a restart canceled before its new run started; a cancel + * after that is reported by the start itself. + */ + readonly progress?: (event: NativeRestartStopProgress) => void; readonly recovery?: { readonly expectAttempt: string; readonly legacyPid?: number; @@ -55,84 +119,225 @@ export async function restartNativeProject(opts: { readonly dependencies?: Partial; }): Promise { const deps = { ...DEFAULTS, ...opts.dependencies }; - return await deps.lock(opts.scope, async (release) => { - let intent = await deps.pending(opts.scope); - const current = await deps.load(opts.scope); - const run = intent?.run ?? current; - if (!run) { - throw new ClosedCliError( - "Native restart has no existing run; use hack up for initial startup." - ); - } - if ( - current && - (current.run !== run.run || - current.planId !== run.planId || - current.owner !== run.owner || - current.namespace !== run.namespace) - ) { - throw new ClosedCliError( - "Native restart mapping changed; no cleanup requested." - ); - } - nativeRestartSelection({ - run, - envName: opts.envName, - profiles: opts.profiles, - }); - if (!current && intent && intent.phase !== "cleaned") { - throw new ClosedCliError( - "Native restart cleanup or down hooks were interrupted; inspect the pending intent before recovery." - ); + const stop = deps.signals(); + try { + return await restartUnderLock({ ...opts, deps, stop }); + } finally { + stop.dispose(); + } +} + +type RestartOptions = Parameters[0]; +type RestartContext = RestartOptions & { + readonly deps: typeof DEFAULTS; + readonly stop: NativeRestartStopSignals; + /** Prints the "stopping" line once, as soon as the old run's cleanup is committed. */ + readonly reportStopping: () => void; + /** Set once the new start owns stop requests; the restart then ignores them. */ + readonly handOff: () => void; +}; + +/** + * The first stop request cancels the restart at the next safe point: before + * cleanup nothing is stopped; once cleanup has begun it runs to completion and + * the new run is not started; after the start takes over, the start stops its + * own run as an interrupted `up` does. Every path ends with the graph stopped + * and data retained, and exits 130 (143 for SIGTERM). + */ +async function restartUnderLock( + opts: RestartOptions & { + readonly deps: typeof DEFAULTS; + readonly stop: NativeRestartStopSignals; + } +): Promise { + let stopping = false; + let cleanupStarted = false; + let handedOff = false; + const reportStopping = () => { + if (!stopping) { + stopping = true; + opts.progress?.({ phase: "stopping" }); } + }; + opts.stop.signal.addEventListener( + "abort", + () => { + if (cleanupStarted && !handedOff) { + reportStopping(); + } + }, + { once: true } + ); + const context: RestartContext = { + ...opts, + reportStopping, + handOff: () => { + handedOff = true; + opts.stop.dispose(); + }, + }; + return await opts.deps.lock(opts.scope, async (release) => { + const { run, intent, current } = await selectRestart(context); await opts.preflight(run, { cleanedRetry: intent?.phase === "cleaned", }); - if (!intent) { - intent = { - phase: "prepared", - run, - finalization: await deps.capture({ scope: opts.scope, run }), - }; - await deps.save({ ...opts.scope, intent }); + if (opts.stop.signal.aborted && intent?.phase !== "cleaned") { + opts.progress?.({ phase: "stopped", summary: CANCELED_BEFORE_CLEANUP }); + return opts.stop.interruptedExit(); } - if (current) { - await opts.down(); - intent = await deps.cleaned({ ...opts.scope, expected: intent }); - } else if (intent.phase !== "cleaned") { - throw new ClosedCliError( - "Native restart cleanup or down hooks were interrupted; inspect the pending intent before recovery." - ); + cleanupStarted = true; + if (opts.stop.signal.aborted) { + reportStopping(); } - const selected = intent; - const recovery = opts.recovery; - if (recovery) { - await deps.recover({ - scope: opts.scope, - run, - token: selected.finalization, - expectAttempt: recovery.expectAttempt, - legacyPid: recovery.legacyPid, - verifyEffects: async () => - await recovery.verifyEffects({ - run, - token: selected.finalization, - }), - }); - await recovery.retirePublisher(run); - } - await deps.wait({ - scope: opts.scope, + const selected = await cleanOldRun({ context, run, intent, current }); + return await startOrStop({ context, run, selected, release }); + }); +} + +async function selectRestart(context: RestartContext): Promise<{ + readonly run: NativeProjectRun; + readonly intent: NativeRestartIntent | null | undefined; + readonly current: NativeProjectRun | null | undefined; +}> { + const { deps } = context; + const intent = await deps.pending(context.scope); + const current = await deps.load(context.scope); + const run = intent?.run ?? current; + if (!run) { + throw new ClosedCliError( + "Native restart has no existing run; use hack up for initial startup." + ); + } + if ( + current && + (current.run !== run.run || + current.planId !== run.planId || + current.owner !== run.owner || + current.namespace !== run.namespace) + ) { + throw new ClosedCliError( + "Native restart mapping changed; no cleanup requested." + ); + } + nativeRestartSelection({ + run, + envName: context.envName, + profiles: context.profiles, + }); + if (!current && intent && intent.phase !== "cleaned") { + throw interruptedCleanup(); + } + return { run, intent, current }; +} + +function interruptedCleanup(): Error { + return new ClosedCliError( + "Native restart cleanup or down hooks were interrupted; inspect the pending intent before recovery." + ); +} + +/** Stop the old run (or resume a cleaned intent) and wait for its frontend's finalizers. */ +async function cleanOldRun(opts: { + readonly context: RestartContext; + readonly run: NativeProjectRun; + readonly intent: NativeRestartIntent | null | undefined; + readonly current: NativeProjectRun | null | undefined; +}): Promise { + const { context, run, current } = opts; + const { deps } = context; + let intent = opts.intent; + if (!intent) { + intent = { + phase: "prepared", + run, + finalization: await deps.capture({ scope: context.scope, run }), + }; + await deps.save({ ...context.scope, intent }); + } + if (current) { + await context.down(); + intent = await deps.cleaned({ ...context.scope, expected: intent }); + } else if (intent.phase !== "cleaned") { + throw interruptedCleanup(); + } + const selected = intent; + const recovery = context.recovery; + if (recovery) { + await deps.recover({ + scope: context.scope, run, token: selected.finalization, - timeoutMs: 150_000, + expectAttempt: recovery.expectAttempt, + legacyPid: recovery.legacyPid, + verifyEffects: async () => + await recovery.verifyEffects({ + run, + token: selected.finalization, + }), }); - return await opts.start({ + await recovery.retirePublisher(run); + } + await deps.wait({ + scope: context.scope, + run, + token: selected.finalization, + timeoutMs: 150_000, + }); + return selected; +} + +/** Start the new run, or end as a stopped project when a stop request came first. */ +async function startOrStop(opts: { + readonly context: RestartContext; + readonly run: NativeProjectRun; + readonly selected: NativeRestartIntent; + readonly release: () => Promise; +}): Promise { + const { context, run, selected } = opts; + const { deps, stop } = context; + // The old run is stopped and finalized; a canceled restart ends here as an + // ordinary stopped project, so the next `hack up` resumes its retained data. + const canceled = async () => { + await deps.remove({ ...context.scope, expected: selected }); + context.reportStopping(); + context.progress?.({ phase: "stopped", summary: STOPPED }); + return stop.interruptedExit(); + }; + if (stop.signal.aborted) { + return await canceled(); + } + let handedOff = false; + let stopRequested = false; + let removed = false; + try { + const code = await context.start({ run, + signal: stop.signal, + onStopSignals: () => { + handedOff = true; + context.handOff(); + }, + onStopRequested: () => { + stopRequested = true; + }, onReady: async () => { - await deps.remove({ ...opts.scope, expected: selected }); - await release(); + await deps.remove({ ...context.scope, expected: selected }); + removed = true; + await opts.release(); }, }); - }); + // Stopped by request before it became ready: the new start confirmed its own + // cleanup, so the project is simply stopped and `hack up` resumes it. + if (stopRequested && !removed) { + await deps.remove({ ...context.scope, expected: selected }); + } + return code; + } catch (error) { + // Before the hand-off the start has run no hook and launched no service of + // the new run, so a refusal caused by this cancel leaves the old run stopped. + if (stop.signal.aborted && !handedOff) { + return await canceled(); + } + throw error; + } } diff --git a/src/backends/native-project-start.ts b/src/backends/native-project-start.ts index 1f022cab7..7b829c652 100644 --- a/src/backends/native-project-start.ts +++ b/src/backends/native-project-start.ts @@ -1641,6 +1641,13 @@ export async function startNativeProject(opts: { readonly signal?: AbortSignal; /** A second stop request: cancels a running down hook and forces cleanup. */ readonly forceSignal?: AbortSignal; + /** + * Called once this start's own SIGINT/SIGTERM handling is installed, before any + * hook, runtime start or service. A caller that handled stop requests until + * now (such as `hack restart`) removes its handlers here; `signal` still + * carries a request that arrived earlier. + */ + readonly onStopSignals?: () => void; /** Foreground progress and the ready line; see {@link NativeStartupProgress}. */ readonly onProgress?: (event: NativeStartupProgress) => void; readonly dependencies?: Partial; @@ -1795,6 +1802,7 @@ export async function startNativeProject(opts: { .update(new Uint8Array(await artifactFile.arrayBuffer())) .digest("hex"); const stopSignals = nativeForegroundStopSignals(opts); + opts.onStopSignals?.(); const controller = stopSignals.controller; controller.signal.addEventListener( "abort", diff --git a/src/commands/project.ts b/src/commands/project.ts index 582c0ecc0..e23cdeec6 100644 --- a/src/commands/project.ts +++ b/src/commands/project.ts @@ -6147,6 +6147,17 @@ async function restartNativeImageProject({ }; readonly dropEnvFingerprint: () => void; }): Promise { + // One "Stopping…" line, whether the restart or its new start sees the request. + let stoppingReported = false; + const progress = (event: NativeStartupProgress): void => { + if (event.phase === "stopping") { + if (stoppingReported) { + return; + } + stoppingReported = true; + } + reportNativeStartupProgress(event); + }; return await restartNativeProject({ scope: startup.scope, envName: startup.envName, @@ -6237,12 +6248,21 @@ async function restartNativeImageProject({ }); } }, - start: ({ run, onReady }) => + progress, + start: ({ run, onReady, signal, onStopSignals, onStopRequested }) => startNativeProject({ ...startup, ...nativeRestartSelection({ run }), restore: run, onReady, + signal, + onStopSignals, + onProgress: (event) => { + if (event.phase === "stopping") { + onStopRequested(); + } + progress(event); + }, }), }); } diff --git a/tests/fixtures/native-restart-signal-worker.ts b/tests/fixtures/native-restart-signal-worker.ts new file mode 100644 index 000000000..ab12deb72 --- /dev/null +++ b/tests/fixtures/native-restart-signal-worker.ts @@ -0,0 +1,96 @@ +// Runs `restartNativeProject` with in-memory owners so a test can deliver real +// signals to this process. argv[2] selects the phase that waits for a signal: +// "preflight", "down", "start" (before the start's own handlers), "starting" +// (after the hand-off, before ready) or "serving" (after ready). Each step prints one line to stdout. +import { restartNativeProject } from "../../src/backends/native-project-restart.ts"; +import type { NativeRestartIntent } from "../../src/backends/native-project-run.ts"; + +const hold = process.argv[2]; +const say = (line: string) => process.stdout.write(`${line}\n`); +const run = { + run: "a".repeat(32), + owner: "b".repeat(32), + namespace: "c".repeat(64), + planId: "d".repeat(64), + effectiveEnvName: "qa", + profiles: ["app"], + aws: null, +}; +const token = { + version: 1 as const, + attempt: "e".repeat(32), + scope: "f".repeat(64), + run: run.run, + owner: run.owner, + namespace: run.namespace, + planId: run.planId, +}; +let pending: NativeRestartIntent | null = null; +/** Announce the phase, then let it run long enough for the signal to arrive. */ +async function phase(name: string): Promise { + say(`phase ${name}`); + if (hold === name) { + await Bun.sleep(1500); + } +} +const code = await restartNativeProject({ + scope: { + projectRoot: "/fixture", + projectDir: "/fixture/.hack", + nativeHome: "/candidate", + branch: null, + }, + preflight: async () => await phase("preflight"), + down: async () => { + await phase("down"); + say("old run stopped"); + }, + progress: (event) => + say(event.phase === "stopping" ? "stopping" : `stopped: ${event.summary}`), + start: async ({ signal, onStopSignals, onStopRequested, onReady }) => { + await phase("start"); + if (signal.aborted) { + throw new Error("start canceled before hand-off"); + } + // The start's own stop handling, as nativeForegroundStopSignals installs it. + const own = Promise.withResolvers(); + process.once("SIGINT", () => { + onStopRequested(); + own.resolve(130); + }); + onStopSignals(); + if (hold === "starting") { + // Stopped by its own owner before it became ready. + await phase("starting"); + } else { + await onReady(); + say("ready"); + await phase("serving"); + } + const exit = await own.promise; + say("new run stopped by its own owner"); + return exit; + }, + dependencies: { + load: async () => (pending?.phase === "cleaned" ? null : run), + pending: async () => pending, + save: async ({ intent }) => { + pending = intent; + }, + cleaned: async ({ expected }) => { + pending = { ...expected, phase: "cleaned" }; + return pending; + }, + remove: async () => { + pending = null; + say("intent removed"); + }, + capture: async () => token, + wait: async () => { + say("old frontend finalized"); + }, + lock: async (_scope, action) => await action(async () => undefined), + }, +}); +say(`exit ${code} listeners=${process.listenerCount("SIGINT")}`); +process.exit(code); diff --git a/tests/native-project-restart.test.ts b/tests/native-project-restart.test.ts index ed9ef5c7b..3fd1e113b 100644 --- a/tests/native-project-restart.test.ts +++ b/tests/native-project-restart.test.ts @@ -1949,3 +1949,305 @@ test("active restart preserves admitted images despite moved tags and refuses ch } } }); + +function stopFixture(abortAt: "preflight" | "down" | "start" | "never") { + const events: string[] = []; + const controller = new AbortController(); + let exit = 130; + let disposed = 0; + const signals = { + signal: controller.signal, + interruptedExit: () => exit, + dispose: () => { + disposed++; + }, + }; + const interrupt = (code = 130) => { + exit = code; + controller.abort(); + }; + let pending: NativeRestartIntent | null = null; + const options: Parameters[0] = { + scope, + preflight: async () => { + events.push("preflight"); + if (abortAt === "preflight") { + interrupt(); + } + }, + down: async () => { + if (abortAt === "down") { + interrupt(); + } + events.push("down"); + }, + progress: (event) => + events.push(event.phase === "stopping" ? "stopping" : "stopped"), + start: async ({ signal, onStopSignals }) => { + events.push("start"); + if (abortAt === "start") { + interrupt(); + } + if (signal.aborted) { + throw new ClosedCliError("canceled before hand-off"); + } + onStopSignals(); + return 0; + }, + dependencies: { + signals: () => signals, + load: async () => run, + pending: async () => pending, + save: async ({ intent }) => { + pending = intent; + events.push("intent"); + }, + cleaned: async ({ expected }) => { + pending = { ...expected, phase: "cleaned" }; + return pending; + }, + remove: async () => { + pending = null; + events.push("remove"); + }, + capture: async () => token, + wait: async () => { + events.push("finalized"); + }, + lock: async (_scope, action) => await action(async () => undefined), + }, + }; + return { + options, + events, + interrupt, + disposed: () => disposed, + pending: () => pending, + }; +} + +test("a stop request during preflight cancels the restart before anything is stopped", async () => { + const f = stopFixture("preflight"); + expect(await restartNativeProject(f.options)).toBe(130); + expect(f.events).toEqual(["preflight", "stopped"]); + expect(f.pending()).toBeNull(); + expect(f.disposed()).toBeGreaterThan(0); +}); + +test("a stop request during cleanup finishes the owned stop and does not start the new run", async () => { + const f = stopFixture("down"); + expect(await restartNativeProject(f.options)).toBe(130); + expect(f.events).toEqual([ + "preflight", + "intent", + "stopping", + "down", + "finalized", + "remove", + "stopped", + ]); + // A plain `hack up` resumes the retained run; no restart stays pending. + expect(f.pending()).toBeNull(); +}); + +test("a stop request before the start's own handlers ends as a stopped project", async () => { + const f = stopFixture("start"); + expect(await restartNativeProject(f.options)).toBe(130); + expect(f.events).toEqual([ + "preflight", + "intent", + "down", + "finalized", + "start", + "stopping", + "remove", + "stopped", + ]); + expect(f.pending()).toBeNull(); +}); + +test("after the hand-off the restart's handlers are gone and the start owns the stop", async () => { + const f = stopFixture("never"); + let disposedAtHandOff = -1; + const start = f.options.start; + const options = { + ...f.options, + start: async (input: Parameters[0]) => + await start({ + ...input, + onStopSignals: () => { + input.onStopSignals(); + disposedAtHandOff = f.disposed(); + // A later request reaches only the start; the restart must not react. + f.interrupt(); + }, + }), + }; + expect(await restartNativeProject(options)).toBe(0); + expect(disposedAtHandOff).toBeGreaterThan(0); + expect(f.events).toEqual([ + "preflight", + "intent", + "down", + "finalized", + "start", + ]); +}); + +test("a new start stopped by request before ready leaves no pending restart", async () => { + const f = stopFixture("never"); + expect( + await restartNativeProject({ + ...f.options, + start: async ({ onStopSignals, onStopRequested }) => { + onStopSignals(); + onStopRequested(); + return 130; + }, + }) + ).toBe(130); + expect(f.pending()).toBeNull(); + expect(f.events).toEqual([ + "preflight", + "intent", + "down", + "finalized", + "remove", + ]); +}); + +test("a new start that fails without a stop request keeps the pending restart", async () => { + const f = stopFixture("never"); + expect( + await restartNativeProject({ + ...f.options, + start: async ({ onStopSignals }) => { + onStopSignals(); + return 1; + }, + }) + ).toBe(1); + expect(f.pending()).toMatchObject({ phase: "cleaned" }); +}); + +test("SIGTERM during cleanup exits 143 after the owned stop", async () => { + const f = stopFixture("never"); + const down = f.options.down; + expect( + await restartNativeProject({ + ...f.options, + down: async () => { + f.interrupt(143); + await down(); + }, + }) + ).toBe(143); + expect(f.events.at(-1)).toBe("stopped"); +}); + +test.each([ + { + hold: "preflight", + lines: [ + "phase preflight", + "stopped: Native restart canceled before cleanup; nothing was stopped.", + ], + absent: ["phase down", "phase start"], + }, + { + hold: "down", + lines: [ + "phase down", + "stopping", + "old run stopped", + "old frontend finalized", + "intent removed", + "stopped: Native project stopped; owned cleanup confirmed.", + ], + absent: ["phase start"], + }, + { + hold: "start", + lines: [ + "phase start", + "stopping", + "intent removed", + "stopped: Native project stopped; owned cleanup confirmed.", + ], + absent: ["ready"], + }, + { + hold: "starting", + lines: [ + "phase starting", + "new run stopped by its own owner", + "intent removed", + ], + absent: ["ready", "stopping", "stopped:"], + }, + { + hold: "serving", + lines: ["ready", "phase serving", "new run stopped by its own owner"], + absent: ["stopping", "stopped:"], + }, +])("the first real SIGINT stops a restart held in $hold, even when inherited as ignored", async ({ + hold, + lines, + absent, +}) => { + // A background job in a non-interactive shell starts with SIGINT ignored; + // the restart must still handle the first one. Its own process group keeps + // the signal from reaching this test runner. + const worker = join( + import.meta.dir, + "fixtures/native-restart-signal-worker.ts" + ); + const child = Bun.spawn( + [ + "/bin/sh", + "-c", + 'trap "" INT; exec "$0" "$1" "$2"', + process.execPath, + worker, + hold, + ], + { stdin: "ignore", stdout: "pipe", stderr: "pipe", detached: true } + ); + const timer = setTimeout(() => child.kill("SIGKILL"), 15_000); + try { + let output = ""; + const reader = child.stdout.getReader(); + const decoder = new TextDecoder(); + while (!output.includes(`phase ${hold}`)) { + const chunk = await reader.read(); + if (chunk.done) { + break; + } + output += decoder.decode(chunk.value); + } + child.kill("SIGINT"); + for (;;) { + const chunk = await reader.read(); + if (chunk.done) { + break; + } + output += decoder.decode(chunk.value); + } + const code = await child.exited; + expect({ code, stderr: await new Response(child.stderr).text() }).toEqual({ + code: 130, + stderr: "", + }); + for (const line of lines) { + expect(output).toContain(line); + } + for (const line of absent) { + expect(output).not.toContain(line); + } + // One "stopping" line at most, and no handler left behind by the restart. + expect(output.split("stopping").length).toBeLessThanOrEqual(2); + expect(output).toContain("exit 130 listeners=0"); + } finally { + clearTimeout(timer); + } +}, 20_000); From b4c9710821d287bc0628c4e1cb07bfce87ef0440 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy <344842+roodboi@users.noreply.github.com> Date: Sat, 7 Nov 2026 15:13:16 -0500 Subject: [PATCH 2/2] fix(native): retire a restart's intent when its new start is canceled early After the stop-signal hand-off, a first Ctrl-C while the new start was still discovering host dependencies or waiting for `runtime up` made `startNativeProject` throw rather than return an interrupted exit, so the restart rethrew and left its pending intent behind although no new graph was admitted; the next plain `hack up` refused. `startNativeProject` now reports a confirmed cancellation: a stop request before any service was requested, with no HTTPS acquisition failure, ends in `NativeStartupCanceledError` with `beforeServices` set once its own cleanup (hooks, finalization) completed. A cleanup failure still replaces it. The restart retires the intent on that outcome and exits with the start's interrupted code; any other failure keeps the pending restart. Refs HACK-1953 Co-Authored-By: Claude Opus 5.5 --- src/backends/native-project-restart.ts | 14 ++++++ src/backends/native-project-start.ts | 51 ++++++++++++++++++-- tests/native-project-restart.test.ts | 47 ++++++++++++++++++ tests/native-project-start.test.ts | 67 ++++++++++++++++++++++++++ 4 files changed, 175 insertions(+), 4 deletions(-) diff --git a/src/backends/native-project-restart.ts b/src/backends/native-project-restart.ts index 745a658a1..3c577eb4c 100644 --- a/src/backends/native-project-restart.ts +++ b/src/backends/native-project-restart.ts @@ -17,6 +17,7 @@ import { saveNativeRestartIntent, withNativeRestartLock, } from "./native-project-run.ts"; +import { NativeStartupCanceledError } from "./native-project-start.ts"; /** * Stop requests of a native restart until its new foreground `up` installs its @@ -338,6 +339,19 @@ async function startOrStop(opts: { if (stop.signal.aborted && !handedOff) { return await canceled(); } + // After the hand-off, a stop request the start confirmed it honored before + // requesting any service (its cleanup completed): the old run stays stopped and + // nothing new was admitted. Any other failure keeps the pending restart. + if ( + handedOff && + error instanceof NativeStartupCanceledError && + error.beforeServices + ) { + await deps.remove({ ...context.scope, expected: selected }); + context.reportStopping(); + context.progress?.({ phase: "stopped", summary: STOPPED }); + return error.interruptedExit; + } throw error; } } diff --git a/src/backends/native-project-start.ts b/src/backends/native-project-start.ts index 7b829c652..d6d2f9042 100644 --- a/src/backends/native-project-start.ts +++ b/src/backends/native-project-start.ts @@ -413,10 +413,44 @@ function httpsVerificationFailed(code: string): Error { } /** Cancellation is not a configuration refusal: it carries no refusal code. */ +/** + * A foreground start ended by a stop request. `beforeServices` is set only when + * the request arrived after the start's own stop handling was installed and + * before any service of the run was requested, and the start's cleanup (hooks, + * finalization) then completed: nothing of the new run was admitted. + */ +export class NativeStartupCanceledError extends ClosedCliError { + readonly interruptedExit: number; + readonly beforeServices: boolean; + constructor(interruptedExit = 130, beforeServices = false) { + super("Native foreground up was canceled; values omitted."); + this.interruptedExit = interruptedExit; + this.beforeServices = beforeServices; + } +} +/** + * The error a failed foreground start reports. A stop request before any service + * was requested is a confirmed cancellation: the run was never admitted. The + * caller's cleanup must still complete, or its own failure replaces this outcome. + */ +function foregroundStartFailure(opts: { + readonly error: unknown; + readonly stopRequested: boolean; + readonly servicesRequested: boolean; + readonly httpsAcquisitionFailure: unknown; + readonly interruptedExit: number; +}): unknown { + if ( + opts.stopRequested && + !opts.servicesRequested && + opts.httpsAcquisitionFailure === undefined + ) { + return new NativeStartupCanceledError(opts.interruptedExit, true); + } + return httpsCleanupFailure(opts.httpsAcquisitionFailure, opts.error); +} function canceled(): Error { - return new ClosedCliError( - "Native foreground up was canceled; values omitted." - ); + return new NativeStartupCanceledError(); } function requireActiveStartup(signal: AbortSignal | undefined): void { @@ -1812,6 +1846,8 @@ export async function startNativeProject(opts: { let hooks: Hooks | undefined; let foregroundStop: NativeForegroundStop | undefined; let failed = false; + // Set before the first service of this run is requested from the runtime. + let servicesRequested = false; let finalization: | Awaited> | undefined; @@ -2126,6 +2162,7 @@ export async function startNativeProject(opts: { progress({ phase: "services", services }); let stopWatching: (() => Promise) | undefined; try { + servicesRequested = true; code = await deps.serve({ runtime: opts.runtime, projectRoot: opts.scope.projectRoot, @@ -2400,7 +2437,13 @@ export async function startNativeProject(opts: { }); } catch (error) { failed = true; - throw httpsCleanupFailure(httpsAcquisitionFailure, error); + throw foregroundStartFailure({ + error, + stopRequested: controller.signal.aborted, + servicesRequested, + httpsAcquisitionFailure, + interruptedExit: stopSignals.interruptedExit(), + }); } finally { cancelAwsExpiry?.(); // The stop listeners stay installed until down.after and the finalization diff --git a/tests/native-project-restart.test.ts b/tests/native-project-restart.test.ts index 3fd1e113b..58b2fef7c 100644 --- a/tests/native-project-restart.test.ts +++ b/tests/native-project-restart.test.ts @@ -27,6 +27,7 @@ import { saveNativeRestartIntent, withNativeRestartLock, } from "../src/backends/native-project-run.ts"; +import { NativeStartupCanceledError } from "../src/backends/native-project-start.ts"; import { ClosedCliError } from "../src/lib/cli-result.ts"; import { expectClosedRefusal, @@ -2116,6 +2117,52 @@ test("a new start stopped by request before ready leaves no pending restart", as ]); }); +test("a start canceled after the hand-off before any service retires the intent so up resumes", async () => { + const f = stopFixture("never"); + expect( + await restartNativeProject({ + ...f.options, + start: async ({ onStopSignals }) => { + onStopSignals(); + // Ctrl-C while discovering host dependencies or waiting for runtime up. + throw new NativeStartupCanceledError(130, true); + }, + }) + ).toBe(130); + expect(f.pending()).toBeNull(); + expect(f.events).toEqual([ + "preflight", + "intent", + "down", + "finalized", + "remove", + "stopping", + "stopped", + ]); +}); + +test.each([ + { name: "uncertain cleanup", error: new Error("lifecycle cleanup failed") }, + { + name: "a cancellation without confirmed cleanup", + error: new NativeStartupCanceledError(130, false), + }, +])("a start that ends in $name after the hand-off keeps the pending restart", async ({ + error, +}) => { + const f = stopFixture("never"); + await expect( + restartNativeProject({ + ...f.options, + start: async ({ onStopSignals }) => { + onStopSignals(); + throw error; + }, + }) + ).rejects.toBe(error); + expect(f.pending()).toMatchObject({ phase: "cleaned" }); +}); + test("a new start that fails without a stop request keeps the pending restart", async () => { const f = stopFixture("never"); expect( diff --git a/tests/native-project-start.test.ts b/tests/native-project-start.test.ts index b99ece5de..3494024cd 100644 --- a/tests/native-project-start.test.ts +++ b/tests/native-project-start.test.ts @@ -40,6 +40,7 @@ import { import { NATIVE_CONFIGURATION_REFUSAL, NATIVE_SHARED_SOURCE_REFUSAL, + NativeStartupCanceledError, type NativeStartupProgress, nativeReadinessArgs, nativeReadySummary, @@ -1163,6 +1164,72 @@ test("cancellation while runtime up waits on a held lease issues no later runtim expect(events).not.toContain("save"); }); +function heldRuntimeUp( + invoke: NonNullable< + Parameters[0]["dependencies"] + >["invoke"], + controller: AbortController +) { + return async (call: Parameters>[0]) => { + if (call.args[0] === "runtime" && call.args[1] === "up") { + queueMicrotask(() => controller.abort()); + await new Promise((_, reject) => + call.signal?.addEventListener( + "abort", + () => reject(new Error("runtime killed while waiting")), + { once: true } + ) + ); + } + return await invoke!(call); + }; +} + +test("a stop request before any service is a confirmed cancellation once cleanup completes", async () => { + const { opts, events } = await fixture(false); + const controller = new AbortController(); + opts.dependencies.invoke = heldRuntimeUp( + opts.dependencies.invoke, + controller + ); + const error = await startNativeProject({ + ...opts, + signal: controller.signal, + }).then( + () => undefined, + (caught: unknown) => caught + ); + expect(error).toBeInstanceOf(NativeStartupCanceledError); + expect(error).toMatchObject({ beforeServices: true, interruptedExit: 130 }); + expect(events.at(-1)).toBe("cleanup"); +}); + +test("a stop request whose cleanup fails is not reported as a confirmed cancellation", async () => { + const { opts } = await fixture(false); + const controller = new AbortController(); + opts.dependencies.invoke = heldRuntimeUp( + opts.dependencies.invoke, + controller + ); + const error = await startNativeProject({ + ...opts, + signal: controller.signal, + before: async () => ({ + cleanup: async () => { + throw new Error("lifecycle cleanup failed"); + }, + ready: async () => undefined, + }), + }).then( + () => undefined, + (caught: unknown) => caught + ); + expect(error).toBeDefined(); + expect( + error instanceof NativeStartupCanceledError && error.beforeServices + ).toBe(false); +}); + test("cancellation after readiness retains mapping when data is preserved", async () => { const { opts, events } = await fixture(); const controller = new AbortController();