diff --git a/docs/diagrams/compose-lifecycle-retirement.mmd b/docs/diagrams/compose-lifecycle-retirement.mmd index 31885dc45..af5f7cecf 100644 --- a/docs/diagrams/compose-lifecycle-retirement.mmd +++ b/docs/diagrams/compose-lifecycle-retirement.mmd @@ -7,8 +7,10 @@ stateDiagram-v2 attached --> captured : Compose exits captured --> signalled : owned mux stop signalled --> commandsRetired : no live command members - signalled --> retained : observation unknown + signalled --> signalled : failed census / retry within budget + signalled --> retained : no census succeeds by deadline signalled --> escalated : 5 s grace expires + signalled --> escalated : second Ctrl-C / identity rechecked escalated --> commandsRetired : no live command members escalated --> retained : deadline or probe error commandsRetired --> wrappersRetired : no live wrapper members @@ -27,6 +29,8 @@ stateDiagram-v2 FIFO EOF is not group absence TERM separately grouped descendants Revalidate member identity before TERM or KILL + Probe EPERM needs census + Delivery EPERM plus live members refuses Zombie-only groups are retired After KILL allow 2 s to confirm retirement end note diff --git a/docs/diagrams/compose-lifecycle-retirement.svg b/docs/diagrams/compose-lifecycle-retirement.svg index 125c4bc6e..3905430cc 100644 --- a/docs/diagrams/compose-lifecycle-retirement.svg +++ b/docs/diagrams/compose-lifecycle-retirement.svg @@ -1,4 +1,4 @@ -Compose tmux lifecycle retirementCompose exitsowned mux stopno live command membersobservation unknown5 s grace expiresno live command membersdeadline or probe errorno live wrapper members5 s grace expiresno live wrapper membersdeadline or probe errorremove owned recordattachedcommandsRetiredescalatedwrappersRetiredwrapperEscalatedretiredcapturedsignalledretainedCapture roots only with matching PID and birthMux acknowledgement is not group exitFIFO EOF is not group absenceTERM separately grouped descendantsRevalidate member identity before TERM or KILLZombie-only groups are retiredAfter KILL allow 2 s to confirm retirementINV unknown is retainedReport unconfirmed retirementKeep dependent helpers \ No newline at end of file +]]>Compose tmux lifecycle retirementCompose exitsowned mux stopno live command membersfailed census / retry withinbudgetno census succeeds bydeadline5 s grace expiressecond Ctrl-C / identityrecheckedno live command membersdeadline or probe errorno live wrapper members5 s grace expiresno live wrapper membersdeadline or probe errorremove owned recordattachedcommandsRetiredescalatedwrappersRetiredwrapperEscalatedretiredcapturedsignalledretainedCapture roots only with matching PID and birthMux acknowledgement is not group exitFIFO EOF is not group absenceTERM separately grouped descendantsRevalidate member identity before TERM or KILLProbe EPERM needs censusDelivery EPERM plus live members refusesZombie-only groups are retiredAfter KILL allow 2 s to confirm retirementINV unknown is retainedReport unconfirmed retirementKeep dependent helpers \ No newline at end of file diff --git a/docs/lifecycle.md b/docs/lifecycle.md index 0f3eaee7c..5f2e3017c 100644 --- a/docs/lifecycle.md +++ b/docs/lifecycle.md @@ -28,16 +28,65 @@ again before signalling. The wrapper forwards TERM to its main command group; Hack sends TERM to separately grouped descendants that the wrapper does not signal. It waits for the detached commands to disappear before retiring the FIFO wrapper and its reader. Each phase allows 5 seconds for graceful -exit, escalates surviving groups once with SIGKILL, then waits up to 2 seconds for -retirement (at most 14 seconds of group waiting across both phases, plus bounded mux -and process-identity queries). The capture allows two 1-second group snapshots -alongside a 2-second identity read; each signal batch checks identities again with -bounded queries. Explicit `hack down` uses the same command-before-wrapper order +exit, escalates surviving groups once with SIGKILL, then gives confirmation a +2-second polling budget. Each `ps` call can itself take up to 2 seconds, including +a final call begun at the polling deadline: these budgets are not hard wall-clock +limits. A failed census is retried within the current observation budget. If no +read succeeds, cleanup stays unconfirmed; a successful live observation still +requires retirement or escalation. Probe EPERM means the group may still exist: on +macOS, even a zombie-only group can return it. Hack uses the process census to +distinguish live members from zombies. TERM or KILL denied with EPERM is benign +only when the following observation proves retirement; live members still cause +a refusal. Failed reads retry within the existing phase budget. +Explicit `hack down` uses the same command-before-wrapper order after interrupting its owned lifecycle panes. Commands that finish their TERM handler sooner do not wait out the grace period. +A second Ctrl-C skips the remaining command grace and requests the single SIGKILL; +identity/ownership checks and post-KILL confirmation still run. An in-flight +bounded `ps` or mux call finishes before that request can take effect. +The foreground guard counts Ctrl-Cs from before Compose starts: two interrupts +while Compose stops containers also skip the later lifecycle command grace. +The wrapper/reader phase keeps its five-second reap window even after a forced +command stop, so the wrapper can remove its private FIFO directory before exiting. + +| Lifecycle shutdown case | With prompt process reads (excluding Compose and mux shutdown) | +| --- | --- | +| Command finishes a one-second TERM handler; wrapper reaps it | About 1 second, plus polling/scheduling | +| Command ignores TERM; wrapper reaps after KILL | About 5 seconds, plus KILL confirmation/polling | +| Command needs KILL; wrapper then also needs KILL | About 10 seconds, plus both KILL confirmations; confirmation polling budgets can add up to 4 seconds | +| Every process observation consumes its timeout | Up to 48 seconds across both group phases, as detailed below; capture and mux checks are additional | + +The conservative 48-second group ceiling includes two initial-observation budgets +of 5 seconds (up to 7 seconds each with one final 2-second read), two grace budgets +(up to 7 seconds each), two confirmation budgets (up to 4 seconds each), and three +signal-identity batches (up to 4 seconds each with one failed-read retry). No census +success within an initial budget refuses cleanup without signalling. Capture adds +up to 4 seconds: two 1-second snapshots run alongside up to two 2-second identity +reads. The normal owned foreground mux-stop path uses a token read and a stop, +each bounded by `HACK_MUX_COMMAND_TIMEOUT_MS` (15 seconds by default); a failed stop +can add one bounded presence check. Thus the worst-case process/mux allowance is +97 seconds before optional identity refresh, rather than 14 seconds. A missing +launch birth adds up to two owner reads, one parallel pane-query batch and two +identity reads: at most another 49 seconds with default mux timeouts. These are +conservative allowances for successful observations near deadlines, not a timer +that every shutdown waits out. Filesystem I/O, OS scheduling, the instance-lock +wait and Compose's own service stop grace are separate. Explicit `down` also sends +an interrupt to each lifecycle window and waits 750 ms before this group path; +its per-window mux calls and subsequent shutdown work are additional. An unreaped zombie has exited and runs no code: a group containing only zombies counts as retired. A live member in the same group still requires shutdown; -failed process observation remains unconfirmed. +failed process observations never count as absence. +Native launch cancellation still permits the owned cleanup to retry a missing +start time: its ownership and artifact checks remain active independently of the +canceled startup deadline. Failed native launches also use a cleanup freshness +check independent of canceled startup inputs: admission, hook ownership, private +directory identity, the saved owner record and the mux token are still checked. +Normal startup and ordinary stop retain their full input freshness checks. + +For value-free retirement diagnostics, set `HACK_LIFECYCLE_RETIREMENT_TRACE=1`. +Stderr records fixed decision labels, PIDs/group IDs, counts and boolean outcomes. +It excludes paths, names, command/env values, ownership tokens and raw errors. +Leave it unset for normal operation. `hack doctor --fix` also captures groups before mux teardown and waits for commands before wrappers when repairing an ownership-proven orphan. An unconfirmed retirement keeps that entry and is reported while repair continues @@ -56,8 +105,13 @@ grant signal authority or make an unrelated live group count as retained work. Zellij does not await process-group retirement; it is scheduled for removal in v5.0.0. -Retrying missing launch-time identity observations during owned foreground cleanup -remains a follow-up: a missing start time can currently leave cleanup unconfirmed. +If the launch missed a pane start time, foreground cleanup re-reads the current +pane in the recorded window while checking the session's ownership token before +and after the query. It fills only a missing birth for that same current pane; +it never replaces a stored birth or borrows identity from a reused PID outside +the window. A failed pane query leaves the original missing birth intact: Hack +still attempts the token-checked mux stop, then retains unconfirmed descendant +state. A replaced owner still refuses the stop. ![Foreground lifecycle retirement](diagrams/compose-lifecycle-retirement.svg) diff --git a/docs/reference/v5-model-coverage.md b/docs/reference/v5-model-coverage.md index 1aabda8e0..7129b9f5b 100644 --- a/docs/reference/v5-model-coverage.md +++ b/docs/reference/v5-model-coverage.md @@ -67,7 +67,7 @@ must be attached to the eventual exact integration revision. | Post-Compose bridge attachment: endpoint placeholder persists or resolves to owned/foreign ID (HACK-2141) | **Coverage gap:** no maintained model of external Docker endpoint attachment timing | `waitForNativeComposeNetworkMembers` retries only owner-issued member-ID refusal, re-acquiring complete proof; `native-compose-ownership.test.ts` covers the three-ID/one-placeholder listing, resolution, persistence, foreign member, cancellation, deadline and nonretryable errors. `native-compose-endpoint-settlement-command.test.ts` drives source CLI `up` through readiness and the prepared guard independently, with a placeholder that resolves on the next query; strict/saved policy is unchanged | Two-second monotonic retry window plus one bounded probe; Docker read consistency and eventual attachment remain external assumptions. `native-config-process-policy` is the live isolated regression; repeated CI runs remain separate evidence. Other effect-adjacent reads still fail closed immediately: the routed-run guard, strict `afterReadiness` re-read, one-off run pre/post-removal checks, saved exec guards and other saved non-observe commands, saved stop/down reads, and file-owner pre/post-effect/readiness checks in `native-compose-file-command.ts`. A detaching-endpoint placeholder was not observed in this investigation; those paths remain known gaps. | | Compose detached startup: short job exits before observation; old completed container replaced between samples | **Coverage gap:** no maintained model of external Docker observation consistency | `waitForComposeStartupState` corroborates identities and classifies only the latest snapshot within the remaining start budget. `compose-startup-state.test.ts` covers sub-second interleavings, replacement, failure and deadline; `project-up-startup-state.test.ts` replays the Compose/Docker boundary. `compose-short-oneshot` checks real repeated replacement and sub-second exit zero | Assumes Docker listings eventually describe current containers; two matching identity listings cannot prevent replacement after the final read. Unit bounds are one job, two identities and a 1 s virtual budget. Real-project acceptance and Docker CI remain separate gates. | | Compose foreground first/second interrupt and lifecycle signal handoff | **Coverage gap:** no maintained model covers the external Compose shutdown protocol | `composeRuntimeBackend.up` isolates the Compose session, forwards each signal once, and waits for child exit/log EOF; `runUpCommand` retires the setup signal owner before attached launch. `tests/compose-foreground-signal.test.ts` covers delayed graceful completion, second interrupt and listener disposal; `tests/e2e/scenarios/compose-foreground-signal.ts` checks both real containers immediately after a process-group SIGINT and CLI exit. | Finite subprocess tests cover one or two interrupts and one child; Docker shutdown success is observed by the live regression, not inferred from child exit. Docker daemon failure, wrapper SIGKILL/crash and an unresponsive Compose client are not qualified. The first interrupt has no forced deadline; Compose owns its configured service stop grace periods. | -| HACK-2184: foreground lifecycle process groups retire before successful cleanup/state removal | **Coverage gap:** no maintained model covers mux acknowledgement versus external group exit | `abortOwnedSession` awaits `createLifecycleSessionRetirement`; capture owned pane descendants before mux teardown, settle command groups before wrapper/reader groups, then remove owned state. `terminateLifecycleProcessGroups` observes absence after TERM and after its one KILL escalation. `tests/lifecycle-group-settlement.test.ts` uses deterministic time/signal boundaries; `tests/project-lifecycle-controller-ownership.test.ts` blocks retirement and verifies cleanup/state cannot complete early. Removing either wait boundary fails its regression. `tests/lifecycle-fifo-retirement.test.ts` exercises real delayed and TERM-resistant FIFO writers plus a separately grouped cooperative worker; only groups outside the wrapper’s forwarded TERM receive an additional identity-checked TERM; `repairProjectLifecycleSessions` uses the same ordered capture/retirement before removing an orphan entry, and `tests/lifecycle-hygiene-retirement.test.ts` gates state removal, retains uncertainty, continues other repairs and exercises a real TERM-resistant FIFO writer; the Docker foreground scenario requires separate app/worker ready markers, polls both named running containers before Ctrl-C, and verifies the one-second TERM handler finished without KILL. `tests/compose-foreground-readiness.test.ts` rejects replayed single-service markers and stale container listings; settlement, absent-session and repair tests cover zombie-only retirement and mixed live members. [Statechart](../diagrams/compose-lifecycle-retirement.svg), [source](../diagrams/compose-lifecycle-retirement.mmd). | Each group phase has 5 s grace plus 2 s confirmation after at most one KILL; two phases total at most 14 s plus bounded mux calls/process observations. ESRCH or a successful census containing only zombie members proves retirement; probe/permission errors retain uncertainty, ownership and dependent helpers. Deterministic cases cover selective TERM for two groups; real FIFO fixtures cover a main command waiting for a separate worker group, and the Docker fixture covers two services and one persistent command. OS scheduling, PID reuse outside the active ownership interval, CLI SIGKILL/crash, escaped descendants and zellij retirement are not qualified by these tests. HACK-2184 retains the follow-up to refresh missing launch-time births through owned cleanup. Zellij tracking is dropped because HACK-2215 replaces the lifecycle wrapper and zellij is scheduled for removal in v5.0.0. `up.after` after successful attached Compose return and explicit down hooks are inherited v4.3.0 ordering, documented in lifecycle.md. | +| HACK-2184: foreground lifecycle process groups retire before successful cleanup/state removal | **Coverage gap:** no maintained model covers mux acknowledgement versus external group exit | `abortOwnedSession` awaits `createLifecycleSessionRetirement`; capture owned pane descendants before mux teardown, settle command groups before wrapper/reader groups, then remove owned state. `terminateLifecycleProcessGroups` observes absence after TERM and after its one KILL escalation. `tests/lifecycle-group-settlement.test.ts` uses deterministic time/signal boundaries; `tests/project-lifecycle-controller-ownership.test.ts` blocks retirement and verifies cleanup/state cannot complete early. Removing either wait boundary fails its regression. `tests/lifecycle-fifo-retirement.test.ts` exercises real delayed and TERM-resistant FIFO writers plus a separately grouped cooperative worker; only groups outside the wrapper’s forwarded TERM receive an additional identity-checked TERM; `repairProjectLifecycleSessions` uses the same ordered capture/retirement before removing an orphan entry, and `tests/lifecycle-hygiene-retirement.test.ts` gates state removal, retains uncertainty, continues other repairs and exercises a real TERM-resistant FIFO writer; the Docker foreground scenario requires separate app/worker ready markers, polls both named running containers before Ctrl-C, and verifies the one-second TERM handler finished without KILL. `tests/compose-foreground-readiness.test.ts` rejects replayed single-service markers and stale container listings; settlement, absent-session and repair tests cover zombie-only retirement and mixed live members. [Statechart](../diagrams/compose-lifecycle-retirement.svg), [source](../diagrams/compose-lifecycle-retirement.mmd). | Each group phase has a 5 s initial-observation retry budget, 5 s grace and 2 s confirmation after one KILL; each census can overshoot by 2 s. Two phases allow up to 48 s including bounded signal-identity retries, plus capture/mux/optional birth refresh (see lifecycle.md timing table). Second Ctrl-C skips command grace, not fences, exit confirmation or the wrapper reap window; the foreground guard also counts interrupts during Compose shutdown. Failed censuses retry within their deadline; no successful census remains unknown. ESRCH or a successful census containing only zombie members proves retirement; probe EPERM falls through to the zombie-aware census (including macOS zombie-only groups). Delivery EPERM is benign only after observed retirement; a live member refuses, and failed reads retry within the same phase deadline. Other probe errors retain uncertainty, ownership and dependent helpers. Deterministic EPERM-to-ESRCH, mixed-live and denied TERM/KILL cases plus a real held-zombie fixture on macOS/Linux cover these branches; Linux execution does not qualify macOS. Deterministic cases cover selective TERM for two groups; real FIFO fixtures cover a main command waiting for a separate worker group, and the Docker fixture covers two services and one persistent command. OS scheduling, PID reuse outside the active ownership interval, CLI SIGKILL/crash, escaped descendants and zellij retirement are not qualified by these tests. Native launch abandonment excludes canceled startup inputs and budget from missing-birth refresh, while rechecking admission, hook ownership, directory pins, saved owner and mux token; normal start/stop retains full input freshness. A portable adapter regression cancels both startup-input authority and budget; a real adapter regression holds and fails the launch census, cancels, then requires owned mux and state removal. The native recovery fixture retries probe EPERM until ESRCH instead of mistaking the error for absence. FIFO and held-zombie files run in both hosted OS pools. Owned foreground cleanup refreshes only missing births through current pane membership with owner-token checks on both sides; stored births and replacement refusals remain strict. tests/lifecycle-cleanup-followup.test.ts and the controller launch-read failure regression cover refresh, second-interrupt propagation and shared-helper fences. Zellij tracking is dropped because HACK-2215 replaces the lifecycle wrapper and zellij is scheduled for removal in v5.0.0. `up.after` after successful attached Compose return and explicit down hooks are inherited v4.3.0 ordering, documented in lifecycle.md. | These rows identify coverage, not closure. A local mutant or spy proves handling, not that the kernel/provider produces the modeled condition. No model establishes diff --git a/docs/versions/v5/getting-started.md b/docs/versions/v5/getting-started.md index 1b5d73720..22aa34933 100644 --- a/docs/versions/v5/getting-started.md +++ b/docs/versions/v5/getting-started.md @@ -99,8 +99,13 @@ Do not paste raw logs into a public issue. For foreground `hack up`, Ctrl-C stops the project instead. With tmux, Hack also waits for lifecycle commands and separately grouped workers: they get TERM and five seconds to finish handling it -before one forced stop. Hack reports unconfirmed cleanup; repeated Ctrl-C does -not skip that wait. Ctrl-C does not run `down.before` or `down.after` hooks; +before one forced stop. A second Ctrl-C skips the remaining command grace; Hack +still checks ownership, confirms exit and lets the wrapper finish cleanup. +Slow process reads can extend shutdown; +Hack retries transient read failures and reports cleanup it cannot confirm. +Exited processes waiting to be reaped count as stopped, including on macOS. +Canceling startup still allows cleanup to retry a missed process start time. +Ctrl-C does not run `down.before` or `down.after` hooks; use explicit `hack down` for those hooks. Attached `up.after` runs only after Compose returns successfully, so it does not run while the project stays attached. diff --git a/scripts/test-shard-plan.ts b/scripts/test-shard-plan.ts index 04204dfeb..5544a955e 100644 --- a/scripts/test-shard-plan.ts +++ b/scripts/test-shard-plan.ts @@ -514,6 +514,10 @@ export const MACOS_POOL_FILES: Readonly> = {}; * added to the Linux pool as well (HACK-2142). */ export const BOTH_POOL_FILES: Readonly> = { + "tests/lifecycle-fifo-retirement.test.ts": + "FIFO retirement needs Linux process groups and macOS zombie-probe EPERM coverage", + "tests/lifecycle-zombie-retirement.test.ts": + "held zombies return probe success on Linux and can return EPERM on macOS", "tests/v4-upgrade.test.ts": "historical upgrade must preserve keys and values on Linux and macOS", "tests/upgrade-installer.test.ts": diff --git a/src/backends/native-authored-host-processes.ts b/src/backends/native-authored-host-processes.ts index 83c31a2d8..a6cbbfb3b 100644 --- a/src/backends/native-authored-host-processes.ts +++ b/src/backends/native-authored-host-processes.ts @@ -7,6 +7,7 @@ import { LIFECYCLE_PROCESS_CLIENT, serveLifecycleProcessDelivery, } from "../lib/lifecycle-process-delivery.ts"; +import { traceLifecycleRetirement } from "../lib/lifecycle-retirement-trace.ts"; import { type LifecycleStateEntry, resolveLifecycleStatePath, @@ -552,6 +553,9 @@ export async function createNativeAuthoredHostProcesses(input: { name: string ) => Promise>>; readonly assertFresh: () => Promise; + /** Admission/hook ownership for this live owner's failed-launch cleanup. + * Must not depend on the canceled startup input acquisition. */ + readonly assertCleanupFresh?: () => Promise; readonly signal: AbortSignal; readonly remaining: () => number; }): Promise { @@ -593,6 +597,7 @@ export async function createNativeAuthoredHostProcesses(input: { let active = true; let began = false; let launchIntent = false; + let abandoningLaunch = false; let abandoned = false; let stopped = false; let ready: Awaited> | undefined; @@ -607,13 +612,19 @@ export async function createNativeAuthoredHostProcesses(input: { await writeExclusive(path, JSON.stringify(owner)); await synchronizeDirectories(directories); const saved = await capture(path); - const check = async () => { + const check = async (assertCurrent = opts.assertFresh) => { + traceLifecycleRetirement("native-owner-check", { active }); if (!active) { return refuse(); } - await opts.assertFresh(); + await assertCurrent(); + traceLifecycleRetirement("native-freshness-check-passed", { + startupInputsRequired: assertCurrent === opts.assertFresh, + }); await recheckDirectories(directories); + traceLifecycleRetirement("native-directories-fresh"); await unchanged(saved); + traceLifecycleRetirement("native-owner-fresh"); }; /** Groups this owner launched, recorded before their launch completes. */ const launchedGroups = new Set(); @@ -635,8 +646,19 @@ export async function createNativeAuthoredHostProcesses(input: { env: {}, definitionHash: opts.semanticHash, assertFresh: async () => { - await check(); - opts.remaining(); + traceLifecycleRetirement("native-controller-fresh", { + abandoningLaunch, + }); + await check( + abandoningLaunch + ? (opts.assertCleanupFresh ?? opts.assertFresh) + : opts.assertFresh + ); + // Startup cancellation must not veto the missing-birth refresh during + // abandonment. Owner/artifact freshness still holds for every check. + if (!abandoningLaunch) { + opts.remaining(); + } }, mux: async () => { const mux = await resolveMux({}); @@ -707,10 +729,20 @@ export async function createNativeAuthoredHostProcesses(input: { * anything unproven stays retained. */ const abandonLaunch = async () => { + abandoningLaunch = true; + traceLifecycleRetirement("native-abandon-begin"); try { const launched = await currentEntry(opts.scope, composeProject); - await controller.abort(); + traceLifecycleRetirement("native-abandon-entry", { + processes: launched?.processes.length ?? 0, + groups: [...launchedGroups], + }); + const result = await controller.abort(); + traceLifecycleRetirement("native-abandon-controller", { + retained: result !== undefined, + }); if ((await currentEntry(opts.scope, composeProject)) !== null) { + traceLifecycleRetirement("native-abandon-retained"); return; } const groups = new Set(launchedGroups); @@ -724,7 +756,11 @@ export async function createNativeAuthoredHostProcesses(input: { await settledGroup(group); } abandoned = true; - } catch { + traceLifecycleRetirement("native-abandon-settled", { + groups: [...groups], + }); + } catch (error) { + traceLifecycleRetirement("native-abandon-failed", {}, error); // Unproven teardown keeps the launch intent and owner retained. } }; diff --git a/src/backends/native-authored-project-start.ts b/src/backends/native-authored-project-start.ts index eb57a40e9..95fb9cd4b 100644 --- a/src/backends/native-authored-project-start.ts +++ b/src/backends/native-authored-project-start.ts @@ -578,6 +578,13 @@ async function prepareHostSelection(opts: { await opts.admission.assertHeld(); await owner.assertFresh(); }, + // A failed launch owns its mux token before startup inputs can be canceled. + // Abort must retain admission and hook ownership, independently of those + // inputs; normal starts and stops still use the full freshness check above. + assertCleanupFresh: async () => { + await opts.admission.assertHeld(); + await owner.assertFresh(); + }, }); return { hooks, lifecycle, persistent, owner, processes }; } diff --git a/src/commands/project.ts b/src/commands/project.ts index eee2c8fb9..c2569eb2e 100644 --- a/src/commands/project.ts +++ b/src/commands/project.ts @@ -2303,11 +2303,11 @@ export function signalCleanupKeepingStartedHelpers(opts: { if (!cleanup) { return null; } - return async () => { + return async (cleanupOpts) => { if (opts.containerStartBegun()) { opts.keepSharedHelpers()?.(); } - return await cleanup(); + return await cleanup(cleanupOpts); }; } @@ -2654,7 +2654,7 @@ export function withSharedHelperCleanup(opts: { keepSharedHelpers: () => { containersRunning = true; }, - cleanup: async () => { + cleanup: async (cleanupOpts) => { const processCleanup = opts.cleanup; let outcome: ProcessCleanupOutcome | null = null; // The session could not be confirmed stopped and was kept: its shared @@ -2666,7 +2666,9 @@ export function withSharedHelperCleanup(opts: { // only clean up while the session is still this start's own. if (processCleanup) { outcome = await runProcessCleanup(async () => { - if ((await processCleanup()) === LIFECYCLE_SESSION_RETAINED) { + if ( + (await processCleanup(cleanupOpts)) === LIFECYCLE_SESSION_RETAINED + ) { sessionRetained = true; } }); @@ -6887,7 +6889,7 @@ async function runUpCommand({ // Compose may have started some services before it failed or timed // out: their helpers stay unless no container may run. await retainHelpersIfContainersMayRun(); - await lifecycleCleanup?.(); + await lifecycleCleanup?.({ force: foregroundTermination.guard?.force }); await removeProjectRuntimeStateEntry({ projectDir: project.projectDir, composeProject: lifecycleComposeProject, diff --git a/src/lib/foreground-termination-guard.ts b/src/lib/foreground-termination-guard.ts index 29db67a4c..980518af7 100644 --- a/src/lib/foreground-termination-guard.ts +++ b/src/lib/foreground-termination-guard.ts @@ -1,16 +1,23 @@ /** * Keep the foreground command's signal owner alive through caller cleanup. * The subprocess runner forwards signals while its child is live; this guard - * only records cancellation, so repeated signals cannot abort cleanup or turn + * records cancellation and requests escalation on a second Ctrl-C, without turning * a lifecycle cleanup's re-raised signal into a second child interrupt. */ export function installForegroundTerminationGuard(): { readonly exitCode: () => 130 | 143 | null; + readonly force: AbortSignal; readonly dispose: () => void; } { let code: 130 | 143 | null = null; + let interrupts = 0; + const force = new AbortController(); const onInterrupt = (): void => { code ??= 130; + interrupts += 1; + if (interrupts >= 2) { + force.abort(); + } }; const onTerminate = (): void => { code ??= 143; @@ -19,6 +26,7 @@ export function installForegroundTerminationGuard(): { process.on("SIGTERM", onTerminate); return { exitCode: () => code, + force: force.signal, dispose: () => { process.off("SIGINT", onInterrupt); process.off("SIGTERM", onTerminate); diff --git a/src/lib/lifecycle-retirement-trace.ts b/src/lib/lifecycle-retirement-trace.ts new file mode 100644 index 000000000..4fa576a27 --- /dev/null +++ b/src/lib/lifecycle-retirement-trace.ts @@ -0,0 +1,63 @@ +type RetirementTraceEvent = + | "native-owner-check" + | "native-freshness-check-passed" + | "native-directories-fresh" + | "native-owner-fresh" + | "native-controller-fresh" + | "native-abandon-begin" + | "native-abandon-entry" + | "native-abandon-controller" + | "native-abandon-retained" + | "native-abandon-settled" + | "native-abandon-failed" + | "abort-begin" + | "abort-refresh-fresh" + | "abort-refresh-token" + | "abort-refreshed" + | "abort-mux-stop-begin" + | "abort-mux-stop-end" + | "abort-retired" + | "abort-failed" + | "birth-refresh-begin" + | "birth-refresh-panes" + | "birth-refresh-identities" + | "capture-begin" + | "capture-reads" + | "capture-complete" + | "retirement-captured" + | "retirement-mux-stopped" + | "group-signal" + | "group-signal-failed" + | "group-census" + | "group-census-failed" + | "group-wait-ended"; + +/** Opt-in diagnostics: only fixed event labels, counts, PIDs and boolean outcomes. */ +export function traceLifecycleRetirement( + event: RetirementTraceEvent, + fields: Readonly> = {}, + error?: unknown +): void { + if (process.env.HACK_LIFECYCLE_RETIREMENT_TRACE !== "1") { + return; + } + const code = + typeof error === "object" && error !== null && "code" in error + ? error.code + : undefined; + console.error( + "[lifecycle-retirement]", + JSON.stringify({ + event, + ...fields, + ...(error === undefined + ? {} + : { + failed: true, + eperm: code === "EPERM", + esrch: code === "ESRCH", + timedOut: code === "ETIMEDOUT", + }), + }) + ); +} diff --git a/src/lib/project-lifecycle-controller.ts b/src/lib/project-lifecycle-controller.ts index 226a862a7..072902f4c 100644 --- a/src/lib/project-lifecycle-controller.ts +++ b/src/lib/project-lifecycle-controller.ts @@ -14,6 +14,7 @@ import { import { buildLifecycleSessionName } from "../mux/session-names.ts"; import { logger } from "../ui/logger.ts"; import { readTextFile } from "./fs.ts"; +import { traceLifecycleRetirement } from "./lifecycle-retirement-trace.ts"; import { appendLifecycleLogRecord, ensurePrivateLifecycleLog, @@ -71,12 +72,15 @@ type StartedLifecycleProcess = { * ownership record, for hack down; callers must then keep anything that * session depends on (such as shared helper holders). */ -export type LifecycleOperationCleanup = () => Promise; +export type LifecycleOperationCleanup = (opts?: { + readonly force?: AbortSignal; +}) => Promise; /** The value an operation cleanup resolves to when it kept the session. */ export const LIFECYCLE_SESSION_RETAINED = "session-retained"; const LIFECYCLE_COMMAND_PID_WAIT_ATTEMPTS = 50; const LIFECYCLE_COMMAND_PID_WAIT_INTERVAL_MS = 20; +const PANE_PID_PATTERN = /^\d+$/; export function resolveLifecycleSessionName(opts: { readonly projectName: string; readonly branch: string | null; @@ -106,6 +110,7 @@ export function installLifecycleSignalCleanup(opts: { } let handlingSignal = false; + const force = new AbortController(); const handlers = new Map void>(); const dispose = (): void => { for (const [signal, handler] of handlers) { @@ -116,10 +121,13 @@ export function installLifecycleSignalCleanup(opts: { for (const signal of ["SIGINT", "SIGTERM"] as const) { const handler = (): void => { if (handlingSignal) { + if (signal === "SIGINT") { + force.abort(); + } return; } handlingSignal = true; - void opts.cleanup?.().finally(() => { + void opts.cleanup?.({ force: force.signal }).finally(() => { dispose(); process.kill(process.pid, signal); }); @@ -262,6 +270,7 @@ export function createLifecycleProcessController(opts: { readonly muxCommandTimeoutMs?: number; /** Process retirement boundary; must confirm groups before state removal. */ readonly retireSession?: ReturnType; + readonly readIdentities?: typeof readProcessIdentities; }): { readonly sessionName: string; startFromCommand: (opts: { @@ -284,7 +293,8 @@ export function createLifecycleProcessController(opts: { }); const startedProcesses: StartedLifecycleProcess[] = []; const retireSession = - opts.retireSession ?? createLifecycleSessionRetirement(); + opts.retireSession ?? + createLifecycleSessionRetirement({ readIdentities: opts.readIdentities }); let backendName: MuxBackendName | null = null; let sessionReady = false; let sessionDispositionResolved = false; @@ -724,6 +734,7 @@ export function createLifecycleProcessController(opts: { composeProject: opts.composeProject, assertFresh: opts.assertFresh, timeoutMs: muxTimeoutMs, + readIdentities: opts.readIdentities, }); }); nextIndex += 1; @@ -767,9 +778,9 @@ export function createLifecycleProcessController(opts: { // holds the instance lock: never acquire it again here. getOperationCleanup: () => createdOwnershipToken - ? async () => { + ? async (cleanupOpts) => { await sessionCreationSettled; - return await abortOwnedSession(); + return await abortOwnedSession(cleanupOpts); } : null, getSignalCleanup: () => operationSignalCleanup, @@ -786,22 +797,22 @@ export function createLifecycleProcessController(opts: { * as replacement does. Creation settles first: it holds the lock until its * own failure cleanup is done. */ - async function starterAbort(): Promise< - undefined | typeof LIFECYCLE_SESSION_RETAINED - > { + async function starterAbort(cleanupOpts?: { + readonly force?: AbortSignal; + }): Promise { await sessionCreationSettled; if (!(createdOwnershipToken && backendName)) { return undefined; } const ownership = opts.sessionOwnership; if (!ownership) { - return await abortOwnedSession(); + return await abortOwnedSession(cleanupOpts); } let result: undefined | typeof LIFECYCLE_SESSION_RETAINED; let outcome: Awaited>; try { outcome = await ownership.runIfOwned(async () => { - result = await abortOwnedSession(); + result = await abortOwnedSession(cleanupOpts); }); } catch { outcome = "undecidable"; @@ -821,28 +832,73 @@ export function createLifecycleProcessController(opts: { } /** Stop this start's created session; the caller holds the instance lock. */ - async function abortOwnedSession(): Promise< - undefined | typeof LIFECYCLE_SESSION_RETAINED - > { + async function abortOwnedSession(cleanupOpts?: { + readonly force?: AbortSignal; + }): Promise { if (!(createdOwnershipToken && backendName)) { return undefined; } const backend = (await opts.mux()).backends.get(backendName); let stopped = false; let failure = ""; + traceLifecycleRetirement("abort-begin", { + backendAvailable: backend?.available === true, + processes: startedProcesses.length, + panePids: startedProcesses.flatMap((process) => + process.panePid ? [process.panePid] : [] + ), + missingBirths: startedProcesses.filter( + (process) => process.panePid && !process.paneBirth + ).length, + }); if (backend?.available) { const ownershipToken = createdOwnershipToken; try { - stopped = await retireSession({ + const processes = await refreshMissingLifecyclePaneBirths({ processes: startedProcesses, - stopSession: () => - killLifecycleSessionWithOwnership({ + readIdentities: opts.readIdentities ?? readProcessIdentities, + readPanes: (windowName) => + readTmuxPanePids({ sessionName, windowName, strict: true }), + assertOwned: async () => { + await opts.assertFresh?.(); + traceLifecycleRetirement("abort-refresh-fresh"); + if ( + (await backend.readLifecycleOwnerToken?.({ + name: sessionName, + })) !== ownershipToken + ) { + traceLifecycleRetirement("abort-refresh-token", { + matched: false, + }); + throw new Error( + "Lifecycle ownership changed before identity refresh" + ); + } + traceLifecycleRetirement("abort-refresh-token", { matched: true }); + }, + }); + traceLifecycleRetirement("abort-refreshed", { + missingBirths: processes.filter( + (process) => process.panePid && !process.paneBirth + ).length, + }); + stopped = await retireSession({ + processes, + stopSession: async () => { + traceLifecycleRetirement("abort-mux-stop-begin"); + const result = await killLifecycleSessionWithOwnership({ backend, sessionName, ownershipToken, - }), + }); + traceLifecycleRetirement("abort-mux-stop-end", { stopped: result }); + return result; + }, + force: cleanupOpts?.force, }); - } catch { + traceLifecycleRetirement("abort-retired", { stopped }); + } catch (error) { + traceLifecycleRetirement("abort-failed", {}, error); failure = "Lifecycle process-group retirement could not be confirmed."; } } @@ -950,6 +1006,7 @@ async function startLifecycleProcess(opts: { readonly composeProject: string; readonly assertFresh?: () => Promise; readonly timeoutMs: number; + readonly readIdentities?: typeof readProcessIdentities; }): Promise<{ readonly name: string; readonly windowName: string; @@ -1022,7 +1079,7 @@ async function startLifecycleProcess(opts: { if (!processGroupId && panePid !== undefined) { processGroupId = await readProcessGroupIdForPid({ pid: panePid }); } - const identities = await readProcessIdentities(); + const identities = await (opts.readIdentities ?? readProcessIdentities)(); const paneBirth = identities.find((row) => row.pid === panePid)?.birth; const processGroupBirth = identities.find( (row) => row.pid === processGroupId @@ -1317,7 +1374,10 @@ async function interruptLifecycleTmuxProcesses(opts: { }) ) ); - const identities = await readProcessIdentities(); + let identities = await readProcessIdentities(); + if (identities.length === 0) { + identities = await readProcessIdentities(); + } const processes = panes.flatMap((pids) => pids.map((panePid) => ({ panePid, @@ -1357,27 +1417,110 @@ async function interruptLifecycleTmuxProcesses(opts: { } } +/** + * Recover only missing birth metadata, never replace a recorded incarnation. + * Both sides of the live pane query must still prove the operation's ownership. + */ +export async function refreshMissingLifecyclePaneBirths(opts: { + readonly processes: readonly StartedLifecycleProcess[]; + readonly readPanes: (windowName: string) => Promise; + readonly readIdentities: typeof readProcessIdentities; + readonly assertOwned: () => Promise; +}): Promise { + const missing = opts.processes.filter( + (process) => process.panePid !== undefined && !process.paneBirth + ); + traceLifecycleRetirement("birth-refresh-begin", { + missingBirths: missing.length, + }); + if (missing.length === 0) { + return opts.processes; + } + await opts.assertOwned(); + const panes = new Map( + await Promise.all( + missing.map( + async (process) => + [ + process.windowName, + // A failed/missing window is not permission to fill a birth. Keep + // the original unknown record so the owned mux stop still runs. + await opts + .readPanes(process.windowName) + .catch((): readonly number[] => []), + ] as const + ) + ) + ); + traceLifecycleRetirement("birth-refresh-panes", { + panePids: [...panes.values()].flat(), + }); + let identities = await opts.readIdentities(); + if (identities.length === 0) { + identities = await opts.readIdentities(); + } + traceLifecycleRetirement("birth-refresh-identities", { + rows: identities.length, + matchingPids: identities + .filter((row) => missing.some((process) => process.panePid === row.pid)) + .map((row) => row.pid), + zombiePids: identities + .filter( + (row) => + row.zombie && missing.some((process) => process.panePid === row.pid) + ) + .map((row) => row.pid), + }); + await opts.assertOwned(); + return opts.processes.map((process) => { + if ( + process.paneBirth || + process.panePid === undefined || + !panes.get(process.windowName)?.includes(process.panePid) + ) { + return process; + } + const row = identities.find( + (identity) => !identity.zombie && identity.pid === process.panePid + ); + return row ? { ...process, paneBirth: row.birth } : process; + }); +} + async function readTmuxPanePids(opts: { readonly sessionName: string; readonly windowName: string; + readonly strict?: boolean; }): Promise { const result = await runMuxCommand( [ "tmux", "list-panes", "-t", - `${opts.sessionName}:${opts.windowName}`, + `${opts.strict ? "=" : ""}${opts.sessionName}:${opts.windowName}`, "-F", "#{pane_pid}", ], { stdin: "ignore" } ); if (result.exitCode !== 0) { + if (opts.strict) { + throw new Error("Could not refresh owned lifecycle pane identity"); + } return []; } - return result.stdout + const lines = result.stdout .split("\n") - .map((line) => Number.parseInt(line.trim(), 10)) + .map((line) => line.trim()) + .filter(Boolean); + if ( + opts.strict && + lines.some((line) => !PANE_PID_PATTERN.test(line) || Number(line) <= 0) + ) { + throw new Error("Invalid owned lifecycle pane identity"); + } + return lines + .map((line) => Number.parseInt(line, 10)) .filter((value) => Number.isInteger(value) && value > 0); } diff --git a/src/lib/project-lifecycle-processes.ts b/src/lib/project-lifecycle-processes.ts index a5fd136ce..fa89937f4 100644 --- a/src/lib/project-lifecycle-processes.ts +++ b/src/lib/project-lifecycle-processes.ts @@ -1,3 +1,4 @@ +import { traceLifecycleRetirement } from "./lifecycle-retirement-trace.ts"; import type { LifecycleStateEntry } from "./lifecycle-runtime.ts"; import { type ProcessIdentityRow, @@ -203,6 +204,13 @@ export async function readProcessSnapshot( /** Terminate ownership-proven lifecycle groups with a bounded TERM/KILL sequence. */ export const LIFECYCLE_TERM_GRACE_MS = 5000; +/** A failed census is unknown and may be retried within the observation budget. */ +class LifecycleCensusUnavailable extends Error {} + +function hasProcessErrorCode(error: unknown, code: string): boolean { + return error instanceof Error && "code" in error && error.code === code; +} + /** A zombie holds a PID but runs no code; only live members keep a group active. */ export function selectLiveLifecycleGroups(opts: { readonly groups: readonly number[]; @@ -229,10 +237,14 @@ export async function assertLifecycleProcessGroupRetired(opts: { try { process.kill(-opts.processGroupId, 0); } catch (error) { - if (error instanceof Error && "code" in error && error.code === "ESRCH") { + if (hasProcessErrorCode(error, "ESRCH")) { return; } - throw error; + // macOS can report EPERM for an unreaped zombie-only group. Only the + // following census can distinguish that benign state from live members. + if (!hasProcessErrorCode(error, "EPERM")) { + throw error; + } } if ( selectLiveLifecycleGroups({ @@ -251,6 +263,8 @@ export async function terminateLifecycleProcessGroups(opts: { readonly alreadySignalled?: boolean; /** Only these groups received shutdown through the mux/wrapper. */ readonly alreadySignalledGroups?: readonly number[]; + /** A repeated interrupt skips grace, never identity checks or confirmation. */ + readonly force?: AbortSignal; readonly beforeSignal?: ( groups: readonly number[] ) => Promise; @@ -286,18 +300,38 @@ export async function terminateLifecycleProcessGroups(opts: { identities: await (opts.readIdentities ?? readProcessIdentities)(), }), }; + const deniedDeliveries = new Map(); const signal = (group: number, name: NodeJS.Signals | 0): boolean => { try { runtime.signal(group, name); + traceLifecycleRetirement("group-signal", { + group, + probe: name === 0, + kill: name === "SIGKILL", + }); return true; } catch (error: unknown) { - if (error instanceof Error && "code" in error && error.code === "ESRCH") { + traceLifecycleRetirement( + "group-signal-failed", + { group, probe: name === 0, kill: name === "SIGKILL" }, + error + ); + if (hasProcessErrorCode(error, "ESRCH")) { return false; } - throw new Error( + const failure = new Error( `Could not confirm lifecycle process group ${group} stopped`, { cause: error } ); + if (hasProcessErrorCode(error, "EPERM")) { + // A probe proves neither liveness nor absence. A denied delivery is + // benign only if the next successful observation proves retirement. + if (name !== 0) { + deniedDeliveries.set(group, failure); + } + return true; + } + throw failure; } }; const living = async (pending: readonly number[]): Promise => { @@ -305,22 +339,90 @@ export async function terminateLifecycleProcessGroups(opts: { if (existing.length === 0) { return []; } - const live = new Set(await runtime.liveGroups(existing)); - return existing.filter((group) => live.has(group)); + let live: Set; + try { + live = new Set(await runtime.liveGroups(existing)); + traceLifecycleRetirement("group-census", { + groups: existing, + live: [...live], + }); + } catch (cause) { + traceLifecycleRetirement( + "group-census-failed", + { groups: existing }, + cause + ); + throw new LifecycleCensusUnavailable( + "Could not inspect live lifecycle process-group members", + { cause } + ); + } + const remaining = existing.filter((group) => live.has(group)); + for (const group of remaining) { + const denied = deniedDeliveries.get(group); + if (denied) { + throw denied; + } + } + return remaining; + }; + const observe = async (pending: readonly number[]) => { + try { + return { groups: await living(pending) }; + } catch (error) { + if (!(error instanceof LifecycleCensusUnavailable)) { + throw error; + } + return { unavailable: error }; + } }; - const wait = async ( - pending: readonly number[], - timeoutMs: number - ): Promise => { + const wait = async ({ + pending, + timeoutMs, + firstObservation = false, + allowForce = false, + }: { + readonly pending: readonly number[]; + readonly timeoutMs: number; + readonly firstObservation?: boolean; + readonly allowForce?: boolean; + }): Promise => { const deadline = runtime.now() + timeoutMs; - let remaining = await living(pending); - while (remaining.length > 0 && runtime.now() < deadline) { - await runtime.sleep(Math.min(100, deadline - runtime.now())); - remaining = await living(remaining); + let remaining = [...pending]; + let observed = false; + let unavailable: LifecycleCensusUnavailable | undefined; + for (;;) { + const sample = await observe(remaining); + if (sample.groups) { + remaining = sample.groups; + observed = true; + if (firstObservation || remaining.length === 0) { + return remaining; + } + } else { + unavailable = sample.unavailable; + } + const forced = allowForce && opts.force?.aborted === true; + if (runtime.now() >= deadline || (observed && forced)) { + traceLifecycleRetirement("group-wait-ended", { + groups: remaining, + observed, + forced, + timeoutMs, + }); + if (!observed) { + throw unavailable; + } + return remaining; + } + await runtime.sleep(Math.min(100, Math.max(0, deadline - runtime.now()))); } - return remaining; }; - const initial = await living(groups); + const initial = await wait({ + pending: groups, + timeoutMs: LIFECYCLE_TERM_GRACE_MS, + firstObservation: true, + }); const signalled = new Set(opts.alreadySignalledGroups); const needsTerm = opts.alreadySignalled ? [] @@ -334,7 +436,11 @@ export async function terminateLifecycleProcessGroups(opts: { signal(group, "SIGTERM"); } } - const remaining = await wait(initial, LIFECYCLE_TERM_GRACE_MS); + const remaining = await wait({ + pending: initial, + timeoutMs: LIFECYCLE_TERM_GRACE_MS, + allowForce: true, + }); const targets = remaining.length > 0 && opts.beforeSignal ? await opts.beforeSignal(remaining) @@ -343,7 +449,7 @@ export async function terminateLifecycleProcessGroups(opts: { await opts.assertFresh?.(); signal(group, "SIGKILL"); } - const unconfirmed = await wait(remaining, 2000); + const unconfirmed = await wait({ pending: remaining, timeoutMs: 2000 }); if (unconfirmed.length > 0) { throw new Error( `Could not confirm lifecycle process groups stopped after SIGKILL: ${unconfirmed.join(", ")}` @@ -359,6 +465,7 @@ type LifecycleSessionRetirementOptions = { readonly processes: readonly LifecyclePaneIdentity[]; readonly stopSession: () => Promise; readonly assertFresh?: () => Promise; + readonly force?: AbortSignal; }; type RetirementBoundary = { readonly readSnapshot: () => Promise; @@ -378,7 +485,7 @@ type CapturedLifecycleGroups = { * the captured groups across a retry if the mux exited but retirement failed. */ export function createLifecycleSessionRetirement(boundary?: { - readonly readSnapshot: () => Promise; + readonly readSnapshot?: () => Promise; readonly readIdentities?: () => Promise; }): (opts: LifecycleSessionRetirementOptions) => Promise { let captured: @@ -394,15 +501,24 @@ export function createLifecycleSessionRetirement(boundary?: { }; return async (opts) => { if (!captured) { + traceLifecycleRetirement("capture-begin", { + processes: opts.processes.length, + }); captured = await captureRetirementGroups({ processes: opts.processes, os, }); } + traceLifecycleRetirement("retirement-captured", { + unknown: captured === "unknown", + missingIdentity: captured === "identity_unavailable", + groups: typeof captured === "object" ? captured.groups : [], + }); if (!(await opts.stopSession())) { captured = undefined; return false; } + traceLifecycleRetirement("retirement-mux-stopped"); if (captured === "unknown") { throw new Error( "Lifecycle mux stopped, but descendant retirement could not be confirmed because process inspection failed" @@ -422,6 +538,7 @@ export function createLifecycleSessionRetirement(boundary?: { beforeSignal, readIdentities: os.readIdentities, assertFresh: opts.assertFresh, + force: opts.force, }); await terminateLifecycleProcessGroups({ processGroupIds: groups.filter((group) => paneGroups.has(group)), @@ -429,6 +546,8 @@ export function createLifecycleSessionRetirement(boundary?: { beforeSignal, readIdentities: os.readIdentities, assertFresh: opts.assertFresh, + // Even a forced command stop leaves the wrapper time to reap children + // and remove its private FIFO directory before wrapper escalation. }); return true; }; @@ -448,8 +567,29 @@ async function captureRetirementGroups(opts: { } const [snapshot, identities] = await Promise.all([ readRetirementSnapshot({ read: opts.os.readSnapshot }), - opts.os.readIdentities().catch(() => []), + readRetirementIdentities(opts.os.readIdentities), ]); + traceLifecycleRetirement("capture-reads", { + snapshotRows: snapshot.length, + identityRows: identities.length, + panePids: opts.processes.flatMap((process) => + process.panePid ? [process.panePid] : [] + ), + livePanePids: identities + .filter( + (row) => + !row.zombie && + opts.processes.some((process) => process.panePid === row.pid) + ) + .map((row) => row.pid), + zombiePanePids: identities + .filter( + (row) => + row.zombie && + opts.processes.some((process) => process.panePid === row.pid) + ) + .map((row) => row.pid), + }); if (snapshot.length === 0 || identities.length === 0) { return "unknown"; } @@ -501,6 +641,11 @@ async function captureRetirementGroups(opts: { ), ]) ); + traceLifecycleRetirement("capture-complete", { + groups, + paneGroups: [...paneGroups], + roots, + }); return { groups, paneGroups, signalledGroups, members }; } @@ -510,7 +655,7 @@ async function verifyRetirementGroups(opts: { readonly members: ReadonlyMap; readonly os: RetirementBoundary; }): Promise { - const births = await opts.os.readIdentities(); + const births = await readRetirementIdentities(opts.os.readIdentities); const active = new Set( selectLiveLifecycleGroups({ groups: opts.pending, identities: births }) ); @@ -538,6 +683,19 @@ async function verifyRetirementGroups(opts: { }); } +/** Retry one failed bounded identity read; no successful read still means unknown. */ +async function readRetirementIdentities( + read: () => Promise +): Promise { + for (let attempt = 0; attempt < 2; attempt++) { + const identities = await read().catch(() => []); + if (identities.length > 0) { + return identities; + } + } + return []; +} + /** Retry one transient foreground observation; neither failure proves absence. */ async function readRetirementSnapshot(opts: { readonly read: () => Promise; diff --git a/tests/e2e/foreground-readiness.ts b/tests/e2e/foreground-readiness.ts index 98111ae2d..e5eb82562 100644 --- a/tests/e2e/foreground-readiness.ts +++ b/tests/e2e/foreground-readiness.ts @@ -1,4 +1,7 @@ -/** Distinct service logs are required; a replay from one service is insufficient. */ +/** + * Compose v2 prefixes logs with `app-1 |` (or plain `app |` without a replica + * suffix). Distinct service lines are required; one service's replay is insufficient. + */ export function bothForegroundServicesLogged(output: string): boolean { const plain = Bun.stripANSI(output); return ( diff --git a/tests/e2e/scenarios/compose-foreground-signal.ts b/tests/e2e/scenarios/compose-foreground-signal.ts index b76d1c7a5..769c191b9 100644 --- a/tests/e2e/scenarios/compose-foreground-signal.ts +++ b/tests/e2e/scenarios/compose-foreground-signal.ts @@ -32,12 +32,13 @@ export const composeForegroundSignalScenario: Scenario = { if (info.exitCode !== 0) { ctx.skip("docker daemon unavailable"); } - for (const persistentUpBefore of [false, true]) { - const cleanExit = join( - ctx.tempRoot, - `lifecycle-clean-exit-${persistentUpBefore}` - ); - const writer = `process.on("SIGTERM", () => setTimeout(async () => { await Bun.write(${JSON.stringify(cleanExit)}, "clean"); process.exit(0); }, 1000)); setInterval(() => {}, 60000)`; + for (const mode of ["plain", "graceful", "forced"] as const) { + const persistentUpBefore = mode !== "plain"; + const forced = mode === "forced"; + const cleanExit = join(ctx.tempRoot, `lifecycle-clean-exit-${mode}`); + const writer = forced + ? 'process.on("SIGTERM", () => {}); setInterval(() => {}, 60000)' + : `process.on("SIGTERM", () => setTimeout(async () => { await Bun.write(${JSON.stringify(cleanExit)}, "clean"); process.exit(0); }, 1000)); setInterval(() => {}, 60000)`; const fixture = await createMonorepoFixture({ parentDir: ctx.tempRoot, withHackConfig: true, @@ -79,7 +80,7 @@ export const composeForegroundSignalScenario: Scenario = { `${fixture.name}.cleanup-released` ); const shimDir = join(ctx.tempRoot, `${fixture.name}.bin`); - if (persistentUpBefore) { + if (forced) { const tmux = Bun.which("tmux"); expect({ that: tmux !== null, @@ -109,9 +110,7 @@ exec ${quoteShellWord(tmux ?? "tmux")} "$@" hackHome: ctx.hackHome, extra: { HACK_RUNTIME_BACKEND: "compose", - ...(persistentUpBefore - ? { PATH: `${shimDir}:${process.env.PATH}` } - : {}), + ...(forced ? { PATH: `${shimDir}:${process.env.PATH}` } : {}), }, }), detached: true, @@ -158,13 +157,13 @@ exec ${quoteShellWord(tmux ?? "tmux")} "$@" } process.kill(-child.pid, "SIGINT"); await interruptDuringCleanup({ - enabled: persistentUpBefore, + enabled: forced, child, entered: cleanupEntered, released: cleanupReleased, }); const code = await child.exited; - if (persistentUpBefore) { + if (persistentUpBefore && !forced) { expect({ that: await Bun.file(cleanExit).exists(), message: @@ -173,7 +172,7 @@ exec ${quoteShellWord(tmux ?? "tmux")} "$@" } const after = await containers(); ctx.log( - `persistent lifecycle=${persistentUpBefore}; foreground exit=${code}; running project containers=${after.stdout.trim() || "none"}; output:\n${await Bun.file(logPath).text()}\n${await errorLog.text()}` + `lifecycle mode=${mode}; foreground exit=${code}; running project containers=${after.stdout.trim() || "none"}; output:\n${await Bun.file(logPath).text()}\n${await errorLog.text()}` ); expect({ that: code === 130, @@ -291,7 +290,8 @@ async function interruptDuringCleanup(opts: { message: "must reach lifecycle teardown after Compose exits", }); // Two more group interrupts arrive while the real tmux teardown is - // paused. Neither may terminate Hack or abandon its owned cleanup. + // paused. They request one forced retirement after the mux returns; neither + // may abandon ownership checks or the post-KILL confirmation. process.kill(-opts.child.pid, "SIGINT"); await Bun.sleep(50); process.kill(-opts.child.pid, "SIGINT"); diff --git a/tests/helpers/native-authored-start-fixture.ts b/tests/helpers/native-authored-start-fixture.ts index 636d8ce7d..5bf3873bc 100644 --- a/tests/helpers/native-authored-start-fixture.ts +++ b/tests/helpers/native-authored-start-fixture.ts @@ -799,10 +799,15 @@ process.exit(await child.exited); try { process.kill(-group, 0); } catch (error) { - if ((error as { code?: unknown }).code === "ESRCH") { + const code = (error as { code?: unknown }).code; + if (code === "ESRCH") { return true; } - throw error; + // macOS can deny a probe while a zombie waits to be reaped. Keep + // waiting for actual absence; EPERM itself proves no retirement. + if (code !== "EPERM") { + throw error; + } } await Bun.sleep(50); } diff --git a/tests/lifecycle-cleanup-followup.test.ts b/tests/lifecycle-cleanup-followup.test.ts new file mode 100644 index 000000000..0ff10ba40 --- /dev/null +++ b/tests/lifecycle-cleanup-followup.test.ts @@ -0,0 +1,194 @@ +import { expect, test } from "bun:test"; +import { mkdtemp, rm } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { + signalCleanupKeepingStartedHelpers, + withSharedHelperCleanup, +} from "../src/commands/project.ts"; +import { installForegroundTerminationGuard } from "../src/lib/foreground-termination-guard.ts"; +import { refreshMissingLifecyclePaneBirths } from "../src/lib/project-lifecycle-controller.ts"; +import { withOwnedProcessListeners } from "./helpers/owned-process-listeners.ts"; + +test("foreground guard requests force only on the second Ctrl-C", async () => { + await withOwnedProcessListeners("SIGINT", () => + withOwnedProcessListeners("SIGTERM", () => { + const guard = installForegroundTerminationGuard(); + try { + process.emit("SIGINT"); + expect(guard.exitCode()).toBe(130); + expect(guard.force.aborted).toBe(false); + process.emit("SIGINT"); + expect(guard.force.aborted).toBe(true); + } finally { + guard.dispose(); + } + }) + ); +}); + +test("shared-helper wrappers preserve the operation's force signal", async () => { + const force = new AbortController(); + const seen: AbortSignal[] = []; + let kept = false; + let rolledBack = false; + const combined = withSharedHelperCleanup({ + cleanup: (opts) => { + if (opts?.force) { + seen.push(opts.force); + } + return Promise.resolve(); + }, + signalCleanup: null, + sharedHelpers: { + configured: true, + rollback: () => { + rolledBack = true; + return Promise.resolve(); + }, + assertCurrent: () => Promise.resolve(), + }, + sessionOwnership: undefined, + }); + const cleanup = signalCleanupKeepingStartedHelpers({ + cleanup: combined.cleanup, + containerStartBegun: () => true, + keepSharedHelpers: () => () => { + kept = true; + combined.keepSharedHelpers?.(); + }, + }); + await cleanup?.({ force: force.signal }); + expect(seen).toEqual([force.signal]); + expect(kept).toBe(true); + expect(rolledBack).toBe(false); +}); + +const processRecord = { + name: "worker", + windowName: "worker", + logPath: "synthetic.log", + panePid: 42, +}; +const pane = { + pid: 42, + parentPid: 1, + processGroupId: 42, + birth: "fresh-pane-birth", + zombie: false, +}; + +test("cleanup refreshes a missing birth from an owned live pane, retrying one empty read", async () => { + let checks = 0; + let reads = 0; + const refreshed = await refreshMissingLifecyclePaneBirths({ + processes: [processRecord], + readPanes: async () => [42], + readIdentities: async () => (++reads === 1 ? [] : [pane]), + assertOwned: async () => { + checks += 1; + }, + }); + expect(refreshed).toEqual([{ ...processRecord, paneBirth: pane.birth }]); + expect(checks).toBe(2); + expect(reads).toBe(2); +}); + +test("missing birth is never filled from a PID outside the current owned window", async () => { + const refreshed = await refreshMissingLifecyclePaneBirths({ + processes: [processRecord], + readPanes: async () => [43], + readIdentities: async () => [pane], + assertOwned: async () => {}, + }); + expect(refreshed).toEqual([processRecord]); +}); + +test("failed pane refresh preserves the unknown record for owned mux cleanup", async () => { + let checks = 0; + const refreshed = await refreshMissingLifecyclePaneBirths({ + processes: [processRecord], + readPanes: async () => { + throw new Error("window is gone"); + }, + readIdentities: async () => [pane], + assertOwned: async () => { + checks += 1; + }, + }); + expect(refreshed).toEqual([processRecord]); + expect(checks).toBe(2); +}); + +test("identity refresh refuses a replacement owner after reading the pane", async () => { + let checks = 0; + await expect( + refreshMissingLifecyclePaneBirths({ + processes: [processRecord], + readPanes: async () => [42], + readIdentities: async () => [pane], + assertOwned: async () => { + if (++checks === 2) { + throw new Error("owner replaced"); + } + }, + }) + ).rejects.toThrow("owner replaced"); +}); + +test("a stored birth is never replaced by cleanup's newer observation", async () => { + const recorded = { ...processRecord, paneBirth: "original-birth" }; + const refreshed = await refreshMissingLifecyclePaneBirths({ + processes: [recorded], + readPanes: async () => { + throw new Error("must not refresh"); + }, + readIdentities: async () => [pane], + assertOwned: async () => {}, + }); + expect(refreshed).toEqual([recorded]); +}); + +test("startup signal cleanup forwards the second interrupt before re-raising", async () => { + const root = await mkdtemp(join(tmpdir(), "hack-cleanup-interrupt-")); + const source = `import { installLifecycleSignalCleanup } from ${JSON.stringify(join(import.meta.dir, "../src/lib/project-lifecycle-controller.ts"))}; +installLifecycleSignalCleanup({ cleanup: async (opts) => { + await Bun.write(${JSON.stringify(join(root, "entered"))}, "yes"); + await new Promise(resolve => opts.force.addEventListener("abort", async () => { + await Bun.write(${JSON.stringify(join(root, "forced"))}, "yes"); resolve(); + }, { once: true })); +}}); +await Bun.write(${JSON.stringify(join(root, "ready"))}, "yes"); +setInterval(() => {}, 1000);`; + const child = Bun.spawn([process.execPath, "--no-env-file", "-e", source], { + stdin: "ignore", + stdout: "ignore", + stderr: "inherit", + }); + const watchdog = setTimeout(() => child.kill("SIGKILL"), 15_000); + const wait = async (name: string) => { + const deadline = performance.now() + 10_000; + while ( + !(await Bun.file(join(root, name)).exists()) && + performance.now() < deadline + ) { + await Bun.sleep(10); + } + expect(await Bun.file(join(root, name)).exists()).toBe(true); + }; + try { + await wait("ready"); + child.kill("SIGINT"); + await wait("entered"); + child.kill("SIGINT"); + expect(await child.exited).toBe(130); + expect(await Bun.file(join(root, "forced")).exists()).toBe(true); + } finally { + clearTimeout(watchdog); + if (child.exitCode === null) { + child.kill("SIGKILL"); + } + await child.exited; + await rm(root, { recursive: true, force: true }); + } +}, 20_000); diff --git a/tests/lifecycle-fifo-retirement.test.ts b/tests/lifecycle-fifo-retirement.test.ts index 93eedcae3..3d46427c9 100644 --- a/tests/lifecycle-fifo-retirement.test.ts +++ b/tests/lifecycle-fifo-retirement.test.ts @@ -12,8 +12,9 @@ for (const behavior of [ "cooperative", "resistant", "separate-worker", + "forced", ] as const) { - const ignoresTerm = behavior === "resistant"; + const ignoresTerm = behavior === "resistant" || behavior === "forced"; test(`FIFO command retirement (${behavior}) preserves graceful shutdown and reaps the reader`, async () => { const root = await mkdtemp(join(tmpdir(), "hack-fifo-retirement-")); const logPath = join(root, "lifecycle.log"); @@ -33,25 +34,28 @@ for (const behavior of [ }) : `process.on("SIGTERM", () => { ${ignoresTerm ? "" : `setTimeout(async () => { await Bun.write(${JSON.stringify(cleanExit)}, "clean"); process.exit(0); }, 1000);`} }); await Bun.write(${JSON.stringify(ready)}, "yes"); setInterval(() => {}, 1000);`; const command = `${quoteShellWord(process.execPath)} --no-env-file -e ${quoteShellWord(code)}`; - const child = Bun.spawn( - [ - "sh", - "-c", - wrapLifecyclePersistentCommand({ - command, - commandPidPath, - logPath, - serviceName: "writer", - }), - ], - { - detached: true, - env: { PATH: "/usr/bin:/bin", TMPDIR: root }, - stdin: "ignore", - stdout: "ignore", - stderr: "ignore", - } - ); + const wrapped = wrapLifecyclePersistentCommand({ + command, + commandPidPath, + logPath, + serviceName: "writer", + }); + // Make the wrapper's private-directory cleanup observable after its child + // exits, so killing the wrapper with the command cannot pass by scheduling. + const delayedWrapper = + behavior === "forced" + ? wrapped.replace( + ' rmdir "$fifo_dir" 2>/dev/null || true', + ' sleep 0.3\n rmdir "$fifo_dir" 2>/dev/null || true' + ) + : wrapped; + const child = Bun.spawn(["sh", "-c", delayedWrapper], { + detached: true, + env: { PATH: "/usr/bin:/bin", TMPDIR: root }, + stdin: "ignore", + stdout: "ignore", + stderr: "ignore", + }); let group: number | undefined; let workerGroup: number | undefined; const watchdog = setTimeout(() => child.kill("SIGKILL"), 25_000); @@ -87,15 +91,33 @@ for (const behavior of [ (row) => row.pid === child.pid )?.birth; expect(paneBirth).toBeDefined(); + const force = new AbortController(); + if (behavior === "forced") { + expect( + (await readdir(root)).filter((name) => + name.startsWith("hack-lifecycle.") + ) + ).toHaveLength(1); + force.abort(); + } expect( await retire({ processes: [{ panePid: child.pid, paneBirth }], + force: force.signal, stopSession: () => { process.kill(-child.pid, "SIGHUP"); return Promise.resolve(true); }, }) ).toBe(true); + if (behavior === "forced") { + expect( + (await readdir(root)).filter((name) => + name.startsWith("hack-lifecycle.") + ), + "forced cleanup must let the wrapper remove its private FIFO directory" + ).toEqual([]); + } expect(await child.exited).toBe(130); expect(await Bun.file(cleanExit).exists()).toBe(!ignoresTerm); expect(() => process.kill(-(group ?? 0), 0)).toThrow(); @@ -127,7 +149,7 @@ for (const behavior of [ } } if (child.exitCode === null) { - process.kill(-child.pid, "SIGKILL"); + killOwnedWrapper(child.pid); } await child.exited; await rm(root, { recursive: true, force: true }); @@ -135,6 +157,18 @@ for (const behavior of [ }, 30_000); } +function killOwnedWrapper(pid: number): void { + try { + process.kill(-pid, "SIGKILL"); + } catch (error) { + if ( + !(error instanceof Error && "code" in error && error.code === "ESRCH") + ) { + throw error; + } + } +} + /** The main command waits for its separately grouped child, but forwards no TERM. */ function separateWorkerCommand(opts: { readonly workerPidPath: string; diff --git a/tests/lifecycle-group-settlement.test.ts b/tests/lifecycle-group-settlement.test.ts index e1435e1da..07853f9e8 100644 --- a/tests/lifecycle-group-settlement.test.ts +++ b/tests/lifecycle-group-settlement.test.ts @@ -28,14 +28,14 @@ function runtime(opts: { }, signal: (group: number, signal: NodeJS.Signals | 0) => { signals.push({ group, signal, at: now }); + if (now >= exitAt) { + throw Object.assign(new Error("absent"), { code: "ESRCH" }); + } if (signal === 0 && opts.probeError) { throw Object.assign(new Error("unavailable"), { code: opts.probeError, }); } - if (now >= exitAt) { - throw Object.assign(new Error("absent"), { code: "ESRCH" }); - } if (signal === "SIGTERM" && opts.termExit !== undefined) { exitAt = now + opts.termExit; } @@ -125,8 +125,8 @@ test("a cooperative command gets its one-second TERM handler without KILL", asyn ]); }); -test("an unreaped zombie-only group is retired without TERM or KILL", async () => { - const os = runtime({}); +test("an EPERM probe with a zombie-only census retires without TERM or KILL", async () => { + const os = runtime({ probeError: "EPERM" }); await terminateLifecycleProcessGroups({ processGroupIds: [42], runtime: { @@ -150,8 +150,8 @@ test("an unreaped zombie-only group is retired without TERM or KILL", async () = expect(os.signals.filter((event) => event.signal !== 0)).toEqual([]); }); -test("a zombie leader does not hide another live group member", async () => { - const os = runtime({ killExit: 0 }); +test("EPERM and a zombie leader do not hide another live group member", async () => { + const os = runtime({ killExit: 0, probeError: "EPERM" }); await terminateLifecycleProcessGroups({ processGroupIds: [42], runtime: { @@ -183,14 +183,128 @@ test("a zombie leader does not hide another live group member", async () => { ]); }); -test("permission errors cannot become absence or grant a kill", async () => { +test("an EPERM probe followed by ESRCH retires after observing the live member", async () => { + const os = runtime({}); + let probes = 0; + let reads = 0; + await terminateLifecycleProcessGroups({ + processGroupIds: [42], + alreadySignalled: true, + runtime: { + ...os, + signal: (_group, name) => { + expect(name).toBe(0); + probes += 1; + throw Object.assign(new Error("probe"), { + code: probes === 1 ? "EPERM" : "ESRCH", + }); + }, + liveGroups: async (groups) => { + reads += 1; + return groups; + }, + }, + }); + expect(probes).toBe(2); + expect(reads).toBe(1); +}); + +test("an EPERM probe retries an unavailable census and accepts the next zombie census", async () => { + const os = runtime({ probeError: "EPERM" }); + let reads = 0; + await terminateLifecycleProcessGroups({ + processGroupIds: [42], + runtime: { + ...os, + liveGroups: async () => { + reads += 1; + if (reads === 1) { + await os.sleep(2000); + throw new Error("ps timeout"); + } + return []; + }, + }, + }); + expect(reads).toBe(2); + expect(os.signals.filter((event) => event.signal !== 0)).toEqual([]); +}); + +test("EPERM with no successful census remains unconfirmed at the deadline", async () => { const os = runtime({ probeError: "EPERM" }); await expect( terminateLifecycleProcessGroups({ processGroupIds: [42], alreadySignalled: true, - runtime: os, + runtime: { + ...os, + liveGroups: async () => { + await os.sleep(2000); + throw new Error("ps timeout"); + }, + }, }) + ).rejects.toThrow("Could not inspect live lifecycle process-group members"); + expect(os.now()).toBeLessThanOrEqual(7000); + expect(os.signals.filter((event) => event.signal !== 0)).toEqual([]); +}); + +for (const delivery of ["SIGTERM", "SIGKILL"] as const) { + for (const state of ["zombie", "live", "unavailable"] as const) { + test(`${delivery} EPERM is benign only after retirement is proved (${state})`, async () => { + const os = runtime({}); + let denied = false; + const stopping = terminateLifecycleProcessGroups({ + processGroupIds: [42], + alreadySignalled: delivery === "SIGKILL", + runtime: { + ...os, + signal: (group, name) => { + os.signal(group, name); + if (name === delivery) { + denied = true; + throw Object.assign(new Error("denied"), { code: "EPERM" }); + } + }, + liveGroups: async (groups) => { + if (!denied || state === "live") { + return groups; + } + if (state === "unavailable") { + await os.sleep(2000); + throw new Error("ps timeout"); + } + return []; + }, + }, + }); + if (state === "zombie") { + await stopping; + } else { + await expect(stopping).rejects.toThrow( + state === "live" + ? "Could not confirm lifecycle process group 42 stopped" + : "Could not inspect live lifecycle process-group members" + ); + } + expect(denied).toBe(true); + expect(os.now()).toBeLessThanOrEqual(9000); + expect( + os.signals.filter((event) => event.signal === delivery) + ).toHaveLength(1); + if (delivery === "SIGTERM") { + expect( + os.signals.filter((event) => event.signal === "SIGKILL") + ).toEqual([]); + } + }); + } +} + +test("unexpected probe errors still fail without signalling", async () => { + const os = runtime({ probeError: "EINVAL" }); + await expect( + terminateLifecycleProcessGroups({ processGroupIds: [42], runtime: os }) ).rejects.toThrow("Could not confirm lifecycle process group 42 stopped"); expect(os.signals.filter((event) => event.signal !== 0)).toEqual([]); }); @@ -208,6 +322,122 @@ test("a lost freshness fence prevents escalation", async () => { expect(os.signals.filter((event) => event.signal !== 0)).toEqual([]); }); +for (const failedRead of [1, 2, 52]) { + test(`a timed-out census at read ${failedRead} is retried inside the observation deadline`, async () => { + const os = runtime({ killExit: 100 }); + let reads = 0; + await terminateLifecycleProcessGroups({ + processGroupIds: [42], + alreadySignalled: true, + runtime: { + ...os, + liveGroups: async (groups) => { + reads += 1; + if (reads === failedRead) { + await os.sleep(2000); + throw new Error("ps timeout"); + } + return groups; + }, + }, + }); + expect(os.absent()).toBe(true); + expect(reads).toBeGreaterThan(failedRead); + expect( + os.signals.filter((event) => event.signal === "SIGKILL") + ).toHaveLength(1); + }); +} + +test("continuous census failures are bounded and never authorize KILL", async () => { + const os = runtime({}); + await expect( + terminateLifecycleProcessGroups({ + processGroupIds: [42], + alreadySignalled: true, + runtime: { + ...os, + liveGroups: async () => { + await os.sleep(2000); + throw new Error("ps timeout"); + }, + }, + }) + ).rejects.toThrow("Could not inspect live lifecycle process-group members"); + expect(os.now()).toBeLessThanOrEqual(7000); + expect(os.signals.filter((event) => event.signal !== 0)).toEqual([]); +}); + +test("a repeated interrupt skips TERM grace but still confirms one KILL", async () => { + const force = new AbortController(); + const os = runtime({ killExit: 100 }); + await terminateLifecycleProcessGroups({ + processGroupIds: [42], + alreadySignalled: true, + force: force.signal, + runtime: { + ...os, + sleep: async (ms) => { + await os.sleep(ms); + if (os.now() >= 300) { + force.abort(); + } + }, + }, + }); + expect(os.signals.filter((event) => event.signal === "SIGKILL")).toEqual([ + { group: 42, signal: "SIGKILL", at: 300 }, + ]); + expect(os.now()).toBe(400); +}); + +test("a force request cannot bypass the ownership fence", async () => { + const force = new AbortController(); + force.abort(); + const os = runtime({ killExit: 0 }); + await expect( + terminateLifecycleProcessGroups({ + processGroupIds: [42], + alreadySignalled: true, + force: force.signal, + runtime: os, + assertFresh: async () => { + throw new Error("owner replaced"); + }, + }) + ).rejects.toThrow("owner replaced"); + expect(os.signals.filter((event) => event.signal !== 0)).toEqual([]); +}); + +test("unknown confirmation after KILL is not mistaken for retirement", async () => { + const os = runtime({}); + let killed = false; + await expect( + terminateLifecycleProcessGroups({ + processGroupIds: [42], + runtime: { + ...os, + signal: (group, signal) => { + os.signal(group, signal); + if (signal === "SIGKILL") { + killed = true; + } + }, + liveGroups: async (groups) => { + if (killed) { + await os.sleep(2000); + throw new Error("ps timeout"); + } + return groups; + }, + }, + }) + ).rejects.toThrow("Could not inspect live lifecycle process-group members"); + expect(os.signals.filter((event) => event.signal === "SIGKILL")).toHaveLength( + 1 + ); +}); + test("one failed foreground snapshot is retried before the owned mux stop", async () => { let reads = 0; let stopped = false; diff --git a/tests/lifecycle-retirement-trace.test.ts b/tests/lifecycle-retirement-trace.test.ts new file mode 100644 index 000000000..962efa1fc --- /dev/null +++ b/tests/lifecycle-retirement-trace.test.ts @@ -0,0 +1,38 @@ +import { expect, spyOn, test } from "bun:test"; +import { traceLifecycleRetirement } from "../src/lib/lifecycle-retirement-trace.ts"; +import { restoreEnv } from "./helpers/env.ts"; + +test("retirement trace is opt-in and never serializes error details", () => { + const previous = process.env.HACK_LIFECYCLE_RETIREMENT_TRACE; + const output = spyOn(console, "error").mockImplementation(() => undefined); + try { + restoreEnv("HACK_LIFECYCLE_RETIREMENT_TRACE", undefined); + traceLifecycleRetirement("abort-failed", { processes: 1 }); + expect(output).not.toHaveBeenCalled(); + process.env.HACK_LIFECYCLE_RETIREMENT_TRACE = "1"; + traceLifecycleRetirement( + "group-signal-failed", + { group: 42, probe: true }, + Object.assign(new Error("synthetic-private-message"), { + code: "EPERM", + cause: "synthetic-private-cause", + path: "/synthetic/private/path", + }) + ); + expect(output).toHaveBeenCalledWith( + "[lifecycle-retirement]", + JSON.stringify({ + event: "group-signal-failed", + group: 42, + probe: true, + failed: true, + eperm: true, + esrch: false, + timedOut: false, + }) + ); + } finally { + output.mockRestore(); + restoreEnv("HACK_LIFECYCLE_RETIREMENT_TRACE", previous); + } +}); diff --git a/tests/lifecycle-zombie-retirement.test.ts b/tests/lifecycle-zombie-retirement.test.ts index 0f85e51cd..a171b03fb 100644 --- a/tests/lifecycle-zombie-retirement.test.ts +++ b/tests/lifecycle-zombie-retirement.test.ts @@ -5,26 +5,29 @@ import { terminateLifecycleProcessGroups, } from "../src/lib/project-lifecycle-processes.ts"; -test.skipIf(process.platform !== "linux")( +const supported = process.platform === "linux" || process.platform === "darwin"; +const python = supported ? Bun.which("python3") : null; + +test.skipIf(!supported || python === null)( "a real zombie-only process group retires before its parent reaps it", async () => { - const python = Bun.which("python3"); - if (!python) { - throw new Error("Zombie retirement regression requires working python3"); - } - // WNOWAIT deliberately keeps the exited child observable to kill(0). The - // parent stays alive to reap it in finally, without relying on PID 1. + // The pipe closes on child exit, without waitid/WNOWAIT (not portable to + // macOS). The parent holds the zombie until finally, then reaps it itself. const source = `import json, os, sys +read_end, write_end = os.pipe() pid = os.fork() if pid == 0: + os.close(read_end) os.setpgid(0, 0) os._exit(0) -os.waitid(os.P_PID, pid, os.WEXITED | os.WNOWAIT) +os.close(write_end) +os.read(read_end, 1) +os.close(read_end) print(json.dumps({"pid": pid}), flush=True) sys.stdin.readline() os.waitpid(pid, 0) `; - const parent = Bun.spawn([python, "-c", source], { + const parent = Bun.spawn([python ?? "python3", "-c", source], { stdin: "pipe", stdout: "pipe", stderr: "pipe", @@ -51,10 +54,22 @@ os.waitpid(pid, 0) throw new Error("Zombie fixture returned an invalid child identity"); } const group = record.pid; - expect(process.kill(-group, 0)).toBe(true); - const row = (await readProcessIdentities()).find( + // macOS may report EPERM for this zombie-only group; Linux succeeds. + try { + expect(process.kill(-group, 0)).toBe(true); + } catch (error) { + expect(error).toHaveProperty("code", "EPERM"); + } + let row = (await readProcessIdentities()).find( (item) => item.pid === group ); + const deadline = performance.now() + 3000; + while (!row?.zombie && performance.now() < deadline) { + await Bun.sleep(25); + row = (await readProcessIdentities()).find( + (item) => item.pid === group + ); + } expect(row?.zombie).toBe(true); expect(row?.processGroupId).toBe(group); const started = performance.now(); @@ -70,6 +85,14 @@ os.waitpid(pid, 0) (await readProcessIdentities()).find((item) => item.pid === group) ?.zombie ).toBe(true); + parent.stdin.end(); + expect(await parent.exited).toBe(0); + expect(() => process.kill(-group, 0)).toThrow(); + try { + process.kill(-group, 0); + } catch (error) { + expect(error).toHaveProperty("code", "ESRCH"); + } } finally { clearTimeout(watchdog); parent.stdin.end(); diff --git a/tests/native-authored-host-processes.test.ts b/tests/native-authored-host-processes.test.ts index 532e2d032..95abff69c 100644 --- a/tests/native-authored-host-processes.test.ts +++ b/tests/native-authored-host-processes.test.ts @@ -22,7 +22,10 @@ import { } from "../src/lib/lifecycle-runtime.ts"; import type { NativeEnvironmentPlan } from "../src/lib/native-env-plan-protocol.ts"; import { selectNativeHostLifecycle } from "../src/lib/native-host-lifecycle-contract.ts"; -import { writeLifecycleMuxFixture } from "./helpers/lifecycle-mux-fixture.ts"; +import { + endLifecycleMuxFixtureGroups, + writeLifecycleMuxFixture, +} from "./helpers/lifecycle-mux-fixture.ts"; const roots = new Set(); let unknown = false; @@ -401,3 +404,96 @@ ownedCase( }); } ); + +ownedCase( + "cancellation with a missing launch birth still ends the owned native mux session", + async (f) => { + const ps = Bun.which("ps"); + expect(ps).not.toBeNull(); + const held = join(f.root, "launch-census-held"); + const release = join(f.root, "launch-census-release"); + await writeFile( + join(f.root, "bin", "ps"), + `#!${process.execPath} +const args = process.argv.slice(2); +if (args[0] === "-A" && !(await Bun.file(${JSON.stringify(held)}).exists())) { + await Bun.write(${JSON.stringify(held)}, "held"); + const deadline = performance.now() + 10_000; + while (!(await Bun.file(${JSON.stringify(release)}).exists()) && performance.now() < deadline) await Bun.sleep(10); + process.exit(0); +} +const child = Bun.spawn([${JSON.stringify(ps)}, ...args], {stdin:"ignore", stdout:"inherit", stderr:"inherit"}); +process.exit(await child.exited); +`, + { mode: 0o700 } + ); + let muxStopped = false; + let stateRemoved = false; + let startupInputsCanceled = false; + let cleanupChecks = 0; + const outcome = await withNativeAuthoredProjectAdmission( + f.scope, + async (admission) => { + const owner = await createNativeAuthoredHostProcesses({ + scope: f.scope, + run, + semanticHash: semantic, + projectName: "fixture", + lifecycle: f.lifecycle, + report: () => f.report, + resolveValues: () => Promise.resolve({ TOKEN: canary }), + assertFresh: async () => { + if (startupInputsCanceled) { + throw new Error("startup input authority was canceled"); + } + await admission.assertHeld(); + }, + assertCleanupFresh: async () => { + cleanupChecks += 1; + await admission.assertHeld(); + }, + signal: f.abort.signal, + remaining: f.remaining, + }); + const starting = owner.start().catch((error: unknown) => error); + try { + const deadline = performance.now() + 10_000; + while ( + !(await Bun.file(held).exists()) && + performance.now() < deadline + ) { + await Bun.sleep(10); + } + expect(await Bun.file(held).exists()).toBe(true); + startupInputsCanceled = true; + f.abort.abort(); + await Bun.write(release, "released"); + expect(await starting).toBeInstanceOf(Error); + // The launch's aborted deadline must not veto owned teardown when + // the new cleanup path needs to retry missing start-time metadata. + muxStopped = !(await Bun.file(f.state).exists()); + stateRemoved = + (await readLifecycleState({ projectDir: f.projectDir })).length === + 0; + if (muxStopped && stateRemoved) { + // The frontend refreshes teardown inputs after the launch settles. + startupInputsCanceled = false; + await owner.stop(); + await owner.close(); + } + } finally { + await Bun.write(release, "released"); + await starting; + await endLifecycleMuxFixtureGroups(f.state); + } + } + ).catch((error: unknown) => error); + expect( + muxStopped, + "canceled launch must stop its owned mux despite a missing birth" + ).toBe(true); + expect(stateRemoved).toBe(true); + expect(cleanupChecks).toBeGreaterThan(0); + expect(outcome).toBeUndefined(); + } +); diff --git a/tests/native-authored-start-fixture.test.ts b/tests/native-authored-start-fixture.test.ts new file mode 100644 index 000000000..496cec449 --- /dev/null +++ b/tests/native-authored-start-fixture.test.ts @@ -0,0 +1,33 @@ +import { expect, spyOn, test } from "bun:test"; +import { createNativeAuthoredStartHarness } from "./helpers/native-authored-start-fixture.ts"; + +const { groupEnded } = createNativeAuthoredStartHarness(); + +test("native cancellation fixture waits through EPERM until the group is reaped", async () => { + let probes = 0; + const probe = spyOn(process, "kill").mockImplementation((pid, signal) => { + expect(pid).toBe(-42); + expect(signal).toBe(0); + probes += 1; + throw Object.assign(new Error("fixture probe"), { + code: probes === 1 ? "EPERM" : "ESRCH", + }); + }); + try { + expect(await groupEnded(42)).toBe(true); + expect(probes).toBe(2); + } finally { + probe.mockRestore(); + } +}); + +test("native cancellation fixture retains unexpected probe failures", async () => { + const probe = spyOn(process, "kill").mockImplementation(() => { + throw Object.assign(new Error("fixture probe"), { code: "EINVAL" }); + }); + try { + await expect(groupEnded(42)).rejects.toThrow("fixture probe"); + } finally { + probe.mockRestore(); + } +}); diff --git a/tests/project-lifecycle-controller-ownership.test.ts b/tests/project-lifecycle-controller-ownership.test.ts index 6bd15a8b3..6aa381cae 100644 --- a/tests/project-lifecycle-controller-ownership.test.ts +++ b/tests/project-lifecycle-controller-ownership.test.ts @@ -229,6 +229,7 @@ async function fixture(backendName: "zellij" | "tmux" = "zellij") { readonly retireSession?: Parameters< typeof createLifecycleProcessController >[0]["retireSession"]; + readonly readIdentities?: typeof readProcessIdentities; }) => createLifecycleProcessController({ mux: resolvedMux, @@ -249,6 +250,7 @@ async function fixture(backendName: "zellij" | "tmux" = "zellij") { return { dispose: () => undefined }; }, ...(opts.retireSession ? { retireSession: opts.retireSession } : {}), + readIdentities: opts.readIdentities, ...(opts.prepareLaunch ? { prepareLaunch: async () => { @@ -301,6 +303,77 @@ async function fixture(backendName: "zellij" | "tmux" = "zellij") { const PROCESS = { name: "helper-client", command: "sleep 600" }; +test("operation cleanup recovers a birth missed by the launch observation", async () => { + const { controller, entries, bin } = await fixture("tmux"); + const previousPath = process.env.PATH; + process.env.PATH = `${bin}:${previousPath ?? ""}`; + await writeFile( + join(bin, "tmux"), + `#!/bin/sh\nif [ "$1" = list-panes ]; then echo ${process.pid}; fi\nexit 0\n`, + { mode: 0o755 } + ); + let reads = 0; + let retired = false; + const force = new AbortController(); + const starter = controller({ + sessionOwnership: undefined, + readIdentities: async () => (++reads === 1 ? [] : readProcessIdentities()), + retireSession: async (opts) => { + expect(opts.processes[0]?.panePid).toBe(process.pid); + expect(opts.processes[0]?.paneBirth).toBeString(); + expect(opts.force).toBe(force.signal); + expect(await opts.stopSession()).toBe(true); + retired = true; + return true; + }, + }); + try { + await starter.startMany({ processes: [PROCESS] }); + await starter.finalize(); + expect((await entries())[0]?.processes[0]?.paneBirth).toBeUndefined(); + await starter.getOperationCleanup()?.({ force: force.signal }); + expect(retired).toBe(true); + expect(await entries()).toEqual([]); + } finally { + await starter.abort(); + restoreEnv("PATH", previousPath); + } +}, 10_000); + +test("a missing-birth window query failure still stops the owned mux and keeps unknown state", async () => { + const { controller, entries, bin, mux } = await fixture("tmux"); + const previousPath = process.env.PATH; + process.env.PATH = `${bin}:${previousPath ?? ""}`; + await writeFile( + join(bin, "tmux"), + `#!/bin/sh\nif [ "$1" = list-panes ]; then echo ${process.pid}; fi\nexit 0\n`, + { mode: 0o755 } + ); + let reads = 0; + let retirementReached = false; + const starter = controller({ + sessionOwnership: undefined, + readIdentities: async () => (++reads === 1 ? [] : readProcessIdentities()), + retireSession: async (opts) => { + retirementReached = true; + expect(opts.processes[0]?.paneBirth).toBeUndefined(); + expect(await opts.stopSession()).toBe(true); + throw new Error("identity unavailable"); + }, + }); + try { + await starter.startMany({ processes: [PROCESS] }); + await starter.finalize(); + await writeFile(join(bin, "tmux"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + expect(await starter.getOperationCleanup()?.()).toBe("session-retained"); + expect(retirementReached).toBe(true); + expect(mux.sessions.has(SESSION)).toBe(false); + expect(await entries()).toHaveLength(1); + } finally { + restoreEnv("PATH", previousPath); + } +}, 10_000); + test("explicit down awaits ordered group retirement before session and state removal", async () => { const { controller, stop, entries, mux, bin } = await fixture("tmux"); const previousPath = process.env.PATH;