From 631d3fc74135196f860e211bc12bd51bc2594121 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Thu, 22 Oct 2026 00:52:05 -0400 Subject: [PATCH 1/4] fix(native): keep the closed exit code when the graph owner's pipes drain late The foreground graph owner drained its child's stdout and stderr for only 250 ms after exit. Under heavy oversubscription the pipe readers could be scheduled after that window, so the closed native error code (or readiness line) was dropped and a generic startup failure was reported. Replace the window with the named, documented NATIVE_EXIT_DRAIN_BOUND_MS (5 s), still measured from the observed exit. EOF still ends a normal drain at once; the bound only ends a drain that a descendant holding the pipes keeps open. Tests: a writer that emits the code only after the stand-in is reaped is kept (fails with the 250 ms window), and a held descendant still settles within the bound while it is alive (fails with a widened drain). Refs HACK-1350 Co-Authored-By: Claude Opus 5.5 (cherry picked from commit 4dfbb6a035c37713d4f3f269a6a4fabbd7c7a870) --- src/backends/native-project-process.ts | 21 +- tests/native-project-process.test.ts | 265 ++++++++++++++++++++----- 2 files changed, 236 insertions(+), 50 deletions(-) diff --git a/src/backends/native-project-process.ts b/src/backends/native-project-process.ts index b898d8f5..ed3c467e 100644 --- a/src/backends/native-project-process.ts +++ b/src/backends/native-project-process.ts @@ -22,6 +22,20 @@ const RUN = /^[a-f0-9]{32}$/; const MAX_OUTPUT = 16 * 1024 * 1024; const MAX_INPUT = 256 * 1024; +/** + * Exit-relative bound on draining the owned child's stdout and stderr. + * + * The native runtime writes its closed error code (and any readiness line) + * before it exits, so those bytes are already in the pipes when the exit is + * observed; with no other writer, EOF follows immediately and the drain ends + * there. This bound only ends a drain that EOF cannot end: a descendant that + * inherited a pipe and keeps it open. It is measured from the observed exit, + * not from spawn, and is deliberately generous so that a heavily + * oversubscribed host still schedules the pipe readers before it expires. + * A 250 ms bound lost the closed code under load (HACK-1350). + */ +export const NATIVE_EXIT_DRAIN_BOUND_MS = 5000; + export type NativeExitDiagnostic = { readonly exitCode: number; readonly nativeCode?: string; @@ -317,14 +331,15 @@ async function serveGraphProcess( } ); // Descendants may inherit a pipe after the owned receiver exits. Its exit, - // not unrelated writers' EOF, bounds draining; final graph inspection remains - // the caller's authority for cleanup, regardless of a successful exit code. + // not unrelated writers' EOF, bounds draining (see NATIVE_EXIT_DRAIN_BOUND_MS); + // final graph inspection remains the caller's authority for cleanup, + // regardless of a successful exit code. const drain = new AbortController(); const owner = new AbortController(); let drainTimer: ReturnType | undefined; void child.exited.then(() => { owner.abort(); - drainTimer = setTimeout(() => drain.abort(), 250); + drainTimer = setTimeout(() => drain.abort(), NATIVE_EXIT_DRAIN_BOUND_MS); }); const failureCodes = readNativeFailureCodes(child.stderr, drain.signal); const failureCode = failureCodes.then((failure) => failure?.code); diff --git a/tests/native-project-process.test.ts b/tests/native-project-process.test.ts index 49e17032..cc4e9195 100644 --- a/tests/native-project-process.test.ts +++ b/tests/native-project-process.test.ts @@ -2,7 +2,10 @@ import { afterEach, expect, test } from "bun:test"; import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { serveNativeProjectGraph } from "../src/backends/native-project-process.ts"; +import { + NATIVE_EXIT_DRAIN_BOUND_MS, + serveNativeProjectGraph, +} from "../src/backends/native-project-process.ts"; import { expectClosedRefusal } from "./helpers/closed-refusal.ts"; const roots: string[] = []; @@ -227,53 +230,221 @@ test("readiness rejection retains identity while observing safe native cleanup c expect(Bun.inspect(diagnostic)).not.toContain("synthetic-private-cleanup"); }); -test("owned exit completes without waiting for descendant-held stdout and stderr", async () => { - const opts = await fixture(` - const keeper = Bun.spawn(["/bin/sh", "-c", "sleep 3"], { - stdin: "ignore", stdout: "inherit", stderr: "inherit", detached: true, - }); - keeper.unref(); - console.log(JSON.stringify({kind:"graph_foreground_ready",run:"${run}"})); - console.error(JSON.stringify({code:"graph_owner_recovery",message:"synthetic-private-detail"})); - process.exit(0); +/** Slack for host load between the stand-in's recorded exit and settlement. */ +const DRAIN_SETTLE_MARGIN_MS = 5000; +/** Fixed, well above the former 250 ms window and well below the named bound. */ +const LATE_CODE_DELAY_MS = 1000; +/** A held descendant always retires by itself, even if this test process dies. */ +const DESCENDANT_CAP_MS = 20_000; +const SETTLEMENT_WAIT_MS = DESCENDANT_CAP_MS + 5000; + +async function waitForFile(path: string, budgetMs: number): Promise { + const deadline = performance.now() + budgetMs; + while (!(await Bun.file(path).exists())) { + if (performance.now() >= deadline) { + return false; + } + await Bun.sleep(10); + } + return true; +} + +function processPresent(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + if ( + error !== null && + typeof error === "object" && + "code" in error && + error.code === "ESRCH" + ) { + return false; + } + throw error; + } +} + +/** + * Run `body`, then release the stand-in's descendant through `release` and + * confirm it finished. An unconfirmed descendant retains the fixture root. + */ +async function settleDescendant(opts: { + readonly root: string; + readonly release: string; + readonly finished: string; + readonly body: () => Promise; +}): Promise { + let failed: unknown; + try { + await opts.body(); + } catch (error: unknown) { + failed = error; + } finally { + await Bun.write(join(opts.root, opts.release), "release"); + } + const completed = await waitForFile( + join(opts.root, opts.finished), + SETTLEMENT_WAIT_MS + ); + if (!completed) { + roots.splice(roots.indexOf(opts.root), 1); + throw new AggregateError( + failed === undefined ? [] : [failed], + `Owned descendant completion is unconfirmed; fixture retained at ${opts.root}.` + ); + } + if (failed !== undefined) { + throw failed; + } +} + +// Keeps the stand-in's stdout and stderr open until the test releases it. +const HOLDING_KEEPER = `import {existsSync, writeFileSync} from "node:fs"; +writeFileSync("keeper-pid", String(process.pid)); +const end = Date.now() + ${DESCENDANT_CAP_MS}; +while (!existsSync("release") && Date.now() < end) await Bun.sleep(20); +writeFileSync("keeper-finished", "finished");`; + +test.each([ + ["ready", 0], + ["early failure", 2], +] as const)( + "%s exit settles within the named drain bound while a descendant holds stdout and stderr", + async (mode, exitCode) => { + const ready = mode === "ready"; + const nativeCode = ready ? "graph_owner_recovery" : "graph_budget"; + const opts = await fixture(` + import { spawn } from "node:child_process"; + import { existsSync, writeFileSync } from "node:fs"; + spawn(process.execPath, ["--eval", ${JSON.stringify(HOLDING_KEEPER)}], { + detached: true, stdio: ["ignore", 1, 2], + }).unref(); + const deadline = Date.now() + 10000; + while (!existsSync("keeper-pid") && Date.now() < deadline) await Bun.sleep(10); + if (!existsSync("keeper-pid")) process.exit(3); + ${ready ? `console.log(JSON.stringify({kind:"graph_foreground_ready",run:"${run}"}));` : ""} + console.error(JSON.stringify({code:"${nativeCode}",message:"synthetic-private-detail"})); + writeFileSync("exited-at", String(Date.now())); + process.exit(${exitCode}); `); - let diagnostic: unknown; - const started = performance.now(); - const result = await serveNativeProjectGraph({ - ...opts, - onReady: async () => {}, - onExitDiagnostic: (value) => { - diagnostic = value; - }, - }); - expect(result).toBe(0); - expect(performance.now() - started).toBeLessThan(1500); - expect(diagnostic).toEqual({ - exitCode: 0, - nativeCode: "graph_owner_recovery", - }); - expect(Bun.inspect(diagnostic)).not.toContain("synthetic-private-detail"); -}, 6000); + const root = opts.projectRoot; + await settleDescendant({ + root, + release: "release", + finished: "keeper-finished", + body: async () => { + let diagnostic: unknown; + let readyCount = 0; + let result: number | undefined; + let caught: unknown; + try { + result = await serveNativeProjectGraph({ + ...opts, + startupTimeoutMs: 30_000, + onReady: async () => { + readyCount++; + }, + onExitDiagnostic: (value) => { + diagnostic = value; + }, + }); + } catch (error: unknown) { + caught = error; + } + const settledAt = Date.now(); + const exitedAt = Number(await Bun.file(join(root, "exited-at")).text()); + const keeper = Number(await Bun.file(join(root, "keeper-pid")).text()); + // The bound, not the descendant's EOF, ended the drain: it is still holding. + expect(processPresent(keeper)).toBe(true); + expect(await Bun.file(join(root, "keeper-finished")).exists()).toBe( + false + ); + expect(settledAt - exitedAt).toBeLessThan( + NATIVE_EXIT_DRAIN_BOUND_MS + DRAIN_SETTLE_MARGIN_MS + ); + if (ready) { + expect(caught).toBeUndefined(); + expect(result).toBe(0); + expect(readyCount).toBe(1); + } else { + expect(readyCount).toBe(0); + const refusal = await expectClosedRefusal( + Promise.reject(caught), + nativeCode + ); + expect(refusal.message).not.toContain("synthetic-private-detail"); + } + expect(diagnostic).toEqual({ exitCode, nativeCode }); + expect(Bun.inspect(diagnostic)).not.toContain( + "synthetic-private-detail" + ); + }, + }); + }, + 60_000 +); -test("early native failure retains its safe code when descendant pipes stay open", async () => { +// Writes the closed code on the stand-in's inherited stderr only after the +// test has observed the stand-in reaped, i.e. after its exit was reported. +const LATE_CODE_WRITER = `import {existsSync, writeFileSync} from "node:fs"; +writeFileSync("writer-pid", String(process.pid)); +const end = Date.now() + ${DESCENDANT_CAP_MS}; +while (!existsSync("exit-observed") && Date.now() < end) await Bun.sleep(10); +if (existsSync("exit-observed")) { + await Bun.sleep(${LATE_CODE_DELAY_MS}); + process.stderr.write(JSON.stringify({code:"graph_budget",message:"synthetic-private-detail"})); +} +writeFileSync("writer-finished", "finished");`; + +test("closed code written after the reported exit is kept regardless of a late reader", async () => { + expect(LATE_CODE_DELAY_MS * 2).toBeLessThanOrEqual( + NATIVE_EXIT_DRAIN_BOUND_MS + ); const opts = await fixture(` - const keeper = Bun.spawn(["/bin/sh", "-c", "sleep 3"], { - stdin: "ignore", stdout: "inherit", stderr: "inherit", detached: true, - }); - keeper.unref(); - console.error(JSON.stringify({code:"graph_budget",message:"synthetic-private-detail"})); - process.exit(2); + import { spawn } from "node:child_process"; + import { existsSync, writeFileSync } from "node:fs"; + spawn(process.execPath, ["--eval", ${JSON.stringify(LATE_CODE_WRITER)}], { + detached: true, stdio: ["ignore", "ignore", 2], + }).unref(); + const deadline = Date.now() + 10000; + while (!existsSync("writer-pid") && Date.now() < deadline) await Bun.sleep(10); + writeFileSync("child-pid", String(process.pid)); + process.exit(existsSync("writer-pid") ? 2 : 3); `); - let ready = false; - const started = performance.now(); - await expect( - serveNativeProjectGraph({ - ...opts, - onReady: async () => { - ready = true; - }, - }) - ).rejects.toThrow("graph_budget"); - expect(ready).toBe(false); - expect(performance.now() - started).toBeLessThan(1500); -}, 6000); + const root = opts.projectRoot; + await settleDescendant({ + root, + release: "exit-observed", + finished: "writer-finished", + body: async () => { + let diagnostic: unknown; + let readyCount = 0; + const outcome = serveNativeProjectGraph({ + ...opts, + startupTimeoutMs: 30_000, + onReady: async () => { + readyCount++; + }, + onExitDiagnostic: (value) => { + diagnostic = value; + }, + }); + void outcome.catch(() => undefined); + expect(await waitForFile(join(root, "child-pid"), 20_000)).toBe(true); + const child = Number(await Bun.file(join(root, "child-pid")).text()); + // Absence means the CLI reaped the stand-in: its exit has been reported. + const deadline = performance.now() + 20_000; + while (processPresent(child) && performance.now() < deadline) { + await Bun.sleep(10); + } + expect(processPresent(child)).toBe(false); + await Bun.write(join(root, "exit-observed"), "observed"); + const refusal = await expectClosedRefusal(outcome, "graph_budget"); + expect(refusal.message).not.toContain("synthetic-private-detail"); + expect(readyCount).toBe(0); + expect(diagnostic).toEqual({ exitCode: 2, nativeCode: "graph_budget" }); + }, + }); +}, 60_000); From 8dca82e62f5f105504b5bc6c99185b51828b75f9 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 01:13:08 -0400 Subject: [PATCH 2/4] fix(native): drain bounded runtime requests to EOF with the shared exit bound Bounded one-shot requests (status, logs, exec, one-off, read and recovery) stopped reading the child's stdout and stderr 250 ms after its exit. Under heavy oversubscription the closed native error code could arrive after that window, so it was dropped and a generic request failure was reported. Share NATIVE_EXIT_DRAIN_BOUND_MS (5 s, measured from the observed exit) from the runtime client with the foreground graph owner, which #396 moved off the same 250 ms window. EOF still ends a normal drain at once; the bound only ends a drain that a descendant holding a pipe keeps open. Both boundaries take an optional exitDrainBoundMs test seam that defaults to the constant. Tests: a stderr writer that emits the closed code only after the stand-in is reaped is kept for status, read, logs and exec (fails with a 250 ms window); the held-descendant cases now settle within the named bound plus a margin (fail with an unbounded drain). The graph owner's late-writer case injects a wide named bound and uses named handshake budgets so host load cannot flake it; its held-descendant cases keep the production bound. Refs HACK-1387 Co-Authored-By: Claude Opus 5.5 --- src/backends/native-project-process.ts | 28 ++-- src/backends/native-runtime-client.ts | 38 ++++- tests/native-project-process.test.ts | 118 +++++++++------- tests/native-runtime-client.test.ts | 184 ++++++++++++++++++++++++- 4 files changed, 295 insertions(+), 73 deletions(-) diff --git a/src/backends/native-project-process.ts b/src/backends/native-project-process.ts index ed3c467e..50b3bf91 100644 --- a/src/backends/native-project-process.ts +++ b/src/backends/native-project-process.ts @@ -12,6 +12,7 @@ import { import type { NativeAuthoredStorageTool } from "./native-authored-storage-tool.ts"; import { invokeNativeRuntime, + NATIVE_EXIT_DRAIN_BOUND_MS, type NativeRuntimeSelection, readNativeFailureCodes, rethrowNativeInputFailure, @@ -22,20 +23,6 @@ const RUN = /^[a-f0-9]{32}$/; const MAX_OUTPUT = 16 * 1024 * 1024; const MAX_INPUT = 256 * 1024; -/** - * Exit-relative bound on draining the owned child's stdout and stderr. - * - * The native runtime writes its closed error code (and any readiness line) - * before it exits, so those bytes are already in the pipes when the exit is - * observed; with no other writer, EOF follows immediately and the drain ends - * there. This bound only ends a drain that EOF cannot end: a descendant that - * inherited a pipe and keeps it open. It is measured from the observed exit, - * not from spawn, and is deliberately generous so that a heavily - * oversubscribed host still schedules the pipe readers before it expires. - * A 250 ms bound lost the closed code under load (HACK-1350). - */ -export const NATIVE_EXIT_DRAIN_BOUND_MS = 5000; - export type NativeExitDiagnostic = { readonly exitCode: number; readonly nativeCode?: string; @@ -61,6 +48,12 @@ type ProcessOptions = { readonly onStopFailure?: () => void; /** Durable capture in the existing frontend owner, before private input delivery. */ readonly onGroup?: (group: number) => Promise; + /** + * Test seam: how long after the owned child's observed exit a pipe a + * descendant still holds keeps being read. Production callers leave it at + * {@link NATIVE_EXIT_DRAIN_BOUND_MS}; EOF ends the drain earlier. + */ + readonly exitDrainBoundMs?: number; }; /** @@ -275,9 +268,11 @@ function nativeSnapshotReady( } function assertProcessSelection(opts: ProcessOptions): void { + const exitDrainBoundMs = opts.exitDrainBoundMs ?? NATIVE_EXIT_DRAIN_BOUND_MS; if ( !(RUN.test(opts.run) && Number.isSafeInteger(opts.startupTimeoutMs)) || opts.startupTimeoutMs < 1 || + !(Number.isSafeInteger(exitDrainBoundMs) && exitDrainBoundMs >= 1) || opts.startupTimeoutMs > 600_000 || (opts.privateInput?.byteLength ?? 0) > MAX_INPUT || canceledAtEntry(opts) @@ -339,7 +334,10 @@ async function serveGraphProcess( let drainTimer: ReturnType | undefined; void child.exited.then(() => { owner.abort(); - drainTimer = setTimeout(() => drain.abort(), NATIVE_EXIT_DRAIN_BOUND_MS); + drainTimer = setTimeout( + () => drain.abort(), + opts.exitDrainBoundMs ?? NATIVE_EXIT_DRAIN_BOUND_MS + ); }); const failureCodes = readNativeFailureCodes(child.stderr, drain.signal); const failureCode = failureCodes.then((failure) => failure?.code); diff --git a/src/backends/native-runtime-client.ts b/src/backends/native-runtime-client.ts index ac456115..30626dfa 100644 --- a/src/backends/native-runtime-client.ts +++ b/src/backends/native-runtime-client.ts @@ -90,6 +90,22 @@ export interface NativeRuntimeSelection { const MAX_OUTPUT_BYTES = 16 * 1024 * 1024; const MAX_PRIVATE_INPUT_BYTES = 256 * 1024; + +/** + * Exit-relative bound on draining an owned native child's stdout and stderr. + * + * The native runtime writes its closed error code (and any response or + * readiness line) before it exits, so those bytes are already in the pipes + * when the exit is observed; with no other writer, EOF follows immediately + * and the drain ends there. This bound only ends a drain that EOF cannot end: + * a descendant that inherited a pipe and keeps it open. It is measured from + * the observed exit, not from spawn, and is deliberately generous so that a + * heavily oversubscribed host still schedules the pipe readers before it + * expires. A 250 ms bound lost the closed code under load for the foreground + * graph owner (HACK-1350) and for bounded one-shot requests (HACK-1387). + * Shared by {@link invokeNativeRuntime} and the foreground graph owner. + */ +export const NATIVE_EXIT_DRAIN_BOUND_MS = 5000; const REQUEST_BUDGET_REFUSAL = "Native runtime request exceeds its input or time budget."; const SYSTEM_ENVIRONMENT = ["PATH", "HOME", "TMPDIR", "USER", "LOGNAME"]; @@ -137,6 +153,12 @@ export async function invokeNativeRuntime(opts: { readonly boundNativeAuthoredReadDrain?: boolean; /** Explicit recovery requests have their own closed argv domain and never accept private input. */ readonly boundNativeAuthoredRecoveryDrain?: "selection" | "cleanup"; + /** + * Test seam for a bound drain: how long after the child's observed exit a + * pipe a descendant still holds keeps being read. Production callers leave + * it at {@link NATIVE_EXIT_DRAIN_BOUND_MS}; EOF ends the drain earlier. + */ + readonly exitDrainBoundMs?: number; }): Promise { if (opts.signal?.aborted) { throw new NativeRuntimeRequestError({ @@ -146,6 +168,7 @@ export async function invokeNativeRuntime(opts: { } const timeoutMs = opts.timeoutMs ?? 180_000; const bound = boundDrainSelection(opts); + const exitDrainBoundMs = exitDrainBound(opts.exitDrainBoundMs); if ( !( validExecResponseSelection(opts.args, opts.serviceExecResponse) && @@ -153,7 +176,9 @@ export async function invokeNativeRuntime(opts: { opts.args, bound, opts.privateInput !== undefined - ) + ) && + Number.isSafeInteger(exitDrainBoundMs) && + exitDrainBoundMs >= 1 ) ) { // Internal invariant: callers build these argv shapes; no user input selects them. @@ -188,11 +213,15 @@ export async function invokeNativeRuntime(opts: { stderr: "pipe", } ); + // A descendant may inherit a pipe after the owned child exits. The child's + // exit, not that writer's EOF, bounds draining (see + // NATIVE_EXIT_DRAIN_BOUND_MS); its own closed code and response are already + // in the pipes at exit, so EOF normally ends the drain long before the bound. const drain = bound === undefined ? undefined : new AbortController(); let drainTimer: ReturnType | undefined; if (drain) { void child.exited.then(() => { - drainTimer = setTimeout(() => drain.abort(), 250); + drainTimer = setTimeout(() => drain.abort(), exitDrainBoundMs); }); } const failureDetails = readNativeFailure(child.stderr, drain?.signal); @@ -259,6 +288,11 @@ export async function invokeNativeRuntime(opts: { } } +/** The exit-relative drain bound in effect: the test seam's value, else the shared constant. */ +function exitDrainBound(selected: number | undefined): number { + return selected ?? NATIVE_EXIT_DRAIN_BOUND_MS; +} + /** One bound output drain per request; the public flags remain the callers' contract. */ type BoundDrain = | "status" diff --git a/tests/native-project-process.test.ts b/tests/native-project-process.test.ts index cc4e9195..4f90a425 100644 --- a/tests/native-project-process.test.ts +++ b/tests/native-project-process.test.ts @@ -2,10 +2,8 @@ import { afterEach, expect, test } from "bun:test"; import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { - NATIVE_EXIT_DRAIN_BOUND_MS, - serveNativeProjectGraph, -} from "../src/backends/native-project-process.ts"; +import { serveNativeProjectGraph } from "../src/backends/native-project-process.ts"; +import { NATIVE_EXIT_DRAIN_BOUND_MS } from "../src/backends/native-runtime-client.ts"; import { expectClosedRefusal } from "./helpers/closed-refusal.ts"; const roots: string[] = []; @@ -232,11 +230,26 @@ test("readiness rejection retains identity while observing safe native cleanup c /** Slack for host load between the stand-in's recorded exit and settlement. */ const DRAIN_SETTLE_MARGIN_MS = 5000; -/** Fixed, well above the former 250 ms window and well below the named bound. */ +/** + * Fixed delay before the descendant writes the closed code, measured from the + * test observing the stand-in reaped: well above the former 250 ms window. + */ const LATE_CODE_DELAY_MS = 1000; +/** + * Injected drain bound for the late-writer case: wide enough that host load + * cannot push the delayed write past it, so the case proves that EOF, not a + * fixed window, ends the drain. The held-descendant cases keep the production + * bound and prove that it still ends a drain EOF cannot. + */ +const LATE_WRITER_DRAIN_BOUND_MS = 25_000; +/** Budget for a handshake file (a pid) to appear on a loaded host. */ +const HANDSHAKE_BUDGET_MS = 20_000; +/** The stand-in's startup deadline; exit, not readiness, ends these cases. */ +const STARTUP_TIMEOUT_MS = 60_000; /** A held descendant always retires by itself, even if this test process dies. */ -const DESCENDANT_CAP_MS = 20_000; +const DESCENDANT_CAP_MS = 60_000; const SETTLEMENT_WAIT_MS = DESCENDANT_CAP_MS + 5000; +const CASE_TIMEOUT_MS = SETTLEMENT_WAIT_MS + STARTUP_TIMEOUT_MS; async function waitForFile(path: string, budgetMs: number): Promise { const deadline = performance.now() + budgetMs; @@ -321,7 +334,7 @@ test.each([ spawn(process.execPath, ["--eval", ${JSON.stringify(HOLDING_KEEPER)}], { detached: true, stdio: ["ignore", 1, 2], }).unref(); - const deadline = Date.now() + 10000; + const deadline = Date.now() + ${HANDSHAKE_BUDGET_MS}; while (!existsSync("keeper-pid") && Date.now() < deadline) await Bun.sleep(10); if (!existsSync("keeper-pid")) process.exit(3); ${ready ? `console.log(JSON.stringify({kind:"graph_foreground_ready",run:"${run}"}));` : ""} @@ -342,7 +355,7 @@ test.each([ try { result = await serveNativeProjectGraph({ ...opts, - startupTimeoutMs: 30_000, + startupTimeoutMs: STARTUP_TIMEOUT_MS, onReady: async () => { readyCount++; }, @@ -383,7 +396,7 @@ test.each([ }, }); }, - 60_000 + CASE_TIMEOUT_MS ); // Writes the closed code on the stand-in's inherited stderr only after the @@ -398,53 +411,58 @@ if (existsSync("exit-observed")) { } writeFileSync("writer-finished", "finished");`; -test("closed code written after the reported exit is kept regardless of a late reader", async () => { - expect(LATE_CODE_DELAY_MS * 2).toBeLessThanOrEqual( - NATIVE_EXIT_DRAIN_BOUND_MS - ); - const opts = await fixture(` +test( + "closed code written after the reported exit is kept regardless of a late reader", + async () => { + expect(LATE_CODE_DELAY_MS).toBeLessThan(LATE_WRITER_DRAIN_BOUND_MS); + const opts = await fixture(` import { spawn } from "node:child_process"; import { existsSync, writeFileSync } from "node:fs"; spawn(process.execPath, ["--eval", ${JSON.stringify(LATE_CODE_WRITER)}], { detached: true, stdio: ["ignore", "ignore", 2], }).unref(); - const deadline = Date.now() + 10000; + const deadline = Date.now() + ${HANDSHAKE_BUDGET_MS}; while (!existsSync("writer-pid") && Date.now() < deadline) await Bun.sleep(10); writeFileSync("child-pid", String(process.pid)); process.exit(existsSync("writer-pid") ? 2 : 3); `); - const root = opts.projectRoot; - await settleDescendant({ - root, - release: "exit-observed", - finished: "writer-finished", - body: async () => { - let diagnostic: unknown; - let readyCount = 0; - const outcome = serveNativeProjectGraph({ - ...opts, - startupTimeoutMs: 30_000, - onReady: async () => { - readyCount++; - }, - onExitDiagnostic: (value) => { - diagnostic = value; - }, - }); - void outcome.catch(() => undefined); - expect(await waitForFile(join(root, "child-pid"), 20_000)).toBe(true); - const child = Number(await Bun.file(join(root, "child-pid")).text()); - // Absence means the CLI reaped the stand-in: its exit has been reported. - const deadline = performance.now() + 20_000; - while (processPresent(child) && performance.now() < deadline) { - await Bun.sleep(10); - } - expect(processPresent(child)).toBe(false); - await Bun.write(join(root, "exit-observed"), "observed"); - const refusal = await expectClosedRefusal(outcome, "graph_budget"); - expect(refusal.message).not.toContain("synthetic-private-detail"); - expect(readyCount).toBe(0); - expect(diagnostic).toEqual({ exitCode: 2, nativeCode: "graph_budget" }); - }, - }); -}, 60_000); + const root = opts.projectRoot; + await settleDescendant({ + root, + release: "exit-observed", + finished: "writer-finished", + body: async () => { + let diagnostic: unknown; + let readyCount = 0; + const outcome = serveNativeProjectGraph({ + ...opts, + startupTimeoutMs: STARTUP_TIMEOUT_MS, + exitDrainBoundMs: LATE_WRITER_DRAIN_BOUND_MS, + onReady: async () => { + readyCount++; + }, + onExitDiagnostic: (value) => { + diagnostic = value; + }, + }); + void outcome.catch(() => undefined); + expect( + await waitForFile(join(root, "child-pid"), HANDSHAKE_BUDGET_MS * 2) + ).toBe(true); + const child = Number(await Bun.file(join(root, "child-pid")).text()); + // Absence means the CLI reaped the stand-in: its exit has been reported. + const deadline = performance.now() + HANDSHAKE_BUDGET_MS; + while (processPresent(child) && performance.now() < deadline) { + await Bun.sleep(10); + } + expect(processPresent(child)).toBe(false); + await Bun.write(join(root, "exit-observed"), "observed"); + const refusal = await expectClosedRefusal(outcome, "graph_budget"); + expect(refusal.message).not.toContain("synthetic-private-detail"); + expect(readyCount).toBe(0); + expect(diagnostic).toEqual({ exitCode: 2, nativeCode: "graph_budget" }); + }, + }); + }, + CASE_TIMEOUT_MS +); diff --git a/tests/native-runtime-client.test.ts b/tests/native-runtime-client.test.ts index b2feb07d..90ca3cf4 100644 --- a/tests/native-runtime-client.test.ts +++ b/tests/native-runtime-client.test.ts @@ -5,6 +5,7 @@ import { join } from "node:path"; import { invokeNativeRuntime, isNativeRuntimeNotRunning, + NATIVE_EXIT_DRAIN_BOUND_MS, NativeRuntimeRequestError, nativeCodesReportRuntimeNotRunning, resolveNativeRuntimeSelection, @@ -37,12 +38,24 @@ const KEEPER_RELEASE_CAP_MS = 120_000; /** Bun timeout for every case here: most spawn the stand-in runtime. */ const CASE_TIMEOUT_MS = 2 * HANDSHAKE_BUDGET_MS; /** - * Upper bound from the stand-in's exit (or the caller's abort) to the request - * settling while a descendant still holds stdout/stderr. The client stops - * draining 250 ms after exit; this is far above that, so load cannot flake it, - * and far below the request timeout, so an unbounded drain still fails. + * Slack for host load between the stand-in's recorded exit (or the caller's + * abort) and the request settling. Added to the named drain bound, the sum + * stays far below the request timeout, so an unbounded drain still fails. */ -const DRAIN_SETTLE_BOUND_MS = 5000; +const DRAIN_SETTLE_MARGIN_MS = 5000; +/** + * Fixed delay before a descendant writes the closed code on the stand-in's + * inherited stderr, measured from the test observing the stand-in reaped. It + * is well above the former 250 ms window, which dropped such a code. + */ +const LATE_CODE_DELAY_MS = 1000; +/** + * Injected drain bound for the late-writer cases: wide enough that host load + * cannot push the delayed write past it, so those cases prove that EOF, not a + * fixed window, ends the drain. The held-descendant cases keep the production + * bound and prove that it still ends a drain EOF cannot. + */ +const LATE_WRITER_DRAIN_BOUND_MS = 25_000; /** Every case gets a named budget; none relies on Bun's 5 s default. */ function test( @@ -69,6 +82,23 @@ async function waitForFile( } } +function processPresent(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + if ( + error !== null && + typeof error === "object" && + "code" in error && + error.code === "ESRCH" + ) { + return false; + } + throw error; + } +} + async function fixture() { const home = await mkdtemp(join(tmpdir(), "hack-native-client-")); roots.push(home); @@ -415,7 +445,7 @@ ${mode === "abort" ? "await Bun.sleep(60_000);" : `(await import('node:fs')).wri // though the unreleased keeper still holds both pipes. expect(settledAt).toBeDefined(); expect((settledAt ?? Number.POSITIVE_INFINITY) - endedAt).toBeLessThan( - DRAIN_SETTLE_BOUND_MS + NATIVE_EXIT_DRAIN_BOUND_MS + DRAIN_SETTLE_MARGIN_MS ); expect(await Bun.file(finished).exists()).toBe(false); const keeperPid = Number(await Bun.file(holding).text()); @@ -449,6 +479,148 @@ ${mode === "abort" ? "await Bun.sleep(60_000);" : `(await import('node:fs')).wri CASE_TIMEOUT_MS ); +const RUN_ID = "a".repeat(32); +const LATE_WRITER_SELECTIONS = { + status: { + args: [ + "graph", + "native", + "control", + "--run-id", + RUN_ID, + "--action", + "status", + "--json", + ], + flags: { boundNativeStatusDrain: true }, + }, + read: { + args: ["graph", "native", "inspect", "--run-id", RUN_ID, "--json"], + flags: { boundNativeAuthoredReadDrain: true }, + }, + logs: { + args: [ + "graph", + "native", + "logs", + "--run-id", + RUN_ID, + "--service", + "web", + "--tail", + "10", + "--json", + ], + flags: { boundNativeAuthoredLogsDrain: true }, + }, + exec: { + args: [ + "graph", + "native", + "exec", + "--run-id", + RUN_ID, + "--service", + "web", + "--json", + "--", + "true", + ], + flags: { boundNativeAuthoredExecDrain: true }, + }, +} as const; + +bunTest.each( + Object.keys(LATE_WRITER_SELECTIONS) as (keyof typeof LATE_WRITER_SELECTIONS)[] +)( + "authored %s keeps a closed code written after the reported exit", + async (selection) => { + const runtime = await fixture(); + const writerPid = join(runtime.home, "writer-pid"); + const childPid = join(runtime.home, "child-pid"); + const exitObserved = join(runtime.home, "exit-observed"); + const finished = join(runtime.home, "finished"); + // The writer inherits only stderr. It writes the closed code after the + // test has seen the stand-in reaped, i.e. after the client observed the + // exit, and then closes the pipe. A fixed post-exit window drops the code; + // draining to EOF keeps it. + const writer = `import {existsSync,writeFileSync} from 'node:fs'; +writeFileSync(${JSON.stringify(writerPid)},String(process.pid)); +const cap=performance.now()+${KEEPER_RELEASE_CAP_MS}; +while(!existsSync(${JSON.stringify(exitObserved)})&&performance.now() undefined); + let failed: unknown; + let completed = false; + try { + await waitForFile(childPid); + const child = Number(await Bun.file(childPid).text()); + expect(Number.isSafeInteger(child) && child > 1).toBe(true); + // Absence means the client reaped the stand-in: its exit was reported. + const deadline = performance.now() + HANDSHAKE_BUDGET_MS; + while (processPresent(child) && performance.now() < deadline) { + await Bun.sleep(10); + } + expect(processPresent(child)).toBe(false); + await Bun.write(exitObserved, "observed"); + const error: unknown = await request.catch((caught: unknown) => caught); + expect(error).toBeInstanceOf(NativeRuntimeRequestError); + expect(error).toMatchObject({ nativeCode: "graph_budget" }); + expect(String(error)).toContain( + "Native runtime request failed (graph_budget)" + ); + expect(String(error)).not.toContain("synthetic-secret-diagnostic"); + } catch (error: unknown) { + failed = error; + } finally { + await Bun.write(exitObserved, "observed"); + await request.catch(() => undefined); + completed = await waitForFile(finished).then( + () => true, + () => false + ); + if (!completed) { + roots.splice(roots.indexOf(runtime.home), 1); + } + } + if (!completed) { + throw new AggregateError( + failed === undefined ? [] : [failed], + "owned writer completion is unconfirmed; fixture retained" + ); + } + if (failed !== undefined) { + throw failed; + } + }, + CASE_TIMEOUT_MS +); + test("managed input uses stdin with EOF and is absent from argv and inherited env", async () => { const runtime = await fixture(); const prior = process.env.HACK_RUNTIME_CLIENT_CANARY; From e4be689ba0f7a4ff452951fc5fc389842e7c5352 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 02:33:32 -0400 Subject: [PATCH 3/4] test(native): hold inherited pipes past the shared drain bound in logs and exec refusals The logs and exec "inherited pipe refuses output" cases spawned a keeper that slept 800 ms: longer than the former 250 ms window, but inside the shared NATIVE_EXIT_DRAIN_BOUND_MS, so its EOF now settled the output and the requests resolved instead of refusing. The keeper now holds the inherited streams until the test releases it after the refusal (self-capped at 60 s), so the drain can only end at the bound, and the case budgets derive from that bound. Refs HACK-1387 Co-Authored-By: Claude Opus 5.5 --- tests/native-authored-project-exec.test.ts | 23 ++++++--- tests/native-authored-project-logs.test.ts | 57 +++++++++++++--------- 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/tests/native-authored-project-exec.test.ts b/tests/native-authored-project-exec.test.ts index 7ded9eff..44c15ff1 100644 --- a/tests/native-authored-project-exec.test.ts +++ b/tests/native-authored-project-exec.test.ts @@ -21,7 +21,10 @@ import { saveNativeAuthoredProjectRun, withNativeAuthoredProjectStatus, } from "../src/backends/native-authored-project-run.ts"; -import { invokeNativeRuntime } from "../src/backends/native-runtime-client.ts"; +import { + invokeNativeRuntime, + NATIVE_EXIT_DRAIN_BOUND_MS, +} from "../src/backends/native-runtime-client.ts"; import { tryNativeAuthoredCommand } from "../src/lib/native-authored-command.ts"; import type { NativeComposeCommandOptions } from "../src/lib/native-compose-command.ts"; import { captureCompletedJobFixtureCommand } from "./e2e/scenarios/native-compose-adoption-job-worktrees.ts"; @@ -34,8 +37,8 @@ import { const DEFAULT_BUDGET_MS = 4000; /** The cancelled request child sleeps this long after writing its identity. */ const REQUEST_SLEEP_MS = 3000; -/** The inherited-stream keeper child lives this long after its parent exits. */ -const KEEPER_SLEEP_MS = 800; +/** An inherited-stream keeper its case never releases still retires after this cap. */ +const KEEPER_HOLD_CAP_MS = 60_000; /** Explicit keeper join allowance on top of its own lifetime. */ const KEEPER_JOIN_MS = 3000; @@ -596,10 +599,15 @@ test( "exited exec request joins finite keeper", async () => { const f = await fixture(), - marker = join(f.runtime.home, "keeper.pid"); + marker = join(f.runtime.home, "keeper.pid"), + release = join(f.runtime.home, "keeper.release"); + // The keeper holds the inherited streams until the test releases it + // after the refusal, so the output cannot settle to EOF and the + // exit-relative drain bound (NATIVE_EXIT_DRAIN_BOUND_MS) ends the drain. + const keeper = `import {existsSync} from 'node:fs';const end=Date.now()+${KEEPER_HOLD_CAP_MS};while(!existsSync(${JSON.stringify(release)})&&Date.now() { - return owned(async () => { - const f = await fixture(), - pidPath = join(f.runtime.home, "keeper.pid"); - await Bun.write( - f.runtime.binary, - `#!${process.execPath}\nconst c=Bun.spawn([process.execPath,'--eval','await Bun.sleep(800)'],{stdin:'ignore',stdout:'inherit',stderr:'inherit'});await Bun.write(${JSON.stringify(pidPath)},String(c.pid));console.log('{}');process.exit(0);\n` - ); - try { - await expect( - invokeNativeRuntime({ - ...f, - cwd: f.scope.projectRoot, - args: f.args, - boundNativeAuthoredLogsDrain: true, - }) - ).rejects.toThrow("did not settle"); - } finally { - await joinKeeper(pidPath); - } - }); -}, 10_000); +/** An inherited-pipe keeper its case never releases still retires after this cap. */ +const KEEPER_HOLD_CAP_MS = 60_000; +test( + "leader exit with an inherited pipe refuses output; the exact finite keeper is joined", + async () => { + return owned(async () => { + const f = await fixture(), + pidPath = join(f.runtime.home, "keeper.pid"), + release = join(f.runtime.home, "keeper.release"); + // The keeper holds the inherited stdout and stderr until the test releases + // it after the refusal, so the leader's output cannot settle to EOF and the + // exit-relative drain bound (NATIVE_EXIT_DRAIN_BOUND_MS) ends the drain. + const keeper = `import {existsSync} from 'node:fs';const end=Date.now()+${KEEPER_HOLD_CAP_MS};while(!existsSync(${JSON.stringify(release)})&&Date.now() { return owned(async () => { From daac38daf7c1eafda960384ec1ca18042c627ad5 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Fri, 23 Oct 2026 11:07:52 -0400 Subject: [PATCH 4/4] test(native): wait for a complete pid in late-writer handshakes The late-writer cases waited only for the stand-in's pid file to exist and then read it. writeFileSync creates the file before it writes the digits, so under load the read could return "": Number("") is 0, kill(0, 0) always succeeds, and the reaped-stand-in wait expired. Both files now read pid handshakes through waitForPid, which waits for a safe-integer pid above 1, and the late-writer stand-ins publish their pid as a torn handshake (empty file, 50 ms gap, then the pid) so an existence-only reader fails every run. The keeper pid reads use the same helper. Refs HACK-1387 Co-Authored-By: Claude Opus 5.5 --- tests/native-project-process.test.ts | 56 +++++++++++++++++++++++++--- tests/native-runtime-client.test.ts | 47 ++++++++++++++++++++--- 2 files changed, 91 insertions(+), 12 deletions(-) diff --git a/tests/native-project-process.test.ts b/tests/native-project-process.test.ts index 4f90a425..5d007846 100644 --- a/tests/native-project-process.test.ts +++ b/tests/native-project-process.test.ts @@ -242,6 +242,12 @@ const LATE_CODE_DELAY_MS = 1000; * bound and prove that it still ends a drain EOF cannot. */ const LATE_WRITER_DRAIN_BOUND_MS = 25_000; +/** + * Gap between creating the stand-in's pid file and writing its digits. It + * makes the empty-file window that a loaded host opens by chance happen on + * every run, so a reader that only waits for existence fails every time. + */ +const TORN_HANDSHAKE_GAP_MS = 50; /** Budget for a handshake file (a pid) to appear on a loaded host. */ const HANDSHAKE_BUDGET_MS = 20_000; /** The stand-in's startup deadline; exit, not readiness, ends these cases. */ @@ -262,6 +268,33 @@ async function waitForFile(path: string, budgetMs: number): Promise { return true; } +/** + * Wait until a fixture's pid handshake holds a complete pid. A fixture's + * writeFileSync creates the file before it writes the digits, so a read that + * only waited for existence can see "": Number("") is 0, and kill(0, 0) + * always succeeds, so the absence wait would never end. Returns undefined + * when no complete pid appears within the budget. + */ +async function waitForPid( + path: string, + budgetMs: number +): Promise { + const deadline = performance.now() + budgetMs; + for (;;) { + const file = Bun.file(path); + if (await file.exists()) { + const pid = Number(await file.text()); + if (Number.isSafeInteger(pid) && pid > 1) { + return pid; + } + } + if (performance.now() >= deadline) { + return undefined; + } + await Bun.sleep(10); + } +} + function processPresent(pid: number): boolean { try { process.kill(pid, 0); @@ -368,9 +401,13 @@ test.each([ } const settledAt = Date.now(); const exitedAt = Number(await Bun.file(join(root, "exited-at")).text()); - const keeper = Number(await Bun.file(join(root, "keeper-pid")).text()); + const keeper = await waitForPid( + join(root, "keeper-pid"), + HANDSHAKE_BUDGET_MS + ); + expect(keeper).toBeDefined(); // The bound, not the descendant's EOF, ended the drain: it is still holding. - expect(processPresent(keeper)).toBe(true); + expect(processPresent(keeper ?? 0)).toBe(true); expect(await Bun.file(join(root, "keeper-finished")).exists()).toBe( false ); @@ -423,6 +460,10 @@ test( }).unref(); const deadline = Date.now() + ${HANDSHAKE_BUDGET_MS}; while (!existsSync("writer-pid") && Date.now() < deadline) await Bun.sleep(10); + // Publish the pid in two steps, as a torn handshake: the reader must wait + // for a complete pid, not just for the file to exist. + writeFileSync("child-pid", ""); + await Bun.sleep(${TORN_HANDSHAKE_GAP_MS}); writeFileSync("child-pid", String(process.pid)); process.exit(existsSync("writer-pid") ? 2 : 3); `); @@ -446,10 +487,13 @@ test( }, }); void outcome.catch(() => undefined); - expect( - await waitForFile(join(root, "child-pid"), HANDSHAKE_BUDGET_MS * 2) - ).toBe(true); - const child = Number(await Bun.file(join(root, "child-pid")).text()); + const child = await waitForPid( + join(root, "child-pid"), + HANDSHAKE_BUDGET_MS * 2 + ); + if (child === undefined) { + throw new Error("Stand-in pid handshake did not complete."); + } // Absence means the CLI reaped the stand-in: its exit has been reported. const deadline = performance.now() + HANDSHAKE_BUDGET_MS; while (processPresent(child) && performance.now() < deadline) { diff --git a/tests/native-runtime-client.test.ts b/tests/native-runtime-client.test.ts index 90ca3cf4..2710cb20 100644 --- a/tests/native-runtime-client.test.ts +++ b/tests/native-runtime-client.test.ts @@ -56,6 +56,12 @@ const LATE_CODE_DELAY_MS = 1000; * bound and prove that it still ends a drain EOF cannot. */ const LATE_WRITER_DRAIN_BOUND_MS = 25_000; +/** + * Gap between creating a stand-in's pid file and writing its digits. It makes + * the empty-file window that a loaded host opens by chance happen on every + * run, so a reader that only waits for existence fails every time. + */ +const TORN_HANDSHAKE_GAP_MS = 50; /** Every case gets a named budget; none relies on Bun's 5 s default. */ function test( @@ -82,6 +88,34 @@ async function waitForFile( } } +/** + * Wait until a fixture's pid handshake holds a complete pid. A fixture's + * writeFileSync creates the file before it writes the digits, so a read that + * only waited for existence can see "": Number("") is 0, and kill(0, 0) + * always succeeds, so an absence wait on it would never end. + */ +async function waitForPid( + path: string, + budgetMs = HANDSHAKE_BUDGET_MS +): Promise { + const deadline = performance.now() + budgetMs; + for (;;) { + const file = Bun.file(path); + if (await file.exists()) { + const pid = Number(await file.text()); + if (Number.isSafeInteger(pid) && pid > 1) { + return pid; + } + } + if (performance.now() >= deadline) { + throw new Error( + `Fixture pid handshake did not complete within ${budgetMs} ms.` + ); + } + await Bun.sleep(10); + } +} + function processPresent(pid: number): boolean { try { process.kill(pid, 0); @@ -448,8 +482,7 @@ ${mode === "abort" ? "await Bun.sleep(60_000);" : `(await import('node:fs')).wri NATIVE_EXIT_DRAIN_BOUND_MS + DRAIN_SETTLE_MARGIN_MS ); expect(await Bun.file(finished).exists()).toBe(false); - const keeperPid = Number(await Bun.file(holding).text()); - expect(Number.isSafeInteger(keeperPid) && keeperPid > 1).toBe(true); + const keeperPid = await waitForPid(holding); expect(() => process.kill(keeperPid, 0)).not.toThrow(); } catch (error: unknown) { failed = error; @@ -543,7 +576,9 @@ bunTest.each( // The writer inherits only stderr. It writes the closed code after the // test has seen the stand-in reaped, i.e. after the client observed the // exit, and then closes the pipe. A fixed post-exit window drops the code; - // draining to EOF keeps it. + // draining to EOF keeps it. The stand-in publishes its pid as a torn + // handshake (an empty file first), so the reader must wait for a complete + // pid rather than for the file to exist. const writer = `import {existsSync,writeFileSync} from 'node:fs'; writeFileSync(${JSON.stringify(writerPid)},String(process.pid)); const cap=performance.now()+${KEEPER_RELEASE_CAP_MS}; @@ -561,6 +596,8 @@ import {existsSync,writeFileSync} from 'node:fs'; spawn(${JSON.stringify(process.execPath)},['--eval',${JSON.stringify(writer)}],{detached:true,stdio:['ignore','ignore',2]}).unref(); const cap=performance.now()+${HANDSHAKE_BUDGET_MS}; while(!existsSync(${JSON.stringify(writerPid)})&&performance.now() 1).toBe(true); + const child = await waitForPid(childPid); // Absence means the client reaped the stand-in: its exit was reported. const deadline = performance.now() + HANDSHAKE_BUDGET_MS; while (processPresent(child) && performance.now() < deadline) {