From 7fd488732dad1b9b689008a694b7bf5ef79bef24 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 19 Oct 2026 11:51:07 -0400 Subject: [PATCH 1/5] feat(lifecycle): ownership-safe shared host helper contract and fixtures Add a library contract for long-lived helpers shared across branches on one loopback port (HACK-1142, WU29). Starters are serialized by the projects-registry lock protocol; a listener is adopted only after its identity endpoint names the expected helper and branch upstream; owned helpers are recorded as PID plus process birth time and spawned behind an exec gate that only opens once the claim is durable, so a lost starter never leaves an unclaimed child. Readiness is the identity answer, bounded by backoff and one deadline, with cancellation stopping the owned child. Release drops one holder and stops an owned helper's birth-verified tree only for the last holder; adopted, foreign and stale-PID processes are never signalled. Failures are a closed, value-free set. Portable process fixtures cover adoption, wrong upstream, foreign listeners, concurrent independent starters, stale/reused PIDs, child restart, cancellation during retry, placeholder liveness, lost parent before and after the gate, descendants, impostor identity, invalid claims and an unavailable census. Co-Authored-By: Claude Opus 5.5 --- docs/lifecycle.md | 5 + docs/reference.md | 1 + docs/reference/shared-host-lifecycle.md | 184 +++++ src/lib/shared-host-lifecycle.ts | 1002 +++++++++++++++++++++++ tests/fixtures/shared-host-helper.ts | 71 ++ tests/fixtures/shared-host-starter.ts | 27 + tests/shared-host-lifecycle.test.ts | 916 +++++++++++++++++++++ 7 files changed, 2206 insertions(+) create mode 100644 docs/reference/shared-host-lifecycle.md create mode 100644 src/lib/shared-host-lifecycle.ts create mode 100644 tests/fixtures/shared-host-helper.ts create mode 100644 tests/fixtures/shared-host-starter.ts create mode 100644 tests/shared-host-lifecycle.test.ts diff --git a/docs/lifecycle.md b/docs/lifecycle.md index 4d0bc15d4..e9761669a 100644 --- a/docs/lifecycle.md +++ b/docs/lifecycle.md @@ -46,6 +46,11 @@ still be reconnecting. When the wrong listener would matter, pair `onConflict: " `lifecycle.up.before` hook that verifies the listener's identity (for example, a request to a known endpoint) and fails when it does not match. +The [shared host helper lifecycle contract](./reference/shared-host-lifecycle.md) defines the +identity-verified alternative: adoption only after the listener names the expected helper and upstream, +exact PID-plus-start-time claims, serialized starters, and release that never signals a foreign +process. It is a library contract today; `singleton` does not use it yet. + ## Config ```json diff --git a/docs/reference.md b/docs/reference.md index ee490977b..ae65b4fcf 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -12,6 +12,7 @@ - [Environment model](./env.md) - [Lifecycle & startup](./lifecycle.md) +- [Shared host helper lifecycle](./reference/shared-host-lifecycle.md) — identity-verified adoption, exact process claims and release rules for helpers shared across branches - [Sessions](./sessions.md) - [Agent-first setup](./guides/agent-first-setup.md) diff --git a/docs/reference/shared-host-lifecycle.md b/docs/reference/shared-host-lifecycle.md new file mode 100644 index 000000000..0b77dfa82 --- /dev/null +++ b/docs/reference/shared-host-lifecycle.md @@ -0,0 +1,184 @@ +# Shared host helper lifecycle contract + +A shared host helper is a long-lived local process that several branches or clients +use through one fixed loopback port: an issuer bridge, a local API proxy, a database +port-forward or an opt-in tunnel. This contract defines how Hack starts, adopts, +reuses and releases such a helper without killing a process it does not own. + +Source: [`src/lib/shared-host-lifecycle.ts`](../../src/lib/shared-host-lifecycle.ts). +Fixtures: [`tests/shared-host-lifecycle.test.ts`](../../tests/shared-host-lifecycle.test.ts). + +Status: this is a library contract with portable process fixtures. No configuration +key or CLI command uses it yet. Consumers (WU18 shared helpers, bounded external +claims and opt-in tunnels) adopt it in their own changes. The existing +`lifecycle.singleton` port check in [Lifecycle & startup](../lifecycle.md) is +unchanged. It adopts any listener on the configured ports; this contract adopts +only a listener that proves its identity. + +## States + +| Axis | Values | Meaning | +| --- | --- | --- | +| Ownership | `owned` | Hack started the helper and holds an exact process claim. | +| | `adopted` | Someone else started it; its identity endpoint matched. Hack never signals it. | +| | `disabled` | The helper is off. Hack takes no lock, writes no claim and starts no child. | +| Phase | `starting` | Owned child spawned and claimed; readiness not yet proven. | +| | `ready` | The identity endpoint answered for the expected helper and upstream. | +| | `failed` | One closed failure reason (below). Terminal for this call. | + +A disabled public tunnel is the `disabled` case: it never spawns a child. + +## Identity and readiness + +The helper serves a JSON identity endpoint on `127.0.0.1:`: + +```json +{ "helper": "issuer-bridge", "upstream": "https://issuer.branch-a.test", "pid": 4242 } +``` + +- `helper` must equal the expected helper kind. Otherwise the listener is foreign. +- `upstream` must equal the upstream this branch requires. Callers derive it per + branch. A shared helper serving another upstream is refused, never retargeted. +- `pid` is optional. When present for an owned helper, it must equal the claimed PID. + A different process answering for the port is foreign. + +Readiness is this identity answer. A live process, an open port or a sleeping +placeholder is never readiness. Each probe has its own timeout and reads at most 4 KiB. + +## Persisted claim + +The claim is `/.claim.json`, owner-only (0600), replaced atomically, +and parsed strictly (exact keys, version 1): + +```json +{"version":1,"helper":"issuer-bridge","port":4100,"ownership":"owned","phase":"ready","pid":4242,"birth":"Mon Oct 19 10:00:00 2026","holders":["branch-a","branch-b"]} +{"version":1,"helper":"issuer-bridge","port":4100,"ownership":"adopted","holders":["branch-a"]} +``` + +- Ownership is the pair `pid` + `birth` (process start time from `ps lstart`). The + claim is live only when the census shows that exact pair. A missing PID, or the + same PID with a different birth time (PID reuse), makes the claim stale. A stale + claim grants nothing. Hack discards it and never signals that PID. +- `holders` names the branches or clients that use the helper. Holders carry over + when a stale owned helper is restarted. +- A malformed, unreadable, symlinked or group/world-accessible claim, or one for a + different port, fails closed with `claim-invalid`. Hack does not start, adopt or + stop anything until an operator inspects it. +- A census that did not observe the calling process (for example `ps` failed) fails + closed with `census-unavailable`. An empty census never reads as "every claimed + process is gone". + +## Start (`ensureSharedHostHelper`) + +1. `enabled: false` returns `disabled` immediately. +2. Take the helper's exclusive lock (`/.lock`, the projects-registry + lock protocol: hard-link publication, dead-owner reclamation by `ESRCH` only). + Competing starters wait in turn, bounded by the deadline. +3. A live owned claim is reused. The starter waits for readiness and adds its holder. + It does not spawn a second helper. If readiness fails, it leaves the helper for + the other holders. +4. Otherwise probe the port: + - identity matches: write an `adopted` claim and return `ready/adopted`; + - wrong upstream: `wrong-upstream`; another helper, a silent or non-identity + listener: `foreign-listener`. Either way the listener keeps running and no + child is spawned; + - connection refused: confirm with a loopback bind probe, then spawn. +5. Spawn through an exec gate in a new session: + `/bin/sh -c 'IFS= read -r hack_gate || exit 0; [ "$hack_gate" = open ] || exit 0; exec "$@" > = { + "foreign-listener": + "The shared helper port is held by a process that is not the expected helper; it was left running.", + "wrong-upstream": + "The shared helper on this port serves a different upstream; it was left running.", + "claim-invalid": + "The persisted shared helper claim is unreadable or belongs to another configuration; inspect and release it before retrying.", + "census-unavailable": + "The process census is unavailable, so helper ownership cannot be verified.", + "lock-unavailable": + "Another starter holds the shared helper lock beyond the deadline.", + "port-probe-failed": "The shared helper port could not be inspected.", + "spawn-failed": "The shared helper could not be started.", + "child-exited": "The shared helper exited before it became ready.", + deadline: "The shared helper did not become ready before the deadline.", + cancelled: "Shared helper startup was cancelled.", +}; + +/** Fixed, value-free message for a closed failure reason. */ +export function sharedHostFailureMessage(reason: SharedHostFailure): string { + return FAILURE_MESSAGES[reason]; +} + +/** Identity a helper must report at its identity endpoint. */ +export type SharedHostIdentity = { + /** Stable helper kind, for example `issuer-bridge`. */ + readonly helper: string; + /** The upstream this branch requires the helper to serve. */ + readonly upstream: string; +}; + +export type SharedHostHelperSpec = { + /** Claim and lock name; `[A-Za-z0-9][A-Za-z0-9_.-]{0,127}`. */ + readonly name: string; + /** Disabled helpers take no lock, write no claim and start no child. */ + readonly enabled: boolean; + /** Caller-owned private directory for the claim and lock. */ + readonly stateDir: string; + /** Loopback port the helper serves. */ + readonly port: number; + /** Absolute path of the JSON identity endpoint, for example `/__hack/identity`. */ + readonly identityPath: string; + readonly expected: SharedHostIdentity; + /** Helper argv; executed directly, never through a shell string. */ + readonly command: readonly string[]; + readonly cwd: string; + /** Complete child environment. The runner environment is never inherited. */ + readonly env: Readonly>; + /** One bound for lock wait, spawn and readiness. */ + readonly deadlineMs: number; + readonly backoff: { readonly initialMs: number; readonly maxMs: number }; + /** Per-request bound for one identity probe. */ + readonly probeTimeoutMs: number; +}; + +export type SharedHostEnsureResult = + | { readonly state: "disabled"; readonly ownership: "disabled" } + | { + readonly state: "ready"; + readonly ownership: "owned" | "adopted"; + readonly endpoint: string; + /** Present for owned helpers only. */ + readonly pid?: number; + readonly spawned: boolean; + } + | { + readonly state: "failed"; + readonly reason: SharedHostFailure; + readonly message: string; + }; + +/** Closed progress events. The observer is awaited (a test and UI seam). */ +export type SharedHostEvent = + | "lock-acquired" + | "claim-stale" + | "claim-reused" + | "adopted" + | "spawned" + | "claimed" + | "gate-opened" + | "probe-retry" + | "ready" + | "stopped-after-failure"; + +export type SharedHostReleaseResult = + | { + readonly state: + | "absent" + | "retained" + | "released-adopted" + | "stale-cleared" + | "stopped" + | "stop-unconfirmed"; + } + | { + readonly state: "failed"; + readonly reason: Extract< + SharedHostFailure, + | "claim-invalid" + | "census-unavailable" + | "lock-unavailable" + | "cancelled" + >; + readonly message: string; + }; + +type OwnedClaim = { + readonly version: 1; + readonly helper: string; + readonly port: number; + readonly ownership: "owned"; + readonly phase: "starting" | "ready"; + readonly pid: number; + readonly birth: string; + readonly holders: readonly string[]; +}; +type AdoptedClaim = { + readonly version: 1; + readonly helper: string; + readonly port: number; + readonly ownership: "adopted"; + readonly holders: readonly string[]; +}; +type Claim = OwnedClaim | AdoptedClaim; + +type ProbeObservation = + | { readonly kind: "absent" } + | { readonly kind: "unidentified" } + | { readonly kind: "foreign" } + | { readonly kind: "wrong-upstream" } + | { readonly kind: "match"; readonly pid: number | null } + | { readonly kind: "cancelled" }; + +const NAME = /^[A-Za-z0-9][A-Za-z0-9_.-]{0,127}$/; +const MAX_CLAIM_BYTES = 8192; +const MAX_IDENTITY_BYTES = 4096; +const MAX_HOLDERS = 256; +const STOP_POLL_MS = 50; +/** + * Exec gate: the helper argv runs only after the starter writes `open`. EOF + * (starter lost before its claim was durable) exits without running it. `exec` + * keeps the PID and birth time recorded in the claim. + */ +const GATE_SCRIPT = + 'IFS= read -r hack_gate || exit 0; [ "$hack_gate" = open ] || exit 0; exec "$@" +): SharedHostReleaseResult { + return { state: "failed", reason, message: FAILURE_MESSAGES[reason] }; +} + +function assertSpec(spec: SharedHostHelperSpec): void { + const valid = + NAME.test(spec.name) && + Number.isInteger(spec.port) && + spec.port > 0 && + spec.port <= 65_535 && + spec.identityPath.startsWith("/") && + spec.command.length > 0 && + spec.deadlineMs > 0 && + spec.probeTimeoutMs > 0 && + spec.backoff.initialMs > 0 && + spec.backoff.maxMs >= spec.backoff.initialMs; + if (!valid) { + throw new Error("Invalid shared host helper specification"); + } +} + +function assertHolder(holder: string): void { + if (!NAME.test(holder)) { + throw new Error("Invalid shared host helper holder"); + } +} + +function claimPath(spec: Pick) { + return join(spec.stateDir, `${spec.name}.claim.json`); +} + +function lockPath(spec: Pick) { + return join(spec.stateDir, `${spec.name}.lock`); +} + +function endpointFor(port: number): string { + return `http://127.0.0.1:${port}`; +} + +function exactKeys(value: Record, keys: readonly string[]) { + const actual = Object.keys(value).sort(); + const wanted = [...keys].sort(); + return ( + actual.length === wanted.length && + actual.every((key, index) => key === wanted[index]) + ); +} + +function parseHolders(value: unknown): readonly string[] | null { + if ( + !Array.isArray(value) || + value.length > MAX_HOLDERS || + !value.every((entry) => typeof entry === "string" && NAME.test(entry)) || + new Set(value).size !== value.length + ) { + return null; + } + return value as string[]; +} + +/** Strict v1 claim parser; any other shape is an uncertain receipt. */ +export function parseSharedHostClaim(text: string): Claim | null { + let value: unknown; + try { + value = JSON.parse(text); + } catch { + return null; + } + if (!isRecord(value) || value.version !== 1) { + return null; + } + const holders = parseHolders(value.holders); + const port = value.port; + if ( + holders === null || + typeof value.helper !== "string" || + !NAME.test(value.helper) || + typeof port !== "number" || + !Number.isInteger(port) || + port <= 0 || + port > 65_535 + ) { + return null; + } + if (value.ownership === "adopted") { + return exactKeys(value, [ + "version", + "helper", + "port", + "ownership", + "holders", + ]) + ? { + version: 1, + helper: value.helper, + port, + ownership: "adopted", + holders, + } + : null; + } + const pid = value.pid; + if ( + value.ownership !== "owned" || + !exactKeys(value, [ + "version", + "helper", + "port", + "ownership", + "phase", + "pid", + "birth", + "holders", + ]) || + (value.phase !== "starting" && value.phase !== "ready") || + typeof pid !== "number" || + !Number.isSafeInteger(pid) || + pid <= 1 || + typeof value.birth !== "string" || + value.birth.length === 0 || + value.birth.length > 64 + ) { + return null; + } + return { + version: 1, + helper: value.helper, + port, + ownership: "owned", + phase: value.phase, + pid, + birth: value.birth, + holders, + }; +} + +type ClaimRead = + | { readonly kind: "absent" } + | { readonly kind: "invalid" } + | { readonly kind: "claim"; readonly claim: Claim }; + +async function readClaim( + spec: Pick +): Promise { + let file: Awaited>; + try { + file = await readReplaceablePrivateFile({ + path: claimPath(spec), + directories: [spec.stateDir], + refusal: () => new ClaimRefusal(), + }); + } catch { + return { kind: "invalid" }; + } + if (file === null) { + return { kind: "absent" }; + } + if (file.text.length > MAX_CLAIM_BYTES || (file.mode & 0o077) !== 0) { + return { kind: "invalid" }; + } + const claim = parseSharedHostClaim(file.text); + if (claim === null || claim.helper !== spec.name) { + return { kind: "invalid" }; + } + return { kind: "claim", claim }; +} + +async function writeClaim( + spec: Pick, + claim: Claim +): Promise { + await replacePrivateFileAtomic({ + path: claimPath(spec), + directories: [spec.stateDir], + refusal: () => new ClaimRefusal(), + text: `${JSON.stringify(claim)}\n`, + }); +} + +async function removeClaim( + spec: Pick +): Promise { + try { + await unlink(claimPath(spec)); + } catch (error: unknown) { + if (!(isRecord(error) && error.code === "ENOENT")) { + throw error; + } + } +} + +/** + * A census is usable only when it observed this process; an empty or failed + * `ps` must never be read as "every claimed process is gone". + */ +async function readCensus(): Promise { + const rows = await readProcessIdentities(); + return rows.some((row) => row.pid === process.pid) ? rows : null; +} + +function isLive( + census: readonly ProcessIdentityRow[], + claim: { readonly pid: number; readonly birth: string } +): boolean { + return census.some( + (row) => row.pid === claim.pid && row.birth === claim.birth + ); +} + +async function readBounded(response: Response): Promise { + const reader = response.body?.getReader(); + if (!reader) { + return ""; + } + const chunks: Uint8Array[] = []; + let size = 0; + try { + while (true) { + const { done, value } = await reader.read(); + if (done) { + break; + } + size += value.byteLength; + if (size > MAX_IDENTITY_BYTES) { + await reader.cancel().catch(() => undefined); + return null; + } + chunks.push(value); + } + } catch { + return null; + } + try { + return new TextDecoder("utf-8", { fatal: true }).decode( + Buffer.concat(chunks) + ); + } catch { + return null; + } +} + +/** Classify the listener on the helper port by its identity endpoint. */ +export async function probeSharedHostIdentity(opts: { + readonly port: number; + readonly identityPath: string; + readonly expected: SharedHostIdentity; + readonly timeoutMs: number; + readonly signal?: AbortSignal; +}): Promise { + const signals = [AbortSignal.timeout(opts.timeoutMs)]; + if (opts.signal) { + signals.push(opts.signal); + } + let response: Response; + try { + response = await fetch(`${endpointFor(opts.port)}${opts.identityPath}`, { + redirect: "manual", + signal: AbortSignal.any(signals), + }); + } catch (error: unknown) { + if (opts.signal?.aborted) { + return { kind: "cancelled" }; + } + // Only a refused connection proves no listener; a timeout or reset means + // something holds the port without answering as the helper. + return isRecord(error) && error.code === "ECONNREFUSED" + ? { kind: "absent" } + : { kind: "unidentified" }; + } + const text = await readBounded(response); + if (opts.signal?.aborted) { + return { kind: "cancelled" }; + } + if (response.status !== 200 || text === null) { + return { kind: "unidentified" }; + } + let body: unknown; + try { + body = JSON.parse(text); + } catch { + return { kind: "unidentified" }; + } + if ( + !isRecord(body) || + typeof body.helper !== "string" || + typeof body.upstream !== "string" + ) { + return { kind: "unidentified" }; + } + if (body.helper !== opts.expected.helper) { + return { kind: "foreign" }; + } + if (body.upstream !== opts.expected.upstream) { + return { kind: "wrong-upstream" }; + } + const pid = + typeof body.pid === "number" && Number.isSafeInteger(body.pid) + ? body.pid + : null; + return { kind: "match", pid }; +} + +/** + * Stop a birth-verified process tree: TERM, bounded grace, then KILL for the + * members that still carry their captured birth time. Nothing is signalled + * when the root no longer matches its claim. + */ +export async function stopVerifiedSharedHostProcess(opts: { + readonly pid: number; + readonly birth: string; + readonly graceMs: number; +}): Promise<"stopped" | "absent" | "unconfirmed" | "census-unavailable"> { + const census = await readCensus(); + if (census === null) { + return "census-unavailable"; + } + if (!isLive(census, opts)) { + return "absent"; + } + const tree = selectProcessTree(census, opts.pid); + const remaining = async (): Promise => { + const live = await readCensus(); + if (live === null) { + return null; + } + return tree.filter((row) => isLive(live, row)); + }; + await signalVerifiedProcessTree(tree, "SIGTERM"); + const termDeadline = performance.now() + opts.graceMs; + let left = await remaining(); + while (left !== null && left.length > 0 && performance.now() < termDeadline) { + await delay(STOP_POLL_MS); + left = await remaining(); + } + if (left !== null && left.length === 0) { + return "stopped"; + } + await signalVerifiedProcessTree(left ?? tree, "SIGKILL"); + const killDeadline = performance.now() + 1000; + left = await remaining(); + while (left !== null && left.length > 0 && performance.now() < killDeadline) { + await delay(STOP_POLL_MS); + left = await remaining(); + } + return left !== null && left.length === 0 ? "stopped" : "unconfirmed"; +} + +function withHolder(holders: readonly string[], holder: string): string[] { + return holders.includes(holder) ? [...holders] : [...holders, holder]; +} + +type EnsureContext = { + readonly spec: SharedHostHelperSpec; + readonly holder: string; + readonly signal?: AbortSignal; + readonly observe: (event: SharedHostEvent) => Promise; + readonly deadline: number; +}; + +function remainingMs(context: EnsureContext): number { + return context.deadline - performance.now(); +} + +type ReadinessOutcome = + | { readonly kind: "ready" } + | { + readonly kind: "failed"; + readonly reason: Extract< + SharedHostFailure, + | "foreign-listener" + | "wrong-upstream" + | "child-exited" + | "deadline" + | "cancelled" + | "census-unavailable" + >; + }; + +/** + * Probe until the identity endpoint answers for the expected helper and, when + * it reports a PID, for the owned process. Liveness alone is never readiness. + */ +async function awaitReadiness( + context: EnsureContext, + owned: { readonly pid: number; readonly birth: string } +): Promise { + const { spec } = context; + let wait = spec.backoff.initialMs; + while (true) { + if (context.signal?.aborted) { + return { kind: "failed", reason: "cancelled" }; + } + const census = await readCensus(); + if (census === null) { + return { kind: "failed", reason: "census-unavailable" }; + } + if (!isLive(census, owned)) { + return { kind: "failed", reason: "child-exited" }; + } + const left = remainingMs(context); + if (left <= 0) { + return { kind: "failed", reason: "deadline" }; + } + const probe = await probeSharedHostIdentity({ + port: spec.port, + identityPath: spec.identityPath, + expected: spec.expected, + timeoutMs: Math.min(spec.probeTimeoutMs, left), + signal: context.signal, + }); + switch (probe.kind) { + case "cancelled": + return { kind: "failed", reason: "cancelled" }; + case "wrong-upstream": + return { kind: "failed", reason: "wrong-upstream" }; + case "foreign": + return { kind: "failed", reason: "foreign-listener" }; + case "match": + if (probe.pid === null || probe.pid === owned.pid) { + return { kind: "ready" }; + } + return { kind: "failed", reason: "foreign-listener" }; + default: + break; + } + const sleep = Math.min(wait, remainingMs(context)); + if (sleep <= 0) { + return { kind: "failed", reason: "deadline" }; + } + await context.observe("probe-retry"); + try { + await delay(sleep, undefined, { signal: context.signal }); + } catch { + return { kind: "failed", reason: "cancelled" }; + } + wait = Math.min(wait * 2, spec.backoff.maxMs); + } +} + +async function ensureLocked( + context: EnsureContext +): Promise { + const { spec, holder } = context; + await context.observe("lock-acquired"); + const census = await readCensus(); + if (census === null) { + return failed("census-unavailable"); + } + const read = await readClaim(spec); + if (read.kind === "invalid") { + return failed("claim-invalid"); + } + let carried: readonly string[] = []; + if (read.kind === "claim") { + const claim = read.claim; + if (claim.port !== spec.port) { + return failed("claim-invalid"); + } + if (claim.ownership === "owned" && isLive(census, claim)) { + await context.observe("claim-reused"); + const outcome = await awaitReadiness(context, claim); + if (outcome.kind === "failed") { + // Other holders may depend on it: a reused helper is never stopped here. + return failed(outcome.reason); + } + await writeClaim(spec, { + ...claim, + phase: "ready", + holders: withHolder(claim.holders, holder), + }); + await context.observe("ready"); + return { + state: "ready", + ownership: "owned", + endpoint: endpointFor(spec.port), + pid: claim.pid, + spawned: false, + }; + } + if (claim.ownership === "owned") { + // Dead or reused PID: the claim no longer proves ownership of anything. + await context.observe("claim-stale"); + } + carried = claim.holders; + } + + const existing = await probeSharedHostIdentity({ + port: spec.port, + identityPath: spec.identityPath, + expected: spec.expected, + timeoutMs: Math.min(spec.probeTimeoutMs, Math.max(1, remainingMs(context))), + signal: context.signal, + }); + switch (existing.kind) { + case "cancelled": + return failed("cancelled"); + case "foreign": + case "unidentified": + return failed("foreign-listener"); + case "wrong-upstream": + return failed("wrong-upstream"); + case "match": + await writeClaim(spec, { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "adopted", + holders: withHolder(carried, holder), + }); + await context.observe("adopted"); + return { + state: "ready", + ownership: "adopted", + endpoint: endpointFor(spec.port), + spawned: false, + }; + default: + break; + } + try { + const occupied = await inspectListeningTcpPorts({ ports: [spec.port] }); + if (occupied.length > 0) { + return failed("foreign-listener"); + } + } catch { + return failed("port-probe-failed"); + } + if (read.kind === "claim") { + await removeClaim(spec); + } + return await spawnOwned(context, carried); +} + +async function spawnOwned( + context: EnsureContext, + carried: readonly string[] +): Promise { + const { spec, holder } = context; + let child: Bun.Subprocess<"pipe", "ignore", "ignore">; + try { + child = Bun.spawn( + ["/bin/sh", "-c", GATE_SCRIPT, "hack-shared-host-gate", ...spec.command], + { + cwd: spec.cwd, + env: { ...spec.env }, + stdin: "pipe", + stdout: "ignore", + stderr: "ignore", + // A new session keeps terminal signals for the starter away from a + // helper that other branches share. + detached: true, + } + ); + } catch { + return failed("spawn-failed"); + } + child.unref(); + await context.observe("spawned"); + const closeGate = () => { + try { + child.stdin.end(); + } catch { + // Already closed; the gate treats EOF as "do not run". + } + }; + const census = await readCensus(); + const row = census?.find((entry) => entry.pid === child.pid); + if (!row) { + closeGate(); + return failed(census === null ? "census-unavailable" : "spawn-failed"); + } + const owned = { pid: row.pid, birth: row.birth }; + const holders = withHolder(carried, holder); + try { + await writeClaim(spec, { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "starting", + pid: owned.pid, + birth: owned.birth, + holders, + }); + } catch { + closeGate(); + return failed("claim-invalid"); + } + await context.observe("claimed"); + if (context.signal?.aborted) { + closeGate(); + await removeClaim(spec); + return failed("cancelled"); + } + try { + child.stdin.write("open\n"); + await child.stdin.flush(); + } finally { + closeGate(); + } + await context.observe("gate-opened"); + const outcome = await awaitReadiness(context, owned); + if (outcome.kind === "failed") { + const stopped = await stopVerifiedSharedHostProcess({ + ...owned, + graceMs: 1000, + }); + // Keep the claim when the stop is unproven, so the process stays recorded. + if (stopped === "stopped" || stopped === "absent") { + await removeClaim(spec); + } + await context.observe("stopped-after-failure"); + return failed(outcome.reason); + } + await writeClaim(spec, { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "ready", + pid: owned.pid, + birth: owned.birth, + holders, + }); + await context.observe("ready"); + return { + state: "ready", + ownership: "owned", + endpoint: endpointFor(spec.port), + pid: owned.pid, + spawned: true, + }; +} + +async function prepareStateDir(stateDir: string): Promise { + await mkdir(stateDir, { recursive: true, mode: 0o700 }); +} + +function isAbort(error: unknown, signal?: AbortSignal): boolean { + return ( + signal?.aborted === true || + (error instanceof Error && error.name === "AbortError") + ); +} + +/** + * Make the shared helper ready for `holder`, adopting an identity-verified + * helper, reusing the owned one, or starting it under an exact claim. + */ +export async function ensureSharedHostHelper(opts: { + readonly spec: SharedHostHelperSpec; + readonly holder: string; + readonly signal?: AbortSignal; + readonly observe?: (event: SharedHostEvent) => void | Promise; +}): Promise { + assertSpec(opts.spec); + assertHolder(opts.holder); + if (!opts.spec.enabled) { + return { state: "disabled", ownership: "disabled" }; + } + if (opts.signal?.aborted) { + return failed("cancelled"); + } + const observer = opts.observe; + const context: EnsureContext = { + spec: opts.spec, + holder: opts.holder, + signal: opts.signal, + observe: async (event) => { + await observer?.(event); + }, + deadline: performance.now() + opts.spec.deadlineMs, + }; + try { + await prepareStateDir(opts.spec.stateDir); + } catch { + return failed("claim-invalid"); + } + try { + return await withProjectsRegistryLock({ + lockPath: lockPath(opts.spec), + label: "shared host helper", + timeoutMs: opts.spec.deadlineMs, + signal: opts.signal, + run: () => ensureLocked(context), + }); + } catch (error: unknown) { + if (isAbort(error, opts.signal)) { + return failed("cancelled"); + } + if (error instanceof ClaimRefusal) { + return failed("claim-invalid"); + } + return failed("lock-unavailable"); + } +} + +/** + * Drop `holder`. The last holder of an owned helper stops it through its + * birth-verified tree; adopted helpers and stale claims are never signalled. + */ +export async function releaseSharedHostHelper(opts: { + readonly spec: Pick; + readonly holder: string; + readonly graceMs?: number; + readonly timeoutMs?: number; + readonly signal?: AbortSignal; +}): Promise { + if (!NAME.test(opts.spec.name)) { + throw new Error("Invalid shared host helper specification"); + } + assertHolder(opts.holder); + try { + await prepareStateDir(opts.spec.stateDir); + } catch { + return releaseFailed("claim-invalid"); + } + try { + return await withProjectsRegistryLock({ + lockPath: lockPath(opts.spec), + label: "shared host helper", + timeoutMs: opts.timeoutMs ?? 10_000, + signal: opts.signal, + run: () => releaseLocked(opts), + }); + } catch (error: unknown) { + if (isAbort(error, opts.signal)) { + return releaseFailed("cancelled"); + } + if (error instanceof ClaimRefusal) { + return releaseFailed("claim-invalid"); + } + return releaseFailed("lock-unavailable"); + } +} + +async function releaseLocked(opts: { + readonly spec: Pick; + readonly holder: string; + readonly graceMs?: number; +}): Promise { + const read = await readClaim(opts.spec); + if (read.kind === "absent") { + return { state: "absent" }; + } + if (read.kind === "invalid") { + return releaseFailed("claim-invalid"); + } + const claim = read.claim; + const holders = claim.holders.filter((entry) => entry !== opts.holder); + if (holders.length > 0) { + await writeClaim(opts.spec, { ...claim, holders }); + return { state: "retained" }; + } + if (claim.ownership === "adopted") { + await removeClaim(opts.spec); + return { state: "released-adopted" }; + } + const stopped = await stopVerifiedSharedHostProcess({ + pid: claim.pid, + birth: claim.birth, + graceMs: opts.graceMs ?? 2000, + }); + switch (stopped) { + case "census-unavailable": + return releaseFailed("census-unavailable"); + case "unconfirmed": + await writeClaim(opts.spec, { ...claim, holders }); + return { state: "stop-unconfirmed" }; + case "absent": + await removeClaim(opts.spec); + return { state: "stale-cleared" }; + default: + await removeClaim(opts.spec); + return { state: "stopped" }; + } +} diff --git a/tests/fixtures/shared-host-helper.ts b/tests/fixtures/shared-host-helper.ts new file mode 100644 index 000000000..54f73cd74 --- /dev/null +++ b/tests/fixtures/shared-host-helper.ts @@ -0,0 +1,71 @@ +/** + * Portable shared-host-helper fixture process for + * tests/shared-host-lifecycle.test.ts. + * + * argv: + * - `identity`: HTTP on 127.0.0.1:; `/__hack/identity` reports + * {helper, upstream, pid}; `/` answers `served:`. + * - `tcp`: a plain TCP listener that never answers (a foreign port owner). + * - `placeholder`: stays alive and never listens (a sleeping placeholder). + * - `parent`: spawns a `placeholder` grandchild, prints its PID, then serves + * like `identity` (a helper with its own child process). + * + * Every mode exits once its private HOME (the test's scratch root) is gone, + * so no fixture outlives its test even when a failing run skips cleanup. + */ +import { existsSync } from "node:fs"; + +const [mode, portText, helper, upstream] = process.argv.slice(2); +const port = Number(portText); +const home = process.env.HOME; +setInterval(() => { + if (!(home && existsSync(home))) { + process.exit(0); + } +}, 250); + +function serveIdentity(): void { + Bun.serve({ + hostname: "127.0.0.1", + port, + fetch(request) { + const url = new URL(request.url); + if (url.pathname === "/__hack/identity") { + return Response.json({ helper, upstream, pid: process.pid }); + } + return new Response(`served:${upstream}`); + }, + }); +} + +switch (mode) { + case "identity": + serveIdentity(); + break; + case "tcp": + Bun.listen({ + hostname: "127.0.0.1", + port, + socket: { + data() { + // Accepts connections and never answers. + }, + }, + }); + break; + case "placeholder": + setInterval(() => undefined, 60_000); + break; + case "parent": { + const child = Bun.spawn( + [process.execPath, import.meta.path, "placeholder", "0", "-", "-"], + { stdin: "ignore", stdout: "ignore", stderr: "ignore" } + ); + console.log(`grandchild ${child.pid}`); + serveIdentity(); + break; + } + default: + process.exit(2); +} +console.log("listening"); diff --git a/tests/fixtures/shared-host-starter.ts b/tests/fixtures/shared-host-starter.ts new file mode 100644 index 000000000..077a9e348 --- /dev/null +++ b/tests/fixtures/shared-host-starter.ts @@ -0,0 +1,27 @@ +/** + * Independent starter client for tests/shared-host-lifecycle.test.ts. + * + * argv: [pause-event] + * Prints `event ` for each lifecycle event and `result ` at the + * end. With a pause event it stops forever after printing that event, so the + * test can kill this parent at a precise point. + */ +import { + ensureSharedHostHelper, + type SharedHostHelperSpec, +} from "../../src/lib/shared-host-lifecycle.ts"; + +const [specText, holder, pauseAt] = process.argv.slice(2); +const spec = JSON.parse(specText ?? "{}") as SharedHostHelperSpec; +const result = await ensureSharedHostHelper({ + spec, + holder: holder ?? "", + observe: async (event) => { + console.log(`event ${event}`); + if (event === pauseAt) { + // An interval keeps the event loop alive until the test kills us. + await new Promise(() => setInterval(() => undefined, 60_000)); + } + }, +}); +console.log(`result ${JSON.stringify(result)}`); diff --git a/tests/shared-host-lifecycle.test.ts b/tests/shared-host-lifecycle.test.ts new file mode 100644 index 000000000..5d6896cd7 --- /dev/null +++ b/tests/shared-host-lifecycle.test.ts @@ -0,0 +1,916 @@ +import { afterEach, expect, test } from "bun:test"; +import { + mkdir, + mkdtemp, + readdir, + readFile, + rm, + writeFile, +} from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join, resolve } from "node:path"; +import { + readProcessIdentities, + selectProcessTree, + signalVerifiedProcessTree, +} from "../src/lib/process-tree.ts"; +import { + ensureSharedHostHelper, + parseSharedHostClaim, + releaseSharedHostHelper, + type SharedHostEvent, + type SharedHostHelperSpec, +} from "../src/lib/shared-host-lifecycle.ts"; +import { restoreEnv } from "./helpers/env.ts"; + +/** + * Portable process fixtures for the shared host helper contract + * (docs/reference/shared-host-lifecycle.md). Every fixture is a local Bun + * process on a loopback port: no Docker, no real HOME, no privileged effect. + * Foreign processes are started and stopped by the test through their own + * handles; the library under test may only signal the helpers it claimed. + */ + +const HELPER = resolve(import.meta.dir, "fixtures/shared-host-helper.ts"); +const STARTER = resolve(import.meta.dir, "fixtures/shared-host-starter.ts"); +const TIMEOUT = 30_000; + +type Foreign = Bun.Subprocess<"ignore", "pipe", "ignore">; +const foreign: Foreign[] = []; +const starters: Bun.Subprocess<"ignore", "pipe", "ignore">[] = []; +const claimedHelpers: { pid: number; birth: string }[] = []; +const roots: string[] = []; + +afterEach(async () => { + for (const proc of [...starters.splice(0), ...foreign.splice(0)]) { + proc.kill("SIGKILL"); + await proc.exited; + } + // Reap every helper the library claimed, through the shared birth-verified + // tree primitives rather than the code under test, so a failing or mutated + // run cannot leak fixture processes onto a shared host. + const owned = [...claimedHelpers.splice(0)]; + for (const root of roots) { + const text = await readFile( + join(root, "state", "issuer-bridge.claim.json"), + "utf8" + ).catch(() => null); + const claim = text === null ? null : parseSharedHostClaim(text); + if (claim?.ownership === "owned") { + owned.push({ pid: claim.pid, birth: claim.birth }); + } + } + const census = await readProcessIdentities(); + for (const identity of owned) { + if ( + census.some( + (row) => row.pid === identity.pid && row.birth === identity.birth + ) + ) { + await signalVerifiedProcessTree( + selectProcessTree(census, identity.pid), + "SIGKILL" + ); + } + } + for (const root of roots.splice(0)) { + await rm(root, { recursive: true, force: true }); + } +}); + +function childEnv(home: string): Record { + return { PATH: process.env.PATH ?? "/usr/bin:/bin", HOME: home }; +} + +async function scratch(): Promise { + const root = await mkdtemp(join(tmpdir(), "hack-shared-host-")); + roots.push(root); + return root; +} + +function freePort(): number { + const probe = Bun.listen({ + hostname: "127.0.0.1", + port: 0, + socket: { + data() { + // Bind probe only. + }, + }, + }); + const port = probe.port; + probe.stop(true); + return port; +} + +function specFor(opts: { + readonly root: string; + readonly port: number; + readonly upstream?: string; + readonly mode?: "identity" | "placeholder" | "parent"; + readonly deadlineMs?: number; +}): SharedHostHelperSpec { + const upstream = opts.upstream ?? "https://issuer.branch-a.test"; + return { + name: "issuer-bridge", + enabled: true, + stateDir: join(opts.root, "state"), + port: opts.port, + identityPath: "/__hack/identity", + expected: { helper: "issuer-bridge", upstream }, + command: [ + process.execPath, + HELPER, + opts.mode ?? "identity", + String(opts.port), + "issuer-bridge", + upstream, + ], + cwd: opts.root, + env: childEnv(opts.root), + deadlineMs: opts.deadlineMs ?? 10_000, + backoff: { initialMs: 25, maxMs: 200 }, + probeTimeoutMs: 500, + }; +} + +function lineReader(stream: ReadableStream) { + const reader = stream.getReader(); + const decoder = new TextDecoder(); + const lines: string[] = []; + let buffer = ""; + return { + lines, + async waitFor(match: (line: string) => boolean): Promise { + const deadline = performance.now() + 15_000; + while (performance.now() < deadline) { + const found = lines.find(match); + if (found !== undefined) { + return found; + } + const { done, value } = await reader.read(); + if (done) { + break; + } + buffer += decoder.decode(value, { stream: true }); + const parts = buffer.split("\n"); + buffer = parts.pop() ?? ""; + lines.push(...parts); + } + const found = lines.find(match); + if (found === undefined) { + throw new Error(`fixture line not observed; saw ${lines.join(" | ")}`); + } + return found; + }, + }; +} + +async function startForeign(opts: { + readonly mode: "identity" | "tcp" | "placeholder"; + readonly port: number; + readonly helper?: string; + readonly upstream?: string; + readonly home: string; +}): Promise { + const proc = Bun.spawn( + [ + process.execPath, + HELPER, + opts.mode, + String(opts.port), + opts.helper ?? "issuer-bridge", + opts.upstream ?? "https://issuer.branch-a.test", + ], + { + env: childEnv(opts.home), + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + } + ); + foreign.push(proc); + await lineReader(proc.stdout).waitFor((line) => line === "listening"); + return proc; +} + +function startStarter(opts: { + readonly spec: SharedHostHelperSpec; + readonly holder: string; + readonly pauseAt?: SharedHostEvent; +}) { + const proc = Bun.spawn( + [ + process.execPath, + STARTER, + JSON.stringify(opts.spec), + opts.holder, + ...(opts.pauseAt ? [opts.pauseAt] : []), + ], + { + env: childEnv(opts.spec.cwd), + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + } + ); + starters.push(proc); + return { proc, output: lineReader(proc.stdout) }; +} + +async function starterResult(starter: ReturnType) { + const line = await starter.output.waitFor((entry) => + entry.startsWith("result ") + ); + return JSON.parse(line.slice("result ".length)) as Record; +} + +async function identityOf(pid: number) { + const row = (await readProcessIdentities()).find( + (entry) => entry.pid === pid + ); + return row ? { pid: row.pid, birth: row.birth } : null; +} + +async function isAlive(identity: { pid: number; birth: string }) { + return (await readProcessIdentities()).some( + (row) => row.pid === identity.pid && row.birth === identity.birth + ); +} + +async function waitGone(identity: { pid: number; birth: string }) { + const deadline = performance.now() + 5000; + while (performance.now() < deadline) { + if (!(await isAlive(identity))) { + return true; + } + await Bun.sleep(50); + } + return false; +} + +async function readClaim(spec: SharedHostHelperSpec) { + const text = await readFile( + join(spec.stateDir, `${spec.name}.claim.json`), + "utf8" + ).catch(() => null); + return text === null ? null : parseSharedHostClaim(text); +} + +async function trackClaimedHelper(spec: SharedHostHelperSpec) { + const claim = await readClaim(spec); + if (claim?.ownership !== "owned") { + throw new Error("expected an owned claim"); + } + const identity = { pid: claim.pid, birth: claim.birth }; + claimedHelpers.push(identity); + return identity; +} + +function recorder() { + const events: SharedHostEvent[] = []; + return { + events, + observe: (event: SharedHostEvent) => { + events.push(event); + }, + }; +} + +test( + "a disabled helper takes no lock, writes no claim and starts no child", + async () => { + const root = await scratch(); + const spec = { ...specFor({ root, port: freePort() }), enabled: false }; + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }); + expect(result).toEqual({ state: "disabled", ownership: "disabled" }); + expect(seen.events).toEqual([]); + expect(await readdir(root)).toEqual([]); + }, + TIMEOUT +); + +test( + "an owned helper becomes ready by identity, serves its branch upstream and is stopped by its last holder", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }); + const owned = await trackClaimedHelper(spec); + expect(result).toMatchObject({ + state: "ready", + ownership: "owned", + spawned: true, + pid: owned.pid, + endpoint: `http://127.0.0.1:${spec.port}`, + }); + expect(seen.events).toContain("claimed"); + expect(seen.events.indexOf("claimed")).toBeLessThan( + seen.events.indexOf("gate-opened") + ); + expect(await (await fetch(`http://127.0.0.1:${spec.port}/`)).text()).toBe( + "served:https://issuer.branch-a.test" + ); + expect(await readClaim(spec)).toMatchObject({ + phase: "ready", + holders: ["branch-a"], + }); + + expect(await releaseSharedHostHelper({ spec, holder: "branch-a" })).toEqual( + { state: "stopped" } + ); + expect(await isAlive(owned)).toBe(false); + expect(await readClaim(spec)).toBeNull(); + }, + TIMEOUT +); + +test( + "a healthy shared helper started elsewhere is adopted and never signalled on release", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const existing = await startForeign({ + mode: "identity", + port: spec.port, + home: root, + }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }); + expect(result).toEqual({ + state: "ready", + ownership: "adopted", + endpoint: `http://127.0.0.1:${spec.port}`, + spawned: false, + }); + expect(seen.events).not.toContain("spawned"); + expect(await (await fetch(`http://127.0.0.1:${spec.port}/`)).text()).toBe( + "served:https://issuer.branch-a.test" + ); + expect(await releaseSharedHostHelper({ spec, holder: "branch-a" })).toEqual( + { state: "released-adopted" } + ); + await Bun.sleep(200); + expect(existing.exitCode).toBeNull(); + expect(await readClaim(spec)).toBeNull(); + }, + TIMEOUT +); + +test( + "a helper serving the wrong upstream is refused and left running", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const existing = await startForeign({ + mode: "identity", + port: spec.port, + upstream: "https://issuer.production.example", + home: root, + }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }); + expect(result).toMatchObject({ state: "failed", reason: "wrong-upstream" }); + expect(JSON.stringify(result)).not.toContain("production"); + expect(seen.events).not.toContain("spawned"); + expect(await readClaim(spec)).toBeNull(); + await Bun.sleep(200); + expect(existing.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "foreign listeners on the helper port are refused and preserved", + async () => { + for (const fixture of [ + { mode: "tcp" as const }, + { mode: "identity" as const, helper: "unrelated-dev-server" }, + ]) { + const root = await scratch(); + const spec = specFor({ root, port: freePort(), deadlineMs: 3000 }); + const existing = await startForeign({ + ...fixture, + port: spec.port, + home: root, + }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }); + expect(result).toMatchObject({ + state: "failed", + reason: "foreign-listener", + }); + expect(seen.events).not.toContain("spawned"); + await Bun.sleep(200); + expect(existing.exitCode).toBeNull(); + } + }, + TIMEOUT +); + +test( + "independent concurrent starters are serialized onto one owned helper; sibling holders and branches are preserved", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const first = startStarter({ spec, holder: "branch-a" }); + const second = startStarter({ spec, holder: "branch-b" }); + const results = await Promise.all([ + starterResult(first), + starterResult(second), + ]); + const owned = await trackClaimedHelper(spec); + expect(results.map((entry) => entry.state)).toEqual(["ready", "ready"]); + expect(results.map((entry) => entry.ownership)).toEqual(["owned", "owned"]); + expect(results.map((entry) => entry.pid)).toEqual([owned.pid, owned.pid]); + expect(results.filter((entry) => entry.spawned === true)).toHaveLength(1); + const spawnedEvents = [first, second].filter((starter) => + starter.output.lines.includes("event spawned") + ); + expect(spawnedEvents).toHaveLength(1); + expect([...((await readClaim(spec))?.holders ?? [])].sort()).toEqual([ + "branch-a", + "branch-b", + ]); + + // A sibling branch with a different upstream cannot take over the helper. + const sibling = await ensureSharedHostHelper({ + spec: specFor({ + root, + port: spec.port, + upstream: "https://issuer.branch-c.test", + }), + holder: "branch-c", + }); + expect(sibling).toMatchObject({ + state: "failed", + reason: "wrong-upstream", + }); + + expect(await releaseSharedHostHelper({ spec, holder: "branch-a" })).toEqual( + { state: "retained" } + ); + expect(await isAlive(owned)).toBe(true); + expect(await (await fetch(`http://127.0.0.1:${spec.port}/`)).text()).toBe( + "served:https://issuer.branch-a.test" + ); + expect(await releaseSharedHostHelper({ spec, holder: "branch-b" })).toEqual( + { state: "stopped" } + ); + expect(await isAlive(owned)).toBe(false); + }, + TIMEOUT +); + +test( + "a stale claim whose PID was reused is discarded without signalling the live process", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const bystander = await startForeign({ + mode: "placeholder", + port: 0, + home: root, + }); + const reused = await identityOf(bystander.pid); + expect(reused).not.toBeNull(); + const staleClaim = { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "ready", + pid: bystander.pid, + birth: "Thu Jan 1 00:00:00 1970", + holders: ["branch-old"], + }; + await mkdir(spec.stateDir, { recursive: true, mode: 0o700 }); + const claimFile = join(spec.stateDir, `${spec.name}.claim.json`); + await writeFile(claimFile, JSON.stringify(staleClaim), { mode: 0o600 }); + + // Release by the last holder must not signal the reused PID. + expect( + await releaseSharedHostHelper({ spec, holder: "branch-old" }) + ).toEqual({ state: "stale-cleared" }); + await Bun.sleep(200); + expect(bystander.exitCode).toBeNull(); + + // A starter treats the same stale claim as no ownership and starts fresh. + await writeFile(claimFile, JSON.stringify(staleClaim), { mode: 0o600 }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }); + const owned = await trackClaimedHelper(spec); + expect(seen.events).toContain("claim-stale"); + expect(result).toMatchObject({ state: "ready", ownership: "owned" }); + expect(owned.pid).not.toBe(bystander.pid); + expect((await readClaim(spec))?.holders).toEqual([ + "branch-old", + "branch-a", + ]); + await Bun.sleep(200); + expect(bystander.exitCode).toBeNull(); + expect(await isAlive(reused ?? owned)).toBe(true); + }, + TIMEOUT +); + +test( + "a crashed owned child is restarted under a fresh claim by the next starter", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const first = await ensureSharedHostHelper({ spec, holder: "branch-a" }); + const crashed = await trackClaimedHelper(spec); + expect(first).toMatchObject({ state: "ready", spawned: true }); + process.kill(crashed.pid, "SIGKILL"); + expect(await waitGone(crashed)).toBe(true); + + const seen = recorder(); + const second = await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }); + const restarted = await trackClaimedHelper(spec); + expect(seen.events).toContain("claim-stale"); + expect(second).toMatchObject({ + state: "ready", + ownership: "owned", + spawned: true, + pid: restarted.pid, + }); + expect(restarted).not.toEqual(crashed); + expect((await readClaim(spec))?.holders).toEqual(["branch-a", "branch-b"]); + expect(await (await fetch(`http://127.0.0.1:${spec.port}/`)).text()).toBe( + "served:https://issuer.branch-a.test" + ); + }, + TIMEOUT +); + +test( + "cancellation during bounded readiness retries stops the owned placeholder and keeps foreign processes", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort(), mode: "placeholder" }); + const bystander = await startForeign({ + mode: "tcp", + port: freePort(), + home: root, + }); + const controller = new AbortController(); + let owned: { pid: number; birth: string } | null = null; + let retries = 0; + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + signal: controller.signal, + observe: async (event) => { + if (event === "claimed") { + owned = await trackClaimedHelper(spec); + } + if (event === "probe-retry") { + retries += 1; + if (retries === 3) { + controller.abort(); + } + } + }, + }); + expect(result).toMatchObject({ state: "failed", reason: "cancelled" }); + expect(retries).toBe(3); + expect(owned).not.toBeNull(); + expect(await isAlive(owned ?? { pid: 0, birth: "" })).toBe(false); + expect(await readClaim(spec)).toBeNull(); + expect(bystander.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "a live placeholder is never readiness: the deadline bounds retries and stops the owned child", + async () => { + const root = await scratch(); + const spec = specFor({ + root, + port: freePort(), + mode: "placeholder", + deadlineMs: 800, + }); + let owned: { pid: number; birth: string } | null = null; + const started = performance.now(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: async (event) => { + if (event === "claimed") { + owned = await trackClaimedHelper(spec); + } + }, + }); + expect(result).toMatchObject({ state: "failed", reason: "deadline" }); + expect(performance.now() - started).toBeLessThan(5000); + expect(await isAlive(owned ?? { pid: 0, birth: "" })).toBe(false); + expect(await readClaim(spec)).toBeNull(); + }, + TIMEOUT +); + +test( + "a reused owned claim is not ready when a different process answers its identity", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort(), deadlineMs: 2000 }); + const claimedProcess = await startForeign({ + mode: "placeholder", + port: 0, + home: root, + }); + const impostor = await startForeign({ + mode: "identity", + port: spec.port, + home: root, + }); + const identity = await identityOf(claimedProcess.pid); + await mkdir(spec.stateDir, { recursive: true, mode: 0o700 }); + await writeFile( + join(spec.stateDir, `${spec.name}.claim.json`), + JSON.stringify({ + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "starting", + pid: claimedProcess.pid, + birth: identity?.birth ?? "", + holders: ["branch-a"], + }), + { mode: 0o600 } + ); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }); + expect(seen.events).toContain("claim-reused"); + expect(result).toMatchObject({ + state: "failed", + reason: "foreign-listener", + }); + expect((await readClaim(spec))?.holders).toEqual(["branch-a"]); + await Bun.sleep(200); + expect(claimedProcess.exitCode).toBeNull(); + expect(impostor.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "a starter lost before its claim opens the gate leaves no helper running", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const starter = startStarter({ + spec, + holder: "branch-a", + pauseAt: "claimed", + }); + await starter.output.waitFor((line) => line === "event claimed"); + const gated = await trackClaimedHelper(spec); + starter.proc.kill("SIGKILL"); + await starter.proc.exited; + expect(await waitGone(gated)).toBe(true); + const port = await fetch(`http://127.0.0.1:${spec.port}/`).catch( + () => null + ); + expect(port).toBeNull(); + + const seen = recorder(); + const next = await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }); + await trackClaimedHelper(spec); + expect(seen.events).toContain("claim-stale"); + expect(next).toMatchObject({ state: "ready", spawned: true }); + }, + TIMEOUT +); + +test( + "a starter lost after readiness leaves a claimed helper that another client reuses and releases", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const starter = startStarter({ + spec, + holder: "branch-a", + pauseAt: "ready", + }); + await starter.output.waitFor((line) => line === "event ready"); + const owned = await trackClaimedHelper(spec); + starter.proc.kill("SIGKILL"); + await starter.proc.exited; + await Bun.sleep(200); + expect(await isAlive(owned)).toBe(true); + + const seen = recorder(); + const reused = await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }); + expect(seen.events).toContain("claim-reused"); + expect(reused).toMatchObject({ + state: "ready", + ownership: "owned", + spawned: false, + pid: owned.pid, + }); + expect(await releaseSharedHostHelper({ spec, holder: "branch-a" })).toEqual( + { state: "retained" } + ); + expect(await releaseSharedHostHelper({ spec, holder: "branch-b" })).toEqual( + { state: "stopped" } + ); + expect(await isAlive(owned)).toBe(false); + }, + TIMEOUT +); + +test( + "release stops the owned helper's own descendants and nothing else", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort(), mode: "parent" }); + const bystander = await startForeign({ + mode: "placeholder", + port: 0, + home: root, + }); + const result = await ensureSharedHostHelper({ spec, holder: "branch-a" }); + expect(result).toMatchObject({ state: "ready", ownership: "owned" }); + const owned = await trackClaimedHelper(spec); + const grandchildRow = (await readProcessIdentities()).find( + (row) => row.parentPid === owned.pid + ); + expect(grandchildRow).toBeDefined(); + const grandchild = { + pid: grandchildRow?.pid ?? 0, + birth: grandchildRow?.birth ?? "", + }; + claimedHelpers.push(grandchild); + expect(await releaseSharedHostHelper({ spec, holder: "branch-a" })).toEqual( + { state: "stopped" } + ); + expect(await isAlive(owned)).toBe(false); + expect(await waitGone(grandchild)).toBe(true); + expect(bystander.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "an unreadable claim fails closed for start and release", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + await mkdir(spec.stateDir, { recursive: true, mode: 0o700 }); + await writeFile( + join(spec.stateDir, `${spec.name}.claim.json`), + '{"version":1,"helper":"issuer-bridge","ownership":"owned"}', + { mode: 0o600 } + ); + const seen = recorder(); + expect( + await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }) + ).toMatchObject({ state: "failed", reason: "claim-invalid" }); + expect(seen.events).not.toContain("spawned"); + expect( + await releaseSharedHostHelper({ spec, holder: "branch-a" }) + ).toMatchObject({ state: "failed", reason: "claim-invalid" }); + }, + TIMEOUT +); + +test( + "an unavailable process census fails closed: no spawn, no signal, claim kept", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const bystander = await startForeign({ + mode: "placeholder", + port: 0, + home: root, + }); + const identity = await identityOf(bystander.pid); + await mkdir(spec.stateDir, { recursive: true, mode: 0o700 }); + const claimFile = join(spec.stateDir, `${spec.name}.claim.json`); + const claim = JSON.stringify({ + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "ready", + pid: bystander.pid, + birth: identity?.birth ?? "", + holders: ["branch-a"], + }); + await writeFile(claimFile, claim, { mode: 0o600 }); + const bin = join(root, "bin"); + await mkdir(bin); + await writeFile(join(bin, "ps"), "#!/bin/sh\nexit 1\n", { mode: 0o755 }); + const savedPath = process.env.PATH; + process.env.PATH = `${bin}:${savedPath ?? "/usr/bin:/bin"}`; + try { + const seen = recorder(); + expect( + await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }) + ).toMatchObject({ state: "failed", reason: "census-unavailable" }); + expect(seen.events).not.toContain("spawned"); + expect( + await releaseSharedHostHelper({ spec, holder: "branch-a" }) + ).toMatchObject({ state: "failed", reason: "census-unavailable" }); + } finally { + restoreEnv("PATH", savedPath); + } + expect(await readFile(claimFile, "utf8")).toBe(claim); + await Bun.sleep(200); + expect(bystander.exitCode).toBeNull(); + }, + TIMEOUT +); + +test("claim parser accepts only the exact v1 shapes", () => { + const owned = { + version: 1, + helper: "issuer-bridge", + port: 4100, + ownership: "owned", + phase: "ready", + pid: 4242, + birth: "Mon Oct 19 10:00:00 2026", + holders: ["branch-a"], + }; + const adopted = { + version: 1, + helper: "issuer-bridge", + port: 4100, + ownership: "adopted", + holders: ["branch-a"], + }; + expect(parseSharedHostClaim(JSON.stringify(owned))).toEqual(owned as never); + expect(parseSharedHostClaim(JSON.stringify(adopted))).toEqual( + adopted as never + ); + for (const invalid of [ + { ...owned, pid: 1 }, + { ...owned, birth: "" }, + { ...owned, phase: "failed" }, + { ...owned, extra: true }, + { ...owned, holders: ["branch-a", "branch-a"] }, + { ...owned, holders: ["../escape"] }, + { ...adopted, pid: 4242 }, + { ...adopted, version: 2 }, + { ...adopted, port: 70_000 }, + ]) { + expect(parseSharedHostClaim(JSON.stringify(invalid))).toBeNull(); + } + expect(parseSharedHostClaim("not json")).toBeNull(); +}); From 5d0f337ff32895596ec84c436023811a76af4e9e Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 19 Oct 2026 12:28:00 -0400 Subject: [PATCH 2/5] fix(lifecycle): keep shared helper ownership exact across time zones and lost starts Read process births in UTC so clients with different TZ values agree on a live owned helper; previously a second client saw it as stale, re-adopted it and the last release leaked it. A stale owned claim whose PID still answers as the helper is now kept (claim-invalid) instead of adopted. Report deadline (not foreign-listener) when lock contention or the deadline cut the port probe short. Stop a helper left in phase starting by a lost starter once it still cannot become ready. Rethrow observer errors after closing the gate or stopping the started helper, and report state errors raised under the lock as claim-invalid/state-unavailable rather than lock-unavailable. Bound the claim read by bytes before reading it. Add a lock barrier to the concurrent-starters fixture and regression tests for every documented check; register the suite in the Linux process job. Refs: HACK-1142 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 2 +- docs/reference/shared-host-lifecycle.md | 135 +++-- src/lib/private-file.ts | 15 +- src/lib/process-tree.ts | 12 +- src/lib/shared-host-lifecycle.ts | 452 ++++++++++++----- tests/fixtures/shared-host-helper.ts | 30 +- tests/fixtures/shared-host-starter.ts | 50 +- tests/shared-host-lifecycle.test.ts | 626 +++++++++++++++++++++++- 8 files changed, 1114 insertions(+), 208 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 987da5080..739a3c1d7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -402,4 +402,4 @@ jobs: - name: Install dependencies run: bun install --frozen-lockfile - name: Run process and terminal regressions - run: bun test tests/daemon-command.test.ts tests/daemon-orphan.test.ts tests/host-exec-lifetime.test.ts tests/host-exec-tty.test.ts tests/shell-observation.test.ts + run: bun test tests/daemon-command.test.ts tests/daemon-orphan.test.ts tests/host-exec-lifetime.test.ts tests/host-exec-tty.test.ts tests/shell-observation.test.ts tests/shared-host-lifecycle.test.ts diff --git a/docs/reference/shared-host-lifecycle.md b/docs/reference/shared-host-lifecycle.md index 0b77dfa82..601bf98c4 100644 --- a/docs/reference/shared-host-lifecycle.md +++ b/docs/reference/shared-host-lifecycle.md @@ -43,27 +43,35 @@ The helper serves a JSON identity endpoint on `127.0.0.1:`: A different process answering for the port is foreign. Readiness is this identity answer. A live process, an open port or a sleeping -placeholder is never readiness. Each probe has its own timeout and reads at most 4 KiB. +placeholder is never readiness. Each probe has its own timeout and reads at most +4 KiB; a larger answer is not an identity. ## Persisted claim -The claim is `/.claim.json`, owner-only (0600), replaced atomically, -and parsed strictly (exact keys, version 1): +The claim is `/.claim.json`, owner-only (0600), at most 8 KiB, +replaced atomically, and parsed strictly (exact keys, version 1): ```json {"version":1,"helper":"issuer-bridge","port":4100,"ownership":"owned","phase":"ready","pid":4242,"birth":"Mon Oct 19 10:00:00 2026","holders":["branch-a","branch-b"]} {"version":1,"helper":"issuer-bridge","port":4100,"ownership":"adopted","holders":["branch-a"]} ``` -- Ownership is the pair `pid` + `birth` (process start time from `ps lstart`). The - claim is live only when the census shows that exact pair. A missing PID, or the - same PID with a different birth time (PID reuse), makes the claim stale. A stale - claim grants nothing. Hack discards it and never signals that PID. +- Ownership is the pair `pid` + `birth`. `birth` is the process start time from + `ps lstart`, always read with `TZ=UTC0` and `LC_ALL=C`. `ps` prints `lstart` in + local time, so without a fixed zone two clients with different `TZ` values, or a + host whose zone changes, would see different births for the same live process. +- The claim is live only when the census shows that exact pair. A missing PID, or + the same PID with a different birth time (PID reuse), makes the claim stale. A + stale claim grants nothing. Hack discards it and never signals that PID. +- One exception: if a stale owned claim's PID still answers the identity endpoint + as the expected helper, ownership is uncertain. Hack keeps the claim and fails + with `claim-invalid`. It neither adopts the helper (which would drop the owned + record and leak the helper on the last release) nor signals it. - `holders` names the branches or clients that use the helper. Holders carry over when a stale owned helper is restarted. -- A malformed, unreadable, symlinked or group/world-accessible claim, or one for a - different port, fails closed with `claim-invalid`. Hack does not start, adopt or - stop anything until an operator inspects it. +- A malformed, oversized, unreadable, symlinked or group/world-accessible claim, or + one for a different port, fails closed with `claim-invalid`. Hack does not start, + adopt or stop anything until an operator inspects it. - A census that did not observe the calling process (for example `ps` failed) fails closed with `census-unavailable`. An empty census never reads as "every claimed process is gone". @@ -73,26 +81,37 @@ and parsed strictly (exact keys, version 1): 1. `enabled: false` returns `disabled` immediately. 2. Take the helper's exclusive lock (`/.lock`, the projects-registry lock protocol: hard-link publication, dead-owner reclamation by `ESRCH` only). - Competing starters wait in turn, bounded by the deadline. + Competing starters wait in turn (`lock-waiting`, then `lock-acquired`). One + deadline bounds the lock wait, the port probe, the spawn and readiness. 3. A live owned claim is reused. The starter waits for readiness and adds its holder. - It does not spawn a second helper. If readiness fails, it leaves the helper for - the other holders. -4. Otherwise probe the port: + It does not spawn a second helper. + - A `ready` helper may serve other holders, so if readiness fails the starter + leaves it running and returns the failure. + - A `starting` claim seen under the lock has lost its starter, because a starter + holds the lock until its helper is ready or stopped. If that helper still + cannot become ready (`deadline`, `child-exited`, `foreign-listener`, + `wrong-upstream`), the starter stops it through its birth-verified tree and + removes the claim. `cancelled` and `census-unavailable` leave it recorded. +4. Otherwise, if the deadline has already passed, return `deadline`. Then probe the + port: - identity matches: write an `adopted` claim and return `ready/adopted`; - - wrong upstream: `wrong-upstream`; another helper, a silent or non-identity - listener: `foreign-listener`. Either way the listener keeps running and no - child is spawned; + - wrong upstream: `wrong-upstream`; another helper, a non-identity answer or a + listener that stays silent for the full probe timeout: `foreign-listener`. + Either way the listener keeps running and no child is spawned; + - a probe whose timeout was shortened by the deadline and then timed out proves + nothing about the listener: `deadline`; - connection refused: confirm with a loopback bind probe, then spawn. 5. Spawn through an exec gate in a new session: `/bin/sh -c 'IFS= read -r hack_gate || exit 0; [ "$hack_gate" = open ] || exit 0; exec "$@" `. - PID reuse within one `lstart` second. Birth times have one-second resolution, the same assumption as the existing lifecycle process-tree helpers. +- Linux `lstart` drift. Linux `ps` computes `lstart` from the boot time, which can + move when the wall clock is changed, so a live helper's birth can change. Start + then fails closed if the helper reports its PID in the identity answer, but a + release would read the claim as stale and clear it without stopping the helper. + macOS records the start time directly and is not affected. - Wiring into `hack.config.json`, `hack up`/`down` or the daemon, and migrating existing `lifecycle.singleton` configurations. - Native (macOS VM) and container-runtime effects. These fixtures qualify the diff --git a/src/lib/private-file.ts b/src/lib/private-file.ts index 3776ef235..6101e353b 100644 --- a/src/lib/private-file.ts +++ b/src/lib/private-file.ts @@ -278,7 +278,10 @@ async function observeReplaceableTarget(opts: { * unreadable file is never mistaken for an empty one and overwritten. */ export async function readReplaceablePrivateFile( - opts: ReplaceablePrivateFileOptions + opts: ReplaceablePrivateFileOptions & { + /** Largest accepted file size in bytes; a larger file throws `refusal()` before it is read. */ + readonly maxBytes?: number; + } ): Promise { const directories = await captureRealDirectories(opts); const observed = await observeReplaceableTarget(opts); @@ -303,11 +306,19 @@ export async function readReplaceablePrivateFile( if ( !opened.isFile() || opened.dev !== observed.dev || - opened.ino !== observed.ino + opened.ino !== observed.ino || + (opts.maxBytes !== undefined && opened.size > opts.maxBytes) ) { throw opts.refusal(); } const text = await file.readFile({ encoding: "utf8" }); + if ( + opts.maxBytes !== undefined && + Buffer.byteLength(text, "utf8") > opts.maxBytes + ) { + // The file grew in place after the size check. + throw opts.refusal(); + } await recheckRealDirectories({ directories, refusal: opts.refusal }); return { text, mode: opened.mode & 0o7777 }; } finally { diff --git a/src/lib/process-tree.ts b/src/lib/process-tree.ts index c354fc612..a6bb346b7 100644 --- a/src/lib/process-tree.ts +++ b/src/lib/process-tree.ts @@ -5,11 +5,19 @@ export type ProcessIdentityRow = { }; const IDENTITY_ROW = /^\s*(\d+)\s+(\d+)\s+(.+?)\s*$/; -/** Capture only process identity and lineage; never command arguments or environment. */ +/** + * Capture only process identity and lineage; never command arguments or environment. + * + * `birth` is the `lstart` start time rendered in UTC with the C locale. `ps` + * prints `lstart` in local time, so without a fixed zone two clients with + * different `TZ` values, or one host whose zone changes, would see different + * births for the same live process. Births may be persisted and compared across + * processes, so the zone is pinned here (POSIX `UTC0` needs no zoneinfo). + */ export async function readProcessIdentities(): Promise { try { const proc = Bun.spawn(["ps", "-A", "-o", "pid=,ppid=,lstart="], { - env: { ...process.env, LC_ALL: "C" }, + env: { ...process.env, LC_ALL: "C", TZ: "UTC0" }, stdin: "ignore", stdout: "pipe", stderr: "ignore", diff --git a/src/lib/shared-host-lifecycle.ts b/src/lib/shared-host-lifecycle.ts index a50187e74..f436b6b49 100644 --- a/src/lib/shared-host-lifecycle.ts +++ b/src/lib/shared-host-lifecycle.ts @@ -29,15 +29,19 @@ import { withProjectsRegistryLock } from "./projects-registry-lock.ts"; * - A listener is only ever adopted after its identity endpoint names the * expected helper and upstream. Anything else on the port is refused and * left running: this module never signals a process it cannot prove it owns. - * - Ownership is a persisted PID plus process birth time. A claim whose PID is + * - Ownership is a persisted PID plus process birth time (`ps lstart`, read in + * UTC so clients with different time zones agree). A claim whose PID is * gone or now belongs to a different process (PID reuse) is stale and is - * discarded without signalling anything. + * discarded without signalling anything. A stale claim whose PID still + * answers as the helper is uncertain: it is kept, never adopted. * - A child is spawned behind an exec gate: it waits on stdin until its claim * is durable. If the starter dies first, the gate reads EOF and exits, so a * spawned helper is never left running without a claim. * - Readiness is the identity endpoint answering for the expected helper, not - * process liveness. Retries back off and are bounded by one deadline; - * cancellation stops the owned child and removes its claim. + * process liveness. Retries back off and are bounded by one deadline that + * also covers the lock wait; cancellation stops the owned child and removes + * its claim. A `starting` claim found under the lock has lost its starter; + * if that helper still cannot become ready, it is stopped. * - Release removes one holder. Only when no holder remains is an owned helper * stopped, through its birth-verified process tree. Adopted helpers are * never signalled. @@ -54,6 +58,7 @@ export type SharedHostFailure = | "foreign-listener" | "wrong-upstream" | "claim-invalid" + | "state-unavailable" | "census-unavailable" | "lock-unavailable" | "port-probe-failed" @@ -68,7 +73,9 @@ const FAILURE_MESSAGES: Readonly> = { "wrong-upstream": "The shared helper on this port serves a different upstream; it was left running.", "claim-invalid": - "The persisted shared helper claim is unreadable or belongs to another configuration; inspect and release it before retrying.", + "The persisted shared helper claim is unreadable, uncertain or belongs to another configuration; inspect and release it before retrying.", + "state-unavailable": + "The shared helper state directory could not be read or written.", "census-unavailable": "The process census is unavailable, so helper ownership cannot be verified.", "lock-unavailable": @@ -135,6 +142,7 @@ export type SharedHostEnsureResult = /** Closed progress events. The observer is awaited (a test and UI seam). */ export type SharedHostEvent = + | "lock-waiting" | "lock-acquired" | "claim-stale" | "claim-reused" @@ -158,16 +166,19 @@ export type SharedHostReleaseResult = } | { readonly state: "failed"; - readonly reason: Extract< - SharedHostFailure, - | "claim-invalid" - | "census-unavailable" - | "lock-unavailable" - | "cancelled" - >; + readonly reason: ReleaseFailure; readonly message: string; }; +type ReleaseFailure = Extract< + SharedHostFailure, + | "claim-invalid" + | "state-unavailable" + | "census-unavailable" + | "lock-unavailable" + | "cancelled" +>; + type OwnedClaim = { readonly version: 1; readonly helper: string; @@ -190,6 +201,8 @@ type Claim = OwnedClaim | AdoptedClaim; type ProbeObservation = | { readonly kind: "absent" } | { readonly kind: "unidentified" } + /** The listener did not answer within this probe's own timeout. */ + | { readonly kind: "timeout" } | { readonly kind: "foreign" } | { readonly kind: "wrong-upstream" } | { readonly kind: "match"; readonly pid: number | null } @@ -214,16 +227,40 @@ class ClaimRefusal extends Error { } } +/** Carries an error thrown by the caller's observer; it is rethrown as is. */ +class ObserverError extends Error { + readonly original: unknown; + constructor(original: unknown) { + super("shared host observer failed"); + this.original = original; + } +} + +/** + * Wraps an error thrown while the helper lock is held, so it is never + * reported as a lock acquisition failure. + */ +class LockedRunError extends Error { + readonly original: unknown; + constructor(original: unknown) { + super("shared host locked operation failed"); + this.original = original; + } +} + +async function runLocked(run: () => Promise): Promise { + try { + return await run(); + } catch (error: unknown) { + throw new LockedRunError(error); + } +} + function failed(reason: SharedHostFailure): SharedHostEnsureResult { return { state: "failed", reason, message: FAILURE_MESSAGES[reason] }; } -function releaseFailed( - reason: Extract< - SharedHostFailure, - "claim-invalid" | "census-unavailable" | "lock-unavailable" | "cancelled" - > -): SharedHostReleaseResult { +function releaseFailed(reason: ReleaseFailure): SharedHostReleaseResult { return { state: "failed", reason, message: FAILURE_MESSAGES[reason] }; } @@ -373,6 +410,7 @@ async function readClaim( path: claimPath(spec), directories: [spec.stateDir], refusal: () => new ClaimRefusal(), + maxBytes: MAX_CLAIM_BYTES, }); } catch { return { kind: "invalid" }; @@ -380,7 +418,8 @@ async function readClaim( if (file === null) { return { kind: "absent" }; } - if (file.text.length > MAX_CLAIM_BYTES || (file.mode & 0o077) !== 0) { + // Only an owner-only receipt is trusted; group/world access makes it uncertain. + if ((file.mode & 0o077) !== 0) { return { kind: "invalid" }; } const claim = parseSharedHostClaim(file.text); @@ -472,7 +511,8 @@ export async function probeSharedHostIdentity(opts: { readonly timeoutMs: number; readonly signal?: AbortSignal; }): Promise { - const signals = [AbortSignal.timeout(opts.timeoutMs)]; + const timeout = AbortSignal.timeout(opts.timeoutMs); + const signals = [timeout]; if (opts.signal) { signals.push(opts.signal); } @@ -486,8 +526,11 @@ export async function probeSharedHostIdentity(opts: { if (opts.signal?.aborted) { return { kind: "cancelled" }; } - // Only a refused connection proves no listener; a timeout or reset means - // something holds the port without answering as the helper. + if (timeout.aborted) { + return { kind: "timeout" }; + } + // Only a refused connection proves no listener; a reset means something + // holds the port without answering as the helper. return isRecord(error) && error.code === "ECONNREFUSED" ? { kind: "absent" } : { kind: "unidentified" }; @@ -496,9 +539,19 @@ export async function probeSharedHostIdentity(opts: { if (opts.signal?.aborted) { return { kind: "cancelled" }; } + if (text === null && timeout.aborted) { + return { kind: "timeout" }; + } if (response.status !== 200 || text === null) { return { kind: "unidentified" }; } + return classifyIdentity(text, opts.expected); +} + +function classifyIdentity( + text: string, + expected: SharedHostIdentity +): ProbeObservation { let body: unknown; try { body = JSON.parse(text); @@ -512,10 +565,10 @@ export async function probeSharedHostIdentity(opts: { ) { return { kind: "unidentified" }; } - if (body.helper !== opts.expected.helper) { + if (body.helper !== expected.helper) { return { kind: "foreign" }; } - if (body.upstream !== opts.expected.upstream) { + if (body.upstream !== expected.upstream) { return { kind: "wrong-upstream" }; } const pid = @@ -662,10 +715,83 @@ async function awaitReadiness( } } +type OwnedIdentity = { readonly pid: number; readonly birth: string }; + +/** + * Readiness failures after which an owned helper that never reached `ready` is + * stopped. `cancelled` and `census-unavailable` leave it recorded instead: a + * cancelled caller or a blind census is not evidence that the start failed. + */ +const ABANDONED_START_FAILURES: ReadonlySet = new Set([ + "child-exited", + "deadline", + "foreign-listener", + "wrong-upstream", +]); + +/** + * Stop an owned helper whose start failed and forget its claim. When the stop + * cannot be confirmed the claim is kept, so the process stays recorded. + */ +async function stopFailedStart(opts: { + readonly context: EnsureContext; + readonly owned: OwnedIdentity; + readonly notify: boolean; +}): Promise { + const stopped = await stopVerifiedSharedHostProcess({ + ...opts.owned, + graceMs: 1000, + }); + if (stopped === "stopped" || stopped === "absent") { + await removeClaim(opts.context.spec); + } + if (opts.notify) { + await opts.context.observe("stopped-after-failure"); + } +} + +/** + * Reuse a live owned helper. A `ready` helper may serve other holders, so a + * failed readiness check never stops it. A `starting` claim seen under the + * lock has no live starter (a starter holds the lock until its helper is + * ready or stopped), so a helper that still cannot become ready is an + * abandoned start and is stopped through its birth-verified tree. + */ +async function reuseOwned( + context: EnsureContext, + claim: OwnedClaim +): Promise { + const { spec, holder } = context; + await context.observe("claim-reused"); + const outcome = await awaitReadiness(context, claim); + if (outcome.kind === "failed") { + if ( + claim.phase === "starting" && + ABANDONED_START_FAILURES.has(outcome.reason) + ) { + await stopFailedStart({ context, owned: claim, notify: true }); + } + return failed(outcome.reason); + } + await writeClaim(spec, { + ...claim, + phase: "ready", + holders: withHolder(claim.holders, holder), + }); + await context.observe("ready"); + return { + state: "ready", + ownership: "owned", + endpoint: endpointFor(spec.port), + pid: claim.pid, + spawned: false, + }; +} + async function ensureLocked( context: EnsureContext ): Promise { - const { spec, holder } = context; + const { spec } = context; await context.observe("lock-acquired"); const census = await readCensus(); if (census === null) { @@ -676,55 +802,87 @@ async function ensureLocked( return failed("claim-invalid"); } let carried: readonly string[] = []; + let staleOwned: OwnedClaim | null = null; if (read.kind === "claim") { const claim = read.claim; if (claim.port !== spec.port) { return failed("claim-invalid"); } if (claim.ownership === "owned" && isLive(census, claim)) { - await context.observe("claim-reused"); - const outcome = await awaitReadiness(context, claim); - if (outcome.kind === "failed") { - // Other holders may depend on it: a reused helper is never stopped here. - return failed(outcome.reason); - } - await writeClaim(spec, { - ...claim, - phase: "ready", - holders: withHolder(claim.holders, holder), - }); - await context.observe("ready"); - return { - state: "ready", - ownership: "owned", - endpoint: endpointFor(spec.port), - pid: claim.pid, - spawned: false, - }; + return await reuseOwned(context, claim); } if (claim.ownership === "owned") { // Dead or reused PID: the claim no longer proves ownership of anything. await context.observe("claim-stale"); + staleOwned = claim; } carried = claim.holders; } + const settled = await adoptOrRefuse(context, { carried, staleOwned }); + if (settled !== null) { + return settled; + } + try { + const occupied = await inspectListeningTcpPorts({ ports: [spec.port] }); + if (occupied.length > 0) { + return failed("foreign-listener"); + } + } catch { + return failed("port-probe-failed"); + } + if (read.kind === "claim") { + await removeClaim(spec); + } + return await spawnOwned(context, carried); +} +/** + * Probe the helper port when no live owned claim exists. Returns the final + * result (adopted, refused, deadline or cancelled), or null when nothing + * answers and the caller may spawn. + */ +async function adoptOrRefuse( + context: EnsureContext, + prior: { + readonly carried: readonly string[]; + readonly staleOwned: OwnedClaim | null; + } +): Promise { + const { spec, holder } = context; + const { carried, staleOwned } = prior; + // The lock wait shares the deadline; never start a probe after it passed. + const left = remainingMs(context); + if (left <= 0) { + return failed("deadline"); + } + const cutByDeadline = left < spec.probeTimeoutMs; const existing = await probeSharedHostIdentity({ port: spec.port, identityPath: spec.identityPath, expected: spec.expected, - timeoutMs: Math.min(spec.probeTimeoutMs, Math.max(1, remainingMs(context))), + timeoutMs: cutByDeadline ? left : spec.probeTimeoutMs, signal: context.signal, }); switch (existing.kind) { case "cancelled": return failed("cancelled"); - case "foreign": + case "timeout": + // A probe cut short by the deadline proves nothing about the listener; + // one that used its full timeout met a listener that does not answer. + return failed(cutByDeadline ? "deadline" : "foreign-listener"); case "unidentified": + case "foreign": return failed("foreign-listener"); case "wrong-upstream": return failed("wrong-upstream"); case "match": + if (staleOwned !== null && existing.pid === staleOwned.pid) { + // The claimed PID answers as the helper, but its birth no longer + // matches: ownership is uncertain. Adopting it would drop the owned + // record (and a later release would leak the helper); stopping it + // could signal a stranger. Keep the claim and fail closed. + return failed("claim-invalid"); + } await writeClaim(spec, { version: 1, helper: spec.name, @@ -740,20 +898,8 @@ async function ensureLocked( spawned: false, }; default: - break; - } - try { - const occupied = await inspectListeningTcpPorts({ ports: [spec.port] }); - if (occupied.length > 0) { - return failed("foreign-listener"); - } - } catch { - return failed("port-probe-failed"); - } - if (read.kind === "claim") { - await removeClaim(spec); + return null; } - return await spawnOwned(context, carried); } async function spawnOwned( @@ -780,7 +926,6 @@ async function spawnOwned( return failed("spawn-failed"); } child.unref(); - await context.observe("spawned"); const closeGate = () => { try { child.stdin.end(); @@ -788,73 +933,83 @@ async function spawnOwned( // Already closed; the gate treats EOF as "do not run". } }; - const census = await readCensus(); - const row = census?.find((entry) => entry.pid === child.pid); - if (!row) { - closeGate(); - return failed(census === null ? "census-unavailable" : "spawn-failed"); - } - const owned = { pid: row.pid, birth: row.birth }; - const holders = withHolder(carried, holder); + let owned: OwnedIdentity | null = null; + let gateOpened = false; try { + await context.observe("spawned"); + const census = await readCensus(); + const row = census?.find((entry) => entry.pid === child.pid); + if (!row) { + closeGate(); + return failed(census === null ? "census-unavailable" : "spawn-failed"); + } + const identity: OwnedIdentity = { pid: row.pid, birth: row.birth }; + const holders = withHolder(carried, holder); await writeClaim(spec, { version: 1, helper: spec.name, port: spec.port, ownership: "owned", phase: "starting", - pid: owned.pid, - birth: owned.birth, + pid: identity.pid, + birth: identity.birth, holders, }); - } catch { - closeGate(); - return failed("claim-invalid"); - } - await context.observe("claimed"); - if (context.signal?.aborted) { - closeGate(); - await removeClaim(spec); - return failed("cancelled"); - } - try { - child.stdin.write("open\n"); - await child.stdin.flush(); - } finally { + owned = identity; + await context.observe("claimed"); + if (context.signal?.aborted) { + closeGate(); + await removeClaim(spec); + return failed("cancelled"); + } + gateOpened = true; + try { + child.stdin.write("open\n"); + await child.stdin.flush(); + } catch { + // The gate died before it could run the helper. + closeGate(); + await stopFailedStart({ context, owned: identity, notify: false }); + return failed("spawn-failed"); + } closeGate(); - } - await context.observe("gate-opened"); - const outcome = await awaitReadiness(context, owned); - if (outcome.kind === "failed") { - const stopped = await stopVerifiedSharedHostProcess({ - ...owned, - graceMs: 1000, + await context.observe("gate-opened"); + const outcome = await awaitReadiness(context, identity); + if (outcome.kind === "failed") { + await stopFailedStart({ context, owned: identity, notify: true }); + return failed(outcome.reason); + } + await writeClaim(spec, { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "ready", + pid: identity.pid, + birth: identity.birth, + holders, }); - // Keep the claim when the stop is unproven, so the process stays recorded. - if (stopped === "stopped" || stopped === "absent") { - await removeClaim(spec); + await context.observe("ready"); + return { + state: "ready", + ownership: "owned", + endpoint: endpointFor(spec.port), + pid: identity.pid, + spawned: true, + }; + } catch (error: unknown) { + // An observer or a state write failed. Never leave a gate waiting while + // this starter lives, nor a running helper the caller was not handed. + closeGate(); + if (owned !== null) { + const recorded = owned; + await (gateOpened + ? stopFailedStart({ context, owned: recorded, notify: false }) + : removeClaim(spec) + ).catch(() => undefined); } - await context.observe("stopped-after-failure"); - return failed(outcome.reason); + throw error; } - await writeClaim(spec, { - version: 1, - helper: spec.name, - port: spec.port, - ownership: "owned", - phase: "ready", - pid: owned.pid, - birth: owned.birth, - holders, - }); - await context.observe("ready"); - return { - state: "ready", - ownership: "owned", - endpoint: endpointFor(spec.port), - pid: owned.pid, - spawned: true, - }; } async function prepareStateDir(stateDir: string): Promise { @@ -868,9 +1023,45 @@ function isAbort(error: unknown, signal?: AbortSignal): boolean { ); } +function hasErrorCode(error: unknown): boolean { + return isRecord(error) && typeof error.code === "string"; +} + +/** + * Map an error to a closed reason. A caller's observer error is rethrown + * unchanged; errors raised while the lock is held are never `lock-unavailable`. + */ +function classifyFailure( + error: unknown, + signal: AbortSignal | undefined +): ReleaseFailure { + if (error instanceof ObserverError) { + throw error.original; + } + if (error instanceof LockedRunError) { + const inner = error.original; + if (inner instanceof ObserverError) { + throw inner.original; + } + if (inner instanceof ClaimRefusal) { + return "claim-invalid"; + } + return isAbort(inner, signal) ? "cancelled" : "state-unavailable"; + } + if (isAbort(error, signal)) { + return "cancelled"; + } + // The lock times out with a plain error; a filesystem error carries a code. + return hasErrorCode(error) ? "state-unavailable" : "lock-unavailable"; +} + /** * Make the shared helper ready for `holder`, adopting an identity-verified * helper, reusing the owned one, or starting it under an exact claim. + * + * An error thrown by `observe` propagates unchanged after cleanup: a gate that + * has not opened is closed and its claim removed, and a helper started by this + * call is stopped. A holder recorded before the error stays recorded. */ export async function ensureSharedHostHelper(opts: { readonly spec: SharedHostHelperSpec; @@ -892,37 +1083,38 @@ export async function ensureSharedHostHelper(opts: { holder: opts.holder, signal: opts.signal, observe: async (event) => { - await observer?.(event); + try { + await observer?.(event); + } catch (error: unknown) { + throw new ObserverError(error); + } }, deadline: performance.now() + opts.spec.deadlineMs, }; try { await prepareStateDir(opts.spec.stateDir); } catch { - return failed("claim-invalid"); + return failed("state-unavailable"); } try { + await context.observe("lock-waiting"); return await withProjectsRegistryLock({ lockPath: lockPath(opts.spec), label: "shared host helper", timeoutMs: opts.spec.deadlineMs, signal: opts.signal, - run: () => ensureLocked(context), + run: () => runLocked(() => ensureLocked(context)), }); } catch (error: unknown) { - if (isAbort(error, opts.signal)) { - return failed("cancelled"); - } - if (error instanceof ClaimRefusal) { - return failed("claim-invalid"); - } - return failed("lock-unavailable"); + return failed(classifyFailure(error, opts.signal)); } } /** * Drop `holder`. The last holder of an owned helper stops it through its * birth-verified tree; adopted helpers and stale claims are never signalled. + * Releasing a holder that is not recorded changes nothing and reports + * `retained` while other holders remain. */ export async function releaseSharedHostHelper(opts: { readonly spec: Pick; @@ -938,7 +1130,7 @@ export async function releaseSharedHostHelper(opts: { try { await prepareStateDir(opts.spec.stateDir); } catch { - return releaseFailed("claim-invalid"); + return releaseFailed("state-unavailable"); } try { return await withProjectsRegistryLock({ @@ -946,16 +1138,10 @@ export async function releaseSharedHostHelper(opts: { label: "shared host helper", timeoutMs: opts.timeoutMs ?? 10_000, signal: opts.signal, - run: () => releaseLocked(opts), + run: () => runLocked(() => releaseLocked(opts)), }); } catch (error: unknown) { - if (isAbort(error, opts.signal)) { - return releaseFailed("cancelled"); - } - if (error instanceof ClaimRefusal) { - return releaseFailed("claim-invalid"); - } - return releaseFailed("lock-unavailable"); + return releaseFailed(classifyFailure(error, opts.signal)); } } diff --git a/tests/fixtures/shared-host-helper.ts b/tests/fixtures/shared-host-helper.ts index 54f73cd74..b88f4cc17 100644 --- a/tests/fixtures/shared-host-helper.ts +++ b/tests/fixtures/shared-host-helper.ts @@ -4,7 +4,11 @@ * * argv: * - `identity`: HTTP on 127.0.0.1:; `/__hack/identity` reports - * {helper, upstream, pid}; `/` answers `served:`. + * {helper, upstream, pid}; `/` answers `served:`; + * `/__fixture/env` lists this process's environment variable names. + * - `stubborn`: like `identity`, but ignores SIGTERM (only SIGKILL stops it). + * - `oversized`: like `identity`, but pads the identity answer past 4 KiB. + * - `exit`: exits immediately (a helper that dies during startup). * - `tcp`: a plain TCP listener that never answers (a foreign port owner). * - `placeholder`: stays alive and never listens (a sleeping placeholder). * - `parent`: spawns a `placeholder` grandchild, prints its PID, then serves @@ -24,14 +28,22 @@ setInterval(() => { } }, 250); -function serveIdentity(): void { +function serveIdentity(opts: { readonly padding?: number } = {}): void { Bun.serve({ hostname: "127.0.0.1", port, fetch(request) { const url = new URL(request.url); if (url.pathname === "/__hack/identity") { - return Response.json({ helper, upstream, pid: process.pid }); + return Response.json({ + helper, + upstream, + pid: process.pid, + ...(opts.padding ? { padding: "x".repeat(opts.padding) } : {}), + }); + } + if (url.pathname === "/__fixture/env") { + return Response.json(Object.keys(process.env).sort()); } return new Response(`served:${upstream}`); }, @@ -42,6 +54,18 @@ switch (mode) { case "identity": serveIdentity(); break; + case "stubborn": + process.on("SIGTERM", () => { + // Ignored on purpose: the stop path must escalate to SIGKILL. + }); + serveIdentity(); + break; + case "oversized": + serveIdentity({ padding: 8192 }); + break; + case "exit": + process.exit(3); + break; case "tcp": Bun.listen({ hostname: "127.0.0.1", diff --git a/tests/fixtures/shared-host-starter.ts b/tests/fixtures/shared-host-starter.ts index 077a9e348..9b0ee3226 100644 --- a/tests/fixtures/shared-host-starter.ts +++ b/tests/fixtures/shared-host-starter.ts @@ -1,27 +1,63 @@ /** * Independent starter client for tests/shared-host-lifecycle.test.ts. * - * argv: [pause-event] + * argv: [options-json] + * options: { pauseAt?: event, resume?: "never" | "stdin", linger?: boolean } + * * Prints `event ` for each lifecycle event and `result ` at the - * end. With a pause event it stops forever after printing that event, so the - * test can kill this parent at a precise point. + * end. With `pauseAt` it stops after printing that event: forever by default + * (so the test can kill this parent at a precise point, possibly while it + * holds the helper lock), or until a line arrives on stdin with + * `resume: "stdin"`. With `linger` it stays alive after printing the result. */ import { ensureSharedHostHelper, + type SharedHostEvent, type SharedHostHelperSpec, } from "../../src/lib/shared-host-lifecycle.ts"; -const [specText, holder, pauseAt] = process.argv.slice(2); +type StarterOptions = { + readonly pauseAt?: SharedHostEvent; + readonly resume?: "never" | "stdin"; + readonly linger?: boolean; +}; + +const [specText, holder, optionsText] = process.argv.slice(2); const spec = JSON.parse(specText ?? "{}") as SharedHostHelperSpec; +const options = JSON.parse(optionsText ?? "{}") as StarterOptions; + +async function waitForStdinLine(): Promise { + const reader = Bun.stdin.stream().getReader(); + const decoder = new TextDecoder(); + try { + while (true) { + const { done, value } = await reader.read(); + if (done || decoder.decode(value, { stream: true }).includes("\n")) { + return; + } + } + } finally { + reader.releaseLock(); + } +} + const result = await ensureSharedHostHelper({ spec, holder: holder ?? "", observe: async (event) => { console.log(`event ${event}`); - if (event === pauseAt) { - // An interval keeps the event loop alive until the test kills us. - await new Promise(() => setInterval(() => undefined, 60_000)); + if (event !== options.pauseAt) { + return; + } + if (options.resume === "stdin") { + await waitForStdinLine(); + return; } + // An interval keeps the event loop alive until the test kills us. + await new Promise(() => setInterval(() => undefined, 60_000)); }, }); console.log(`result ${JSON.stringify(result)}`); +if (options.linger) { + setInterval(() => undefined, 60_000); +} diff --git a/tests/shared-host-lifecycle.test.ts b/tests/shared-host-lifecycle.test.ts index 5d6896cd7..87c44a907 100644 --- a/tests/shared-host-lifecycle.test.ts +++ b/tests/shared-host-lifecycle.test.ts @@ -1,5 +1,6 @@ import { afterEach, expect, test } from "bun:test"; import { + chmod, mkdir, mkdtemp, readdir, @@ -14,6 +15,7 @@ import { selectProcessTree, signalVerifiedProcessTree, } from "../src/lib/process-tree.ts"; +import { withProjectsRegistryLock } from "../src/lib/projects-registry-lock.ts"; import { ensureSharedHostHelper, parseSharedHostClaim, @@ -37,7 +39,7 @@ const TIMEOUT = 30_000; type Foreign = Bun.Subprocess<"ignore", "pipe", "ignore">; const foreign: Foreign[] = []; -const starters: Bun.Subprocess<"ignore", "pipe", "ignore">[] = []; +const starters: Bun.Subprocess<"pipe", "pipe", "ignore">[] = []; const claimedHelpers: { pid: number; birth: string }[] = []; const roots: string[] = []; @@ -107,7 +109,15 @@ function specFor(opts: { readonly root: string; readonly port: number; readonly upstream?: string; - readonly mode?: "identity" | "placeholder" | "parent"; + readonly mode?: + | "identity" + | "placeholder" + | "parent" + | "stubborn" + | "oversized" + | "exit"; + readonly backoff?: SharedHostHelperSpec["backoff"]; + readonly probeTimeoutMs?: number; readonly deadlineMs?: number; }): SharedHostHelperSpec { const upstream = opts.upstream ?? "https://issuer.branch-a.test"; @@ -129,16 +139,39 @@ function specFor(opts: { cwd: opts.root, env: childEnv(opts.root), deadlineMs: opts.deadlineMs ?? 10_000, - backoff: { initialMs: 25, maxMs: 200 }, - probeTimeoutMs: 500, + backoff: opts.backoff ?? { initialMs: 25, maxMs: 200 }, + probeTimeoutMs: opts.probeTimeoutMs ?? 500, }; } +/** + * Reads fixture stdout continuously, so `lines` is current even while the + * test is not waiting (a negative check can inspect what was printed). + */ function lineReader(stream: ReadableStream) { const reader = stream.getReader(); const decoder = new TextDecoder(); const lines: string[] = []; let buffer = ""; + let closed = false; + const pump = (async () => { + try { + while (true) { + const { done, value } = await reader.read(); + if (done) { + break; + } + buffer += decoder.decode(value, { stream: true }); + const parts = buffer.split("\n"); + buffer = parts.pop() ?? ""; + lines.push(...parts); + } + } catch { + // The fixture was killed; keep what was read. + } finally { + closed = true; + } + })(); return { lines, async waitFor(match: (line: string) => boolean): Promise { @@ -148,14 +181,10 @@ function lineReader(stream: ReadableStream) { if (found !== undefined) { return found; } - const { done, value } = await reader.read(); - if (done) { + if (closed) { break; } - buffer += decoder.decode(value, { stream: true }); - const parts = buffer.split("\n"); - buffer = parts.pop() ?? ""; - lines.push(...parts); + await Promise.race([pump, Bun.sleep(10)]); } const found = lines.find(match); if (found === undefined) { @@ -167,7 +196,7 @@ function lineReader(stream: ReadableStream) { } async function startForeign(opts: { - readonly mode: "identity" | "tcp" | "placeholder"; + readonly mode: "identity" | "tcp" | "placeholder" | "oversized"; readonly port: number; readonly helper?: string; readonly upstream?: string; @@ -198,6 +227,12 @@ function startStarter(opts: { readonly spec: SharedHostHelperSpec; readonly holder: string; readonly pauseAt?: SharedHostEvent; + /** Resume a paused starter with {@link resumeStarter} instead of never. */ + readonly resumable?: boolean; + /** Keep the starter process alive after it prints its result. */ + readonly linger?: boolean; + /** The starter client's own time zone. */ + readonly tz?: string; }) { const proc = Bun.spawn( [ @@ -205,11 +240,18 @@ function startStarter(opts: { STARTER, JSON.stringify(opts.spec), opts.holder, - ...(opts.pauseAt ? [opts.pauseAt] : []), + JSON.stringify({ + pauseAt: opts.pauseAt, + resume: opts.resumable ? "stdin" : "never", + linger: opts.linger === true, + }), ], { - env: childEnv(opts.spec.cwd), - stdin: "ignore", + env: { + ...childEnv(opts.spec.cwd), + ...(opts.tz ? { TZ: opts.tz } : {}), + }, + stdin: "pipe", stdout: "pipe", stderr: "ignore", } @@ -218,6 +260,11 @@ function startStarter(opts: { return { proc, output: lineReader(proc.stdout) }; } +async function resumeStarter(starter: ReturnType) { + starter.proc.stdin.write("resume\n"); + await starter.proc.stdin.flush(); +} + async function starterResult(starter: ReturnType) { const line = await starter.output.waitFor((entry) => entry.startsWith("result ") @@ -435,8 +482,20 @@ test( async () => { const root = await scratch(); const spec = specFor({ root, port: freePort() }); - const first = startStarter({ spec, holder: "branch-a" }); + // Barrier: the first starter holds the lock until it is resumed, and the + // second is already contending for it, so both runs overlap for certain. + const first = startStarter({ + spec, + holder: "branch-a", + pauseAt: "lock-acquired", + resumable: true, + }); + await first.output.waitFor((line) => line === "event lock-acquired"); const second = startStarter({ spec, holder: "branch-b" }); + await second.output.waitFor((line) => line === "event lock-waiting"); + await Bun.sleep(300); + expect(second.output.lines).not.toContain("event lock-acquired"); + await resumeStarter(first); const results = await Promise.all([ starterResult(first), starterResult(second), @@ -643,7 +702,7 @@ test( ); test( - "a reused owned claim is not ready when a different process answers its identity", + "a reused ready claim is not ready when a different process answers its identity, and neither process is signalled", async () => { const root = await scratch(); const spec = specFor({ root, port: freePort(), deadlineMs: 2000 }); @@ -666,7 +725,7 @@ test( helper: spec.name, port: spec.port, ownership: "owned", - phase: "starting", + phase: "ready", pid: claimedProcess.pid, birth: identity?.birth ?? "", holders: ["branch-a"], @@ -914,3 +973,536 @@ test("claim parser accepts only the exact v1 shapes", () => { } expect(parseSharedHostClaim("not json")).toBeNull(); }); + +async function writeClaimFile( + spec: SharedHostHelperSpec, + value: unknown, + opts: { readonly mode?: number; readonly padding?: number } = {} +) { + await mkdir(spec.stateDir, { recursive: true, mode: 0o700 }); + const path = join(spec.stateDir, `${spec.name}.claim.json`); + const text = `${JSON.stringify(value)}${" ".repeat(opts.padding ?? 0)}`; + await writeFile(path, text, { mode: 0o600 }); + await chmod(path, opts.mode ?? 0o600); + return { path, text }; +} + +async function processGroupOf(pid: number): Promise { + const proc = Bun.spawn(["ps", "-o", "pgid=", "-p", String(pid)], { + env: { PATH: process.env.PATH ?? "/usr/bin:/bin", LC_ALL: "C" }, + stdin: "ignore", + stdout: "pipe", + stderr: "ignore", + }); + const text = await new Response(proc.stdout).text(); + await proc.exited; + return Number(text.trim()); +} + +test( + "clients in different time zones agree on an owned helper's birth: it is reused, stays owned and is stopped by its last holder", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + // POSIX zone strings need no zoneinfo, so the zones differ on minimal hosts too. + const first = startStarter({ spec, holder: "branch-a", tz: "UTC0" }); + expect(await starterResult(first)).toMatchObject({ + state: "ready", + ownership: "owned", + spawned: true, + }); + const owned = await trackClaimedHelper(spec); + const second = startStarter({ spec, holder: "branch-b", tz: "HST10" }); + expect(await starterResult(second)).toMatchObject({ + state: "ready", + ownership: "owned", + spawned: false, + pid: owned.pid, + }); + expect(second.output.lines).toContain("event claim-reused"); + expect(second.output.lines).not.toContain("event claim-stale"); + expect(second.output.lines).not.toContain("event adopted"); + expect(await readClaim(spec)).toMatchObject({ + ownership: "owned", + pid: owned.pid, + birth: owned.birth, + holders: ["branch-a", "branch-b"], + }); + expect(await releaseSharedHostHelper({ spec, holder: "branch-a" })).toEqual( + { state: "retained" } + ); + expect(await releaseSharedHostHelper({ spec, holder: "branch-b" })).toEqual( + { state: "stopped" } + ); + expect(await isAlive(owned)).toBe(false); + }, + TIMEOUT +); + +test( + "a starter lost after opening the gate leaves a starting helper that the next starter stops when it never becomes ready", + async () => { + const root = await scratch(); + const spec = specFor({ + root, + port: freePort(), + mode: "placeholder", + deadlineMs: 1500, + }); + const starter = startStarter({ + spec, + holder: "branch-a", + pauseAt: "gate-opened", + }); + await starter.output.waitFor((line) => line === "event gate-opened"); + const abandoned = await trackClaimedHelper(spec); + starter.proc.kill("SIGKILL"); + await starter.proc.exited; + expect(await readClaim(spec)).toMatchObject({ phase: "starting" }); + expect(await isAlive(abandoned)).toBe(true); + + const seen = recorder(); + const next = await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }); + expect(next).toMatchObject({ state: "failed", reason: "deadline" }); + expect(seen.events).toContain("claim-reused"); + expect(seen.events).toContain("stopped-after-failure"); + expect(await waitGone(abandoned)).toBe(true); + expect(await readClaim(spec)).toBeNull(); + + // The helper port is free again for a fresh, working start. + const fresh = await ensureSharedHostHelper({ + spec: specFor({ root, port: spec.port }), + holder: "branch-c", + }); + await trackClaimedHelper(spec); + expect(fresh).toMatchObject({ state: "ready", spawned: true }); + }, + TIMEOUT +); + +test( + "a lock wait that uses up the deadline reports deadline, never a foreign listener, and adopts nothing", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort(), deadlineMs: 600 }); + const healthy = await startForeign({ + mode: "identity", + port: spec.port, + home: root, + }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: async (event) => { + seen.observe(event); + if (event === "lock-acquired") { + // Stands in for lock contention that consumed the whole deadline. + await Bun.sleep(700); + } + }, + }); + expect(result).toMatchObject({ state: "failed", reason: "deadline" }); + expect(seen.events).not.toContain("adopted"); + expect(await readClaim(spec)).toBeNull(); + expect(healthy.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "a port probe cut short by the deadline reports deadline; a full-length silent probe is a foreign listener", + async () => { + const root = await scratch(); + const port = freePort(); + const silent = await startForeign({ mode: "tcp", port, home: root }); + const cut = await ensureSharedHostHelper({ + spec: specFor({ root, port, deadlineMs: 600, probeTimeoutMs: 5000 }), + holder: "branch-a", + }); + expect(cut).toMatchObject({ state: "failed", reason: "deadline" }); + const full = await ensureSharedHostHelper({ + spec: specFor({ root, port, deadlineMs: 5000, probeTimeoutMs: 300 }), + holder: "branch-a", + }); + expect(full).toMatchObject({ state: "failed", reason: "foreign-listener" }); + expect(silent.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "a stale owned claim whose PID still answers as the helper is kept, not adopted, and nothing is signalled", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const answering = await startForeign({ + mode: "identity", + port: spec.port, + home: root, + }); + const { path, text } = await writeClaimFile(spec, { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "ready", + pid: answering.pid, + birth: "Thu Jan 1 00:00:00 1970", + holders: ["branch-a"], + }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }); + expect(result).toMatchObject({ state: "failed", reason: "claim-invalid" }); + expect(seen.events).toContain("claim-stale"); + expect(seen.events).not.toContain("adopted"); + expect(await readFile(path, "utf8")).toBe(text); + await Bun.sleep(200); + expect(answering.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "a claim for another port, a group-readable claim and an oversized claim fail closed", + async () => { + const root = await scratch(); + const bystander = await startForeign({ + mode: "placeholder", + port: 0, + home: root, + }); + const identity = await identityOf(bystander.pid); + const cases = [ + { label: "other port", portOffset: 1, ownership: "owned" }, + { label: "group-readable", mode: 0o640, ownership: "owned" }, + { label: "oversized", padding: 9000, ownership: "adopted" }, + ] as const; + for (const entry of cases) { + const spec = specFor({ root: await scratch(), port: freePort() }); + const portOffset = "portOffset" in entry ? entry.portOffset : 0; + const claim = + entry.ownership === "owned" + ? { + version: 1, + helper: spec.name, + port: spec.port + portOffset, + ownership: "owned", + phase: "ready", + pid: bystander.pid, + birth: identity?.birth ?? "", + holders: ["branch-a"], + } + : { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "adopted", + holders: ["branch-a"], + }; + const { path, text } = await writeClaimFile(spec, claim, { + mode: "mode" in entry ? entry.mode : undefined, + padding: "padding" in entry ? entry.padding : undefined, + }); + const seen = recorder(); + expect({ + label: entry.label, + result: await ensureSharedHostHelper({ + spec, + holder: "branch-b", + observe: seen.observe, + }), + }).toMatchObject({ + label: entry.label, + result: { state: "failed", reason: "claim-invalid" }, + }); + expect(seen.events).not.toContain("spawned"); + expect(seen.events).not.toContain("claim-reused"); + if (portOffset === 0) { + expect( + await releaseSharedHostHelper({ spec, holder: "branch-a" }) + ).toMatchObject({ state: "failed", reason: "claim-invalid" }); + } + expect(await readFile(path, "utf8")).toBe(text); + } + await Bun.sleep(200); + expect(bystander.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "a helper that exits during startup reports child-exited and leaves no claim", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort(), mode: "exit" }); + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }); + expect(result).toMatchObject({ state: "failed", reason: "child-exited" }); + expect(seen.events).toContain("gate-opened"); + expect(seen.events).toContain("stopped-after-failure"); + expect(await readClaim(spec)).toBeNull(); + }, + TIMEOUT +); + +test( + "cancellation after the claim and before the gate never runs the helper", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const controller = new AbortController(); + let gated: { pid: number; birth: string } | null = null; + const seen = recorder(); + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + signal: controller.signal, + observe: async (event) => { + seen.observe(event); + if (event === "claimed") { + gated = await trackClaimedHelper(spec); + controller.abort(); + } + }, + }); + expect(result).toMatchObject({ state: "failed", reason: "cancelled" }); + expect(seen.events).not.toContain("gate-opened"); + expect(await waitGone(gated ?? { pid: 0, birth: "" })).toBe(true); + expect(await readClaim(spec)).toBeNull(); + expect( + await fetch(`http://127.0.0.1:${spec.port}/`).catch(() => null) + ).toBeNull(); + }, + TIMEOUT +); + +test( + "a helper that ignores SIGTERM is stopped by SIGKILL after the grace period", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort(), mode: "stubborn" }); + expect( + await ensureSharedHostHelper({ spec, holder: "branch-a" }) + ).toMatchObject({ state: "ready", ownership: "owned" }); + const owned = await trackClaimedHelper(spec); + expect( + await releaseSharedHostHelper({ spec, holder: "branch-a", graceMs: 200 }) + ).toEqual({ state: "stopped" }); + expect(await isAlive(owned)).toBe(false); + expect(await readClaim(spec)).toBeNull(); + }, + TIMEOUT +); + +test( + "an owned helper runs in its own session with only the caller's environment", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const saved = process.env.HACK_SHARED_HOST_RUNNER_ONLY; + process.env.HACK_SHARED_HOST_RUNNER_ONLY = "runner-value"; + try { + expect( + await ensureSharedHostHelper({ spec, holder: "branch-a" }) + ).toMatchObject({ state: "ready", ownership: "owned" }); + } finally { + restoreEnv("HACK_SHARED_HOST_RUNNER_ONLY", saved); + } + const owned = await trackClaimedHelper(spec); + const keys = (await ( + await fetch(`http://127.0.0.1:${spec.port}/__fixture/env`) + ).json()) as string[]; + expect(keys).toContain("HOME"); + expect(keys).not.toContain("HACK_SHARED_HOST_RUNNER_ONLY"); + const helperGroup = await processGroupOf(owned.pid); + expect(helperGroup).toBe(owned.pid); + expect(helperGroup).not.toBe(await processGroupOf(process.pid)); + }, + TIMEOUT +); + +test( + "cancellation interrupts the readiness backoff sleep", + async () => { + const root = await scratch(); + const spec = specFor({ + root, + port: freePort(), + mode: "placeholder", + deadlineMs: 20_000, + backoff: { initialMs: 5000, maxMs: 5000 }, + }); + const controller = new AbortController(); + let abortedAt = 0; + let owned: { pid: number; birth: string } | null = null; + const result = await ensureSharedHostHelper({ + spec, + holder: "branch-a", + signal: controller.signal, + observe: async (event) => { + if (event === "claimed") { + owned = await trackClaimedHelper(spec); + } + if (event === "probe-retry") { + setTimeout(() => { + abortedAt = performance.now(); + controller.abort(); + }, 50); + } + }, + }); + expect(result).toMatchObject({ state: "failed", reason: "cancelled" }); + expect(abortedAt).toBeGreaterThan(0); + expect(performance.now() - abortedAt).toBeLessThan(2500); + expect(await isAlive(owned ?? { pid: 0, birth: "" })).toBe(false); + }, + TIMEOUT +); + +test( + "release waits for the helper lock and fails closed when it is held past the timeout", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + expect( + await ensureSharedHostHelper({ spec, holder: "branch-a" }) + ).toMatchObject({ state: "ready", ownership: "owned" }); + const owned = await trackClaimedHelper(spec); + const claimFile = join(spec.stateDir, `${spec.name}.claim.json`); + const before = await readFile(claimFile, "utf8"); + const blocked = await withProjectsRegistryLock({ + lockPath: join(spec.stateDir, `${spec.name}.lock`), + label: "test holder", + run: () => + releaseSharedHostHelper({ spec, holder: "branch-a", timeoutMs: 300 }), + }); + expect(blocked).toMatchObject({ + state: "failed", + reason: "lock-unavailable", + }); + expect(await isAlive(owned)).toBe(true); + expect(await readFile(claimFile, "utf8")).toBe(before); + expect(await releaseSharedHostHelper({ spec, holder: "branch-a" })).toEqual( + { state: "stopped" } + ); + }, + TIMEOUT +); + +test( + "an identity answer larger than 4 KiB is not identity: the listener is refused and preserved", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const oversized = await startForeign({ + mode: "oversized", + port: spec.port, + home: root, + }); + const seen = recorder(); + expect( + await ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: seen.observe, + }) + ).toMatchObject({ state: "failed", reason: "foreign-listener" }); + expect(seen.events).not.toContain("adopted"); + expect(await readClaim(spec)).toBeNull(); + await Bun.sleep(200); + expect(oversized.exitCode).toBeNull(); + }, + TIMEOUT +); + +test( + "a stop that cannot be confirmed keeps the claim so the process stays recorded", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + // The starter stays alive and is then frozen, so the helper it spawned + // cannot be reaped: after SIGKILL it remains a zombie in the census. + const starter = startStarter({ spec, holder: "branch-a", linger: true }); + expect(await starterResult(starter)).toMatchObject({ + state: "ready", + ownership: "owned", + }); + const owned = await trackClaimedHelper(spec); + starter.proc.kill("SIGSTOP"); + try { + expect( + await releaseSharedHostHelper({ + spec, + holder: "branch-a", + graceMs: 100, + }) + ).toEqual({ state: "stop-unconfirmed" }); + expect(await readClaim(spec)).toMatchObject({ + ownership: "owned", + pid: owned.pid, + birth: owned.birth, + holders: [], + }); + } finally { + starter.proc.kill("SIGCONT"); + } + }, + TIMEOUT +); + +test( + "an observer error is rethrown after the waiting gate is closed and its claim removed", + async () => { + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const children = async () => + (await readProcessIdentities()) + .filter((row) => row.parentPid === process.pid) + .map((row) => ({ pid: row.pid, birth: row.birth })); + const before = new Set((await children()).map((row) => row.pid)); + let gates: { pid: number; birth: string }[] = []; + const failure = new Error("observer failed on purpose"); + await expect( + ensureSharedHostHelper({ + spec, + holder: "branch-a", + observe: async (event) => { + if (event === "spawned") { + // Each census sees its own short-lived `ps` child; the gate is + // the new child present in two consecutive censuses. + const first = await children(); + const second = await children(); + gates = first.filter( + (row) => + !before.has(row.pid) && + second.some( + (entry) => entry.pid === row.pid && entry.birth === row.birth + ) + ); + claimedHelpers.push(...gates); + throw failure; + } + }, + }) + ).rejects.toBe(failure); + expect(gates.length).toBeGreaterThan(0); + for (const gate of gates) { + expect(await waitGone(gate)).toBe(true); + } + expect(await readClaim(spec)).toBeNull(); + }, + TIMEOUT +); From d056fc85e491e28b055b88ef050299e3f2771433 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 19 Oct 2026 12:40:35 -0400 Subject: [PATCH 3/5] fix(lifecycle): confirm helper port absence with a bind probe on every platform The hosted Linux job showed that a refused loopback fetch does not carry the macOS ECONNREFUSED code there, so every free port was misread as a foreign listener. A failed connection now never proves absence by its error code: the loopback bind probe decides (occupied is foreign-listener, free spawns). Add a listener that drops every connection to the foreign-listener fixture. Refs: HACK-1142 Co-Authored-By: Claude Opus 5.5 --- docs/reference/shared-host-lifecycle.md | 7 +++++-- src/lib/shared-host-lifecycle.ts | 14 +++++++------- tests/fixtures/shared-host-helper.ts | 16 ++++++++++++++++ tests/shared-host-lifecycle.test.ts | 3 ++- 4 files changed, 30 insertions(+), 10 deletions(-) diff --git a/docs/reference/shared-host-lifecycle.md b/docs/reference/shared-host-lifecycle.md index 601bf98c4..bd55812aa 100644 --- a/docs/reference/shared-host-lifecycle.md +++ b/docs/reference/shared-host-lifecycle.md @@ -100,7 +100,10 @@ replaced atomically, and parsed strictly (exact keys, version 1): Either way the listener keeps running and no child is spawned; - a probe whose timeout was shortened by the deadline and then timed out proves nothing about the listener: `deadline`; - - connection refused: confirm with a loopback bind probe, then spawn. + - connection failed (refused, reset or unreachable): confirm with a loopback + bind probe. An occupied port is `foreign-listener`; a free port is spawned. + The refused-connection error code differs by platform, so the error itself + never proves absence. 5. Spawn through an exec gate in a new session: `/bin/sh -c 'IFS= read -r hack_gate || exit 0; [ "$hack_gate" = open ] || exit 0; exec "$@" undefined, 60_000); break; diff --git a/tests/shared-host-lifecycle.test.ts b/tests/shared-host-lifecycle.test.ts index 87c44a907..960850cd8 100644 --- a/tests/shared-host-lifecycle.test.ts +++ b/tests/shared-host-lifecycle.test.ts @@ -196,7 +196,7 @@ function lineReader(stream: ReadableStream) { } async function startForeign(opts: { - readonly mode: "identity" | "tcp" | "placeholder" | "oversized"; + readonly mode: "identity" | "tcp" | "reset" | "placeholder" | "oversized"; readonly port: number; readonly helper?: string; readonly upstream?: string; @@ -450,6 +450,7 @@ test( async () => { for (const fixture of [ { mode: "tcp" as const }, + { mode: "reset" as const }, { mode: "identity" as const, helper: "unrelated-dev-server" }, ]) { const root = await scratch(); From 73b09904651d4358812f7c3f7b89fce36f8602b3 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 19 Oct 2026 13:44:16 -0400 Subject: [PATCH 4/5] fix(lifecycle): count an unreaped zombie helper as stopped The "stop that cannot be confirmed" test froze its starter with Subprocess.kill("SIGSTOP"). On macOS, the pinned Bun 1.4.2 delivers SIGCONT for that call, so the starter kept running, reaped the helper after TERM, and release reported `stopped`. CI uses the pinned Bun, so the test failed there; local runs passed only on an unpinned Bun 1.4.3. The scenario also showed a product defect. The census counted a zombie (an exited, unreaped process) as live. A stop that left the helper a zombie reported `stop-unconfirmed`. A `ready` claim whose helper crashed under a suspended starter looked live, so other clients could not restart the helper until that starter resumed. A zombie runs no code and holds no listener, and its PID cannot be reused until it is reaped. The census now reads `ps` state and treats zombies as exited. The unconfirmed-stop test now injects the census and signal boundary through a narrow `processes` seam on release, because no real process can portably survive SIGKILL. A separate real-process test freezes the starter with process.kill and asserts that the helper is an unreaped zombie and that release reports `stopped`. Refs HACK-1142 Co-Authored-By: Claude Opus 5.5 --- docs/reference/shared-host-lifecycle.md | 9 ++- src/lib/process-tree.ts | 16 ++++- src/lib/shared-host-lifecycle.ts | 64 +++++++++++++++--- tests/shared-host-lifecycle.test.ts | 89 ++++++++++++++++++++++--- 4 files changed, 152 insertions(+), 26 deletions(-) diff --git a/docs/reference/shared-host-lifecycle.md b/docs/reference/shared-host-lifecycle.md index bd55812aa..3652190b6 100644 --- a/docs/reference/shared-host-lifecycle.md +++ b/docs/reference/shared-host-lifecycle.md @@ -63,6 +63,12 @@ replaced atomically, and parsed strictly (exact keys, version 1): - The claim is live only when the census shows that exact pair. A missing PID, or the same PID with a different birth time (PID reuse), makes the claim stale. A stale claim grants nothing. Hack discards it and never signals that PID. +- A zombie (a process that has exited but that its parent has not reaped yet, + `ps` state `Z`) counts as exited. It runs no code and holds no listener, and its + PID cannot be reused until it is reaped. So a stop that leaves the helper a zombie + is `stopped`, and a claim whose helper is a zombie is stale. Otherwise a helper + that crashed under a suspended starter would block every restart until that + starter resumed. - One exception: if a stale owned claim's PID still answers the identity endpoint as the expected helper, ownership is uncertain. Hack keeps the claim and fails with `claim-invalid`. It neither adopts the helper (which would drop the owned @@ -205,7 +211,8 @@ Linux pass is a receipt only once that hosted job has run it. | lost parent after ready | The helper stays claimed; another client reuses it; the release by both holders stops it. | | owned descendants | Release stops the helper and its grandchild; a bystander survives. | | SIGTERM ignored | Release escalates to SIGKILL after the grace period: `stopped`. | -| stop unconfirmed | A helper that stays in the census after KILL (a zombie under a frozen parent) gives `stop-unconfirmed` and keeps the claim. | +| stop unconfirmed | With a simulated census and signal boundary (no real process can portably survive KILL), a tree that stays in the census gets TERM then KILL, gives `stop-unconfirmed` and keeps the claim with no holders. | +| unreaped zombie | A helper whose starter is frozen with `SIGSTOP` exits on TERM and stays an unreaped zombie; release reports `stopped` and removes the claim. | | session and environment | The helper leads its own process group and does not see a runner-only variable. | | impostor identity | A live `ready` claim whose port is answered by another PID fails `foreign-listener`; neither process is signalled. | | invalid claim / no census | Malformed, other-port, group-readable and oversized claims, and an unavailable census, fail closed; nothing is spawned or signalled; the claim is kept. | diff --git a/src/lib/process-tree.ts b/src/lib/process-tree.ts index a6bb346b7..d51ba0fb7 100644 --- a/src/lib/process-tree.ts +++ b/src/lib/process-tree.ts @@ -2,8 +2,14 @@ export type ProcessIdentityRow = { readonly pid: number; readonly parentPid: number; readonly birth: string; + /** + * The process has exited but its parent has not reaped it yet (`ps` state + * `Z`). A zombie runs no code and holds no listener; its PID cannot be + * reused until it is reaped. + */ + readonly zombie: boolean; }; -const IDENTITY_ROW = /^\s*(\d+)\s+(\d+)\s+(.+?)\s*$/; +const IDENTITY_ROW = /^\s*(\d+)\s+(\d+)\s+(\S+)\s+(.+?)\s*$/; /** * Capture only process identity and lineage; never command arguments or environment. @@ -13,10 +19,13 @@ const IDENTITY_ROW = /^\s*(\d+)\s+(\d+)\s+(.+?)\s*$/; * different `TZ` values, or one host whose zone changes, would see different * births for the same live process. Births may be persisted and compared across * processes, so the zone is pinned here (POSIX `UTC0` needs no zoneinfo). + * + * Zombie rows are kept and flagged, not dropped: each caller decides whether + * an exited, unreaped process still counts for its purpose. */ export async function readProcessIdentities(): Promise { try { - const proc = Bun.spawn(["ps", "-A", "-o", "pid=,ppid=,lstart="], { + const proc = Bun.spawn(["ps", "-A", "-o", "pid=,ppid=,stat=,lstart="], { env: { ...process.env, LC_ALL: "C", TZ: "UTC0" }, stdin: "ignore", stdout: "pipe", @@ -34,7 +43,8 @@ export async function readProcessIdentities(): Promise { { pid: Number(match[1]), parentPid: Number(match[2]), - birth: (match[3] ?? "").replace(/\s+/g, " "), + birth: (match[4] ?? "").replace(/\s+/g, " "), + zombie: (match[3] ?? "").startsWith("Z"), }, ] : []; diff --git a/src/lib/shared-host-lifecycle.ts b/src/lib/shared-host-lifecycle.ts index c28eb8236..8671e2c15 100644 --- a/src/lib/shared-host-lifecycle.ts +++ b/src/lib/shared-host-lifecycle.ts @@ -31,9 +31,10 @@ import { withProjectsRegistryLock } from "./projects-registry-lock.ts"; * left running: this module never signals a process it cannot prove it owns. * - Ownership is a persisted PID plus process birth time (`ps lstart`, read in * UTC so clients with different time zones agree). A claim whose PID is - * gone or now belongs to a different process (PID reuse) is stale and is - * discarded without signalling anything. A stale claim whose PID still - * answers as the helper is uncertain: it is kept, never adopted. + * gone, an unreaped zombie, or now belongs to a different process (PID + * reuse) is stale and is discarded without signalling anything. A stale + * claim whose PID still answers as the helper is uncertain: it is kept, + * never adopted. * - A child is spawned behind an exec gate: it waits on stdin until its claim * is durable. If the starter dies first, the gate reads EOF and exits, so a * spawned helper is never left running without a claim. @@ -454,13 +455,47 @@ async function removeClaim( } } +/** + * The process census and signal boundary. Production callers omit it and get + * {@link systemProcesses}; tests replace it to simulate states real processes + * cannot produce portably, such as a process that stays in the census after + * SIGKILL. + */ +export type SharedHostProcesses = { + /** Every process row, including zombies; `[]` when `ps` failed. */ + readonly census: () => Promise; + /** Signal the members of a captured tree that still carry their birth. */ + readonly signalTree: ( + tree: readonly ProcessIdentityRow[], + signal: NodeJS.Signals + ) => Promise; +}; + +const systemProcesses: SharedHostProcesses = { + census: readProcessIdentities, + signalTree: signalVerifiedProcessTree, +}; + /** * A census is usable only when it observed this process; an empty or failed * `ps` must never be read as "every claimed process is gone". + * + * Zombies are left out: an exited process that its parent has not reaped yet + * runs no code, holds no listener or open file, and cannot be stopped any + * further. Its PID cannot be reused until it is reaped, so treating it as + * exited can never confuse it with another process. Counting it as live would + * report a finished stop as unconfirmed, and would make a `ready` claim whose + * helper crashed under a suspended (for example Ctrl-Z) starter look live, so + * no other client could restart that helper until the starter resumed. */ -async function readCensus(): Promise { - const rows = await readProcessIdentities(); - return rows.some((row) => row.pid === process.pid) ? rows : null; +async function readCensus( + processes: SharedHostProcesses = systemProcesses +): Promise { + const rows = await processes.census(); + if (!rows.some((row) => row.pid === process.pid)) { + return null; + } + return rows.filter((row) => !row.zombie); } function isLive( @@ -581,14 +616,17 @@ function classifyIdentity( /** * Stop a birth-verified process tree: TERM, bounded grace, then KILL for the * members that still carry their captured birth time. Nothing is signalled - * when the root no longer matches its claim. + * when the root no longer matches its claim. A member that has exited counts + * as stopped even before its parent reaps it (see {@link readCensus}). */ export async function stopVerifiedSharedHostProcess(opts: { readonly pid: number; readonly birth: string; readonly graceMs: number; + readonly processes?: SharedHostProcesses; }): Promise<"stopped" | "absent" | "unconfirmed" | "census-unavailable"> { - const census = await readCensus(); + const processes = opts.processes ?? systemProcesses; + const census = await readCensus(processes); if (census === null) { return "census-unavailable"; } @@ -597,13 +635,13 @@ export async function stopVerifiedSharedHostProcess(opts: { } const tree = selectProcessTree(census, opts.pid); const remaining = async (): Promise => { - const live = await readCensus(); + const live = await readCensus(processes); if (live === null) { return null; } return tree.filter((row) => isLive(live, row)); }; - await signalVerifiedProcessTree(tree, "SIGTERM"); + await processes.signalTree(tree, "SIGTERM"); const termDeadline = performance.now() + opts.graceMs; let left = await remaining(); while (left !== null && left.length > 0 && performance.now() < termDeadline) { @@ -613,7 +651,7 @@ export async function stopVerifiedSharedHostProcess(opts: { if (left !== null && left.length === 0) { return "stopped"; } - await signalVerifiedProcessTree(left ?? tree, "SIGKILL"); + await processes.signalTree(left ?? tree, "SIGKILL"); const killDeadline = performance.now() + 1000; left = await remaining(); while (left !== null && left.length > 0 && performance.now() < killDeadline) { @@ -1122,6 +1160,8 @@ export async function releaseSharedHostHelper(opts: { readonly graceMs?: number; readonly timeoutMs?: number; readonly signal?: AbortSignal; + /** Test seam for the census and signals; production callers omit it. */ + readonly processes?: SharedHostProcesses; }): Promise { if (!NAME.test(opts.spec.name)) { throw new Error("Invalid shared host helper specification"); @@ -1149,6 +1189,7 @@ async function releaseLocked(opts: { readonly spec: Pick; readonly holder: string; readonly graceMs?: number; + readonly processes?: SharedHostProcesses; }): Promise { const read = await readClaim(opts.spec); if (read.kind === "absent") { @@ -1171,6 +1212,7 @@ async function releaseLocked(opts: { pid: claim.pid, birth: claim.birth, graceMs: opts.graceMs ?? 2000, + processes: opts.processes, }); switch (stopped) { case "census-unavailable": diff --git a/tests/shared-host-lifecycle.test.ts b/tests/shared-host-lifecycle.test.ts index 960850cd8..f6b684783 100644 --- a/tests/shared-host-lifecycle.test.ts +++ b/tests/shared-host-lifecycle.test.ts @@ -22,6 +22,7 @@ import { releaseSharedHostHelper, type SharedHostEvent, type SharedHostHelperSpec, + type SharedHostProcesses, } from "../src/lib/shared-host-lifecycle.ts"; import { restoreEnv } from "./helpers/env.ts"; @@ -1431,35 +1432,101 @@ test( test( "a stop that cannot be confirmed keeps the claim so the process stays recorded", + async () => { + // No real process can portably survive SIGKILL, so the census and signal + // boundary is simulated: the claimed helper and its child stay in every + // census whatever is sent to them. + const root = await scratch(); + const spec = specFor({ root, port: freePort() }); + const birth = "Thu Jan 1 00:00:00 1970"; + const helper = { pid: 4242, parentPid: 1, birth, zombie: false }; + const child = { pid: 4243, parentPid: 4242, birth, zombie: false }; + const self = { + pid: process.pid, + parentPid: process.ppid, + birth, + zombie: false, + }; + await writeClaimFile(spec, { + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "ready", + pid: helper.pid, + birth, + holders: ["branch-a"], + }); + const signals: [NodeJS.Signals, number[]][] = []; + const processes: SharedHostProcesses = { + census: async () => [self, helper, child], + signalTree: async (tree, signal) => { + signals.push([signal, tree.map((row) => row.pid)]); + }, + }; + expect( + await releaseSharedHostHelper({ + spec, + holder: "branch-a", + graceMs: 100, + processes, + }) + ).toEqual({ state: "stop-unconfirmed" }); + // TERM, then KILL, each to the whole captured tree and nothing else. + expect(signals).toEqual([ + ["SIGTERM", [helper.pid, child.pid]], + ["SIGKILL", [helper.pid, child.pid]], + ]); + // The claim keeps the exact process recorded, now with no holders. + expect(await readClaim(spec)).toEqual({ + version: 1, + helper: spec.name, + port: spec.port, + ownership: "owned", + phase: "ready", + pid: helper.pid, + birth, + holders: [], + }); + }, + TIMEOUT +); + +test( + "an exited helper its parent has not reaped yet is stopped, not unconfirmed", async () => { const root = await scratch(); const spec = specFor({ root, port: freePort() }); // The starter stays alive and is then frozen, so the helper it spawned - // cannot be reaped: after SIGKILL it remains a zombie in the census. + // cannot be reaped: once it exits on SIGTERM it is a zombie in the census. const starter = startStarter({ spec, holder: "branch-a", linger: true }); expect(await starterResult(starter)).toMatchObject({ state: "ready", ownership: "owned", }); const owned = await trackClaimedHelper(spec); - starter.proc.kill("SIGSTOP"); + // process.kill, not Subprocess.kill: Bun 1.4.2 on macOS delivers SIGCONT + // for Subprocess.kill("SIGSTOP"), which would leave the starter running. + process.kill(starter.proc.pid, "SIGSTOP"); try { expect( await releaseSharedHostHelper({ spec, holder: "branch-a", - graceMs: 100, + graceMs: 1000, }) - ).toEqual({ state: "stop-unconfirmed" }); - expect(await readClaim(spec)).toMatchObject({ - ownership: "owned", - pid: owned.pid, - birth: owned.birth, - holders: [], - }); + ).toEqual({ state: "stopped" }); + expect(await readClaim(spec)).toBeNull(); + // The helper really is an unreaped zombie, so this run exercised the + // zombie path rather than an ordinary reaped exit. + const row = (await readProcessIdentities()).find( + (entry) => entry.pid === owned.pid && entry.birth === owned.birth + ); + expect(row).toMatchObject({ parentPid: starter.proc.pid, zombie: true }); } finally { - starter.proc.kill("SIGCONT"); + process.kill(starter.proc.pid, "SIGCONT"); } + expect(await waitGone(owned)).toBe(true); }, TIMEOUT ); From e2dab3a82e9f02831fc1fb00fe0dc66faa00faf1 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Mon, 19 Oct 2026 14:20:04 -0400 Subject: [PATCH 5/5] docs(lifecycle): require one shared stateDir and name per helper port Clients of a helper port coordinate only through the claim and lock under `/`. The previous JSDoc called stateDir a "caller-owned private directory", which suggested one directory per caller. A client with its own directory finds no claim. It adopts the running owned helper, and the owner's last release then stops a helper that client still holds. Its starts are also not serialized with the owner's. The spec JSDoc, the module contract and the reference doc now state that every client of one port must pass the same stateDir and name, derived from the helper and port rather than the branch. Refs HACK-1142 Co-Authored-By: Claude Opus 5.5 --- docs/reference/shared-host-lifecycle.md | 11 +++++++++++ src/lib/shared-host-lifecycle.ts | 24 ++++++++++++++++++++---- 2 files changed, 31 insertions(+), 4 deletions(-) diff --git a/docs/reference/shared-host-lifecycle.md b/docs/reference/shared-host-lifecycle.md index 3652190b6..e07b83b03 100644 --- a/docs/reference/shared-host-lifecycle.md +++ b/docs/reference/shared-host-lifecycle.md @@ -56,6 +56,17 @@ replaced atomically, and parsed strictly (exact keys, version 1): {"version":1,"helper":"issuer-bridge","port":4100,"ownership":"adopted","holders":["branch-a"]} ``` +The lock is `/.lock`. Every branch or client that uses one helper +port must pass the same `stateDir` and `name`: the state directory belongs to the +helper, not to each caller. The claim and lock are the only coordination between +clients, and Hack cannot see a claim in another directory. A client with a +different `stateDir` or `name` takes a different lock and finds no claim. If the +running owned helper answers its identity check, that client adopts it as if +someone else had started it. The owner's last release then stops the helper while +the other client still holds it, and the two clients' starts are not serialized. +Derive both values from the helper and its port, never from the branch or +checkout. + - Ownership is the pair `pid` + `birth`. `birth` is the process start time from `ps lstart`, always read with `TZ=UTC0` and `LC_ALL=C`. `ps` prints `lstart` in local time, so without a fixed zone two clients with different `TZ` values, or a diff --git a/src/lib/shared-host-lifecycle.ts b/src/lib/shared-host-lifecycle.ts index 8671e2c15..20a47d7e6 100644 --- a/src/lib/shared-host-lifecycle.ts +++ b/src/lib/shared-host-lifecycle.ts @@ -24,8 +24,10 @@ import { withProjectsRegistryLock } from "./projects-registry-lock.ts"; * - Ownership is `owned` (Hack started it and holds an exact process claim), * `adopted` (an identity-verified helper someone else started) or `disabled` * (no lock, no claim, no child). Phases are `starting`, `ready` and `failed`. - * - Starters for one helper are serialized by an exclusive lock in the - * caller's private state directory; only one of them may spawn. + * - Starters for one helper are serialized by an exclusive lock next to its + * claim; only one of them may spawn. Every client of one helper port must + * therefore use the same `stateDir` and `name` (see + * {@link SharedHostHelperSpec.stateDir}). * - A listener is only ever adopted after its identity endpoint names the * expected helper and upstream. Anything else on the port is refused and * left running: this module never signals a process it cannot prove it owns. @@ -102,11 +104,25 @@ export type SharedHostIdentity = { }; export type SharedHostHelperSpec = { - /** Claim and lock name; `[A-Za-z0-9][A-Za-z0-9_.-]{0,127}`. */ + /** + * Claim and lock name; `[A-Za-z0-9][A-Za-z0-9_.-]{0,127}`. Every client of + * one helper port must use the same name (see {@link stateDir}). + */ readonly name: string; /** Disabled helpers take no lock, write no claim and start no child. */ readonly enabled: boolean; - /** Caller-owned private directory for the claim and lock. */ + /** + * Private (owner-only) directory that holds this helper's claim and lock. + * + * It is shared, not per caller: every branch or client that uses the helper + * on `port` must pass the same `stateDir` and `name`. The claim and lock are + * the only coordination between clients, and nothing here can see a claim in + * another directory. A client with a different `stateDir` or `name` takes a + * different lock and finds no claim. If the running owned helper answers its + * identity check, that client adopts it, so the owner's last release stops a + * helper this client still holds, and their starts are not serialized. + * Derive both values from the helper and its port, never from the branch. + */ readonly stateDir: string; /** Loopback port the helper serves. */ readonly port: number;