From 061638b667a5f4c9cb76ac6df8c8f14a613c04b0 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 23:15:20 -0400 Subject: [PATCH 1/7] test: restore leaked listeners at file boundaries and pin guard-only leak fixes The global-state guard now puts the guarded process listeners back at each file boundary after reporting the change, so a finished file's leftover handler is no longer run or consumed by a later test's process.emit and charged to that test (HACK-1416). Only the difference is edited, so a kept once listener stays a once listener. The Loki follow also gets a test that removes its SIGINT listener through a real process.emit("SIGINT") inside a fixture that owns every SIGINT listener. The routing refused-teardown case restores only the spies its fixture installed instead of the global mock.restore(), and runs through the file's fixture gate (HACK-1388). The gate and the unknown-lifetime teardown are now shared by the gated runner and that case, which pins the runner's HACK_HOME release. File generation's PATH restore under an unknown lifetime and the protected-files settlement veto's process.kill restore get deterministic assertions. Refs HACK-1416, HACK-1388 Co-Authored-By: Claude Opus 5.5 --- .ai/skills/hack-repo-verify/SKILL.md | 8 +- tests/global-state-guard.test.ts | 81 +++++++- tests/helpers/global-state-guard-preload.ts | 16 +- tests/helpers/global-state-guard.ts | 76 +++++++- tests/helpers/owned-process-listeners.ts | 32 +++ tests/helpers/retained-routing-adoption.ts | 17 +- tests/loki-short-lived-logs.test.ts | 23 +++ ...e-compose-adoption-file-generation.test.ts | 33 +++- ...ompose-adoption-routing-generation.test.ts | 184 +++++++++++------- ...ive-config-protected-files-fixture.test.ts | 2 + 10 files changed, 387 insertions(+), 85 deletions(-) create mode 100644 tests/helpers/owned-process-listeners.ts diff --git a/.ai/skills/hack-repo-verify/SKILL.md b/.ai/skills/hack-repo-verify/SKILL.md index 2c83dba60..86a4ee8c9 100644 --- a/.ai/skills/hack-repo-verify/SKILL.md +++ b/.ai/skills/hack-repo-verify/SKILL.md @@ -69,8 +69,12 @@ every `bun test` run: a test or file that leaves the environment, cwd, umask, exit code, a guarded global function or a process listener changed fails with a `[global-state-guard]` message naming the file. The guard restores the environment, cwd, umask and exit code so later files run clean; it keeps -replaced functions and listeners, which may still protect unsettled work. Fix -the named file's restore (`try`/`finally` or `afterEach`), not the later failures. +replaced functions, which may still protect unsettled work. It keeps a test's +leftover listeners until its file ends, then puts the file's listeners back, so +a later `process.emit` is not blamed for them. Fix the named file's restore +(`try`/`finally` or `afterEach`), not the later failures. To emit a real signal +in a test, use `withOwnedProcessListeners` from +`tests/helpers/owned-process-listeners.ts`. Restore environment keys with `restoreEnv` from `tests/helpers/env.ts`: Bun keeps a deleted `HTTP_PROXY`-style key for `fetch` and `Bun.spawn` without `env`, and the guard reports that as `native env changed`. diff --git a/tests/global-state-guard.test.ts b/tests/global-state-guard.test.ts index e3ff41fc0..fbe249375 100644 --- a/tests/global-state-guard.test.ts +++ b/tests/global-state-guard.test.ts @@ -16,7 +16,9 @@ import { diffGlobalState, diffNativeEnv, restoreGlobalState, + restoreProcessListeners, } from "./helpers/global-state-guard.ts"; +import { withOwnedProcessListeners } from "./helpers/owned-process-listeners.ts"; const PRELOAD = resolve( import.meta.dir, @@ -133,6 +135,52 @@ test("a replaced function or listener is reported but left for its owner", () => expect(diffGlobalState(before)).toEqual([]); }); +test("the file-boundary listener restore removes leftovers, adds back missing listeners and keeps a kept once listener", async () => { + await withOwnedProcessListeners("SIGUSR2", () => { + let keptOnceCalls = 0; + const keptOnce = () => { + keptOnceCalls += 1; + }; + const removed = () => undefined; + const leftover = () => undefined; + const secondLeftover = () => undefined; + process.once("SIGUSR2", keptOnce); + process.on("SIGUSR2", removed); + try { + const before = captureGlobalState(); + process.removeListener("SIGUSR2", removed); + process.on("SIGUSR2", leftover); + process.once("SIGUSR2", secondLeftover); + expect(diffGlobalState(before)).toContain( + "process 'SIGUSR2' listeners changed (2 -> 3)" + ); + + expect(restoreProcessListeners(before)).toEqual(["SIGUSR2"]); + + expect(sameItems(process.listeners("SIGUSR2"), [keptOnce, removed])).toBe( + true + ); + expect(diffGlobalState(before)).toEqual([]); + expect(restoreProcessListeners(before)).toEqual([]); + // The kept listener was never re-added, so it is still a `once`. + process.emit("SIGUSR2"); + expect(keptOnceCalls).toBe(1); + expect(sameItems(process.listeners("SIGUSR2"), [removed])).toBe(true); + } finally { + for (const listener of [keptOnce, removed, leftover, secondLeftover]) { + process.removeListener("SIGUSR2", listener); + } + } + }); +}); + +function sameItems(left: readonly unknown[], right: readonly unknown[]) { + return ( + left.length === right.length && + left.every((item, index) => item === right[index]) + ); +} + const LEAK_IN_TEST = `import { test } from "bun:test"; test("sets PATH and fails before restoring", () => { const saved = process.env.PATH; @@ -159,7 +207,24 @@ test("sets a proxy and deletes it, which Bun's fetch never sees", () => { }); `; +// A once listener that this file never removes. Without the file-boundary +// restore, it stays in the process and a later test that emits SIGUSR2 +// consumes it and is blamed for the change (HACK-1416). +const LEAK_LISTENER = `import { test } from "bun:test"; +process.once("SIGUSR2", () => undefined); +test("passes but leaves a SIGUSR2 listener installed", () => {}); +`; + const LATER = `import { expect, test } from "bun:test"; +test("a later test that emits SIGUSR2 is not charged with an earlier listener", () => { + const own = () => undefined; + process.on("SIGUSR2", own); + try { + process.emit("SIGUSR2"); + } finally { + process.off("SIGUSR2", own); + } +}); test("a later file still finds executables on PATH", () => { const child = Bun.spawnSync(["hack-guard-probe"], { env: { ...process.env } }); expect(child.exitCode).toBe(0); @@ -206,6 +271,10 @@ async function runNestedSuite(opts: { join(suite, "c-leak-native-proxy.test.ts"), LEAK_NATIVE_PROXY ); + await writeFile( + join(suite, "c2-leak-signal-listener.test.ts"), + LEAK_LISTENER + ); await writeFile(join(suite, "d-later.test.ts"), LATER); const child = Bun.spawn( [ @@ -214,6 +283,7 @@ async function runNestedSuite(opts: { "./a-leak-in-test.test.ts", "./b-leak-in-hooks.test.ts", "./c-leak-native-proxy.test.ts", + "./c2-leak-signal-listener.test.ts", "./d-later.test.ts", ], { @@ -256,7 +326,16 @@ test("the guard names each leaking file, restores state, and later files pass", expect(run.output).toContain( "c-leak-native-proxy.test.ts left Bun's native environment changed after the file finished: native env HTTP_PROXY changed." ); - expect(run.output).toContain("2 test file(s) leaked process state"); + expect(run.output).toContain( + "c2-leak-signal-listener.test.ts left process state changed after the file finished: process 'SIGUSR2' listeners changed (0 -> 1). Restored the environment, cwd and process listeners" + ); + // The leftover listener was removed at the file boundary, so the later + // test that emits SIGUSR2 is not blamed for it. + expect(run.output).not.toContain("a test in d-later.test.ts"); + expect(run.output).toContain( + "(pass) a later test that emits SIGUSR2 is not charged with an earlier listener" + ); + expect(run.output).toContain("3 test file(s) leaked process state"); expect(run.output).not.toContain("127.0.0.1:9"); expect(run.output).not.toContain("Executable not found"); expect(run.output).toContain("LATER-FILE-PASSED"); diff --git a/tests/helpers/global-state-guard-preload.ts b/tests/helpers/global-state-guard-preload.ts index a7041435d..b7b88791e 100644 --- a/tests/helpers/global-state-guard-preload.ts +++ b/tests/helpers/global-state-guard-preload.ts @@ -2,8 +2,9 @@ * `bun test` preload (see `bunfig.toml`): fails loudly when a test or a test * file leaves process-global state changed, names the file, and restores the * environment, working directory, umask and exit code so later files do not - * fail for an unrelated reason. Replaced functions and process listeners are - * reported but kept; `restoreGlobalState` explains why. + * fail for an unrelated reason. Replaced functions are reported but kept; + * `restoreGlobalState` explains why. Process listeners are reported and kept + * per test, and put back at the file boundary (`restoreProcessListeners`). * * Two checks run: * - Per test: the state before the global `beforeEach` must match the state @@ -14,8 +15,11 @@ * - Per file: when Bun loads the next test file, the state must match the * state before the previous file loaded. This catches module-scope and * `beforeAll`/`afterAll` leaks, including an `afterAll` that never ran its - * restore. A mismatch is reported at once, the pre-file environment is restored, - * and the run fails from the preload `afterAll` with every leaking file. + * restore. A mismatch is reported at once, the pre-file environment and + * process listeners are restored, and the run fails from the preload + * `afterAll` with every leaking file. Restoring the listeners keeps a + * finished file's leftover handler from being run or consumed by a later + * test's `process.emit`, which would blame that later test (HACK-1416). * This check also compares Bun's native environment (one short `/usr/bin/env` * child per file), which keeps a proxy key that a test set and then deleted * although `process.env` no longer shows it. @@ -37,6 +41,7 @@ import { type GlobalStateSnapshot, type NativeEnv, restoreGlobalState, + restoreProcessListeners, } from "./global-state-guard.ts"; /** @@ -92,9 +97,10 @@ function checkFileBoundary(): void { if (changes.length > 0) { reportFileLeak( `${displayPath(currentFile)} left process state changed after the file finished: ${changes.join("; ")}`, - "Restored the environment and cwd from before the file so later files run clean." + "Restored the environment, cwd and process listeners from before the file so later files run clean." ); restoreGlobalState(fileBaseline); + restoreProcessListeners(fileBaseline); } checkNativeEnv(); fileBaseline = captureGlobalState(); diff --git a/tests/helpers/global-state-guard.ts b/tests/helpers/global-state-guard.ts index 948de7aff..e7a4eda28 100644 --- a/tests/helpers/global-state-guard.ts +++ b/tests/helpers/global-state-guard.ts @@ -15,7 +15,11 @@ * because fixtures may hold secret-shaped values. */ -/** Process events whose listener lists are compared (reported, not restored). */ +/** + * Process events whose listener lists are compared. A change is reported per + * test and per file, and restored only at the file boundary (see + * {@link restoreProcessListeners}). + */ export const GUARDED_PROCESS_EVENTS = [ "SIGINT", "SIGTERM", @@ -278,11 +282,12 @@ function restoreEnvObject(snapshot: GlobalStateSnapshot): void { * * Replaced functions (`Bun.spawn`, `process.kill`, `fetch`, timers), process * listeners and `process.platform` are reported by {@link diffGlobalState} - * but deliberately not restored. A leak there usually comes from a test that - * timed out while its work is still running, and that fake is what keeps the - * leftover work away from real processes, signals, the network and the + * but deliberately not restored here. A leak there usually comes from a test + * that timed out while its work is still running, and that fake is what keeps + * the leftover work away from real processes, signals, the network and the * Docker socket. Restoring the real function under that work could let it - * reach the host. + * reach the host. Listeners are the exception at a file boundary; see + * {@link restoreProcessListeners}. */ export function restoreGlobalState(snapshot: GlobalStateSnapshot): void { restoreEnvObject(snapshot); @@ -295,6 +300,67 @@ export function restoreGlobalState(snapshot: GlobalStateSnapshot): void { process.exitCode = snapshot.exitCode; } +function countListeners(listeners: readonly Listener[]): Map { + const counts = new Map(); + for (const listener of listeners) { + counts.set(listener, (counts.get(listener) ?? 0) + 1); + } + return counts; +} + +/** + * Puts the guarded process listeners back to `snapshot` and returns the events + * it changed. The preload calls this at a file boundary after it has reported + * the change, so a listener a finished file left behind cannot run (or be + * consumed by) a later file's `process.emit` and get that later test blamed + * (HACK-1416). + * + * Only the difference is edited: a listener added since the snapshot is + * removed and a snapshot listener that went missing is added back with + * `process.on`. Listeners present in both stay untouched, so a `once` + * listener there keeps its `once` behavior; Bun does not expose `once`, so a + * missing listener always comes back as a persistent one, at the end of the + * list. + * + * Unlike replaced functions, removing a leftover listener cannot let unsettled + * work reach the host: it only stops that work from reacting to a signal or + * exit event of the shared test process. + */ +export function restoreProcessListeners( + snapshot: GlobalStateSnapshot +): GuardedProcessEvent[] { + const restored: GuardedProcessEvent[] = []; + for (const event of GUARDED_PROCESS_EVENTS) { + const was = snapshot.listeners.get(event) ?? []; + const now = process.listeners(event) as Listener[]; + if (sameListeners(was, now)) { + continue; + } + restored.push(event); + const wanted = countListeners(was); + const present = countListeners(now); + for (const [listener, count] of present) { + for ( + let extra = count - (wanted.get(listener) ?? 0); + extra > 0; + extra-- + ) { + process.removeListener(event, listener); + } + } + const kept = countListeners(process.listeners(event) as Listener[]); + for (const listener of was) { + const left = kept.get(listener) ?? 0; + if (left > 0) { + kept.set(listener, left - 1); + } else { + process.on(event, listener); + } + } + } + return restored; +} + /** * The environment Bun itself uses for `fetch` proxies and for `Bun.spawn` * without an `env` option, as key to value. diff --git a/tests/helpers/owned-process-listeners.ts b/tests/helpers/owned-process-listeners.ts new file mode 100644 index 000000000..5b310766e --- /dev/null +++ b/tests/helpers/owned-process-listeners.ts @@ -0,0 +1,32 @@ +/** + * Lets a test emit a real process event without reaching listeners it does not + * own. + * + * Every test file shares one Bun process, so `process.emit("SIGINT")` in a + * test runs every SIGINT listener in that process, including a leftover from + * another file. {@link withOwnedProcessListeners} removes the listeners that + * are present, runs the callback, and adds them back afterwards, so the + * callback sees only the listeners it installs itself. + * + * Listeners the callback leaves behind are kept, so the test's own assertions + * and the global-state guard still see them. Bun does not expose `once`, so a + * saved `once` listener comes back as a persistent one. Between tests the + * guarded events normally have no listeners (the guard fails a file that + * leaves one), so the saved list is normally empty. + */ +export async function withOwnedProcessListeners( + event: NodeJS.Signals | "exit", + run: () => Promise | T +): Promise { + const saved = process.listeners(event); + for (const listener of saved) { + process.removeListener(event, listener); + } + try { + return await run(); + } finally { + for (const listener of saved) { + process.on(event, listener); + } + } +} diff --git a/tests/helpers/retained-routing-adoption.ts b/tests/helpers/retained-routing-adoption.ts index c6d1f17a4..f70aedb37 100644 --- a/tests/helpers/retained-routing-adoption.ts +++ b/tests/helpers/retained-routing-adoption.ts @@ -552,6 +552,11 @@ else { return originalKill.call(process, pid, signal); }), ]; + const restoreDoubles = () => { + for (const spy of spies) { + spy.mockRestore(); + } + }; const receiptPath = join( root, ".hack/.internal/legacy-compose-adoption-v1/receipt.json" @@ -682,14 +687,20 @@ else { releaseEnvironment: () => { restoreEnv("HACK_HOME", previousHome); }, + /** Whether this fixture's own global doubles are still installed. */ + doublesInstalled: () => process.kill !== originalKill, + /** + * Restores this fixture's own spies, and no other spy in the shared Bun + * process, whatever the lifetime says. Only for a case that has proven no + * work it started is still running; {@link cleanup} is the gated path. + */ + restoreDoubles, cleanup: async () => { restoreEnv("HACK_HOME", previousHome); if (!lifetime.canRestore()) { return refuse(); } - for (const spy of spies) { - spy.mockRestore(); - } + restoreDoubles(); await rm(outer, { recursive: true, force: true }); }, }; diff --git a/tests/loki-short-lived-logs.test.ts b/tests/loki-short-lived-logs.test.ts index f5a5eb439..864205fa1 100644 --- a/tests/loki-short-lived-logs.test.ts +++ b/tests/loki-short-lived-logs.test.ts @@ -20,6 +20,7 @@ import { lokiLogs, lokiSnapshot, } from "../src/ui/loki-logs.ts"; +import { withOwnedProcessListeners } from "./helpers/owned-process-listeners.ts"; // HACK-1277: a one-shot Compose service that exits quickly must not make // Loki-backed `hack logs` look empty while `docker compose logs` has output. @@ -573,3 +574,25 @@ test("Loki follow stops on SIGINT and leaves no listener behind", async () => { } ); }); + +test("Loki follow drops its SIGINT listener through a real SIGINT emit", async () => { + // The fixture owns every SIGINT listener, so the emit reaches only the + // follow's handler and not another file's or test's. + await withOwnedProcessListeners("SIGINT", () => + withFakeLokiTail( + () => undefined, + async ({ baseUrl, sockets }) => { + captureStdout(); + const exit = lokiLogs({ baseUrl, ...FOLLOW_PARAMS }); + await waitFor(() => sockets.length === 1, "the tail connection"); + expect(process.listenerCount("SIGINT")).toBe(1); + process.emit("SIGINT"); + // The signal path itself removes the `once` listener, before the + // tail closes and the follow's `finally` runs. + expect(process.listenerCount("SIGINT")).toBe(0); + expect(await exit).toBe(0); + expect(process.listenerCount("SIGINT")).toBe(0); + } + ) + ); +}); diff --git a/tests/native-compose-adoption-file-generation.test.ts b/tests/native-compose-adoption-file-generation.test.ts index cbb04dae8..f1fee675e 100644 --- a/tests/native-compose-adoption-file-generation.test.ts +++ b/tests/native-compose-adoption-file-generation.test.ts @@ -172,10 +172,13 @@ else {const source=JSON.parse(await Bun.stdin.text());const {schema_version,...p pathReplaced = true; process.env.PATH = root; }); -afterEach(async () => { - // PATH is restored even when the lifetime is unknown: a PATH left pointing - // at this fixture root hides git and bun from every later file in the Bun - // process (HACK-1327). The root and spawn spy stay retained. +/** + * The per-case teardown, run from `afterEach`. PATH is restored even when the + * lifetime is unknown: a PATH left pointing at this fixture root hides git and + * bun from every later file in the Bun process (HACK-1327). The root and spawn + * spy stay retained, and later cases in this file refuse. + */ +async function releaseRetainedFileFixture() { if (pathReplaced) { pathReplaced = false; restoreEnv("PATH", priorPath); @@ -189,6 +192,28 @@ afterEach(async () => { spawnSpy?.mockRestore(); spawnSpy = undefined; await rm(root, { recursive: true, force: true }); +} +afterEach(releaseRetainedFileFixture); +test("an unknown lifetime still restores PATH before the teardown refuses", async () => { + expect(process.env.PATH).toBe(root); + const owned = lifetime; + const refused = retainedRoutingFixtureLifetime(Date.now() + 5000); + refused.retain(); + lifetime = refused; + try { + await expect(releaseRetainedFileFixture()).rejects.toThrow( + "Retained-file fixture callback or child is unsettled" + ); + expect(uncertain).toBe(true); + expect(process.env.PATH).toBe(priorPath); + // The refusal kept the spawn spy for the unsettled work. + expect(Bun.spawn).not.toBe(originalSpawn); + } finally { + // Only this case's synthetic lifetime was unknown; its own lifetime ends + // normally in afterEach, which restores the spy and removes the root. + uncertain = false; + lifetime = owned; + } }); async function save() { await writeFile(join(root, "fixture.json"), JSON.stringify(fixture)); diff --git a/tests/native-compose-adoption-routing-generation.test.ts b/tests/native-compose-adoption-routing-generation.test.ts index 0906d7054..479772246 100644 --- a/tests/native-compose-adoption-routing-generation.test.ts +++ b/tests/native-compose-adoption-routing-generation.test.ts @@ -1,4 +1,4 @@ -import { test as boundedTest, expect, mock, spyOn } from "bun:test"; +import { test as boundedTest, expect, spyOn } from "bun:test"; import * as fs from "node:fs/promises"; import { readFile, rm } from "node:fs/promises"; import { join } from "node:path"; @@ -37,9 +37,86 @@ const SINGLE_OPERATION_TEST_BUDGET_MS = 180_000; * but well inside the product's default caller deadline: a slow but unchanged * operation. */ const SLOW_OPERATION_ADVANCE_MS = 16_000; -let h: Awaited>; +type RoutingFixture = Awaited>; +type RoutingFixtureLifetime = ReturnType; +let h: RoutingFixture; let fixtureActive = false; let fixtureUncertain = false; +/** + * The file's fixture gate. A case refuses while an earlier case's fixture is + * still active or its lifetime is unknown, because that fixture's global + * doubles may still be installed. `run` calls `restored()` only once every + * global double and root it created is restored; otherwise the gate stays + * closed and every later gated case in this file refuses. + */ +const gatedTest = ( + name: string, + run: (restored: () => void) => Promise, + timeoutMs: number +) => + boundedTest( + name, + async () => { + if (fixtureActive || fixtureUncertain) { + fixtureUncertain = true; + throw new Error( + "Prior retained routing fixture lifetime is unknown; values omitted." + ); + } + fixtureActive = true; + let isRestored = false; + try { + await run(() => { + isRestored = true; + }); + } finally { + if (isRestored) { + fixtureActive = false; + } else { + fixtureUncertain = true; + } + } + }, + timeoutMs + ); +/** + * Ends a gated case's fixture. With a known lifetime it runs the full + * teardown. With an unknown lifetime it keeps the spies and root for the + * unsettled work but still restores HACK_HOME, so later files never inherit + * the fixture home (HACK-1327). `failure` is set whenever not everything was + * restored. + */ +async function endRoutingFixture( + lifetime: RoutingFixtureLifetime, + fixture: RoutingFixture | undefined +): Promise<{ readonly restored: boolean; readonly failure?: unknown }> { + if (!lifetime.canRestore()) { + fixture?.releaseEnvironment(); + return { + restored: false, + failure: new Error( + "Retained routing fixture lifetime is unknown; values omitted." + ), + }; + } + if (fixture === undefined) { + // Setup did not issue a complete fixture; do not reuse its global context. + lifetime.retain(); + return { + restored: false, + failure: new Error( + "Retained routing fixture setup is incomplete; values omitted." + ), + }; + } + try { + await fixture.cleanup(); + return { restored: true }; + } catch (error: unknown) { + lifetime.retain(); + return { restored: false, failure: error }; + } +} const test = ( name: string, run: () => Promise, @@ -52,68 +129,31 @@ const test = ( } = {} ) => { const timeoutMs = opts.timeoutMs ?? 30_000; - return boundedTest( + return gatedTest( name, - async () => { - if (fixtureActive || fixtureUncertain) { - fixtureUncertain = true; - throw new Error( - "Prior retained routing fixture lifetime is unknown; values omitted." - ); - } - fixtureActive = true; + async (restored) => { const lifetime = retainedRoutingFixtureLifetime(Date.now() + timeoutMs); - let issued = false; + let fixture: RoutingFixture | undefined; let failed = false; let failure: unknown; try { h = await retainedRoutingFixture(lifetime); - issued = true; + fixture = h; await run(); } catch (error: unknown) { failed = true; failure = error; - } finally { - if (!lifetime.canRestore()) { - fixtureUncertain = true; - // Keep the spies and root for the unsettled work, but never leave - // HACK_HOME pointing at this fixture for later files (HACK-1327). - if (issued) { - h.releaseEnvironment(); - } - if (!failed) { - failed = true; - failure = new Error( - "Retained routing fixture lifetime is unknown; values omitted." - ); - } - } else if (issued) { - try { - await h.cleanup(); - fixtureActive = false; - } catch (error: unknown) { - lifetime.retain(); - fixtureUncertain = true; - if (!failed) { - failed = true; - failure = error; - } - } - } else { - // Setup did not issue a complete fixture; do not reuse its global context. - lifetime.retain(); - fixtureUncertain = true; - if (!failed) { - failed = true; - failure = new Error( - "Retained routing fixture setup is incomplete; values omitted." - ); - } - } + } + const end = await endRoutingFixture(lifetime, fixture); + if (end.restored) { + restored(); } if (failed) { throw failure; } + if (!end.restored) { + throw end.failure; + } }, timeoutMs ); @@ -864,25 +904,39 @@ boundedTest( }, 1000 ); -boundedTest( +gatedTest( "a refused teardown still restores HACK_HOME so later files never inherit the fixture home", - async () => { + async (restored) => { const before = process.env.HACK_HOME; - const lifetime = retainedRoutingFixtureLifetime(Date.now() + 5000); - const fixture = await retainedRoutingFixture(lifetime); + // Stands in for another fixture's spy that is kept on purpose while its + // settlement is unknown: this case must restore only its own spies. + const foreign = spyOn(privateState, "readPrivate"); try { - expect(process.env.HACK_HOME).toBe(fixture.home); - fixture.releaseEnvironment(); - expect(process.env.HACK_HOME).toBe(before); - process.env.HACK_HOME = fixture.home; - lifetime.retain(); - await expect(fixture.cleanup()).rejects.toThrow(); - expect(process.env.HACK_HOME).toBe(before); + const lifetime = retainedRoutingFixtureLifetime(Date.now() + 5000); + const fixture = await retainedRoutingFixture(lifetime); + try { + expect(process.env.HACK_HOME).toBe(fixture.home); + lifetime.retain(); + // The gated runner's own teardown path for an unknown lifetime. + const end = await endRoutingFixture(lifetime, fixture); + expect(end.restored).toBe(false); + expect(end.failure).toBeInstanceOf(Error); + expect(process.env.HACK_HOME).toBe(before); + expect(fixture.doublesInstalled()).toBe(true); + process.env.HACK_HOME = fixture.home; + await expect(fixture.cleanup()).rejects.toThrow(); + expect(process.env.HACK_HOME).toBe(before); + expect(fixture.doublesInstalled()).toBe(true); + } finally { + restoreEnv("HACK_HOME", before); + fixture.restoreDoubles(); + await rm(fixture.outer, { recursive: true, force: true }); + restored(); + } + expect(fixture.doublesInstalled()).toBe(false); + expect(privateState.readPrivate).toBe(foreign); } finally { - restoreEnv("HACK_HOME", before); - // The refused teardown kept the fixture spies; nothing else is mocked here. - mock.restore(); - await rm(fixture.outer, { recursive: true, force: true }); + foreign.mockRestore(); } }, 5000 diff --git a/tests/native-config-protected-files-fixture.test.ts b/tests/native-config-protected-files-fixture.test.ts index fde295a42..b4900ba30 100644 --- a/tests/native-config-protected-files-fixture.test.ts +++ b/tests/native-config-protected-files-fixture.test.ts @@ -780,6 +780,7 @@ test("closed-pipe survivor refuses completion and permanently vetoes restoration `await Bun.write(${JSON.stringify(pidPath)},String(process.pid));await Bun.sleep(1500);` ); const settlement = createCompletedJobFixtureSettlement(); + const unmockedKill = process.kill; const original = process.kill.bind(process), groups: number[] = []; const signal = spyOn(process, "kill").mockImplementation((pid, kind) => { @@ -851,4 +852,5 @@ test("closed-pipe survivor refuses completion and permanently vetoes restoration // Restore it here: the vetoed teardown would otherwise leave process.kill // replaced for every later file in the Bun process (HACK-1327). signal.mockRestore(); + expect(process.kill).toBe(unmockedKill); }); From d49d2252fa5d7ea668651d89fc4ff36a09875176 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 23:17:16 -0400 Subject: [PATCH 2/7] fix(native): reject Hack ownership label namespaces in runtime-core's label review runtime-core's own Compose label review rejected only the caddy, traefik, hack and com.docker.compose prefixes, so a service label in io.hack-local.*, io.hack.native-config.*, io.hack.native-file.* or io.hack.storage-witness.* passed the candidate review if the TypeScript classifier was bypassed. Add a Rust mirror of HACK_OWNERSHIP_LABEL_NAMESPACES and reject those namespaces (and their bare roots) in the review, never exempted by a routing or dependency cache declaration. A TypeScript parity test keeps the mirror identical to the TypeScript source of truth. Refs HACK-1409 Co-Authored-By: Claude Opus 5.5 --- .../runtime-core/src/project/compose/mod.rs | 1 + .../src/project/compose/ownership_labels.rs | 63 +++++++++++++++++++ .../src/project/compose/service.rs | 13 ++-- .../runtime-core/tests/project_contract.rs | 55 ++++++++++++++++ src/lib/hack-ownership-labels.ts | 6 +- tests/hack-ownership-labels.test.ts | 34 +++++++++- 6 files changed, 163 insertions(+), 9 deletions(-) create mode 100644 packages/runtime-core/src/project/compose/ownership_labels.rs diff --git a/packages/runtime-core/src/project/compose/mod.rs b/packages/runtime-core/src/project/compose/mod.rs index ca8d52f57..be2935aa0 100644 --- a/packages/runtime-core/src/project/compose/mod.rs +++ b/packages/runtime-core/src/project/compose/mod.rs @@ -3,6 +3,7 @@ use serde_json::{Map, Value}; mod dependency_cache; mod fields; mod graph; +mod ownership_labels; pub(super) mod routing; mod service; use fields::*; diff --git a/packages/runtime-core/src/project/compose/ownership_labels.rs b/packages/runtime-core/src/project/compose/ownership_labels.rs new file mode 100644 index 000000000..198c72317 --- /dev/null +++ b/packages/runtime-core/src/project/compose/ownership_labels.rs @@ -0,0 +1,63 @@ +//! Label namespaces that Hack writes as resource ownership evidence. + +/// Container, network and volume label namespaces that Hack writes to prove +/// which resources it owns. This mirrors `HACK_OWNERSHIP_LABEL_NAMESPACES` in +/// `src/lib/hack-ownership-labels.ts`, the source of truth; +/// `tests/hack-ownership-labels.test.ts` fails when the two lists differ. +/// +/// A project must not declare a label in these namespaces, because the native +/// runtime and native-config Compose trust them as ownership evidence. The +/// TypeScript classifier rejects such labels first; the candidate's own label +/// review rejects them again so a bypassed classifier cannot pass one through. +pub(super) const HACK_OWNERSHIP_LABEL_NAMESPACES: [&str; 4] = [ + "io.hack-local.", + "io.hack.native-config.", + "io.hack.native-file.", + "io.hack.storage-witness.", +]; + +/// Whether `key` is inside a Hack ownership namespace or is a namespace root +/// without its trailing dot (for example `io.hack-local`). +pub(super) fn is_hack_ownership_label(key: &str) -> bool { + HACK_OWNERSHIP_LABEL_NAMESPACES + .iter() + .any(|namespace| key.starts_with(namespace) || namespace.strip_suffix('.') == Some(key)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ownership_namespaces_and_roots_are_hack_ownership_labels() { + for key in [ + "io.hack-local.owner", + "io.hack-local", + "io.hack.native-config.owner", + "io.hack.native-config", + "io.hack.native-file.token", + "io.hack.native-file", + "io.hack.storage-witness.carrier", + "io.hack.storage-witness", + ] { + assert!(is_hack_ownership_label(key), "{key}"); + } + } + + #[test] + fn near_miss_keys_are_not_hack_ownership_labels() { + for key in [ + "io.hack", + "io.hack.", + "io.hacker.owner", + "io.hack-localx.owner", + "io.hack.native-configx", + "io.hack.storage", + "io.hack.custom", + "example.io.hack-local.owner", + "IO.HACK-LOCAL.OWNER", + ] { + assert!(!is_hack_ownership_label(key), "{key}"); + } + } +} diff --git a/packages/runtime-core/src/project/compose/service.rs b/packages/runtime-core/src/project/compose/service.rs index 875c562a6..5cc51846a 100644 --- a/packages/runtime-core/src/project/compose/service.rs +++ b/packages/runtime-core/src/project/compose/service.rs @@ -805,11 +805,14 @@ pub(super) fn compile( } if routing_invalid || labels.keys().any(|key| { - !(dependency_cache.is_some() && super::dependency_cache::recognized(key)) - && !(routing.is_some() && super::routing::recognized(key)) - && ["caddy", "traefik", "hack", "com.docker.compose"] - .iter() - .any(|prefix| key == prefix || key.starts_with(&format!("{prefix}."))) + // Hack's ownership namespaces are never exempt: no routing or + // dependency cache declaration may carry ownership evidence. + super::ownership_labels::is_hack_ownership_label(key) + || (!(dependency_cache.is_some() && super::dependency_cache::recognized(key)) + && !(routing.is_some() && super::routing::recognized(key)) + && ["caddy", "traefik", "hack", "com.docker.compose"] + .iter() + .any(|prefix| key == prefix || key.starts_with(&format!("{prefix}.")))) }) { diagnostics.push(Diagnostic::error( diff --git a/packages/runtime-core/tests/project_contract.rs b/packages/runtime-core/tests/project_contract.rs index 00199a38b..440cf3217 100644 --- a/packages/runtime-core/tests/project_contract.rs +++ b/packages/runtime-core/tests/project_contract.rs @@ -523,6 +523,61 @@ fn unsupported_privileged_external_and_unknown_fields_block_enrollment() { assert!(fixture.codes().contains(&"external_network".into())); } +/// The candidate's own label review rejects Hack's ownership namespaces (the +/// list `HACK_OWNERSHIP_LABEL_NAMESPACES` declares in TypeScript), so a +/// project cannot carry ownership evidence even when the TypeScript classifier +/// is bypassed. Routing and dependency cache declarations never exempt them. +#[test] +fn hack_ownership_label_namespaces_block_enrollment() { + let owner_label = |fixture: &Fixture| { + fixture + .codes() + .iter() + .any(|code| code == "external_route_or_owner_label") + }; + for key in [ + "io.hack-local.owner", + "io.hack.native-config.owner", + "io.hack.native-file.token", + "io.hack.storage-witness.carrier", + ] { + for labels in [ + format!(" labels: {{'{key}': owner-canary}}\n"), + format!(" labels: ['{key}=owner-canary']\n"), + format!( + " labels:\n caddy: web.hack\n caddy.reverse_proxy: '{{{{upstreams 3000}}}}'\n caddy.tls: internal\n '{key}': owner-canary\n" + ), + ] { + let fixture = Fixture::new(&format!("{BASIC}{labels}")); + let plan = fixture.plan(); + assert!(owner_label(&fixture), "{key}: {labels}"); + assert!(!plan.plan.enrollment_compatible, "{key}"); + assert!( + !serde_json::to_string(&plan) + .unwrap() + .contains("owner-canary") + ); + assert_eq!( + project::enroll(&fixture.candidate, fixture.options(), &plan.plan_id) + .unwrap_err() + .code, + "incompatible_compose" + ); + assert!(!fixture.candidate.state_root.exists()); + } + } + // Near misses outside every ownership namespace stay reviewable. Closer + // misses (a longer first segment, or another segment under the same + // reverse-DNS root) are pinned by the `ownership_labels` unit tests; they + // stay out of this file because the TypeScript parity scan reads every + // such key here as a label Hack writes. + let fixture = Fixture::new(&format!( + "{BASIC} labels: {{example.io.hack-local.owner: a, io-hack-local.owner: b, io_hack.native-config.owner: c}}\n" + )); + assert!(!owner_label(&fixture)); + assert!(fixture.plan().plan.enrollment_compatible); +} + #[test] fn explicit_external_network_replacement_is_owned_internal_and_review_bound() { let original = diff --git a/src/lib/hack-ownership-labels.ts b/src/lib/hack-ownership-labels.ts index 2cbc1d146..2bfd5e94a 100644 --- a/src/lib/hack-ownership-labels.ts +++ b/src/lib/hack-ownership-labels.ts @@ -18,7 +18,11 @@ * (`owner`, `generation`, `helper`, `carrier`). * * A project must not declare a label in these namespaces, because their - * consumers trust them as ownership evidence. Add a namespace here before + * consumers trust them as ownership evidence. The TypeScript Compose + * classifier rejects them, and runtime-core's own label review rejects them + * again from a Rust mirror of this list + * (`packages/runtime-core/src/project/compose/ownership_labels.rs`) that the + * same test keeps identical. Add a namespace here, and to the mirror, before * writing a label in it. */ export const HACK_OWNERSHIP_LABEL_NAMESPACES = [ diff --git a/tests/hack-ownership-labels.test.ts b/tests/hack-ownership-labels.test.ts index dea0628c5..47901d64c 100644 --- a/tests/hack-ownership-labels.test.ts +++ b/tests/hack-ownership-labels.test.ts @@ -5,9 +5,10 @@ import { HACK_OWNERSHIP_LABEL_NAMESPACES } from "../src/lib/hack-ownership-label /** * Pins `HACK_OWNERSHIP_LABEL_NAMESPACES` to the label keys Hack actually - * writes and verifies. The scan reads the TypeScript CLI and the Rust - * runtime-core sources (read-only); the two files that only restate the list - * are left out, so the list cannot satisfy itself. + * writes and verifies, and to runtime-core's Rust mirror of the list. The scan + * reads the TypeScript CLI and the Rust runtime-core sources (read-only); the + * files that only restate the list are left out, so the list cannot satisfy + * itself. */ const REPO = resolve(import.meta.dir, ".."); @@ -17,9 +18,12 @@ const PATTERNS = [ "packages/*/src/**/*.rs", "packages/*/tests/**/*.rs", ]; +const RUST_MIRROR = + "packages/runtime-core/src/project/compose/ownership_labels.rs"; const RESTATEMENTS = new Set([ "src/lib/hack-ownership-labels.ts", "src/lib/native-compose-backend-features.ts", + RUST_MIRROR, ]); const KEY = /\bio\.hack[A-Za-z0-9._-]*/g; @@ -62,3 +66,27 @@ test("every declared ownership namespace is written or verified somewhere", asyn ); expect(unused).toEqual([]); }); + +/** The string literals of the Rust `HACK_OWNERSHIP_LABEL_NAMESPACES` array. */ +async function rustMirrorNamespaces(): Promise { + const text = await readFile(join(REPO, RUST_MIRROR), "utf8"); + const body = + /const HACK_OWNERSHIP_LABEL_NAMESPACES: \[&str; (\d+)\] = \[([^\]]*)\];/.exec( + text + ); + expect(body).not.toBeNull(); + // Line comments are not entries, even when they quote a namespace. + const entries = (body?.[2] ?? "").replace(/\/\/[^\n]*/g, ""); + const literals = [...entries.matchAll(/"([^"]*)"/g)].map( + (match) => match[1] ?? "" + ); + // The declared array length must agree with the literals that were read. + expect(literals.length).toBe(Number(body?.[1])); + return literals; +} + +test("runtime-core's label review rejects exactly the declared ownership namespaces", async () => { + expect(await rustMirrorNamespaces()).toEqual([ + ...HACK_OWNERSHIP_LABEL_NAMESPACES, + ]); +}); From 7ef8395aedced0624bfe7dfd881fedca5a41d904 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 23:28:44 -0400 Subject: [PATCH 3/7] fix(native): keep observed storage refusal reasons past the carrier deadline and pin the guard exclusion Follow-ups to #421 (HACK-1361): - Record every publication stop of a recorded storage command through one stopPublication/refusePublication pair, so the bounded await, the child publication catch, the guard and settlement all refuse with the first cause and issued boundary. - Keep an owner check's issued boundary when the command record replaces it with its fixed refusal (publication and owner creation). - Tag untagged policy and state mismatches in the Docker storage carrier: a re-inspected carrier whose creation time or policy changed reports helper-policy-stability; a saved material binding that changed during a carrier step reports private-state. Both keep that reason after the carrier deadline instead of reading as storage-budget. - Extract the pre-spawn guard storage proof into nativeComposeGuardStorageProof and pin that only the carrier proof extends the start deadline. Diagnostics only: no check, deadline, retention or recovery rule changes. Refs HACK-1413 Co-Authored-By: Claude Opus 5.5 --- docs/reference/native-compose-commands.md | 7 +- .../native-compose-storage-xattr-carrier.md | 9 +- src/lib/native-compose-command.ts | 47 +++- .../native-compose-storage-command-record.ts | 20 +- .../native-compose-storage-witness-docker.ts | 219 ++++++++++-------- .../helpers/native-compose-storage-docker.ts | 9 +- ...ive-compose-storage-command-record.test.ts | 114 ++++++++- ...e-storage-policy-stability-command.test.ts | 50 ++++ ...ive-compose-storage-witness-docker.test.ts | 113 +++++++++ ...ompose-storage-witness-xattr-owner.test.ts | 80 +++++++ 10 files changed, 545 insertions(+), 123 deletions(-) create mode 100644 tests/native-compose-storage-policy-stability-command.test.ts diff --git a/docs/reference/native-compose-commands.md b/docs/reference/native-compose-commands.md index 18a99f021..f5fa69f01 100644 --- a/docs/reference/native-compose-commands.md +++ b/docs/reference/native-compose-commands.md @@ -347,8 +347,11 @@ expectation admission/write, journal, enrollment read, provision, root discovery seed, verification, closing root proof, completion write and publication. Carrier transport additionally distinguishes target, create, policy, start, response and cleanup; policy reasons identify shape, command, labels, host policy, mount -cardinality, program/storage mounts and policy stability. A policy refusal keeps -its policy reason even when it is caught after the carrier's deadline. A timeout, +cardinality, program/storage mounts and policy stability. A re-inspected carrier +whose creation time or policy changed reports `helper-policy-stability`, and a +saved material binding that changed during a carrier step reports +`private-state`. A policy or state refusal keeps its reason even when it is +caught after the carrier's deadline. A timeout, budget or otherwise unclassified refusal that finds its carrier's storage budget exhausted, and not cancelled, reports `storage-budget`, which names `HACK_NATIVE_STORAGE_TIMEOUT_MS`; a start phase diff --git a/docs/reference/native-compose-storage-xattr-carrier.md b/docs/reference/native-compose-storage-xattr-carrier.md index d4a57ff00..7a6cd9470 100644 --- a/docs/reference/native-compose-storage-xattr-carrier.md +++ b/docs/reference/native-compose-storage-xattr-carrier.md @@ -429,9 +429,12 @@ Budgets are layered and all finite: A timeout, budget or otherwise unclassified refusal that finds its carrier's own deadline passed (and the carrier not cancelled) reports the closed effect reason `storage-budget`; a probe clipped to the remaining budget times out exactly at -that end. A refusal with its own observed reason, such as a helper policy -refusal, keeps that reason even when it is caught after the deadline, including -when a recorded command replaces it with its fixed command refusal. Cancellation +that end. A refusal with its own observed reason keeps that reason even when it +is caught after the deadline, including when a recorded command or its command +record replaces it with a fixed refusal. Such reasons include a helper policy +refusal, `helper-policy-stability` for a re-inspected carrier whose creation +time or policy changed, and `private-state` for a saved material binding that +changed during a carrier step. Cancellation keeps `probe-cancel`. The reason changes no check, retention or recovery rule. Only the original in-memory writer can advance its exact sequence. Publication diff --git a/src/lib/native-compose-command.ts b/src/lib/native-compose-command.ts index 163f78e2f..0e3a11934 100644 --- a/src/lib/native-compose-command.ts +++ b/src/lib/native-compose-command.ts @@ -1174,6 +1174,32 @@ async function finalizeNativeComposeStop(opts: { } } +/** + * The pre-spawn guard's storage proof in a native `up`/`restart` start phase. + * Only each carrier proof (carrier creation and witness verification) runs in + * the start deadline's storage exclusion, passed to `verify` as `excludeProof`. + * The proof's freshness checks and saved-state reads around it consume the + * start budget like any other start-phase work. `takeAdjacentProof` answers, + * once per proof, whether this guard may reuse the immediately preceding proof. + */ +export function nativeComposeGuardStorageProof(opts: { + readonly verify: ( + generation: G, + options: NativeComposeCommandStorageVerifyOptions + ) => Promise; + readonly generation: G; + readonly excludeStorage: (work: () => Promise) => Promise; + readonly takeAdjacentProof: () => boolean; +}): () => Promise { + return () => { + const adjacentProof = opts.takeAdjacentProof(); + return opts.verify(opts.generation, { + ...(adjacentProof ? { adjacentProof } : {}), + excludeProof: opts.excludeStorage, + }); + }; +} + export async function assertNativeComposeEffectOwned(opts: { readonly assertFresh: () => Promise; readonly assertOwned: () => Promise; @@ -1724,17 +1750,16 @@ async function executePreparedGeneration(opts: { assertFresh, assertOwned: () => ownership.assertOwned(), verifyStorage: storage - ? () => { - const adjacentProof = adjacentGuardProof; - adjacentGuardProof = false; - // Only each carrier proof is excluded; the guard's - // freshness checks and saved-state reads consume the - // start budget like any other start-phase work. - return storage.verify(generation, { - ...(adjacentProof ? { adjacentProof } : {}), - excludeProof: start.excludeStorage, - }); - } + ? nativeComposeGuardStorageProof({ + verify: storage.verify, + generation, + excludeStorage: start.excludeStorage, + takeAdjacentProof: () => { + const adjacentProof = adjacentGuardProof; + adjacentGuardProof = false; + return adjacentProof; + }, + }) : undefined, }), }); diff --git a/src/lib/native-compose-storage-command-record.ts b/src/lib/native-compose-storage-command-record.ts index 11dfc1287..b6954b031 100644 --- a/src/lib/native-compose-storage-command-record.ts +++ b/src/lib/native-compose-storage-command-record.ts @@ -2,6 +2,7 @@ import { createHash, randomBytes } from "node:crypto"; import { rename } from "node:fs/promises"; import { join } from "node:path"; import { isRecord } from "./guards.ts"; +import { copyNativeComposeEffectRefusal } from "./native-compose-effect-diagnostics.ts"; import type { NativeComposeMaterialBinding } from "./native-compose-generation.ts"; import { type HeldDirectory, @@ -382,6 +383,17 @@ type OwnerState = { export type NativeComposeStorageCommandOwner = Readonly>; export type NativeComposeStorageArmedCommand = Readonly>; const owners = new WeakMap(); +/** The same fixed refusal. Only an issued owner boundary of `source`, such as an + * owner check's saved-state refusal, is kept for the effect diagnostic; it grants + * nothing and changes no check, invalidation or retention. */ +function refuseFrom(source: unknown): never { + try { + return refuse(); + } catch (error) { + copyNativeComposeEffectRefusal(source, error as Error); + throw error; + } +} const commands = new WeakMap< NativeComposeStorageArmedCommand, { @@ -439,9 +451,9 @@ async function publish(owner: OwnerState, record: RecordV2): Promise { owner.saved = saved; owner.record = record; owner.published(saved.info); - } catch { + } catch (error) { owner.active = false; - return refuse(); + return refuseFrom(error); } finally { owner.busy = false; } @@ -490,8 +502,8 @@ export async function createNativeComposeStorageCommandOwner(opts: { }); published(saved.info); return handle; - } catch { - return refuse(); + } catch (error) { + return refuseFrom(error); } } export async function armNativeComposeStorageCommand(opts: { diff --git a/src/lib/native-compose-storage-witness-docker.ts b/src/lib/native-compose-storage-witness-docker.ts index e63abe200..4c83a0d97 100644 --- a/src/lib/native-compose-storage-witness-docker.ts +++ b/src/lib/native-compose-storage-witness-docker.ts @@ -144,6 +144,56 @@ type Files = { function same(left: unknown, right: unknown): boolean { return JSON.stringify(left) === JSON.stringify(right); } +/** + * The same fixed refusal for a mismatch that no carrier deadline can produce, + * tagged with its observed reason. Untagged, such a refusal has no observed + * cause, so one caught after its carrier deadline would be reported as + * `storage-budget`. It grants nothing and changes no check, deadline or retention. + */ +function refuseObserved( + stage: NativeComposeEffectRefusal["stage"], + reason: "private-state" | "helper-policy-stability" +): never { + try { + return refuse(); + } catch (error) { + retainNativeComposeEffectRefusal(error, { stage, reason }); + throw error; + } +} +/** + * Refuse unless the saved material binding is unchanged across a carrier step. + * A change is saved private state, so it reports `private-state` at `stage`, + * like a binding read that itself refuses. + */ +export function assertNativeComposeStorageBindingUnchanged(opts: { + readonly stage: NativeComposeEffectRefusal["stage"]; + readonly before: NativeComposeMaterialBinding; + readonly after: NativeComposeMaterialBinding; +}): void { + if (!same(opts.before, opts.after)) { + refuseObserved(opts.stage, "private-state"); + } +} +/** + * Refuse unless a re-inspected helper carrier keeps the creation time and fixed + * policy captured after its creation. A change reports `helper-policy-stability` + * at `storage-helper-policy`, the stage of every other carrier policy refusal. + * Callers check the carrier's state separately. + */ +export function assertNativeComposeStorageCarrierStable( + observed: NativeComposeStorageDockerCarrier, + known: { readonly createdAt: string; readonly policy: string } +): void { + if ( + !( + observed.createdAt === known.createdAt && + nativeComposeStorageDockerCarrierPolicy(observed) === known.policy + ) + ) { + refuseObserved("storage-helper-policy", "helper-policy-stability"); + } +} function lifetime(context: Context): void { if (context.signal.aborted) { refuse(); @@ -1072,6 +1122,23 @@ export async function runNativeComposeStorageDockerCommand(supplied: { // The first refusal that stopped publication; only its issued boundary is kept. let publicationSource: unknown; const cancellation = new AbortController(); + // Every path that stops publication records it here, and every later refusal + // of this command is the same fixed refusal with the first cause and issued + // boundary, so no single path can drop the observed reason. + const stopPublication = ( + error: unknown, + cause = storageCommandRefusal(error) + ): void => { + publicationRefused = true; + publicationCause ??= cause; + publicationSource ??= error; + if (opts.originalCommand) { + invalidateNativeComposeStorageCommandOwner(opts.originalCommand.owner); + } + cancellation.abort(); + }; + const refusePublication = (): never => + refuseCommand(publicationCause, publicationSource); try { await opts.assertAdmitted(); const timeoutMs = opts.cleanup @@ -1089,7 +1156,7 @@ export async function runNativeComposeStorageDockerCommand(supplied: { return refuseCommand("probe-budget"); } if (publicationRefused) { - return refuseCommand(publicationCause, publicationSource); + return refusePublication(); } if ( !(recordingSettlement || opts.cleanup) && @@ -1110,12 +1177,7 @@ export async function runNativeComposeStorageDockerCommand(supplied: { new Promise((_, reject) => { timer = setTimeout( () => { - publicationRefused = true; - publicationCause ??= "probe-budget"; - if (record) { - invalidateNativeComposeStorageCommandOwner(record.owner); - } - cancellation.abort(); + stopPublication(undefined, "probe-budget"); reject( new Error( "Original command publication unavailable; values omitted." @@ -1129,14 +1191,8 @@ export async function runNativeComposeStorageDockerCommand(supplied: { guard(); return result; } catch (error) { - publicationRefused = true; - publicationCause ??= storageCommandRefusal(error); - publicationSource ??= error; - if (record) { - invalidateNativeComposeStorageCommandOwner(record.owner); - } - cancellation.abort(); - return refuseCommand(publicationCause, publicationSource); + stopPublication(error); + return refusePublication(); } finally { if (timer !== undefined) { clearTimeout(timer); @@ -1282,11 +1338,7 @@ export async function runNativeComposeStorageDockerCommand(supplied: { return refuse(); } } catch (error) { - publicationRefused = true; - publicationCause ??= storageCommandRefusal(error); - publicationSource ??= error; - invalidateNativeComposeStorageCommandOwner(record.owner); - cancellation.abort(); + stopPublication(error); } }, onExit: (event) => { @@ -1301,7 +1353,7 @@ export async function runNativeComposeStorageDockerCommand(supplied: { settled = true; if (record) { if (publicationRefused) { - return refuseCommand(publicationCause, publicationSource); + return refusePublication(); } if (!(armed && exit)) { return refuse(); @@ -1462,9 +1514,7 @@ export async function createNativeComposeDockerStorageXattrCarrier( stopped: false, }); const after = await current(context, selection); - if (!same(before, after)) { - return refuse(); - } + assertNativeComposeStorageBindingUnchanged({ stage, before, after }); lifetime(context); return { binding: after, target }; } catch (error) { @@ -1607,9 +1657,11 @@ export async function createNativeComposeDockerStorageXattrCarrier( tool(); await recheckDirectories(held); const latest = await current(context, selection, true); - if (!same(before, latest)) { - return refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: latest, + }); const result = await runNativeComposeStorageDockerCommand({ context, files, @@ -1643,9 +1695,11 @@ export async function createNativeComposeDockerStorageXattrCarrier( assertAdmitted: async () => { await engine(probe(), context); tool(); - if (!same(before, await current(context, selection, true))) { - refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: await current(context, selection, true), + }); }, }); if ( @@ -1662,9 +1716,7 @@ export async function createNativeComposeDockerStorageXattrCarrier( return refuse(); } const after = await current(context, selection, true); - if (!same(before, after)) { - return refuse(); - } + assertNativeComposeStorageBindingUnchanged({ stage, before, after }); } finally { await Promise.allSettled(held.map((entry) => entry.file.close())); } @@ -1737,14 +1789,11 @@ export async function createNativeComposeDockerStorageXattrCarrier( check: async () => { tool(); await checkFiles(files); - if ( - !same( - before, - await current(context, input.target, !input.readonly) - ) - ) { - return refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: await current(context, input.target, !input.readonly), + }); }, published: (info) => { files.captures[recordIndex] = { path: recordPath, info }; @@ -1757,9 +1806,11 @@ export async function createNativeComposeDockerStorageXattrCarrier( await checkFiles(files); tool(); const admitted = await current(context, input.target, !input.readonly); - if (!same(before, admitted)) { - return refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: admitted, + }); stage = "storage-helper-create"; const created = await runNativeComposeStorageDockerCommand({ context, @@ -1782,14 +1833,11 @@ export async function createNativeComposeDockerStorageXattrCarrier( assertAdmitted: async () => { await engine(probe(), context); tool(); - if ( - !same( - before, - await current(context, input.target, !input.readonly) - ) - ) { - refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: await current(context, input.target, !input.readonly), + }); }, }); const id = created.stdout.trim(); @@ -1870,10 +1918,9 @@ export async function createNativeComposeDockerStorageXattrCarrier( } // The holder observation just closed with the same engine-ID fence. const ready = await inspectCarrier(false, true); + assertNativeComposeStorageCarrierStable(ready, known); if ( !( - ready.createdAt === known.createdAt && - nativeComposeStorageDockerCarrierPolicy(ready) === known.policy && ready.state.Running === false && ready.state.Pid === 0 && ready.state.Status === "created" @@ -1883,9 +1930,11 @@ export async function createNativeComposeDockerStorageXattrCarrier( } tool(); const final = await current(context, input.target, !input.readonly); - if (!same(before, final)) { - return refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: final, + }); stage = "storage-helper-start"; const result = await runNativeComposeStorageDockerCommand({ context, @@ -1917,11 +1966,9 @@ export async function createNativeComposeDockerStorageXattrCarrier( } await checkFiles(files); const ready = await inspectCarrier(); + assertNativeComposeStorageCarrierStable(ready, owned); if ( !( - ready.createdAt === owned.createdAt && - nativeComposeStorageDockerCarrierPolicy(ready) === - owned.policy && ready.state.Running === false && ready.state.Pid === 0 && ready.state.Status === "created" @@ -1931,14 +1978,11 @@ export async function createNativeComposeDockerStorageXattrCarrier( } await engine(probe(), context); tool(); - if ( - !same( - before, - await current(context, input.target, !input.readonly) - ) - ) { - refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: await current(context, input.target, !input.readonly), + }); }, }); stage = "storage-helper-response"; @@ -1951,10 +1995,9 @@ export async function createNativeComposeDockerStorageXattrCarrier( } stage = "storage-helper-policy"; const stopped = await inspectCarrier(true); + assertNativeComposeStorageCarrierStable(stopped, known); if ( !( - stopped.createdAt === known.createdAt && - nativeComposeStorageDockerCarrierPolicy(stopped) === known.policy && stopped.state.Running === false && stopped.state.Pid === 0 && stopped.state.Status === "exited" && @@ -1969,23 +2012,19 @@ export async function createNativeComposeDockerStorageXattrCarrier( return refuse(); } const repeated = await inspectCarrier(true); - if ( - !( - repeated.createdAt === known.createdAt && - nativeComposeStorageDockerCarrierPolicy(repeated) === - known.policy && - same(repeated.state, stopped.state) - ) - ) { + assertNativeComposeStorageCarrierStable(repeated, known); + if (!same(repeated.state, stopped.state)) { return refuse(); } stage = "storage-helper-cleanup"; await checkFiles(files); tool(); const cleanupAuthority = await current(context, input.target); - if (!same(before, cleanupAuthority)) { - return refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: cleanupAuthority, + }); const removed = await runNativeComposeStorageDockerCommand({ context, files, @@ -2002,21 +2041,17 @@ export async function createNativeComposeDockerStorageXattrCarrier( beforeSpawn: tool, assertAdmitted: async () => { const finalCarrier = await inspectCarrier(true); - if ( - !( - finalCarrier.createdAt === owned.createdAt && - nativeComposeStorageDockerCarrierPolicy(finalCarrier) === - owned.policy && - same(finalCarrier.state, stopped.state) - ) - ) { + assertNativeComposeStorageCarrierStable(finalCarrier, owned); + if (!same(finalCarrier.state, stopped.state)) { refuse(); } await engine(probe(true), context); tool(); - if (!same(before, await current(context, input.target))) { - refuse(); - } + assertNativeComposeStorageBindingUnchanged({ + stage, + before, + after: await current(context, input.target), + }); }, }); if ( diff --git a/tests/helpers/native-compose-storage-docker.ts b/tests/helpers/native-compose-storage-docker.ts index c65abf414..742d8fb7a 100644 --- a/tests/helpers/native-compose-storage-docker.ts +++ b/tests/helpers/native-compose-storage-docker.ts @@ -119,7 +119,14 @@ if(storageArgs[0]==="container"&&storageArgs[1]==="ls"&&argument("--format")===" if(storageArgs[0]==="container"&&storageArgs[1]==="inspect"){ const format=argument("--format"),selected=storageArgs.slice(storageArgs.indexOf("--format")+2),carriers=await readRows(storageCarriers); if(format.includes(".HostConfig")){ - for(const id of selected){const row=carriers.find(value=>value.id===id);if(!row)process.exit(1);emit(row);}process.exit(0); + // Fault: the Nth helper policy inspection observes a replaced carrier (a new creation time). + const replaced=storageRoot+"/fault-carrier-replaced",seen=storageRoot+"/carrier-policy-inspects"; + for(const id of selected){const row=carriers.find(value=>value.id===id);if(!row)process.exit(1); + if(await Bun.file(replaced).exists()){ + const count=Number(await Bun.file(seen).exists()?await Bun.file(seen).text():0)+1;await Bun.write(seen,String(count)); + if(count===Number(await Bun.file(replaced).text())){emit({...row,createdAt:"2026-01-01T00:00:00.000Z"});continue;} + } + emit(row);}process.exit(0); } if(format.includes(".Mounts")){ const main=await workload();for(const id of selected){ diff --git a/tests/native-compose-storage-command-record.test.ts b/tests/native-compose-storage-command-record.test.ts index be96ec3a4..045659211 100644 --- a/tests/native-compose-storage-command-record.test.ts +++ b/tests/native-compose-storage-command-record.test.ts @@ -200,6 +200,7 @@ async function fixture() { }; let source = "original", failChild = false; + let checkRefusal: unknown; const owner = await createNativeComposeStorageCommandOwner({ directory, binding: { @@ -217,6 +218,9 @@ async function fixture() { if (source !== "original") { throw new Error("private source canary"); } + if (checkRefusal !== undefined) { + throw checkRefusal; + } }, published: () => { const record = parseNativeComposeStorageCommandRecord( @@ -257,8 +261,22 @@ async function fixture() { failChild: () => { failChild = true; }, + /** The owner check refuses with this error from now on. */ + refuseCheck: (error: unknown) => { + checkRefusal = error; + }, }; } +/** A fixed refusal with an issued owner boundary, as a carrier check makes. */ +function issuedRefusal( + diagnostic: Parameters[1] +): Error { + const error = new Error( + "Native storage xattr proof refused; values omitted." + ); + retainNativeComposeEffectRefusal(error, diagnostic); + return error; +} test( "v2 is closed and never promotes a copied or legacy original owner", ownedCase(async () => { @@ -535,12 +553,15 @@ test( 30_000 ); test.each([ - ["before arming", 1], - ["after arming", 2], - ["after the stopped child is published", 3], + ["before arming", 1, false], + ["after arming", 2, false], + ["after the stopped child is published", 3, false], + // Thrown before the bounded await sees it: only the child-publication catch + // records it as the publication-stopping refusal. + ["thrown synchronously after the stopped child is published", 3, true], ] as const)( "a helper policy refusal %s keeps its own reason after the carrier deadline", - (_, failingCall) => + (_, failingCall, synchronous) => ownedCase(async () => { const f = await fixture(), marker = join(f.root, "effect"), @@ -556,18 +577,19 @@ test.each([ beforeSpawn: () => {}, // The carrier's admission re-inspects the helper; the chosen call // observes a policy refusal with its own issued boundary. - assertAdmitted: async () => { + assertAdmitted: () => { calls += 1; if (calls === failingCall) { - const policy = new Error( - "Native storage xattr proof refused; values omitted." - ); - retainNativeComposeEffectRefusal(policy, { + const policy = issuedRefusal({ stage: "storage-helper-policy", reason: "helper-host-policy", }); - throw policy; + if (synchronous) { + throw policy; + } + return Promise.reject(policy); } + return Promise.resolve(); }, originalCommand: { owner: f.owner, kind: "create", carrier: null }, }).catch((error: unknown) => error); @@ -598,6 +620,78 @@ test.each([ })(), 30_000 ); +test( + "an owner check refusal while arming keeps its own reason after the carrier deadline", + ownedCase(async () => { + const f = await fixture(), + marker = join(f.root, "effect"), + signal = new AbortController().signal, + deadline = Date.now() + 5000; + let calls = 0; + // The saved material binding changed: the owner check refuses the arm + // publication, inside the runner's bounded publication await. + f.refuseCheck( + issuedRefusal({ stage: "storage-helper-start", reason: "private-state" }) + ); + try { + const caught = await runNativeComposeStorageDockerCommand({ + context: { signal, deadline }, + files: f.files, + args: ["/bin/sh", "-c", 'printf complete > "$1"', "owned", marker], + timeoutMs: 5000, + beforeSpawn: () => {}, + assertAdmitted: () => { + calls += 1; + return Promise.resolve(); + }, + originalCommand: { owner: f.owner, kind: "create", carrier: null }, + }).catch((error: unknown) => error); + expect(caught).toBeInstanceOf(Error); + expect((caught as Error).message).toContain("values omitted"); + // Refused while arming: no later admission and no spawned target. + expect(calls).toBe(1); + expect(existsSync(marker)).toBe(false); + expect(nativeComposeStorageCommandOwnerConfirmed(f.owner)).toBe(false); + expect(nativeComposeEffectRefusal(caught)).toEqual({ + stage: "storage-helper-start", + reason: "private-state", + }); + expect( + nativeComposeStorageCarrierBudgetReason({ + error: caught, + signal, + deadline, + now: () => deadline + 1, + }) + ).toBe("private-state"); + } finally { + if (!nativeComposeStorageCommandOwnerConfirmed(f.owner)) { + retained.add(f.root); + } + } + }), + 30_000 +); +test( + "a record publication refused by its owner check keeps the check's issued boundary", + ownedCase(async () => { + const f = await fixture(); + f.refuseCheck( + issuedRefusal({ stage: "storage-helper-create", reason: "private-state" }) + ); + const caught = await f.arm().catch((error: unknown) => error); + expect((caught as Error).message).toBe( + "Native storage xattr proof refused; values omitted." + ); + expect(nativeComposeEffectRefusal(caught)).toEqual({ + stage: "storage-helper-create", + reason: "private-state", + }); + // The writer stays permanently invalidated, exactly as before. + expect(nativeComposeStorageCommandOwnerConfirmed(f.owner)).toBe(false); + retained.add(f.root); + }) +); test( "failed arm directory sync prevents the target spawn rather than proving no-spawn from a record", ownedCase(async () => { diff --git a/tests/native-compose-storage-policy-stability-command.test.ts b/tests/native-compose-storage-policy-stability-command.test.ts new file mode 100644 index 000000000..7ab54c1ae --- /dev/null +++ b/tests/native-compose-storage-policy-stability-command.test.ts @@ -0,0 +1,50 @@ +import { expect, test } from "bun:test"; +import { join } from "node:path"; +import { fixture, invoke } from "./helpers/native-compose-command.ts"; + +const storage = { data: { kind: "persistent", scope: "worktree" } } as const; + +// The first `up` enrolls cold storage through real helper carriers on the +// synthetic daemon. Its first carrier is policy-inspected after creation (1), +// before start (2), at start admission (3), after it stopped (4), once more +// (5) and at removal admission (6). The chosen inspection observes a replaced +// carrier: a new creation time under the same ID. That mismatch has its own +// observed reason, so even a refusal caught after the carrier deadline cannot +// be reported as the storage budget. +test.each([ + ["before its start", 2], + ["at its start admission", 3], + ["after it stopped", 4], + ["on its repeated stopped inspection", 5], + ["at its removal admission", 6], +] as const)( + "a helper carrier replaced %s refuses with its policy stability reason", + async (_, inspection) => { + const root = await fixture("", false, { storage, noHooks: true }); + await Bun.write(join(root, "fault-carrier-replaced"), String(inspection)); + const result = await invoke( + root, + ["up", "--detach", "--json"], + 30_000, + 120_000 + ); + expect( + Number(await Bun.file(join(root, "carrier-policy-inspects")).text()) + ).toBe(inspection); + expect(result.code).toBe(1); + expect(JSON.parse(result.stdout)).toMatchObject({ + ok: false, + error: { + detail: { + native_compose_effect_refusal: { + stage: "storage-helper-policy", + reason: "helper-policy-stability", + }, + }, + }, + }); + // No workload started. + expect(await Bun.file(join(root, "engine")).exists()).toBe(false); + }, + 150_000 +); diff --git a/tests/native-compose-storage-witness-docker.test.ts b/tests/native-compose-storage-witness-docker.test.ts index ac414d72d..5314931ea 100644 --- a/tests/native-compose-storage-witness-docker.test.ts +++ b/tests/native-compose-storage-witness-docker.test.ts @@ -16,6 +16,8 @@ import { } from "../src/lib/native-compose-generation.ts"; import { NativeComposeOwnershipError } from "../src/lib/native-compose-ownership.ts"; import { + assertNativeComposeStorageBindingUnchanged, + assertNativeComposeStorageCarrierStable, assertNativeComposeStorageDockerImage, nativeComposeStorageCarrierBudgetReason, } from "../src/lib/native-compose-storage-witness-docker.ts"; @@ -1192,6 +1194,117 @@ test("a helper policy refusal caught after its carrier deadline keeps its policy ).toBe("storage-budget"); }); +function refusalOf(check: () => void): unknown { + try { + check(); + } catch (error: unknown) { + return error; + } + return undefined; +} + +test("a re-inspected carrier whose creation time or policy changed keeps a policy stability reason after its deadline", () => { + // Fake clock past the carrier deadline; the carrier was not cancelled. + const deadline = 5_000_000; + const live = new AbortController(); + const selected = input(); + const inspect = ( + change: (value: ReturnType["value"]) => void + ) => { + const fake = carrier(selected); + change(fake.value); + return checkNativeComposeStorageDockerCarrier({ + value: fake.value, + input: selected, + program: fake.program, + imageIds: [selected.artifact.imageId], + }); + }; + const created = inspect(() => {}); + const known = { + createdAt: created.createdAt, + policy: nativeComposeStorageDockerCarrierPolicy(created), + }; + // An unchanged carrier passes; its state is checked by each caller. + expect(() => + assertNativeComposeStorageCarrierStable( + inspect((value) => { + value.state = { Status: "exited", Running: false }; + }), + known + ) + ).not.toThrow(); + for (const changed of [ + inspect((value) => { + value.createdAt = "2026-10-08T12:00:01Z"; + }), + // An admitted equivalent form that still changes the captured policy. + inspect((value) => { + (value.host as Record).Devices = null; + }), + ]) { + const caught = refusalOf(() => + assertNativeComposeStorageCarrierStable(changed, known) + ); + expect((caught as Error).message).toBe( + "Native storage xattr proof refused; values omitted." + ); + expect(nativeComposeEffectRefusal(caught)).toEqual({ + stage: "storage-helper-policy", + reason: "helper-policy-stability", + }); + expect( + nativeComposeStorageCarrierBudgetReason({ + error: caught, + signal: live.signal, + deadline, + now: () => deadline + 30_000, + }) + ).toBe("helper-policy-stability"); + } +}); + +test("a saved binding that changed across a carrier step keeps a private-state reason after its deadline", () => { + const deadline = 5_000_000; + const live = new AbortController(); + const before = { + generationId: "d".repeat(32), + pendingToken: null, + } as unknown as NativeComposeMaterialBinding; + expect(() => + assertNativeComposeStorageBindingUnchanged({ + stage: "storage-helper-start", + before, + after: structuredClone(before), + }) + ).not.toThrow(); + const caught = refusalOf(() => + assertNativeComposeStorageBindingUnchanged({ + stage: "storage-helper-start", + before, + after: { + ...before, + pendingToken: "e".repeat(32), + } as NativeComposeMaterialBinding, + }) + ); + expect((caught as Error).message).toBe( + "Native storage xattr proof refused; values omitted." + ); + expect(nativeComposeEffectRefusal(caught)).toEqual({ + stage: "storage-helper-start", + reason: "private-state", + }); + expect( + nativeComposeStorageCarrierBudgetReason({ + error: caught, + signal: live.signal, + deadline, + now: () => deadline + 30_000, + }) + ).toBe("private-state"); +}); + test("an exhausted xattr carrier lifetime refuses with the storage budget reason", () => { const artifact: NativeComposeStorageXattrArtifact = { version: 1, diff --git a/tests/native-compose-storage-witness-xattr-owner.test.ts b/tests/native-compose-storage-witness-xattr-owner.test.ts index 2073dd0ea..d309d292e 100644 --- a/tests/native-compose-storage-witness-xattr-owner.test.ts +++ b/tests/native-compose-storage-witness-xattr-owner.test.ts @@ -13,8 +13,10 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import { createNativeComposeStartDeadline } from "../src/lib/compose-startup-budget.ts"; import { assertNativeComposeEffectOwned, + nativeComposeGuardStorageProof, runOneOff, } from "../src/lib/native-compose-command.ts"; import { @@ -1888,6 +1890,84 @@ test("runEffect captures the original fresh carrier factory across a long phase } }); +test("a guard storage proof extends the start deadline by its carrier proof only, never by its freshness checks", async () => { + const { store, transport, generation } = await rememberedActive(); + const command = commandPorts(transport); + // Fake monotonic clock: each freshness check takes 7 s, each carrier 3 s. + let monotonic = 0; + let excluding = 0; + let armed = false; + const steps: string[] = []; + const createCarrier = command.ports.carrier; + command.ports.carrier = async (opts) => { + if (armed) { + steps.push(excluding > 0 ? "carrier-excluded" : "carrier-counted"); + monotonic += 3000; + } + return await createCarrier(opts); + }; + await store.withMutation(async (mutation) => { + const storage = await prepareNativeComposeCommandStorage({ + store, + mutation, + operation: "up", + selected: [selection], + signal: transport.controller.signal, + ports: command.ports, + assertFresh: async () => { + if (armed) { + steps.push(excluding > 0 ? "fresh-excluded" : "fresh-counted"); + monotonic += 7000; + } + }, + }); + if (!storage) { + throw new Error("Expected retained storage"); + } + const start = createNativeComposeStartDeadline({ + timeoutMs: 10_000, + storageTimeoutMs: 90_000, + wallNow: () => 1_000_000, + monotonicNow: () => monotonic, + }); + const excludeStorage = (work: () => Promise): Promise => + start.excludeStorage(async () => { + excluding += 1; + try { + return await work(); + } finally { + excluding -= 1; + } + }); + let adjacentAnswers = 0; + armed = true; + await nativeComposeGuardStorageProof({ + verify: storage.verify, + generation, + excludeStorage, + takeAdjacentProof: () => { + adjacentAnswers += 1; + return false; + }, + })(); + armed = false; + expect(adjacentAnswers).toBe(1); + // The enrolled volume's carrier proof is the only excluded span. + expect(steps).toEqual([ + "fresh-counted", + "carrier-excluded", + "fresh-counted", + ]); + // Only that carrier's 3 s moved the deadline; the guard's 14 s of + // freshness checks consumed the start budget. + expect(start.deadline()).toBe(1_000_000 + 10_000 + 3000); + }); + // Admission's proof plus the guard's: each verified the retained witness. + expect( + helperOperations(transport).filter((call) => call === "verify") + ).toHaveLength(2); +}); + test("durable tagged Expected precedes one cold provision/seed and fresh proof before workload", async () => { const { store, transport, reference } = await active(); expect(reference).toMatchObject({ From 3867b9b7ad3254f5d1d44c86167707c45317fc85 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Sat, 24 Oct 2026 02:20:31 -0400 Subject: [PATCH 4/7] test(native): fail when the guard's start-budget exclusion widens past its carrier proof The pre-spawn guard storage proof excludes only its carrier proofs from the start deadline. The existing CLI wiring test proves carrier time is excluded, but re-widening the exclusion at the executePreparedGeneration call site (to cover the whole storage.verify) passed every test. A new CLI test slows the guard storage proof's freshness check, which runs outside every carrier span, past the start budget and asserts that the owned child's admission check then refuses before the workload spawns. Refs HACK-1413 Co-Authored-By: Claude Opus 5.5 --- ...ve-compose-start-storage-exclusion.test.ts | 142 ++++++++++++++---- 1 file changed, 111 insertions(+), 31 deletions(-) diff --git a/tests/native-compose-start-storage-exclusion.test.ts b/tests/native-compose-start-storage-exclusion.test.ts index 71787001d..103dd5293 100644 --- a/tests/native-compose-start-storage-exclusion.test.ts +++ b/tests/native-compose-start-storage-exclusion.test.ts @@ -6,6 +6,9 @@ import { fixture, invoke } from "./helpers/native-compose-command.ts"; const START_BUDGET_MS = 3000; // Longer than the whole start budget, far below the per-carrier storage budget. const GUARD_CARRIER_DELAY_MS = 4000; +// Longer than the whole start budget, far below the storage exclusion cap. +const GUARD_FRESHNESS_DELAY_MS = 4000; +const STORAGE_BUDGET_MS = 120_000; function phaseRows(text: string) { return text @@ -21,38 +24,62 @@ function phaseRows(text: string) { ); } +/** + * Stand-in source that sets `guardStorageOpen` from the CLI's own phase trace, + * which the CLI writes synchronously to its stderr file before any stand-in it + * spawns runs: true only while a `guard.storage` span is open. + */ +function guardStorageOpenSource(stderrFile: string): string { + return `let guardStorageOpen=false; +if(await Bun.file(${JSON.stringify(stderrFile)}).exists()){ + const open=new Map(); + for(const line of (await Bun.file(${JSON.stringify(stderrFile)}).text()).split("\\n")){ + if(!line.startsWith('{"diagnostic":"native-compose-phase",'))continue; + const row=JSON.parse(line);if(row.boundary==="begin")open.set(row.span,row.phase);else open.delete(row.span); + } + guardStorageOpen=[...open.values()].includes("guard.storage"); +}`; +} + +async function injectAfter(opts: { + readonly file: string; + readonly anchor: string; + readonly source: string; +}) { + const text = await readFile(opts.file, "utf8"); + expect(text.includes(opts.anchor)).toBe(true); + await writeFile( + opts.file, + text.replace(opts.anchor, `${opts.anchor}\n${opts.source}`) + ); +} + +async function countLines(file: string) { + return (await readFile(file, "utf8").catch(() => "")) + .split("\n") + .filter(Boolean).length; +} + // Failure injection on the real source CLI: only the pre-spawn guard's storage -// carrier is slowed. The stand-in reads the CLI's own phase trace (written -// synchronously to its stderr file before the carrier spawns Docker) and delays -// a carrier dependency check (helper `image inspect`) only while a -// `guard.storage` span is open. Enrollment and finalization proofs are not -// delayed. Without the start deadline's storage exclusion, the guard alone -// would spend the start budget and the workload would never be admitted. +// carrier is slowed. The Docker stand-in delays a carrier dependency check +// (helper `image inspect`) only while a `guard.storage` span is open. +// Enrollment and finalization proofs are not delayed. Without the start +// deadline's storage exclusion, the guard alone would spend the start budget +// and the workload would never be admitted. test("a pre-spawn guard storage proof longer than the start budget does not consume it", async () => { const root = await fixture("", false, { noHooks: true, storage: { data: { kind: "persistent", scope: "worktree" } }, }); const stderrFile = join(root, "cli-stderr"); - const docker = join(root, "docker"); - const source = await readFile(docker, "utf8"); - const anchor = "const args=process.argv.slice(2);"; - expect(source.includes(anchor)).toBe(true); - await writeFile( - docker, - source.replace( - anchor, - `${anchor} -if(args[0]==="image"&&args[1]==="inspect"&&await Bun.file(${JSON.stringify(stderrFile)}).exists()){ - const open=new Map(); - for(const line of (await Bun.file(${JSON.stringify(stderrFile)}).text()).split("\\n")){ - if(!line.startsWith('{"diagnostic":"native-compose-phase",'))continue; - const row=JSON.parse(line);if(row.boundary==="begin")open.set(row.span,row.phase);else open.delete(row.span); - } - if([...open.values()].includes("guard.storage")){await appendFile(root+"/guard-carrier-delays","1\\n");await Bun.sleep(${GUARD_CARRIER_DELAY_MS});} -}` - ) - ); + await injectAfter({ + file: join(root, "docker"), + anchor: "const args=process.argv.slice(2);", + source: `if(args[0]==="image"&&args[1]==="inspect"){ +${guardStorageOpenSource(stderrFile)} + if(guardStorageOpen){await appendFile(root+"/guard-carrier-delays","1\\n");await Bun.sleep(${GUARD_CARRIER_DELAY_MS});} +}`, + }); const result = await invoke( root, ["up", "--detach", "--json"], @@ -61,12 +88,9 @@ if(args[0]==="image"&&args[1]==="inspect"&&await Bun.file(${JSON.stringify(stder { HACK_NATIVE_COMPOSE_PHASE_TRACE: "1" }, { stderrFile } ); - const delays = ( - await readFile(join(root, "guard-carrier-delays"), "utf8").catch(() => "") - ) - .split("\n") - .filter(Boolean).length; - expect(delays).toBeGreaterThanOrEqual(1); + expect( + await countLines(join(root, "guard-carrier-delays")) + ).toBeGreaterThanOrEqual(1); // The delayed guard storage proof outlasted the whole start budget. const guard = phaseRows(result.stderr).filter( (row) => row.phase === "guard.storage" && row.boundary === "end" @@ -82,3 +106,59 @@ if(args[0]==="image"&&args[1]==="inspect"&&await Bun.file(${JSON.stringify(stder (await readFile(join(root, "storage-events"), "utf8")).split("\n") ).toContain("workload"); }, 300_000); + +// The converse wiring check: only carrier proofs are excluded, so the guard +// storage proof's own freshness check (outside every carrier span) still +// consumes the start budget. The compiler stand-in, which that freshness check +// re-runs, is slowed once, only while a `guard.storage` span is open. If the +// guard's whole storage proof were excluded again, this delay would extend the +// deadline and the workload would start. +test("a pre-spawn guard storage freshness check longer than the start budget consumes it", async () => { + const root = await fixture("", false, { + noHooks: true, + storage: { data: { kind: "persistent", scope: "worktree" } }, + }); + const stderrFile = join(root, "cli-stderr"); + const delays = join(root, "guard-freshness-delays"); + await injectAfter({ + file: join(root, "compiler"), + anchor: 'import {createHash} from "node:crypto";', + source: `${guardStorageOpenSource(stderrFile)} +if(guardStorageOpen&&!(await Bun.file(${JSON.stringify(delays)}).exists())){await Bun.write(${JSON.stringify(delays)},"1\\n");await Bun.sleep(${GUARD_FRESHNESS_DELAY_MS});}`, + }); + const result = await invoke( + root, + ["up", "--detach", "--json"], + START_BUDGET_MS, + 240_000, + { + HACK_NATIVE_COMPOSE_PHASE_TRACE: "1", + // A slow shared host must not exhaust enrollment's carrier first. + HACK_NATIVE_STORAGE_TIMEOUT_MS: String(STORAGE_BUDGET_MS), + }, + { stderrFile } + ); + expect(await countLines(delays)).toBe(1); + const guard = phaseRows(result.stderr).filter( + (row) => row.phase === "guard.storage" && row.boundary === "end" + ); + expect(guard.length).toBeGreaterThanOrEqual(1); + expect(guard[0]?.durationMs ?? 0).toBeGreaterThan(START_BUDGET_MS); + // The start budget ran out, so the owned child's admission check after the + // guard refused before the workload was spawned. + expect(result.code).not.toBe(0); + expect(JSON.parse(result.stdout)).toMatchObject({ + ok: false, + error: { + detail: { + native_compose_effect_refusal: { + stage: "compose-child", + reason: "private-uncertain", + }, + }, + }, + }); + expect( + (await readFile(join(root, "storage-events"), "utf8")).split("\n") + ).not.toContain("workload"); +}, 300_000); From a58f6b9f5556e29725b778023ca392f9bc26ea5a Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Sat, 24 Oct 2026 11:13:07 -0400 Subject: [PATCH 5/7] test(native): keep the guard freshness exclusion test decisive on a slow host The freshness wiring test used the 3 s start budget of the carrier test, so on a slow host the rest of the start phase alone could spend it and a re-widened exclusion would still be refused. It now gives the start phase 12 s, delays every compiler run inside the guard storage proof by 5 s (below the compiler's 10 s timeout) and checks that those delays alone outlast the budget. A larger per-carrier storage budget keeps enrollment from exhausting its carrier first. Refs HACK-1413 Co-Authored-By: Claude Opus 5.5 --- ...ve-compose-start-storage-exclusion.test.ts | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/tests/native-compose-start-storage-exclusion.test.ts b/tests/native-compose-start-storage-exclusion.test.ts index 103dd5293..917b03de9 100644 --- a/tests/native-compose-start-storage-exclusion.test.ts +++ b/tests/native-compose-start-storage-exclusion.test.ts @@ -6,9 +6,15 @@ import { fixture, invoke } from "./helpers/native-compose-command.ts"; const START_BUDGET_MS = 3000; // Longer than the whole start budget, far below the per-carrier storage budget. const GUARD_CARRIER_DELAY_MS = 4000; -// Longer than the whole start budget, far below the storage exclusion cap. -const GUARD_FRESHNESS_DELAY_MS = 4000; -const STORAGE_BUDGET_MS = 120_000; +// The freshness test's start budget is longer, so that the rest of the start +// phase fits in it even on a slow shared host. Its guard storage proof re-runs +// the compiler stand-in several times outside every carrier span; each of those +// runs is delayed below the compiler's own 10 s timeout, and together they +// outlast that start budget. The per-carrier storage budget is raised so that a +// slow host cannot exhaust an enrollment carrier first. +const FRESHNESS_START_BUDGET_MS = 12_000; +const GUARD_FRESHNESS_DELAY_MS = 5000; +const FRESHNESS_STORAGE_BUDGET_MS = 300_000; function phaseRows(text: string) { return text @@ -108,11 +114,12 @@ ${guardStorageOpenSource(stderrFile)} }, 300_000); // The converse wiring check: only carrier proofs are excluded, so the guard -// storage proof's own freshness check (outside every carrier span) still -// consumes the start budget. The compiler stand-in, which that freshness check -// re-runs, is slowed once, only while a `guard.storage` span is open. If the -// guard's whole storage proof were excluded again, this delay would extend the -// deadline and the workload would start. +// storage proof's own freshness checks (outside every carrier span) still +// consume the start budget. The compiler stand-in, which those freshness checks +// re-run, is slowed only while a `guard.storage` span is open. If the guard's +// whole storage proof were excluded again (re-widened at the call site or in +// the helper), these delays would extend the deadline and the workload would +// start. test("a pre-spawn guard storage freshness check longer than the start budget consumes it", async () => { const root = await fixture("", false, { noHooks: true, @@ -124,26 +131,28 @@ test("a pre-spawn guard storage freshness check longer than the start budget con file: join(root, "compiler"), anchor: 'import {createHash} from "node:crypto";', source: `${guardStorageOpenSource(stderrFile)} -if(guardStorageOpen&&!(await Bun.file(${JSON.stringify(delays)}).exists())){await Bun.write(${JSON.stringify(delays)},"1\\n");await Bun.sleep(${GUARD_FRESHNESS_DELAY_MS});}`, +if(guardStorageOpen){await import("node:fs/promises").then(m=>m.appendFile(${JSON.stringify(delays)},"1\\n"));await Bun.sleep(${GUARD_FRESHNESS_DELAY_MS});}`, }); const result = await invoke( root, ["up", "--detach", "--json"], - START_BUDGET_MS, - 240_000, + FRESHNESS_START_BUDGET_MS, + 420_000, { HACK_NATIVE_COMPOSE_PHASE_TRACE: "1", - // A slow shared host must not exhaust enrollment's carrier first. - HACK_NATIVE_STORAGE_TIMEOUT_MS: String(STORAGE_BUDGET_MS), + HACK_NATIVE_STORAGE_TIMEOUT_MS: String(FRESHNESS_STORAGE_BUDGET_MS), }, { stderrFile } ); - expect(await countLines(delays)).toBe(1); + // The delayed compiler runs alone outlast the whole start budget. + expect((await countLines(delays)) * GUARD_FRESHNESS_DELAY_MS).toBeGreaterThan( + FRESHNESS_START_BUDGET_MS + ); const guard = phaseRows(result.stderr).filter( (row) => row.phase === "guard.storage" && row.boundary === "end" ); expect(guard.length).toBeGreaterThanOrEqual(1); - expect(guard[0]?.durationMs ?? 0).toBeGreaterThan(START_BUDGET_MS); + expect(guard[0]?.durationMs ?? 0).toBeGreaterThan(FRESHNESS_START_BUDGET_MS); // The start budget ran out, so the owned child's admission check after the // guard refused before the workload was spawned. expect(result.code).not.toBe(0); @@ -161,4 +170,4 @@ if(guardStorageOpen&&!(await Bun.file(${JSON.stringify(delays)}).exists())){awai expect( (await readFile(join(root, "storage-events"), "utf8")).split("\n") ).not.toContain("workload"); -}, 300_000); +}, 480_000); From 9983c97b2e1cd08a6a61c4336ef3f92b5df0da60 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Sat, 24 Oct 2026 12:29:32 -0400 Subject: [PATCH 6/7] chore: mutation probe, re-widen the guard storage exclusion (reverted next) Temporary hosted-CI mutation probe for HACK-1413 item (2): the guard closure is wrapped in start.excludeStorage at the executePreparedGeneration call site and the helper's inner exclusion is identity, so the whole storage.verify is excluded again. The freshness wiring test is expected to fail on this commit. The next commit reverts it exactly. Refs HACK-1413 Co-Authored-By: Claude Opus 5.5 --- src/lib/native-compose-command.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/native-compose-command.ts b/src/lib/native-compose-command.ts index 0e3a11934..65598029e 100644 --- a/src/lib/native-compose-command.ts +++ b/src/lib/native-compose-command.ts @@ -1750,16 +1750,16 @@ async function executePreparedGeneration(opts: { assertFresh, assertOwned: () => ownership.assertOwned(), verifyStorage: storage - ? nativeComposeGuardStorageProof({ + ? ((guard: () => Promise) => () => start.excludeStorage(guard))(nativeComposeGuardStorageProof({ verify: storage.verify, generation, - excludeStorage: start.excludeStorage, + excludeStorage: (work) => work(), takeAdjacentProof: () => { const adjacentProof = adjacentGuardProof; adjacentGuardProof = false; return adjacentProof; }, - }) + })) : undefined, }), }); From e532eb18e3e2d5d48c27ae9fd15e6d4866bf09dd Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Sat, 24 Oct 2026 12:40:58 -0400 Subject: [PATCH 7/7] chore: revert the guard storage exclusion mutation probe Restores executePreparedGeneration's guard closure exactly as it was before the hosted-CI mutation probe: only each carrier proof runs in the start deadline's storage exclusion. Refs HACK-1413 Co-Authored-By: Claude Opus 5.5 --- src/lib/native-compose-command.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/lib/native-compose-command.ts b/src/lib/native-compose-command.ts index 65598029e..0e3a11934 100644 --- a/src/lib/native-compose-command.ts +++ b/src/lib/native-compose-command.ts @@ -1750,16 +1750,16 @@ async function executePreparedGeneration(opts: { assertFresh, assertOwned: () => ownership.assertOwned(), verifyStorage: storage - ? ((guard: () => Promise) => () => start.excludeStorage(guard))(nativeComposeGuardStorageProof({ + ? nativeComposeGuardStorageProof({ verify: storage.verify, generation, - excludeStorage: (work) => work(), + excludeStorage: start.excludeStorage, takeAdjacentProof: () => { const adjacentProof = adjacentGuardProof; adjacentGuardProof = false; return adjacentProof; }, - })) + }) : undefined, }), });