From 67753a727ed180f9828abdbba7accdcaec534b99 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 20 Oct 2026 11:14:47 -0400 Subject: [PATCH 1/7] test(runtime-core): bind relay publisher test ports in the publisher The stream relay contract probed a free loopback port by binding 127.0.0.1:0 and dropping the listener before the publisher bound it. Under parallel tests another socket could take the port first; the publisher then failed bind() and exited 73 (EX_CANTCREAT) before writing its readiness line, which the helper reported as "readiness EOF" (CI run 39284084093). `stream-relay --publish 0` now binds a kernel-chosen loopback port and reports it as "ready \n". Explicit ports and Unix frontends keep the bare "ready\n" line, and port 0 stays refused (64) in every other form, so existing callers are unchanged. Every test that used the released probe now publishes on port 0 or holds the port it needs, and the control-startup test drops its exit-73 retry. Refs HACK-1284 Co-Authored-By: Claude Opus 5.5 --- packages/runtime-core/guest/stream-relay.c | 27 ++- .../tests/stream_relay_contract.rs | 208 +++++++++++++----- 2 files changed, 178 insertions(+), 57 deletions(-) diff --git a/packages/runtime-core/guest/stream-relay.c b/packages/runtime-core/guest/stream-relay.c index 2c76a4cc0..a5b5ebb89 100644 --- a/packages/runtime-core/guest/stream-relay.c +++ b/packages/runtime-core/guest/stream-relay.c @@ -228,8 +228,22 @@ static int stop_owned(long pid, long start) { return 78; #endif } +/* + * Publisher readiness contract: after the frontend and optional control socket + * are bound, the publisher writes exactly one readiness line to stdout and keeps + * serving. A Unix frontend or an explicit TCP port (1-65535) writes "ready\n", + * unchanged for existing callers. `--publish 0` (exactly "0") asks the kernel for + * an ephemeral loopback port while the listener is bound, closing the window in + * which a caller-probed free port can be taken by another process; that form + * writes "ready \n" with the bound decimal port. Port 0 is accepted only by + * `--publish`; the relay and `--publish-unix` forms keep rejecting it with 64. + * Exit 73 still means the frontend could not be bound (or a bound Unix frontend + * path could not be verified), and no readiness line is written for any refused + * startup. + */ int main(int argc, char **argv) { char *publish_args[8]; + int ephemeral_port = 0; struct sockaddr_un control = {0}; struct stat control_owned, receipt_owned, frontend_receipt_owned; char receipt_path[PATH_MAX] = {0}; @@ -263,7 +277,9 @@ int main(int argc, char **argv) { } /* Both publishers share upstream parsing; the Unix frontend has no TCP port. */ publish_args[0] = argv[0]; publish_args[1] = argv[3]; - publish_args[2] = "127.0.0.1"; publish_args[3] = publish_unix ? "1" : argv[2]; + ephemeral_port = !publish_unix && !strcmp(argv[2], "0"); + publish_args[2] = "127.0.0.1"; + publish_args[3] = publish_unix || ephemeral_port ? "1" : argv[2]; publish_args[4] = argv[5]; publish_args[5] = "--reservation"; publish_args[6] = argv[4]; publish_args[7] = NULL; argv = publish_args; argc = 7; publish_mode = 1; @@ -297,7 +313,7 @@ int main(int argc, char **argv) { } if (argc != option) return 64; target.sin_family = AF_INET; - target.sin_port = htons((uint16_t)port); + target.sin_port = ephemeral_port ? 0 : htons((uint16_t)port); local.sun_family = AF_UNIX; memcpy(local.sun_path, argv[1], strlen(argv[1]) + 1); if (publish_mode) { upstream = local; if (private_upstream(¤t)) return 78; } @@ -338,7 +354,12 @@ int main(int argc, char **argv) { receipt_path, &receipt_owned, &receipt_bound)) failed = 1; } } - if (!failed) { puts("ready"); fflush(stdout); } + struct sockaddr_in bound = {0}; + socklen_t bound_size = sizeof(bound); + if (!failed && ephemeral_port && (getsockname(listener, (struct sockaddr *)&bound, &bound_size) || + bound_size != sizeof(bound) || bound.sin_family != AF_INET || !bound.sin_port)) failed = 1; + if (!failed && ephemeral_port) { printf("ready %u\n", (unsigned)ntohs(bound.sin_port)); fflush(stdout); } + else if (!failed) { puts("ready"); fflush(stdout); } while (!stopping && !failed) { struct pollfd descriptors[4 + 2 * CONNECTIONS]; descriptors[3 + 2 * CONNECTIONS] = (struct pollfd){control_fd, POLLIN, 0}; diff --git a/packages/runtime-core/tests/stream_relay_contract.rs b/packages/runtime-core/tests/stream_relay_contract.rs index 6c72f5264..476abd41f 100644 --- a/packages/runtime-core/tests/stream_relay_contract.rs +++ b/packages/runtime-core/tests/stream_relay_contract.rs @@ -23,6 +23,8 @@ struct Relay { child: Child, root: PathBuf, socket: PathBuf, + /// The TCP port a `--publish` publisher reported or was given; None otherwise. + port: Option, } impl Relay { fn start(port: u16, idle: u32) -> Self { @@ -93,6 +95,7 @@ impl Relay { child, root, socket, + port: None, } } fn connect(&self) -> UnixStream { @@ -576,9 +579,6 @@ fn malformed_reservations_refuse_before_socket_creation() { fs::remove_dir_all(root).unwrap(); } -fn publisher(socket: &std::path::Path, token: &str, port: u16) -> Relay { - controlled_publisher(socket, token, port, None) -} fn controlled_publisher( socket: &std::path::Path, token: &str, @@ -676,9 +676,15 @@ fn try_frontend_publisher( startup.child = Some(command.stdout(Stdio::piped()).spawn().unwrap()); let mut output = startup.child.as_mut().unwrap().stdout.take().unwrap(); let deadline = Instant::now() + Duration::from_secs(3); - let expected = b"ready\n"; + // Port 0 asks a TCP publisher to bind an ephemeral port and report it as + // "ready \n"; every other form keeps the bare "ready\n" line. + let ephemeral = port == 0 && !unix; + let mut line = Vec::with_capacity(READINESS_LIMIT); // Read one byte per readiness notification: no unbounded read_line after POLLIN. - for expected_byte in expected { + while line.last() != Some(&b'\n') { + if line.len() == READINESS_LIMIT { + return Err(startup.failure("readiness marker")); + } loop { let remaining = deadline.saturating_duration_since(Instant::now()); if remaining.is_zero() { @@ -705,21 +711,57 @@ fn try_frontend_publisher( } let mut byte = [0]; match output.read(&mut byte) { - Ok(1) if byte[0] == *expected_byte => break, + Ok(1) => { + line.push(byte[0]); + break; + } Err(error) if error.kind() == std::io::ErrorKind::Interrupted => continue, Ok(0) => return Err(startup.failure("readiness EOF")), _ => return Err(startup.failure("readiness marker")), } } } + let published = match (ephemeral, line.as_slice()) { + (false, b"ready\n") => (!unix).then_some(port), + (true, reported) => match parse_reported_port(reported) { + Some(reported) => Some(reported), + None => return Err(startup.failure("readiness marker")), + }, + _ => return Err(startup.failure("readiness marker")), + }; let root = std::mem::take(&mut startup.root); Ok(Relay { child: startup.child.take().unwrap(), socket: root.join(if unix { "frontend.sock" } else { "unused" }), root, + port: published, }) } +/// "ready 65535\n" is the longest valid readiness line. +const READINESS_LIMIT: usize = 12; + +/// Parses "ready \n" with a canonical, nonzero decimal port. +fn parse_reported_port(line: &[u8]) -> Option { + let digits = line.strip_prefix(b"ready ")?.strip_suffix(b"\n")?; + if digits.is_empty() || digits[0] == b'0' || !digits.iter().all(u8::is_ascii_digit) { + return None; + } + std::str::from_utf8(digits).ok()?.parse::().ok() +} + +/// Starts a TCP publisher on a kernel-chosen port that stays bound from the +/// publisher's bind() onward, so parallel tests cannot take it first. +fn ephemeral_publisher( + socket: &std::path::Path, + token: &str, + control_token: Option<&str>, +) -> (Relay, u16) { + let publication = controlled_publisher(socket, token, 0, control_token); + let port = publication.port.expect("TCP publication reports its port"); + (publication, port) +} + #[test] fn publisher_startup_reports_port_conflict_and_reaps_child() { let backend = TcpListener::bind("127.0.0.1:0").unwrap(); @@ -758,12 +800,73 @@ fn publisher_startup_reports_port_conflict_and_reaps_child() { assert!(upstream.socket.exists()); } -fn free_loopback_port() -> u16 { - TcpListener::bind("127.0.0.1:0") - .unwrap() - .local_addr() - .unwrap() - .port() +#[test] +fn ephemeral_publication_reports_its_bound_port_and_explicit_ports_keep_bare_ready() { + let backend = TcpListener::bind("127.0.0.1:0").unwrap(); + let token = "dddddddddddddddddddddddddddddddd"; + let guard = Relay::start_options( + backend.local_addr().unwrap().port(), + 10000, + None, + Some(token), + None, + ); + // Only the exact `--publish 0` form selects an ephemeral port. + for (mode, endpoint) in [("--publish", "00"), ("--publish-unix", "0")] { + let refused = Command::new(BINARY) + .args([mode, endpoint]) + .arg(&guard.socket) + .args([token, "10000"]) + .stdin(Stdio::null()) + .output() + .unwrap(); + assert_eq!(refused.status.code(), Some(64), "{mode} {endpoint}"); + assert!(refused.stdout.is_empty()); + } + let refused = Command::new(BINARY) + .arg(guard.root.join("relay-port-zero.sock")) + .args(["127.0.0.1", "0", "1000"]) + .stdin(Stdio::null()) + .output() + .unwrap(); + assert_eq!(refused.status.code(), Some(64)); + assert!(!guard.root.join("relay-port-zero.sock").exists()); + + let (mut first, first_port) = ephemeral_publisher(&guard.socket, token, None); + let (mut second, second_port) = ephemeral_publisher(&guard.socket, token, None); + assert_ne!(first_port, second_port); + for port in [first_port, second_port] { + // The reported port is the publisher's live listener, not a released probe. + assert_eq!( + TcpListener::bind(("127.0.0.1", port)).unwrap_err().kind(), + io::ErrorKind::AddrInUse + ); + std::net::TcpStream::connect(("127.0.0.1", port)).unwrap(); + } + first.stop(); + second.stop(); + + // Explicit ports keep the bare "ready\n" line for existing callers. Picking a + // concrete port needs a released probe, so retry only on the publisher's + // confirmed bind refusal (no readiness line, exit 73) and never otherwise. + let mut attempts = 0; + let explicit = loop { + attempts += 1; + let probe = TcpListener::bind("127.0.0.1:0").unwrap(); + let port = probe.local_addr().unwrap().port(); + drop(probe); + match try_frontend_publisher(&guard.socket, token, port, None, false) { + Ok(publication) => break publication, + Err(failure) + if attempts < 3 + && failure.stage == "readiness EOF" + && failure.status.and_then(|status| status.code()) == Some(73) => {} + Err(failure) => panic!("explicit-port publisher refused: {failure:?}"), + } + }; + let port = explicit.port.expect("explicit TCP port recorded"); + std::net::TcpStream::connect(("127.0.0.1", port)).unwrap(); + assert!(guard.socket.exists()); } #[test] @@ -777,8 +880,7 @@ fn loopback_publication_streams_without_exposing_handshake_and_releases_port() { Some(token), None, ); - let port = free_loopback_port(); - let mut publication = publisher(&guard.socket, token, port); + let (mut publication, port) = ephemeral_publisher(&guard.socket, token, None); let payload = vec![0x5a; 2 * 1024 * 1024]; let expected = payload.clone(); let worker = thread::spawn(move || { @@ -835,8 +937,7 @@ fn publisher_refuses_port_conflicts_and_stale_reused_upstream() { Some(73) ); assert!(std::net::TcpStream::connect(("127.0.0.1", busy_port)).is_ok()); - let port = free_loopback_port(); - let _publication = publisher(&guard.socket, old, port); + let (_publication, port) = ephemeral_publisher(&guard.socket, old, None); guard.stop(); let _replacement = Relay::start_options( backend.local_addr().unwrap().port(), @@ -915,17 +1016,27 @@ fn publisher_refuses_nonprivate_or_aliased_upstream_before_listening() { fs::create_dir(&root).unwrap(); let path = root.join("socket"); let _listener = UnixListener::bind(&path).unwrap(); - let port = free_loopback_port(); + // Hold the requested port for the whole test: a publisher that reached bind() + // would exit 73 on it, so exit 78 proves the refusal precedes listening without + // probing a released port that a parallel test could have taken. + let occupied = TcpListener::bind("127.0.0.1:0").unwrap(); + occupied.set_nonblocking(true).unwrap(); + let port = occupied.local_addr().unwrap().port(); let attempt = |upstream: &std::path::Path| { let result = Command::new(BINARY) .arg("--publish") .arg(port.to_string()) .arg(upstream) .args(["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "10000"]) - .status() + .stdin(Stdio::null()) + .output() .unwrap(); - assert_eq!(result.code(), Some(78)); - assert!(std::net::TcpStream::connect(("127.0.0.1", port)).is_err()); + assert_eq!(result.status.code(), Some(78)); + assert!(result.stdout.is_empty()); + assert_eq!( + occupied.accept().unwrap_err().kind(), + std::io::ErrorKind::WouldBlock + ); }; fs::set_permissions(&path, fs::Permissions::from_mode(0o777)).unwrap(); attempt(&path); @@ -955,8 +1066,7 @@ fn private_control_stops_only_matching_publisher_and_preserves_replaced_path() { fs::set_permissions(&upstream, fs::Permissions::from_mode(0o700)).unwrap(); let token = "aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"; for replace in [false, true] { - let port = free_loopback_port(); - let mut publication = controlled_publisher(&upstream, token, port, Some(token)); + let (mut publication, port) = ephemeral_publisher(&upstream, token, Some(token)); let path = publication.root.join("control.sock"); assert_eq!( fs::metadata(&path).unwrap().permissions().mode() & 0o777, @@ -1030,44 +1140,34 @@ fn control_startup_refuses_occupied_and_nonprivate_paths_without_retaining_publi fs::write(&control, b"foreign").unwrap(); let control_identity = fs::symlink_metadata(&control).unwrap(); let frontend = root.join("frontend"); - // The TCP leg publishes on a port that free_loopback_port() has already - // released, so a parallel test may re-take it before the relay binds and the - // relay then exits 73 at bind instead of reaching the control path. Retry a - // bounded number of times on a fresh port; the 70/78 expectations stay exact. - const PORT_ATTEMPTS: u32 = 3; for unix in [false, true] { for (mode, expected_exit) in [(0o700, 70), (0o755, 78)] { fs::set_permissions(&root, fs::Permissions::from_mode(mode)).unwrap(); - let mut attempt = 1; - let (pid, output) = loop { - let mut command = Command::new(BINARY); - command - .arg(if unix { "--publish-unix" } else { "--publish" }) - .arg(if unix { - frontend.as_os_str().to_owned() - } else { - free_loopback_port().to_string().into() - }) - .arg(&upstream) - .args(["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "10000", "--control"]) - .arg(&control) - .arg("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::piped()); - let child = command.spawn().unwrap(); - let pid = child.id(); - let output = child.wait_with_output().unwrap(); - if !unix && output.status.code() == Some(73) && attempt < PORT_ATTEMPTS { - attempt += 1; - continue; - } - break (pid, output); - }; + let mut command = Command::new(BINARY); + command + .arg(if unix { "--publish-unix" } else { "--publish" }) + // TCP port 0 binds an ephemeral port inside the publisher, so the TCP + // leg always reaches the control path instead of racing a parallel + // test for a released port and exiting 73 at bind. + .arg(if unix { + frontend.as_os_str().to_owned() + } else { + "0".into() + }) + .arg(&upstream) + .args(["aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa", "10000", "--control"]) + .arg(&control) + .arg("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb") + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()); + let child = command.spawn().unwrap(); + let pid = child.id(); + let output = child.wait_with_output().unwrap(); assert_eq!( output.status.code(), Some(expected_exit), - "unix={unix} mode={mode:o} attempt={attempt}" + "unix={unix} mode={mode:o}" ); assert!(output.stdout.is_empty()); let mut status = 0; From 524d07e2de8f969120d073343935f6e84b4ce767 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 20 Oct 2026 15:06:10 -0400 Subject: [PATCH 2/7] test(runtime-core): bound the refusal waits in the ephemeral publication contract The port-0 refusal cases waited on the child with output(), so a publisher that wrongly admitted "--publish 00" kept serving and hung the test run instead of failing it (found by mutating the exact-"0" check). Poll the child against a 3s deadline, then kill and reap it and fail with the admitted command. Refs HACK-1284 Co-Authored-By: Claude Opus 5.5 --- .../tests/stream_relay_contract.rs | 62 ++++++++++++++----- 1 file changed, 46 insertions(+), 16 deletions(-) diff --git a/packages/runtime-core/tests/stream_relay_contract.rs b/packages/runtime-core/tests/stream_relay_contract.rs index 476abd41f..02584c656 100644 --- a/packages/runtime-core/tests/stream_relay_contract.rs +++ b/packages/runtime-core/tests/stream_relay_contract.rs @@ -800,6 +800,37 @@ fn publisher_startup_reports_port_conflict_and_reaps_child() { assert!(upstream.socket.exists()); } +/// Runs a command that must refuse promptly and returns its exit code and stdout. +/// A child that is still running after the deadline was admitted instead of +/// refused; it is killed and reaped so the test fails rather than hangs. +fn bounded_refusal(command: &mut Command) -> (Option, Vec) { + let mut child = command + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .spawn() + .unwrap(); + let deadline = Instant::now() + Duration::from_secs(3); + let status = loop { + if let Some(status) = child.try_wait().unwrap() { + break status; + } + if Instant::now() >= deadline { + let _ = child.kill(); + let _ = child.wait(); + panic!("command was admitted instead of refused: {command:?}"); + } + thread::sleep(Duration::from_millis(5)); + }; + let mut stdout = Vec::new(); + child + .stdout + .take() + .unwrap() + .read_to_end(&mut stdout) + .unwrap(); + (status.code(), stdout) +} + #[test] fn ephemeral_publication_reports_its_bound_port_and_explicit_ports_keep_bare_ready() { let backend = TcpListener::bind("127.0.0.1:0").unwrap(); @@ -813,23 +844,22 @@ fn ephemeral_publication_reports_its_bound_port_and_explicit_ports_keep_bare_rea ); // Only the exact `--publish 0` form selects an ephemeral port. for (mode, endpoint) in [("--publish", "00"), ("--publish-unix", "0")] { - let refused = Command::new(BINARY) - .args([mode, endpoint]) - .arg(&guard.socket) - .args([token, "10000"]) - .stdin(Stdio::null()) - .output() - .unwrap(); - assert_eq!(refused.status.code(), Some(64), "{mode} {endpoint}"); - assert!(refused.stdout.is_empty()); + let (code, stdout) = bounded_refusal( + Command::new(BINARY) + .args([mode, endpoint]) + .arg(&guard.socket) + .args([token, "10000"]), + ); + assert_eq!(code, Some(64), "{mode} {endpoint}"); + assert!(stdout.is_empty()); } - let refused = Command::new(BINARY) - .arg(guard.root.join("relay-port-zero.sock")) - .args(["127.0.0.1", "0", "1000"]) - .stdin(Stdio::null()) - .output() - .unwrap(); - assert_eq!(refused.status.code(), Some(64)); + let (code, stdout) = bounded_refusal( + Command::new(BINARY) + .arg(guard.root.join("relay-port-zero.sock")) + .args(["127.0.0.1", "0", "1000"]), + ); + assert_eq!(code, Some(64)); + assert!(stdout.is_empty()); assert!(!guard.root.join("relay-port-zero.sock").exists()); let (mut first, first_port) = ephemeral_publisher(&guard.socket, token, None); From ad5f1cccc9be8c39a9dcdecfdd743032c2a33a4c Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 20 Oct 2026 10:51:23 -0400 Subject: [PATCH 3/7] test(env): start symlink-refusal attempts lazily so neither rejects unobserved The negative control for a symlinked .hack/.env built both refusal attempts in an array literal, so upsertDotEnvValue and removeDotEnvKey ran concurrently. When removeDotEnvKey's refusal settled first, its promise had no handler yet while the loop awaited the first attempt. Bun reports that as an unhandled rejection, fails the test and runs afterEach, which removes the temp root while the test body is still running; the body's later readFile of outside.env then fails with ENOENT as "Unhandled error between tests" (HACK-1292). caught() now takes a thunk, so every operation in this file starts inside the await that observes it, and the loop passes thunks. No product change: the refusal paths in private-file.ts await all their work before rejecting. Refs HACK-1292 Co-Authored-By: Claude Opus 5.5 --- tests/private-env-file.test.ts | 38 +++++++++++++++++++++------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/private-env-file.test.ts b/tests/private-env-file.test.ts index 798792cae..dabf01520 100644 --- a/tests/private-env-file.test.ts +++ b/tests/private-env-file.test.ts @@ -102,8 +102,14 @@ function expectClosedRefusal(error: unknown, paths: readonly string[]): void { } } -async function caught(promise: Promise): Promise { - return await promise.then( +/** + * Starts `run` and settles it to its rejection (or null). Taking a thunk rather + * than a promise keeps every operation started inside the await that observes + * it: a promise started earlier can reject while the test awaits something + * else, which Bun reports as an unhandled rejection (HACK-1292). + */ +async function caught(run: () => Promise): Promise { + return await run().then( () => null, (error: unknown) => error ); @@ -204,13 +210,17 @@ test("negative control: a symlinked .hack/.env is refused without reading or wri const project = await createProject(); await symlink(project.outsideFile, project.envFile); + // Each attempt starts only inside the caught() that awaits it, so the two + // refusals cannot race each other. for (const attempt of [ - upsertDotEnvValue({ - envFile: project.envFile, - key: SENTINEL_KEY, - value: SENTINEL_VALUE, - }), - removeDotEnvKey({ envFile: project.envFile, key: "OUTSIDE_SENTINEL" }), + () => + upsertDotEnvValue({ + envFile: project.envFile, + key: SENTINEL_KEY, + value: SENTINEL_VALUE, + }), + () => + removeDotEnvKey({ envFile: project.envFile, key: "OUTSIDE_SENTINEL" }), ]) { expectClosedRefusal(await caught(attempt), [ project.envFile, @@ -229,7 +239,7 @@ test("negative control: a dangling .hack/.env symlink is refused and its target await symlink(missingTarget, project.envFile); expectClosedRefusal( - await caught( + await caught(() => upsertDotEnvValue({ envFile: project.envFile, key: SENTINEL_KEY, @@ -248,7 +258,7 @@ test("negative control: an env overlay file that is a symlink is refused", async await symlink(project.outsideFile, overlayFile); expectClosedRefusal( - await caught( + await caught(() => upsertDotEnvValue({ envFile: overlayFile, key: SENTINEL_KEY, @@ -273,7 +283,7 @@ test("negative control: a symlinked .hack directory is refused and the linked di const envFile = resolve(projectDir, PROJECT_ENV_FILENAME); expectClosedRefusal( - await caught( + await caught(() => upsertDotEnvValue({ envFile, key: SENTINEL_KEY, value: SENTINEL_VALUE }) ), [envFile, outsideDir] @@ -285,7 +295,7 @@ test("a directory at the .hack/.env path is refused with the same closed diagnos const project = await createProject(); await mkdir(project.envFile); expectClosedRefusal( - await caught( + await caught(() => upsertDotEnvValue({ envFile: project.envFile, key: SENTINEL_KEY, @@ -343,7 +353,7 @@ test("negative control: materializeProjectEnv refuses a symlinked .hack/.env tar await symlink(project.outsideFile, project.envFile); expectClosedRefusal( - await caught( + await caught(() => materializeProjectEnv({ projectRoot: project.projectRoot, projectDir: project.projectDir, @@ -373,7 +383,7 @@ test("negative control: replacePrivateFileAtomic itself refuses a symlinked targ // cannot mask a missing check in the replace step. for (const path of [project.envFile, danglingLink]) { expectClosedRefusal( - await caught( + await caught(() => replacePrivateFileAtomic({ path, directories: [project.projectDir], From 62da1fc161e27008a761ac0884f66c28b84b5294 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 20 Oct 2026 15:08:35 -0400 Subject: [PATCH 4/7] test(adoption): give v14 compound routing operations the product's caller deadline The compound v14 routing-generation tests gave every harness operation a fixed 15 s caller deadline, while `hack up`/`down` give a retained operation the 90 s Compose startup budget. In CI run 39289575897 the fourth lifecycle operation outlived the 15 s deadline under suite load, and the product's fail-closed deadline guard in mutateRetainedContainers refused as E_LEGACY_ADOPTION_CHANGED, the same code as a real binding change. No binding had changed, and the 32.9 s "store-close" phase in that log was the four lifecycle operations: the test had no phase marker between them. Each retained routing operation also costs more than the one before it: every route-claim scan rereads every prior attempt and revalidates every held directory, so the fourth lifecycle operation of a fresh fixture is the slowest. - operation() takes a closed caller deadline (the 15 s default or the product's DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS) and records value-free timing of the last operation: elapsed ms, outcome, refusal code, whether the caller deadline had expired and whether any synthetic binding fact had moved. - The compound lifecycle and rollback tests pass the product's caller deadline to every operation, mark a phase after each operation, attribute a failure with the last operation's timing, and get a budget that covers the bounds of the operations they run in sequence. - Three regression tests use a synthetic clock advance after the first probe: an unchanged operation slower than 15 s completes under the product's deadline; the same operation under the 15 s default refuses as CHANGED with an expired deadline and an unchanged binding; a real volume birth drift still refuses as CHANGED before the deadline. Refs HACK-1283 Co-Authored-By: Claude Opus 5.5 --- tests/helpers/retained-routing-adoption.ts | 101 ++++++++-- ...ompose-adoption-routing-generation.test.ts | 190 ++++++++++++++++-- 2 files changed, 255 insertions(+), 36 deletions(-) diff --git a/tests/helpers/retained-routing-adoption.ts b/tests/helpers/retained-routing-adoption.ts index c5e5ff52d..d8d630905 100644 --- a/tests/helpers/retained-routing-adoption.ts +++ b/tests/helpers/retained-routing-adoption.ts @@ -10,6 +10,7 @@ import { } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; +import type { DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS } from "../../src/lib/compose-startup-budget.ts"; import { isRecord } from "../../src/lib/guards.ts"; import { openLegacyComposeAdoptedGenerationStore } from "../../src/lib/native-compose-adoption-generation.ts"; import { parseLegacyComposeAdoptionReceipt } from "../../src/lib/native-compose-adoption-receipt.ts"; @@ -24,6 +25,29 @@ import { restoreEnv } from "./env.ts"; export const ROUTING_CANARY = "synthetic-retained-sql-row"; /** Closed set: the product's 15 s default proof deadline or its 60 s cap. */ export type RetainedRoutingStoreTimeoutMs = 15_000 | 60_000; +/** Closed set of caller deadlines for one `operation()`: the harness's 15 s + * default, kept by the deadline-refusal controls, or the product's own default + * caller deadline for a retained operation (`hack up`/`down` pass the 90 s + * Compose startup budget to `withMutation`). */ +export type RetainedRoutingOperationDeadlineMs = + | 15_000 + | typeof DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS; +/** Value-free timing of the most recent `operation()` call. A retained mutation + * refuses as `E_LEGACY_ADOPTION_CHANGED` both when its observed binding differs + * from the recorded one and when its caller deadline expires (the product's + * fail-closed deadline guard). `deadlineExpired` and `bindingUnchanged` (whether + * every synthetic binding fact except the running state still equals the + * fixture's original) tell the two apart without exposing any value. */ +export interface RetainedRoutingOperationTiming { + readonly operation: "start" | "stop"; + readonly preparation: boolean; + readonly deadlineMs: RetainedRoutingOperationDeadlineMs; + readonly elapsedMs: number; + readonly outcome: "returned" | "refused"; + readonly code?: string; + readonly deadlineExpired: boolean; + readonly bindingUnchanged: boolean; +} export const ROUTING_IDS = { db: "a".repeat(64), web: "b".repeat(64), @@ -531,6 +555,14 @@ else { root, ".hack/.internal/legacy-compose-adoption-v1/receipt.json" ); + let lastOperation: RetainedRoutingOperationTiming | undefined; + /** Every synthetic binding fact except the running state, which lifecycle + * operations legitimately change. Compared, never reported. */ + const bindingFacts = () => { + const { running: _running, ...facts } = model; + return JSON.stringify(facts); + }; + const originalBinding = bindingFacts(); return { root, outer, @@ -543,6 +575,8 @@ else { hooks, commands, effects, + /** Timing of the most recent `operation()` call, for failure attribution. */ + lastOperation: () => lastOperation, receiptPath, canRestore: lifetime.canRestore, track: lifetime.track, @@ -577,9 +611,33 @@ else { readonly recover?: boolean; readonly numeric?: boolean; readonly preparation?: boolean; + /** The caller deadline for this one operation. Deadline-refusal controls + * keep the 15 s default; a compound lifecycle test passes the product's + * default caller deadline, so a slow but unchanged operation under suite + * load is not refused by a harness deadline shorter than the product's. */ + readonly deadlineMs?: RetainedRoutingOperationDeadlineMs; } = {} ) => { - const deadline = Date.now() + 15_000; + const deadlineMs = opts.deadlineMs ?? 15_000; + const started = Date.now(); + const deadline = started + deadlineMs; + const record = (outcome: "returned" | "refused", error?: unknown) => { + const now = Date.now(); + const code = + isRecord(error) && typeof error.code === "string" + ? error.code + : undefined; + lastOperation = { + operation, + preparation: opts.preparation === true, + deadlineMs, + elapsedMs: now - started, + outcome, + ...(code ? { code } : {}), + deadlineExpired: now >= deadline, + bindingUnchanged: bindingFacts() === originalBinding, + }; + }; const run = async ( input: Parameters[0]["run"]>[0] ) => @@ -590,23 +648,30 @@ else { operation, deadline, }); - return opts.preparation - ? await store.withPreparationStop({ - generation, - binary: compiler, - deadline, - recover: opts.recover, - run, - }) - : await store.withMutation({ - generation, - operation, - services: [], - binary: compiler, - deadline, - recover: opts.recover, - run, - }); + try { + const code = opts.preparation + ? await store.withPreparationStop({ + generation, + binary: compiler, + deadline, + recover: opts.recover, + run, + }) + : await store.withMutation({ + generation, + operation, + services: [], + binary: compiler, + deadline, + recover: opts.recover, + run, + }); + record("returned"); + return code; + } catch (error: unknown) { + record("refused", error); + throw error; + } }, cleanup: async () => { if (!lifetime.canRestore()) { diff --git a/tests/native-compose-adoption-routing-generation.test.ts b/tests/native-compose-adoption-routing-generation.test.ts index 62a2fcf2d..d6a382ed6 100644 --- a/tests/native-compose-adoption-routing-generation.test.ts +++ b/tests/native-compose-adoption-routing-generation.test.ts @@ -2,6 +2,7 @@ import { test as boundedTest, expect, spyOn } from "bun:test"; import * as fs from "node:fs/promises"; import { readFile } from "node:fs/promises"; import { join } from "node:path"; +import { DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS } from "../src/lib/compose-startup-budget.ts"; import { isRecord } from "../src/lib/guards.ts"; import * as admission from "../src/lib/native-compose-adoption-compiler.ts"; import { LegacyComposeAdoptedGenerationError } from "../src/lib/native-compose-adoption-generation.ts"; @@ -16,16 +17,38 @@ import { retainedRoutingFixtureLifetime, } from "./helpers/retained-routing-adoption.ts"; -/** The compound lifecycle test's per-test budget and its store proof deadline: - * the product's 60 s cap. */ -const COMPOUND_BUDGET_MS = 60_000; +/** The compound tests' store proof deadline: the product's 60 s cap. */ +const COMPOUND_STORE_TIMEOUT_MS = 60_000; +/** The caller deadline of each compound operation: the product's own default + * (`hack up`/`down` pass the 90 s Compose startup budget to `withMutation`). */ +const COMPOUND_OPERATION_DEADLINE_MS = DEFAULT_COMPOSE_STARTUP_TIMEOUT_MS; +/** A compound test's budget: six `COMPOUND_OPERATION_DEADLINE_MS` (540 s), for at + * most five caller-bounded operations plus preparation, publication and the + * lease or rollback. It must not be shorter than the bounds of the operations it + * runs in sequence: a 60 s budget let suite load end a slow but unchanged run + * before its operations' own deadlines, and the timed-out fixture then failed every + * later test in this file as an unknown lifetime. A hosted run takes about 40 s. */ +const COMPOUND_TEST_BUDGET_MS = 540_000; +/** A single-operation regression test's budget: two operation deadlines (180 s), + * for preparation, the one caller-bounded operation and a publication. */ +const SINGLE_OPERATION_TEST_BUDGET_MS = 180_000; +/** A synthetic clock advance past the harness's 15 s default operation deadline + * but well inside the product's default caller deadline: a slow but unchanged + * operation. */ +const SLOW_OPERATION_ADVANCE_MS = 16_000; let h: Awaited>; let fixtureActive = false; let fixtureUncertain = false; const test = ( name: string, run: () => Promise, - opts: { readonly timeoutMs?: 30_000 | 60_000 } = {} + opts: { + readonly timeoutMs?: + | 30_000 + | 60_000 + | typeof SINGLE_OPERATION_TEST_BUDGET_MS + | typeof COMPOUND_TEST_BUDGET_MS; + } = {} ) => { const timeoutMs = opts.timeoutMs ?? 30_000; return boundedTest( @@ -214,8 +237,10 @@ test("publication diagnostics distinguish compiler refusal after switching from }); /** Per-phase timing for the compound lifecycle tests. Each marker logs the elapsed * and per-phase ms with the probe counts, and `attribute` names the phase in - * progress when a failure (such as a proof-deadline overrun) surfaces. Only timings - * and counts are added to the already value-free diagnostic; values stay omitted. */ + * progress and the last operation's timing (including whether its caller deadline + * had expired) when a failure (such as a proof-deadline overrun) surfaces. Only + * timings, counts and the refusal code are added to the already value-free + * diagnostic; values stay omitted. */ function compoundPhases(fixture: string) { const started = Date.now(); let lastPhase = "enter"; @@ -249,7 +274,7 @@ function compoundPhases(fixture: string) { attribute: (error: unknown) => { const now = Date.now(); return new Error( - `${fixture} failed ${now - lastPhaseAt} ms after phase "${lastPhase}" (${now - started} ms elapsed of ${COMPOUND_BUDGET_MS} ms budget): ${String(error)}`, + `${fixture} failed ${now - lastPhaseAt} ms after phase "${lastPhase}" (${now - started} ms elapsed of ${COMPOUND_TEST_BUDGET_MS} ms budget; last operation ${JSON.stringify(h.lastOperation() ?? null)}): ${String(error)}`, { cause: error } ); }, @@ -264,7 +289,10 @@ async function publishStoppedV14( expect(generation.report.adoption_generation_version).toBe(14); expect((await h.receipt()).routingHandoff).toBe("held"); expect( - await h.operation(store, generation, "stop", { preparation: true }) + await h.operation(store, generation, "stop", { + preparation: true, + deadlineMs: COMPOUND_OPERATION_DEADLINE_MS, + }) ).toBe(0); phase("preparation-stop-return"); await store.publish({ generation, binary: h.compiler }); @@ -310,7 +338,14 @@ function volumeInspections() { // 37999833592). It is split into the two tests below, each on a fresh fixture: the // first covers publication, the lifecycle operations and the saved lease; the // second covers rollback after a lifecycle cycle. Both open their store at the 60 s -// product cap. The deadline-refusal controls in this file keep the 15 s default. +// product cap and give each operation the product's default caller deadline. With +// the harness's former fixed 15 s operation deadline, the fourth lifecycle operation +// under suite load expired it and the product's fail-closed deadline guard refused +// as E_LEGACY_ADOPTION_CHANGED (HACK-1283, run 39289575897); no binding had changed. +// Each retained routing operation also costs more than the one before it, because +// every route-claim scan rereads every prior attempt in the fixture's fresh home. +// Their per-test budget is `COMPOUND_TEST_BUDGET_MS`. The deadline-refusal controls +// in this file keep the 15 s default. test( "v14 keeps original resources and data through stop/publication/up/down/up and saved open", async () => { @@ -319,15 +354,21 @@ test( ); phase("prepare-enter"); const { store, generation } = await prepare({ - timeoutMs: COMPOUND_BUDGET_MS, + timeoutMs: COMPOUND_STORE_TIMEOUT_MS, }); phase("prepare-return"); try { const active = await publishStoppedV14(phase, store, generation); - for (const operation of ["start", "stop", "start", "stop"] as const) { - expect(await h.operation(store, active, operation)).toBe(0); + for (const [index, operation] of ( + ["start", "stop", "start", "stop"] as const + ).entries()) { + expect( + await h.operation(store, active, operation, { + deadlineMs: COMPOUND_OPERATION_DEADLINE_MS, + }) + ).toBe(0); + phase(`lifecycle-${index + 1}-${operation}-return`); } - phase("four-lifecycle-operations-return"); await store.withLease({ generation: active, run: async (input) => { @@ -351,7 +392,7 @@ test( // One full cold preparation/publication, four lifecycle operations and a saved lease. // Canonical bracketed scans add four probes per stable binding observation. }, - { timeoutMs: COMPOUND_BUDGET_MS } + { timeoutMs: COMPOUND_TEST_BUDGET_MS } ); test( "v14 rollback after publication and one up/down restores originals and keeps data and the routing hold", @@ -361,15 +402,19 @@ test( ); phase("prepare-enter"); const { store, generation } = await prepare({ - timeoutMs: COMPOUND_BUDGET_MS, + timeoutMs: COMPOUND_STORE_TIMEOUT_MS, }); phase("prepare-return"); try { const active = await publishStoppedV14(phase, store, generation); for (const operation of ["start", "stop"] as const) { - expect(await h.operation(store, active, operation)).toBe(0); + expect( + await h.operation(store, active, operation, { + deadlineMs: COMPOUND_OPERATION_DEADLINE_MS, + }) + ).toBe(0); + phase(`lifecycle-${operation}-return`); } - phase("lifecycle-cycle-return"); await store.rollback(); phase("rollback-return"); expect( @@ -393,7 +438,116 @@ test( // One full cold preparation/publication, one lifecycle cycle and rollback. // Canonical bracketed scans add four probes per stable binding observation. }, - { timeoutMs: COMPOUND_BUDGET_MS } + { timeoutMs: COMPOUND_TEST_BUDGET_MS } +); +/** Runs one harness operation while a synthetic clock jumps + * `SLOW_OPERATION_ADVANCE_MS` ahead after the operation's first probe, so the + * operation is slow by the product's own clock but otherwise unchanged. */ +async function slowOperation(run: () => Promise): Promise { + const realNow = Date.now; + let offset = 0; + const clock = spyOn(Date, "now").mockImplementation(() => realNow() + offset); + h.hooks.afterProbe = async () => { + offset = SLOW_OPERATION_ADVANCE_MS; + }; + try { + return await run(); + } finally { + h.hooks.afterProbe = undefined; + clock.mockRestore(); + } +} +test( + "HACK-1283: an unchanged operation slower than 15 s completes under the product's default caller deadline", + async () => { + const { store, generation } = await prepare({ + timeoutMs: COMPOUND_STORE_TIMEOUT_MS, + }); + try { + expect( + await slowOperation(() => + h.operation(store, generation, "stop", { + preparation: true, + deadlineMs: COMPOUND_OPERATION_DEADLINE_MS, + }) + ) + ).toBe(0); + expect(h.lastOperation()).toMatchObject({ + outcome: "returned", + deadlineMs: COMPOUND_OPERATION_DEADLINE_MS, + deadlineExpired: false, + bindingUnchanged: true, + }); + expect(h.lastOperation()?.elapsedMs).toBeGreaterThanOrEqual( + SLOW_OPERATION_ADVANCE_MS + ); + expect(h.effects).toHaveLength(1); + expect((await h.receipt()).pendingOperation).toBeNull(); + await store.publish({ generation, binary: h.compiler }); + expect((await h.receipt()).publication?.phase).toBe("active"); + } finally { + await store.close(); + } + }, + { timeoutMs: SINGLE_OPERATION_TEST_BUDGET_MS } +); +test( + "HACK-1283: the same slow operation under the 15 s default refuses as CHANGED only because its deadline expired", + async () => { + const { store, generation } = await prepare({ + timeoutMs: COMPOUND_STORE_TIMEOUT_MS, + }); + try { + await red( + slowOperation(() => + h.operation(store, generation, "stop", { preparation: true }) + ) + ); + expect(h.lastOperation()).toMatchObject({ + outcome: "refused", + code: "E_LEGACY_ADOPTION_CHANGED", + deadlineMs: 15_000, + deadlineExpired: true, + // Nothing in the synthetic binding moved: "changed" here is the deadline. + bindingUnchanged: true, + }); + expect((await h.receipt()).routingHandoff).toBe("held"); + } finally { + await store.close(); + } + }, + { timeoutMs: SINGLE_OPERATION_TEST_BUDGET_MS } +); +test( + "HACK-1283: a real binding change still refuses as CHANGED within the product's default caller deadline", + async () => { + const { store, generation } = await prepare({ + timeoutMs: COMPOUND_STORE_TIMEOUT_MS, + }); + try { + h.hooks.afterEffect = async () => { + h.model.volumeBirth = "2026-02-02T01:02:03Z"; + }; + await red( + h.operation(store, generation, "stop", { + preparation: true, + deadlineMs: COMPOUND_OPERATION_DEADLINE_MS, + }) + ); + expect(h.lastOperation()).toMatchObject({ + outcome: "refused", + code: "E_LEGACY_ADOPTION_CHANGED", + deadlineMs: COMPOUND_OPERATION_DEADLINE_MS, + deadlineExpired: false, + bindingUnchanged: false, + }); + expect(h.effects).toHaveLength(1); + } finally { + h.hooks.afterEffect = undefined; + await store.close(); + } + }, + { timeoutMs: SINGLE_OPERATION_TEST_BUDGET_MS } ); test("read-only routing phase refuses volume birth drift at its complete final binding", async () => { const { store, generation } = await prepare(); From d9d6376d6437aa50db3f0cd43977fa3e2eb71c41 Mon Sep 17 00:00:00 2001 From: Dimitri Kennedy Date: Tue, 20 Oct 2026 10:48:15 -0400 Subject: [PATCH 5/7] fix: fail local gates on a Bun pin mismatch and run native plan CLI checks locally Root test, check and typecheck, and the CLI package's test, check and typecheck, now run `bun run check:bun-version` first. It fails when the running Bun differs from the package.json packageManager pin that CI installs, unless HACK_ALLOW_BUN_VERSION_MISMATCH=1 is set deliberately. Turbo passes that variable through so package tasks honor it. `bun run check:native-plan-cli` runs the compiled-bundle CLI checks that previously ran only in CI's Config compiler job. A wiring test fails when a scripts/check-native-*-plan-cli.ts is missing from it or from CI. Refs HACK-1290 Refs HACK-1294 Co-Authored-By: Claude Opus 5.5 --- .ai/skills/hack-repo-verify/SKILL.md | 20 +++- AGENTS.md | 8 +- package.json | 8 +- packages/cli/package.json | 6 +- scripts/check-bun-version.ts | 105 ++++++++++++++++ scripts/check-native-plan-clis.ts | 70 +++++++++++ tests/bun-version-guard.test.ts | 171 +++++++++++++++++++++++++++ tests/native-plan-cli-wiring.test.ts | 75 ++++++++++++ turbo.json | 1 + 9 files changed, 455 insertions(+), 9 deletions(-) create mode 100644 scripts/check-bun-version.ts create mode 100644 scripts/check-native-plan-clis.ts create mode 100644 tests/bun-version-guard.test.ts create mode 100644 tests/native-plan-cli-wiring.test.ts diff --git a/.ai/skills/hack-repo-verify/SKILL.md b/.ai/skills/hack-repo-verify/SKILL.md index 4b01408db..bd075c080 100644 --- a/.ai/skills/hack-repo-verify/SKILL.md +++ b/.ai/skills/hack-repo-verify/SKILL.md @@ -33,8 +33,24 @@ For native setup use root `mise.toml`, the Bun pin in `package.json`, and `bun install --frozen-lockfile`. Native gates are `bun run typecheck`, `bun run check`, `bun run test`. Focused CLI checks: `bun run --cwd packages/cli typecheck` and `bun run --cwd packages/cli check`; lint changed tests/scripts explicitly with -`bunx ultracite check `. Rust default gates are `bun run check:local` and -`bun run test:local`. Add `--all-features` to equivalent Cargo commands for feature +`bunx ultracite check `. These entry points first run +`bun run check:bun-version`, which fails when the running Bun differs from the +`packageManager` pin that CI installs. A Bun ahead of mise's shims on `PATH` +(for example mise's `latest` install) bypasses the checkout's `mise.toml`; use +`mise x bun@ -- bun run