From 9e2ea3f0e3614ca4bfe57fba57920ab5e1d79289 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Fri, 28 Aug 2026 21:26:59 +0200 Subject: [PATCH 1/4] fix(pr-proof): resolve real Cargo executable outside version-manager shims The 1602-parentless-worker-inventory case runner's resolveCargo() rejects rustup's symlink-proxy (basename after realpath becomes "rustup") but had no way to reach the real toolchain cargo in a Cloud sandbox whose PATH contains only shims. When RUSTUP_HOME is unset and no PATH entry ends in /.cargo/bin or /.local/share/mise/shims, the toolchain walk gets no home to work from and the resolver throws. Broaden the resolver: - Detect known shim directories (rustup/mise/asdf /shims/, .volta/bin/) explicitly and reject them alongside the basename check. - Ask the version manager for the toolchain-selected cargo path via `rustup which cargo`, `mise which cargo`, `asdf which cargo`. This is the authoritative resolution when only shims sit on PATH. - Infer likely rustup homes from HOME, CARGO_HOME, RUSTUP_HOME, and asdf shim locations in addition to the existing mise/.cargo hints. - Extend the hardcoded system-install fallbacks with /root/.cargo/bin/ and /home/daytona/.cargo/bin/ so a rustup install without any PATH hint is still found. - On failure, include the ordered list of attempted paths so the next debug pass has ground truth instead of just a rejection message. The resolver now takes an options bag with injectable dependencies so unit tests can drive it with a mocked filesystem and a stubbed version manager. Main() is guarded by the invoked-as-CLI check so importing the module for tests does not execute the probe pipeline. Add resolve-cargo.test.mjs with `node --test` coverage for: - rustup symlink proxy skipped, real toolchain cargo picked up - `rustup which cargo` output preferred over a shim on PATH - shim-only PATH throws with the diagnostic listing - direct non-shim cargo on PATH short-circuits the fallbacks - isShimPath heuristics for rustup, mise, asdf, and volta layouts --- .../resolve-cargo.test.mjs | 153 +++++++++++++++++ .../1602-parentless-worker-inventory/run.mjs | 160 +++++++++++++++--- 2 files changed, 294 insertions(+), 19 deletions(-) create mode 100644 tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs diff --git a/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs new file mode 100644 index 0000000000..f5c372cecd --- /dev/null +++ b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs @@ -0,0 +1,153 @@ +// Unit tests for the Cargo resolver used by the 1602 PR-proof case runner. +// +// Run with: +// node --test tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs + +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { chmod, mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises'; +import os from 'node:os'; +import path from 'node:path'; + +import { isShimPath, resolveCargo } from './run.mjs'; + +async function makeExecutable(filePath, body = '#!/bin/sh\nexit 0\n') { + await mkdir(path.dirname(filePath), { recursive: true }); + await writeFile(filePath, body); + await chmod(filePath, 0o755); +} + +async function makeTmpRoot(prefix) { + const raw = await mkdtemp(path.join(os.tmpdir(), prefix)); + // macOS resolves /var to /private/var — normalize once so string comparisons + // against paths inside `root` line up with the resolver's realpath output. + return realpath(raw); +} + +test('isShimPath recognizes rustup, mise, asdf, and volta shim directories', () => { + assert.equal(isShimPath('/home/x/.local/share/mise/shims/cargo'), true); + assert.equal(isShimPath('/home/x/.asdf/shims/cargo'), true); + assert.equal(isShimPath('/root/.rustup/shims/cargo'), true); + assert.equal(isShimPath('/home/x/.volta/bin/cargo'), true); + + // Real toolchain binaries — mise's installs/ and asdf's installs/ contain + // the real cargo, not a shim. Do not reject them. + assert.equal(isShimPath('/home/x/.local/share/mise/installs/rust/1.75.0/bin/cargo'), false); + assert.equal(isShimPath('/home/x/.asdf/installs/rust/1.75.0/bin/cargo'), false); + assert.equal(isShimPath('/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo'), false); + assert.equal(isShimPath('/usr/local/cargo/bin/cargo'), false); +}); + +test('resolveCargo skips a rustup symlink-proxy and walks toolchain dirs', async () => { + const root = await makeTmpRoot('resolve-cargo-proxy-'); + try { + // rustup on PATH: real `rustup` binary, plus a `cargo` symlink pointing at + // rustup. Basename check rejects it (realpath's basename is `rustup`). + const cargoBin = path.join(root, 'cargo-bin'); + await makeExecutable(path.join(cargoBin, 'rustup')); + await symlink(path.join(cargoBin, 'rustup'), path.join(cargoBin, 'cargo')); + + const toolchainCargo = path.join( + root, + '.rustup', + 'toolchains', + 'stable-x86_64-unknown-linux-gnu', + 'bin', + 'cargo' + ); + await makeExecutable(toolchainCargo); + + // Force the toolchain walk by making `rustup which cargo` fail. + const runOnce = async () => ({ code: 1, signal: null, stdout: '', stderr: 'no toolchain' }); + + const resolved = await resolveCargo({ + env: { PATH: cargoBin, HOME: root }, + pathEntries: [cargoBin], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, toolchainCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo prefers the path printed by `rustup which cargo`', async () => { + const root = await makeTmpRoot('resolve-cargo-rustup-'); + try { + // Put shims in a proper `shims/` dir so loop 1 rejects them. + const shimsDir = path.join(root, 'shims'); + await makeExecutable(path.join(shimsDir, 'rustup')); + await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho shim\n'); + + const realCargo = path.join(root, 'toolchains', 'stable', 'bin', 'cargo'); + await makeExecutable(realCargo); + + const runOnce = async (command, args) => { + if (path.basename(command) === 'rustup' && args.join(' ') === 'which cargo') { + return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; + } + return { code: 1, signal: null, stdout: '', stderr: '' }; + }; + + const resolved = await resolveCargo({ + env: { PATH: shimsDir, HOME: root }, + pathEntries: [shimsDir], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, realCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo throws a diagnostic listing when nothing resolves', async () => { + const root = await makeTmpRoot('resolve-cargo-empty-'); + try { + const shimsDir = path.join(root, '.local', 'share', 'mise', 'shims'); + await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho mise-shim\n'); + + const runOnce = async () => ({ code: 1, signal: null, stdout: '', stderr: '' }); + + await assert.rejects( + resolveCargo({ + env: { PATH: shimsDir, HOME: root }, + pathEntries: [shimsDir], + runOnce, + extraSystemPaths: [], + }), + (error) => { + assert.match(error.message, /could not resolve a real Cargo executable/); + assert.match(error.message, /attempts:/); + assert.match(error.message, /rejected: shim path/); + return true; + } + ); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo returns a direct non-shim cargo on PATH when present', async () => { + const root = await makeTmpRoot('resolve-cargo-direct-'); + try { + const binDir = path.join(root, 'bin'); + const cargo = path.join(binDir, 'cargo'); + await makeExecutable(cargo); + + const runOnce = async () => { + throw new Error('should not be called when PATH already has a real cargo'); + }; + + const resolved = await resolveCargo({ + env: { PATH: binDir, HOME: root }, + pathEntries: [binDir], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, cargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); diff --git a/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs b/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs index 5ede5a470c..6ad60b0ac4 100644 --- a/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs +++ b/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs @@ -4,6 +4,7 @@ import { spawn } from 'node:child_process'; import { constants as fsConstants } from 'node:fs'; import { access, readFile, readdir, realpath, writeFile } from 'node:fs/promises'; import path from 'node:path'; +import { pathToFileURL } from 'node:url'; const CASE_ID = '1602-parentless-worker-inventory'; const MARKER = 'RELAY_PR_PROOF_OBSERVATION='; @@ -46,39 +47,158 @@ async function isExecutable(filePath) { } } -async function resolveCargo() { - const pathEntries = (process.env.PATH ?? '').split(path.delimiter).filter(Boolean); +// A shim is a version-manager stub that dispatches to a real toolchain based +// on the current directory / env. Even when it happens to work end-to-end, +// invoking the toolchain directly is more predictable in a sandbox that has +// nothing but a bare PATH, and avoids surprises like "cargo not found in +// toolchain X" or shim env-loading failures. +export function isShimPath(candidate) { + const normalized = candidate.replaceAll('\\', '/'); + // Any `.../shims/` path is a version-manager shim (rustup, mise, asdf). + if (/\/shims\/[^/]+$/.test(normalized)) return true; + // Volta wraps invocations through the binaries it installs in .volta/bin. + if (/\/\.volta\/bin\/[^/]+$/.test(normalized)) return true; + return false; +} + +async function defaultRunOnce(command, args) { + return new Promise((resolve) => { + const child = spawn(command, args, { + stdio: ['ignore', 'pipe', 'pipe'], + env: process.env, + }); + let stdout = ''; + let stderr = ''; + child.stdout.on('data', (chunk) => { + stdout += chunk.toString('utf8'); + }); + child.stderr.on('data', (chunk) => { + stderr += chunk.toString('utf8'); + }); + child.on('error', () => resolve({ code: 1, signal: null, stdout, stderr })); + child.on('close', (code, signal) => resolve({ code: code ?? 1, signal, stdout, stderr })); + }); +} + +export async function resolveCargo(options = {}) { + const { + env = process.env, + pathEntries = (env.PATH ?? '').split(path.delimiter).filter(Boolean), + isExecutable: isExec = isExecutable, + realpath: realpathFn = (p) => realpath(p).catch(() => p), + readdir: readdirFn = (p) => readdir(p).catch(() => []), + runOnce = defaultRunOnce, + extraSystemPaths = [ + '/usr/local/cargo/bin/cargo', + '/opt/rust/bin/cargo', + '/root/.cargo/bin/cargo', + '/home/daytona/.cargo/bin/cargo', + ], + } = options; + + const attempts = []; + + // 1) A direct non-shim cargo on PATH. Rustup's symlink proxy is rejected by + // the basename check (realpath's basename is "rustup"); hard-linked or + // scripted shims (mise/asdf/volta/rustup) are rejected by isShimPath. for (const entry of pathEntries) { const candidate = path.join(entry, 'cargo'); - if (!(await isExecutable(candidate))) continue; - const resolved = await realpath(candidate).catch(() => candidate); - if (path.basename(resolved) === 'cargo') return resolved; + if (!(await isExec(candidate))) continue; + const resolved = await realpathFn(candidate); + if (path.basename(resolved) !== 'cargo') { + attempts.push(`${candidate} -> ${resolved} (rejected: proxy, not named cargo)`); + continue; + } + if (isShimPath(candidate) || isShimPath(resolved)) { + attempts.push(`${resolved} (rejected: shim path)`); + continue; + } + return resolved; } + // 2) Ask an installed version manager for the toolchain-selected cargo. + // `rustup which cargo` (and mise/asdf equivalents) prints the resolved + // real binary. This is the reliable path in Cloud sandboxes where only + // shims sit on PATH. + for (const [tool, args] of [ + ['rustup', ['which', 'cargo']], + ['mise', ['which', 'cargo']], + ['asdf', ['which', 'cargo']], + ]) { + let binary = null; + for (const entry of pathEntries) { + const candidate = path.join(entry, tool); + if (await isExec(candidate)) { + binary = candidate; + break; + } + } + if (!binary) { + attempts.push(`${tool}: not found on PATH`); + continue; + } + let result; + try { + result = await runOnce(binary, args); + } catch (error) { + attempts.push(`${tool} ${args.join(' ')}: threw ${error?.message ?? error}`); + continue; + } + if (!result || result.code !== 0) { + attempts.push(`${tool} ${args.join(' ')}: exit ${result?.code ?? 'unknown'}`); + continue; + } + const printed = (result.stdout || '') + .split(/\r?\n/) + .map((line) => line.trim()) + .find((line) => line.length > 0); + if (!printed) { + attempts.push(`${tool} ${args.join(' ')}: no output`); + continue; + } + if (!(await isExec(printed))) { + attempts.push(`${tool} ${args.join(' ')} -> ${printed} (not executable)`); + continue; + } + if (path.basename(printed) !== 'cargo' || isShimPath(printed)) { + attempts.push(`${tool} ${args.join(' ')} -> ${printed} (rejected: still a shim)`); + continue; + } + return printed; + } + + // 3) Enumerate rustup toolchains under any home we can plausibly infer. const homes = new Set(); for (const entry of pathEntries) { const normalized = entry.replaceAll('\\', '/'); - for (const suffix of ['/.local/share/mise/shims', '/.cargo/bin']) { + for (const suffix of ['/.local/share/mise/shims', '/.cargo/bin', '/.asdf/shims']) { if (normalized.endsWith(suffix)) homes.add(normalized.slice(0, -suffix.length)); } } - if (process.env.RUSTUP_HOME) { - homes.add(path.dirname(path.resolve(process.env.RUSTUP_HOME))); - } + if (env.HOME) homes.add(env.HOME); + if (env.CARGO_HOME) homes.add(path.dirname(path.resolve(env.CARGO_HOME))); + if (env.RUSTUP_HOME) homes.add(path.dirname(path.resolve(env.RUSTUP_HOME))); for (const home of homes) { const toolchains = path.join(home, '.rustup', 'toolchains'); - const entries = await readdir(toolchains).catch(() => []); - for (const entry of entries.sort().reverse()) { + const entries = await readdirFn(toolchains); + for (const entry of entries.slice().sort().reverse()) { const candidate = path.join(toolchains, entry, 'bin', 'cargo'); - if (await isExecutable(candidate)) return candidate; + if (await isExec(candidate)) return candidate; + attempts.push(`${candidate} (missing/not executable)`); } } - for (const candidate of ['/usr/local/cargo/bin/cargo', '/opt/rust/bin/cargo']) { - if (await isExecutable(candidate)) return candidate; + // 4) System installs. + for (const candidate of extraSystemPaths) { + if (!(await isExec(candidate))) continue; + const resolved = await realpathFn(candidate); + if (path.basename(resolved) === 'cargo' && !isShimPath(resolved)) return resolved; + attempts.push(`${candidate} -> ${resolved} (rejected)`); } - throw new Error('could not resolve a real Cargo executable outside a version-manager shim'); + + const detail = attempts.length ? `; attempts: ${attempts.join('; ')}` : ''; + throw new Error(`could not resolve a real Cargo executable outside a version-manager shim${detail}`); } function run(command, args, options = {}) { @@ -479,7 +599,9 @@ async function main() { ); } -main().catch((error) => { - console.error(error instanceof Error ? error.message : String(error)); - process.exitCode = 1; -}); +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main().catch((error) => { + console.error(error instanceof Error ? error.message : String(error)); + process.exitCode = 1; + }); +} From a44fd0ab2bfbab21091907a748bee0c837169db3 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 29 Aug 2026 07:11:30 +0200 Subject: [PATCH 2/4] fix(pr-proof): time out hung which-cargo probes and cover the branches Two cubic review threads on fix/pr-proof-cargo-shim: 1) resolveCargo step 2 spawned `rustup|mise|asdf which cargo` with no timeout. A hung shim (file-lock or network wait) left the promise pending, and the workflow only failed at its outer timeout. Wrap each runOnce call with a 5s race (configurable via `probeTimeoutMs`) and kill the underlying child in `defaultRunOnce` when a `timeoutMs` option is supplied. On timeout, log via the `log` option and fall through to the next resolver rather than blocking the whole probe. 2) resolve-cargo.test.mjs only exercised rustup (step 2) and the empty / direct-PATH shapes. Add coverage for the mise-which and asdf-which branches, the CARGO_HOME / RUSTUP_HOME / asdf-shims home-inference variants of step 3, and a hung `mise which cargo` that must fall through to a working asdf resolver via the new timeout path. All 11 tests pass locally with `node --test`. Co-Authored-By: Claude Opus 4.7 --- .../resolve-cargo.test.mjs | 198 ++++++++++++++++++ .../1602-parentless-worker-inventory/run.mjs | 55 ++++- 2 files changed, 249 insertions(+), 4 deletions(-) diff --git a/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs index f5c372cecd..128c2aa4b7 100644 --- a/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs +++ b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs @@ -151,3 +151,201 @@ test('resolveCargo returns a direct non-shim cargo on PATH when present', async await rm(root, { recursive: true, force: true }); } }); + +test('resolveCargo prefers the path printed by `mise which cargo`', async () => { + const root = await makeTmpRoot('resolve-cargo-mise-'); + try { + const shimsDir = path.join(root, '.local', 'share', 'mise', 'shims'); + await makeExecutable(path.join(shimsDir, 'mise')); + await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho mise-shim\n'); + + const realCargo = path.join(root, '.local', 'share', 'mise', 'installs', 'rust', '1.75.0', 'bin', 'cargo'); + await makeExecutable(realCargo); + + const runOnce = async (command, args) => { + if (path.basename(command) === 'mise' && args.join(' ') === 'which cargo') { + return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; + } + return { code: 1, signal: null, stdout: '', stderr: '' }; + }; + + const resolved = await resolveCargo({ + env: { PATH: shimsDir, HOME: root }, + pathEntries: [shimsDir], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, realCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo prefers the path printed by `asdf which cargo`', async () => { + const root = await makeTmpRoot('resolve-cargo-asdf-'); + try { + const shimsDir = path.join(root, '.asdf', 'shims'); + await makeExecutable(path.join(shimsDir, 'asdf')); + await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho asdf-shim\n'); + + const realCargo = path.join(root, '.asdf', 'installs', 'rust', '1.75.0', 'bin', 'cargo'); + await makeExecutable(realCargo); + + const runOnce = async (command, args) => { + if (path.basename(command) === 'asdf' && args.join(' ') === 'which cargo') { + return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; + } + return { code: 1, signal: null, stdout: '', stderr: '' }; + }; + + const resolved = await resolveCargo({ + env: { PATH: shimsDir, HOME: root }, + pathEntries: [shimsDir], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, realCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo step 3 infers home from CARGO_HOME when PATH lacks shims', async () => { + const root = await makeTmpRoot('resolve-cargo-cargohome-'); + try { + const toolchainCargo = path.join( + root, + '.rustup', + 'toolchains', + 'stable-x86_64-unknown-linux-gnu', + 'bin', + 'cargo' + ); + await makeExecutable(toolchainCargo); + + // Empty PATH (nothing to probe), no HOME, but CARGO_HOME points at + // `/.cargo`. `dirname(CARGO_HOME)` becomes `root`, so the toolchain + // walk under `/.rustup/toolchains` finds the cargo. + const runOnce = async () => { + throw new Error('should not be called; no tools on PATH'); + }; + + const resolved = await resolveCargo({ + env: { PATH: '', CARGO_HOME: path.join(root, '.cargo') }, + pathEntries: [], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, toolchainCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo step 3 infers home from RUSTUP_HOME when PATH lacks shims', async () => { + const root = await makeTmpRoot('resolve-cargo-rustuphome-'); + try { + const toolchainCargo = path.join( + root, + '.rustup', + 'toolchains', + 'nightly-x86_64-unknown-linux-gnu', + 'bin', + 'cargo' + ); + await makeExecutable(toolchainCargo); + + const runOnce = async () => { + throw new Error('should not be called; no tools on PATH'); + }; + + // RUSTUP_HOME is `/.rustup`; dirname gives `root`, so the toolchain + // walk under `/.rustup/toolchains` succeeds. + const resolved = await resolveCargo({ + env: { PATH: '', RUSTUP_HOME: path.join(root, '.rustup') }, + pathEntries: [], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, toolchainCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo step 3 infers home from an asdf shims PATH entry', async () => { + const root = await makeTmpRoot('resolve-cargo-asdfhome-'); + try { + // asdf shims dir sits on PATH but has no cargo shim in it, and no asdf + // binary — so steps 1 and 2 both fall through. Step 3 must strip the + // `/.asdf/shims` suffix to infer `` as a home and walk toolchains. + const shimsDir = path.join(root, '.asdf', 'shims'); + await mkdir(shimsDir, { recursive: true }); + + const toolchainCargo = path.join( + root, + '.rustup', + 'toolchains', + 'stable-x86_64-unknown-linux-gnu', + 'bin', + 'cargo' + ); + await makeExecutable(toolchainCargo); + + const runOnce = async () => ({ code: 1, signal: null, stdout: '', stderr: '' }); + + const resolved = await resolveCargo({ + env: { PATH: shimsDir }, + pathEntries: [shimsDir], + runOnce, + extraSystemPaths: [], + }); + assert.equal(resolved, toolchainCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + +test('resolveCargo times out a hung version-manager probe and falls through to the next resolver', async () => { + const root = await makeTmpRoot('resolve-cargo-hung-'); + try { + // Shims dir carries mise (which will hang) and asdf (which will succeed). + // rustup is absent, so it fails fast; mise hangs, timeout kicks in; asdf + // then answers with the real cargo. If the timeout doesn't fire, the + // test itself would time out on `node --test`. + const shimsDir = path.join(root, 'shims'); + await makeExecutable(path.join(shimsDir, 'mise')); + await makeExecutable(path.join(shimsDir, 'asdf')); + + const realCargo = path.join(root, 'installs', 'rust', 'bin', 'cargo'); + await makeExecutable(realCargo); + + const logged = []; + const runOnce = async (command, args) => { + if (path.basename(command) === 'mise') { + // Never resolves — simulates a hung shim (lock, network wait). + return new Promise(() => {}); + } + if (path.basename(command) === 'asdf' && args.join(' ') === 'which cargo') { + return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; + } + return { code: 1, signal: null, stdout: '', stderr: '' }; + }; + + const resolved = await resolveCargo({ + env: { PATH: shimsDir, HOME: root }, + pathEntries: [shimsDir], + runOnce, + probeTimeoutMs: 75, + log: (message) => logged.push(message), + extraSystemPaths: [], + }); + assert.equal(resolved, realCargo); + assert.ok( + logged.some((line) => line.includes('mise which cargo') && line.includes('timed out')), + `expected a diagnosable timeout log line; got: ${JSON.stringify(logged)}` + ); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); diff --git a/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs b/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs index 6ad60b0ac4..2d6154a0a5 100644 --- a/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs +++ b/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs @@ -61,7 +61,8 @@ export function isShimPath(candidate) { return false; } -async function defaultRunOnce(command, args) { +async function defaultRunOnce(command, args, options = {}) { + const { timeoutMs } = options; return new Promise((resolve) => { const child = spawn(command, args, { stdio: ['ignore', 'pipe', 'pipe'], @@ -69,14 +70,31 @@ async function defaultRunOnce(command, args) { }); let stdout = ''; let stderr = ''; + let timedOut = false; + let timer = null; + if (typeof timeoutMs === 'number' && timeoutMs > 0) { + timer = setTimeout(() => { + timedOut = true; + try { + child.kill('SIGTERM'); + } catch { + // The child may already be gone; nothing to do. + } + }, timeoutMs); + if (typeof timer.unref === 'function') timer.unref(); + } + const finish = (result) => { + if (timer) clearTimeout(timer); + resolve(result); + }; child.stdout.on('data', (chunk) => { stdout += chunk.toString('utf8'); }); child.stderr.on('data', (chunk) => { stderr += chunk.toString('utf8'); }); - child.on('error', () => resolve({ code: 1, signal: null, stdout, stderr })); - child.on('close', (code, signal) => resolve({ code: code ?? 1, signal, stdout, stderr })); + child.on('error', () => finish({ code: 1, signal: null, stdout, stderr, timedOut })); + child.on('close', (code, signal) => finish({ code: code ?? 1, signal, stdout, stderr, timedOut })); }); } @@ -88,6 +106,11 @@ export async function resolveCargo(options = {}) { realpath: realpathFn = (p) => realpath(p).catch(() => p), readdir: readdirFn = (p) => readdir(p).catch(() => []), runOnce = defaultRunOnce, + // A hung version-manager shim (lock, network wait) must not wedge the + // whole probe. 5s is generous for a PATH lookup; a real hang means the + // shim itself is broken and the next resolver deserves a turn. + probeTimeoutMs = 5000, + log = (message) => console.error(message), extraSystemPaths = [ '/usr/local/cargo/bin/cargo', '/opt/rust/bin/cargo', @@ -139,11 +162,35 @@ export async function resolveCargo(options = {}) { } let result; try { - result = await runOnce(binary, args); + const timeoutSentinel = Symbol('probe-timeout'); + let timer = null; + const timeoutPromise = new Promise((resolve) => { + timer = setTimeout(() => resolve(timeoutSentinel), probeTimeoutMs); + if (typeof timer.unref === 'function') timer.unref(); + }); + try { + const raced = await Promise.race([ + runOnce(binary, args, { timeoutMs: probeTimeoutMs }), + timeoutPromise, + ]); + if (raced === timeoutSentinel) { + log(`[resolveCargo] ${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms; treating as failed probe and trying next resolver`); + attempts.push(`${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms`); + continue; + } + result = raced; + } finally { + if (timer) clearTimeout(timer); + } } catch (error) { attempts.push(`${tool} ${args.join(' ')}: threw ${error?.message ?? error}`); continue; } + if (result?.timedOut) { + log(`[resolveCargo] ${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms; treating as failed probe and trying next resolver`); + attempts.push(`${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms`); + continue; + } if (!result || result.code !== 0) { attempts.push(`${tool} ${args.join(' ')}: exit ${result?.code ?? 'unknown'}`); continue; From afbeaa55f1c127b80d3c0f4a08e1eb0bef3c6857 Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 29 Aug 2026 07:12:55 +0200 Subject: [PATCH 3/4] test(pr-proof): cover the extraSystemPaths fallback in resolveCargo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cubic thread 1 also called out that step 4 (`extraSystemPaths`) had no coverage — every existing test passed `extraSystemPaths: []`. Add a targeted test with empty PATH, no HOME/CARGO_HOME/RUSTUP_HOME, and one fixture path in extraSystemPaths; assert it resolves. 12 tests pass locally with `node --test`. Co-Authored-By: Claude Opus 4.7 --- .../resolve-cargo.test.mjs | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs index 128c2aa4b7..0f06946054 100644 --- a/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs +++ b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs @@ -306,6 +306,30 @@ test('resolveCargo step 3 infers home from an asdf shims PATH entry', async () = } }); +test('resolveCargo step 4 falls back to extraSystemPaths when nothing else resolves', async () => { + const root = await makeTmpRoot('resolve-cargo-system-'); + try { + const systemCargo = path.join(root, 'opt', 'cargo', 'bin', 'cargo'); + await makeExecutable(systemCargo); + + // Empty PATH, no HOME, nothing in step 3 — only the system-path fallback + // can resolve. Point extraSystemPaths at the fixture and expect a hit. + const runOnce = async () => { + throw new Error('should not be called; no tools on PATH'); + }; + + const resolved = await resolveCargo({ + env: { PATH: '' }, + pathEntries: [], + runOnce, + extraSystemPaths: [systemCargo], + }); + assert.equal(resolved, systemCargo); + } finally { + await rm(root, { recursive: true, force: true }); + } +}); + test('resolveCargo times out a hung version-manager probe and falls through to the next resolver', async () => { const root = await makeTmpRoot('resolve-cargo-hung-'); try { From 636d001775156667e518f6654b50af85184b1ffd Mon Sep 17 00:00:00 2001 From: kjgbot Date: Sat, 29 Aug 2026 16:26:55 +0200 Subject: [PATCH 4/4] fix(pr-proof): replace cargo resolver with fail-fast execFileSync probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous resolveCargo layered shim detection, version-manager `which cargo` invocations, and a manual timeout + sentinel race + SIGTERM/SIGKILL escalation. Every layer was a place for cubic to find a real bug: a sentinel race leaked timers, `probeTimeoutMs` was unbounded/unnormalized, SIGTERM-ignoring shims outlived the probe, `cancelledByParent` was lost, and the timeout test itself hit `node --test`'s harness cancel. Rewrite as a fail-fast probe. Enumerate candidates in preference order (CARGO_HOME, ~/.cargo, ~/.rustup/toolchains/*, every cargo on PATH, documented system fallbacks), then for each one run `execFileSync(candidate, ['--version'], { timeout: 3000, killSignal: 'SIGKILL' })`. Node's execFileSync `timeout` + SIGKILL is the OS calling kill(9), so a hung/blocking child cannot out-live the probe. The ground truth is the `/^cargo \d+\.\d+\.\d+/` regex on stdout — no shim heuristics needed. Every failure mode (timeout, non-zero exit, unexpected output) collapses to "try the next candidate." When none work, throw CargoNotResolvableError carrying the attempts array. Drops: isShimPath, defaultRunOnce, probeTimeoutMs racing, rustup/mise/asdf which invocations, HOME/CARGO_HOME/RUSTUP_HOME suffix-stripping, isExec async helper. Tests now cover: PATH hit, CARGO_HOME hit, rustup toolchain enumeration, extraSystemPaths fallback, unexpected-output-then-skip, all-timeout-then-CargoNotResolvableError, and cross-source dedup. Co-Authored-By: Claude Opus 4.7 --- .../resolve-cargo.test.mjs | 417 ++++++------------ .../1602-parentless-worker-inventory/run.mjs | 248 +++-------- 2 files changed, 194 insertions(+), 471 deletions(-) diff --git a/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs index 0f06946054..1bdf22906d 100644 --- a/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs +++ b/tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs @@ -1,149 +1,65 @@ -// Unit tests for the Cargo resolver used by the 1602 PR-proof case runner. +// Unit tests for the fail-fast Cargo resolver used by the 1602 PR-proof +// case runner. +// +// Every test drops a real (fake) executable script into a temp directory and +// exercises the resolver against it. The resolver's timeout is enforced by +// Node's execFileSync `timeout` + `killSignal: 'SIGKILL'`, so a probe that +// blocks forever is guaranteed to die within the budget. // // Run with: // node --test tests/relayflows/cases/1602-parentless-worker-inventory/resolve-cargo.test.mjs import test from 'node:test'; import assert from 'node:assert/strict'; -import { chmod, mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises'; +import { chmod, mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises'; import os from 'node:os'; import path from 'node:path'; -import { isShimPath, resolveCargo } from './run.mjs'; +import { CargoNotResolvableError, resolveCargo } from './run.mjs'; -async function makeExecutable(filePath, body = '#!/bin/sh\nexit 0\n') { +async function makeExecutable(filePath, body) { await mkdir(path.dirname(filePath), { recursive: true }); await writeFile(filePath, body); await chmod(filePath, 0o755); } -async function makeTmpRoot(prefix) { - const raw = await mkdtemp(path.join(os.tmpdir(), prefix)); - // macOS resolves /var to /private/var — normalize once so string comparisons - // against paths inside `root` line up with the resolver's realpath output. - return realpath(raw); +async function makeFakeCargo(filePath, { version = '1.75.0' } = {}) { + await makeExecutable( + filePath, + `#!/bin/sh\nif [ "$1" = "--version" ]; then\n echo "cargo ${version} (fake)"\nfi\n` + ); } -test('isShimPath recognizes rustup, mise, asdf, and volta shim directories', () => { - assert.equal(isShimPath('/home/x/.local/share/mise/shims/cargo'), true); - assert.equal(isShimPath('/home/x/.asdf/shims/cargo'), true); - assert.equal(isShimPath('/root/.rustup/shims/cargo'), true); - assert.equal(isShimPath('/home/x/.volta/bin/cargo'), true); - - // Real toolchain binaries — mise's installs/ and asdf's installs/ contain - // the real cargo, not a shim. Do not reject them. - assert.equal(isShimPath('/home/x/.local/share/mise/installs/rust/1.75.0/bin/cargo'), false); - assert.equal(isShimPath('/home/x/.asdf/installs/rust/1.75.0/bin/cargo'), false); - assert.equal(isShimPath('/root/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/bin/cargo'), false); - assert.equal(isShimPath('/usr/local/cargo/bin/cargo'), false); -}); - -test('resolveCargo skips a rustup symlink-proxy and walks toolchain dirs', async () => { - const root = await makeTmpRoot('resolve-cargo-proxy-'); - try { - // rustup on PATH: real `rustup` binary, plus a `cargo` symlink pointing at - // rustup. Basename check rejects it (realpath's basename is `rustup`). - const cargoBin = path.join(root, 'cargo-bin'); - await makeExecutable(path.join(cargoBin, 'rustup')); - await symlink(path.join(cargoBin, 'rustup'), path.join(cargoBin, 'cargo')); - - const toolchainCargo = path.join( - root, - '.rustup', - 'toolchains', - 'stable-x86_64-unknown-linux-gnu', - 'bin', - 'cargo' - ); - await makeExecutable(toolchainCargo); - - // Force the toolchain walk by making `rustup which cargo` fail. - const runOnce = async () => ({ code: 1, signal: null, stdout: '', stderr: 'no toolchain' }); - - const resolved = await resolveCargo({ - env: { PATH: cargoBin, HOME: root }, - pathEntries: [cargoBin], - runOnce, - extraSystemPaths: [], - }); - assert.equal(resolved, toolchainCargo); - } finally { - await rm(root, { recursive: true, force: true }); - } -}); - -test('resolveCargo prefers the path printed by `rustup which cargo`', async () => { - const root = await makeTmpRoot('resolve-cargo-rustup-'); - try { - // Put shims in a proper `shims/` dir so loop 1 rejects them. - const shimsDir = path.join(root, 'shims'); - await makeExecutable(path.join(shimsDir, 'rustup')); - await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho shim\n'); - - const realCargo = path.join(root, 'toolchains', 'stable', 'bin', 'cargo'); - await makeExecutable(realCargo); - - const runOnce = async (command, args) => { - if (path.basename(command) === 'rustup' && args.join(' ') === 'which cargo') { - return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; - } - return { code: 1, signal: null, stdout: '', stderr: '' }; - }; - - const resolved = await resolveCargo({ - env: { PATH: shimsDir, HOME: root }, - pathEntries: [shimsDir], - runOnce, - extraSystemPaths: [], - }); - assert.equal(resolved, realCargo); - } finally { - await rm(root, { recursive: true, force: true }); - } -}); - -test('resolveCargo throws a diagnostic listing when nothing resolves', async () => { - const root = await makeTmpRoot('resolve-cargo-empty-'); - try { - const shimsDir = path.join(root, '.local', 'share', 'mise', 'shims'); - await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho mise-shim\n'); +async function makeHangingCargo(filePath) { + // Ignore SIGTERM so a hung probe survives a soft signal; only SIGKILL takes + // it out. Sleep in a small loop so `trap` can service the signal ignore. + await makeExecutable( + filePath, + '#!/bin/sh\ntrap "" TERM\nwhile true; do sleep 60; done\n' + ); +} - const runOnce = async () => ({ code: 1, signal: null, stdout: '', stderr: '' }); +async function makeWrongOutputCargo(filePath) { + await makeExecutable(filePath, '#!/bin/sh\necho "not cargo at all"\n'); +} - await assert.rejects( - resolveCargo({ - env: { PATH: shimsDir, HOME: root }, - pathEntries: [shimsDir], - runOnce, - extraSystemPaths: [], - }), - (error) => { - assert.match(error.message, /could not resolve a real Cargo executable/); - assert.match(error.message, /attempts:/); - assert.match(error.message, /rejected: shim path/); - return true; - } - ); - } finally { - await rm(root, { recursive: true, force: true }); - } -}); +async function makeTmpRoot(prefix) { + const raw = await mkdtemp(path.join(os.tmpdir(), prefix)); + // macOS resolves /var to /private/var — normalize so string comparisons + // against paths inside the returned root line up. + return realpath(raw); +} -test('resolveCargo returns a direct non-shim cargo on PATH when present', async () => { - const root = await makeTmpRoot('resolve-cargo-direct-'); +test('resolveCargo returns a direct hit on PATH when `cargo --version` looks right', async () => { + const root = await makeTmpRoot('resolve-cargo-path-'); try { const binDir = path.join(root, 'bin'); const cargo = path.join(binDir, 'cargo'); - await makeExecutable(cargo); + await makeFakeCargo(cargo); - const runOnce = async () => { - throw new Error('should not be called when PATH already has a real cargo'); - }; - - const resolved = await resolveCargo({ - env: { PATH: binDir, HOME: root }, + const resolved = resolveCargo({ + env: { PATH: binDir }, pathEntries: [binDir], - runOnce, extraSystemPaths: [], }); assert.equal(resolved, cargo); @@ -152,66 +68,26 @@ test('resolveCargo returns a direct non-shim cargo on PATH when present', async } }); -test('resolveCargo prefers the path printed by `mise which cargo`', async () => { - const root = await makeTmpRoot('resolve-cargo-mise-'); - try { - const shimsDir = path.join(root, '.local', 'share', 'mise', 'shims'); - await makeExecutable(path.join(shimsDir, 'mise')); - await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho mise-shim\n'); - - const realCargo = path.join(root, '.local', 'share', 'mise', 'installs', 'rust', '1.75.0', 'bin', 'cargo'); - await makeExecutable(realCargo); - - const runOnce = async (command, args) => { - if (path.basename(command) === 'mise' && args.join(' ') === 'which cargo') { - return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; - } - return { code: 1, signal: null, stdout: '', stderr: '' }; - }; - - const resolved = await resolveCargo({ - env: { PATH: shimsDir, HOME: root }, - pathEntries: [shimsDir], - runOnce, - extraSystemPaths: [], - }); - assert.equal(resolved, realCargo); - } finally { - await rm(root, { recursive: true, force: true }); - } -}); - -test('resolveCargo prefers the path printed by `asdf which cargo`', async () => { - const root = await makeTmpRoot('resolve-cargo-asdf-'); +test('resolveCargo prefers CARGO_HOME/bin/cargo when it works', async () => { + const root = await makeTmpRoot('resolve-cargo-cargohome-'); try { - const shimsDir = path.join(root, '.asdf', 'shims'); - await makeExecutable(path.join(shimsDir, 'asdf')); - await makeExecutable(path.join(shimsDir, 'cargo'), '#!/bin/sh\necho asdf-shim\n'); - - const realCargo = path.join(root, '.asdf', 'installs', 'rust', '1.75.0', 'bin', 'cargo'); - await makeExecutable(realCargo); - - const runOnce = async (command, args) => { - if (path.basename(command) === 'asdf' && args.join(' ') === 'which cargo') { - return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; - } - return { code: 1, signal: null, stdout: '', stderr: '' }; - }; + const cargoHome = path.join(root, '.cargo'); + const cargo = path.join(cargoHome, 'bin', 'cargo'); + await makeFakeCargo(cargo); - const resolved = await resolveCargo({ - env: { PATH: shimsDir, HOME: root }, - pathEntries: [shimsDir], - runOnce, + const resolved = resolveCargo({ + env: { PATH: '', CARGO_HOME: cargoHome }, + pathEntries: [], extraSystemPaths: [], }); - assert.equal(resolved, realCargo); + assert.equal(resolved, cargo); } finally { await rm(root, { recursive: true, force: true }); } }); -test('resolveCargo step 3 infers home from CARGO_HOME when PATH lacks shims', async () => { - const root = await makeTmpRoot('resolve-cargo-cargohome-'); +test('resolveCargo enumerates HOME/.rustup/toolchains/*/bin/cargo', async () => { + const root = await makeTmpRoot('resolve-cargo-rustup-'); try { const toolchainCargo = path.join( root, @@ -221,19 +97,11 @@ test('resolveCargo step 3 infers home from CARGO_HOME when PATH lacks shims', as 'bin', 'cargo' ); - await makeExecutable(toolchainCargo); + await makeFakeCargo(toolchainCargo); - // Empty PATH (nothing to probe), no HOME, but CARGO_HOME points at - // `/.cargo`. `dirname(CARGO_HOME)` becomes `root`, so the toolchain - // walk under `/.rustup/toolchains` finds the cargo. - const runOnce = async () => { - throw new Error('should not be called; no tools on PATH'); - }; - - const resolved = await resolveCargo({ - env: { PATH: '', CARGO_HOME: path.join(root, '.cargo') }, + const resolved = resolveCargo({ + env: { PATH: '', HOME: root }, pathEntries: [], - runOnce, extraSystemPaths: [], }); assert.equal(resolved, toolchainCargo); @@ -242,133 +110,106 @@ test('resolveCargo step 3 infers home from CARGO_HOME when PATH lacks shims', as } }); -test('resolveCargo step 3 infers home from RUSTUP_HOME when PATH lacks shims', async () => { - const root = await makeTmpRoot('resolve-cargo-rustuphome-'); +test('resolveCargo falls back to extraSystemPaths when nothing else works', async () => { + const root = await makeTmpRoot('resolve-cargo-system-'); try { - const toolchainCargo = path.join( - root, - '.rustup', - 'toolchains', - 'nightly-x86_64-unknown-linux-gnu', - 'bin', - 'cargo' - ); - await makeExecutable(toolchainCargo); - - const runOnce = async () => { - throw new Error('should not be called; no tools on PATH'); - }; + const systemCargo = path.join(root, 'opt', 'cargo', 'bin', 'cargo'); + await makeFakeCargo(systemCargo); - // RUSTUP_HOME is `/.rustup`; dirname gives `root`, so the toolchain - // walk under `/.rustup/toolchains` succeeds. - const resolved = await resolveCargo({ - env: { PATH: '', RUSTUP_HOME: path.join(root, '.rustup') }, + const resolved = resolveCargo({ + env: { PATH: '' }, pathEntries: [], - runOnce, - extraSystemPaths: [], + extraSystemPaths: [systemCargo], }); - assert.equal(resolved, toolchainCargo); + assert.equal(resolved, systemCargo); } finally { await rm(root, { recursive: true, force: true }); } }); -test('resolveCargo step 3 infers home from an asdf shims PATH entry', async () => { - const root = await makeTmpRoot('resolve-cargo-asdfhome-'); +test('resolveCargo skips a candidate that prints unexpected --version output and picks the next one', async () => { + const root = await makeTmpRoot('resolve-cargo-badoutput-'); try { - // asdf shims dir sits on PATH but has no cargo shim in it, and no asdf - // binary — so steps 1 and 2 both fall through. Step 3 must strip the - // `/.asdf/shims` suffix to infer `` as a home and walk toolchains. - const shimsDir = path.join(root, '.asdf', 'shims'); - await mkdir(shimsDir, { recursive: true }); - - const toolchainCargo = path.join( - root, - '.rustup', - 'toolchains', - 'stable-x86_64-unknown-linux-gnu', - 'bin', - 'cargo' - ); - await makeExecutable(toolchainCargo); - - const runOnce = async () => ({ code: 1, signal: null, stdout: '', stderr: '' }); - - const resolved = await resolveCargo({ - env: { PATH: shimsDir }, - pathEntries: [shimsDir], - runOnce, + // First candidate on PATH is a shim that prints garbage. + const badDir = path.join(root, 'bad', 'bin'); + const badCargo = path.join(badDir, 'cargo'); + await makeWrongOutputCargo(badCargo); + + // Second candidate on PATH is the real thing. + const goodDir = path.join(root, 'good', 'bin'); + const goodCargo = path.join(goodDir, 'cargo'); + await makeFakeCargo(goodCargo); + + const resolved = resolveCargo({ + env: { PATH: `${badDir}${path.delimiter}${goodDir}` }, + pathEntries: [badDir, goodDir], extraSystemPaths: [], }); - assert.equal(resolved, toolchainCargo); + assert.equal(resolved, goodCargo); } finally { await rm(root, { recursive: true, force: true }); } }); -test('resolveCargo step 4 falls back to extraSystemPaths when nothing else resolves', async () => { - const root = await makeTmpRoot('resolve-cargo-system-'); - try { - const systemCargo = path.join(root, 'opt', 'cargo', 'bin', 'cargo'); - await makeExecutable(systemCargo); - - // Empty PATH, no HOME, nothing in step 3 — only the system-path fallback - // can resolve. Point extraSystemPaths at the fixture and expect a hit. - const runOnce = async () => { - throw new Error('should not be called; no tools on PATH'); - }; +test( + 'resolveCargo throws CargoNotResolvableError when every candidate hangs past the timeout', + { timeout: 15_000 }, + async () => { + const root = await makeTmpRoot('resolve-cargo-hung-'); + try { + const hungA = path.join(root, 'a', 'cargo'); + const hungB = path.join(root, 'b', 'cargo'); + await makeHangingCargo(hungA); + await makeHangingCargo(hungB); + + const timeoutMs = 400; + const started = Date.now(); + let threw; + try { + resolveCargo({ + env: { PATH: `${path.dirname(hungA)}${path.delimiter}${path.dirname(hungB)}` }, + pathEntries: [path.dirname(hungA), path.dirname(hungB)], + timeoutMs, + extraSystemPaths: [], + }); + } catch (error) { + threw = error; + } + const elapsed = Date.now() - started; - const resolved = await resolveCargo({ - env: { PATH: '' }, - pathEntries: [], - runOnce, - extraSystemPaths: [systemCargo], - }); - assert.equal(resolved, systemCargo); - } finally { - await rm(root, { recursive: true, force: true }); + assert.ok(threw instanceof CargoNotResolvableError, `expected CargoNotResolvableError, got ${threw}`); + assert.equal(threw.attempts.length, 2, 'both hung candidates must be recorded'); + for (const attempt of threw.attempts) { + assert.match(attempt.reason, /killed by SIGKILL after 400ms/); + } + assert.match(threw.message, /could not resolve a working cargo executable/); + // Two 400ms probes + SIGKILL should be under 5s even on a busy runner. + assert.ok(elapsed < 5000, `probes must respect timeout budget, elapsed=${elapsed}ms`); + } finally { + await rm(root, { recursive: true, force: true }); + } } -}); +); -test('resolveCargo times out a hung version-manager probe and falls through to the next resolver', async () => { - const root = await makeTmpRoot('resolve-cargo-hung-'); +test('resolveCargo dedupes identical candidates across enumeration sources', async () => { + const root = await makeTmpRoot('resolve-cargo-dedupe-'); try { - // Shims dir carries mise (which will hang) and asdf (which will succeed). - // rustup is absent, so it fails fast; mise hangs, timeout kicks in; asdf - // then answers with the real cargo. If the timeout doesn't fire, the - // test itself would time out on `node --test`. - const shimsDir = path.join(root, 'shims'); - await makeExecutable(path.join(shimsDir, 'mise')); - await makeExecutable(path.join(shimsDir, 'asdf')); - - const realCargo = path.join(root, 'installs', 'rust', 'bin', 'cargo'); - await makeExecutable(realCargo); - - const logged = []; - const runOnce = async (command, args) => { - if (path.basename(command) === 'mise') { - // Never resolves — simulates a hung shim (lock, network wait). - return new Promise(() => {}); - } - if (path.basename(command) === 'asdf' && args.join(' ') === 'which cargo') { - return { code: 0, signal: null, stdout: `${realCargo}\n`, stderr: '' }; - } - return { code: 1, signal: null, stdout: '', stderr: '' }; - }; - - const resolved = await resolveCargo({ - env: { PATH: shimsDir, HOME: root }, - pathEntries: [shimsDir], - runOnce, - probeTimeoutMs: 75, - log: (message) => logged.push(message), - extraSystemPaths: [], + const cargoHome = path.join(root, '.cargo'); + const cargo = path.join(cargoHome, 'bin', 'cargo'); + await makeFakeCargo(cargo); + + // CARGO_HOME/bin/cargo, HOME/.cargo/bin/cargo, and the PATH-based candidate + // all resolve to the same string. Dedup must not double-execute it. + const resolved = resolveCargo({ + env: { + PATH: path.join(cargoHome, 'bin'), + CARGO_HOME: cargoHome, + HOME: root, + }, + pathEntries: [path.join(cargoHome, 'bin')], + extraSystemPaths: [cargo], }); - assert.equal(resolved, realCargo); - assert.ok( - logged.some((line) => line.includes('mise which cargo') && line.includes('timed out')), - `expected a diagnosable timeout log line; got: ${JSON.stringify(logged)}` - ); + assert.equal(resolved, cargo); } finally { await rm(root, { recursive: true, force: true }); } diff --git a/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs b/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs index 2d6154a0a5..cbb78a68ae 100644 --- a/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs +++ b/tests/relayflows/cases/1602-parentless-worker-inventory/run.mjs @@ -1,8 +1,8 @@ #!/usr/bin/env node -import { spawn } from 'node:child_process'; -import { constants as fsConstants } from 'node:fs'; -import { access, readFile, readdir, realpath, writeFile } from 'node:fs/promises'; +import { execFileSync, spawn } from 'node:child_process'; +import { accessSync, constants as fsConstants, readdirSync } from 'node:fs'; +import { readFile, writeFile } from 'node:fs/promises'; import path from 'node:path'; import { pathToFileURL } from 'node:url'; @@ -38,214 +38,96 @@ function proofChildEnvironment() { return env; } -async function isExecutable(filePath) { - try { - await access(filePath, fsConstants.X_OK); - return true; - } catch { - return false; +export class CargoNotResolvableError extends Error { + constructor(attempts) { + const detail = attempts + .map((a) => `${a.path} (${a.reason})`) + .join('; '); + super( + `could not resolve a working cargo executable; attempted: ${detail || '(no candidates)'}` + ); + this.name = 'CargoNotResolvableError'; + this.attempts = attempts; } } -// A shim is a version-manager stub that dispatches to a real toolchain based -// on the current directory / env. Even when it happens to work end-to-end, -// invoking the toolchain directly is more predictable in a sandbox that has -// nothing but a bare PATH, and avoids surprises like "cargo not found in -// toolchain X" or shim env-loading failures. -export function isShimPath(candidate) { - const normalized = candidate.replaceAll('\\', '/'); - // Any `.../shims/` path is a version-manager shim (rustup, mise, asdf). - if (/\/shims\/[^/]+$/.test(normalized)) return true; - // Volta wraps invocations through the binaries it installs in .volta/bin. - if (/\/\.volta\/bin\/[^/]+$/.test(normalized)) return true; - return false; -} - -async function defaultRunOnce(command, args, options = {}) { - const { timeoutMs } = options; - return new Promise((resolve) => { - const child = spawn(command, args, { - stdio: ['ignore', 'pipe', 'pipe'], - env: process.env, - }); - let stdout = ''; - let stderr = ''; - let timedOut = false; - let timer = null; - if (typeof timeoutMs === 'number' && timeoutMs > 0) { - timer = setTimeout(() => { - timedOut = true; - try { - child.kill('SIGTERM'); - } catch { - // The child may already be gone; nothing to do. - } - }, timeoutMs); - if (typeof timer.unref === 'function') timer.unref(); - } - const finish = (result) => { - if (timer) clearTimeout(timer); - resolve(result); - }; - child.stdout.on('data', (chunk) => { - stdout += chunk.toString('utf8'); - }); - child.stderr.on('data', (chunk) => { - stderr += chunk.toString('utf8'); - }); - child.on('error', () => finish({ code: 1, signal: null, stdout, stderr, timedOut })); - child.on('close', (code, signal) => finish({ code: code ?? 1, signal, stdout, stderr, timedOut })); - }); -} - -export async function resolveCargo(options = {}) { +// Ground truth: a candidate is a real cargo iff `cargo --version` prints +// `cargo ` within a bounded budget. Node's execFileSync `timeout` + +// `killSignal: 'SIGKILL'` is the OS calling kill(9), so a hung/blocking child +// can't out-live the probe — no sentinel race, no SIGTERM→SIGKILL escalation, +// no shim-vs-real heuristics. +export function resolveCargo(options = {}) { const { env = process.env, pathEntries = (env.PATH ?? '').split(path.delimiter).filter(Boolean), - isExecutable: isExec = isExecutable, - realpath: realpathFn = (p) => realpath(p).catch(() => p), - readdir: readdirFn = (p) => readdir(p).catch(() => []), - runOnce = defaultRunOnce, - // A hung version-manager shim (lock, network wait) must not wedge the - // whole probe. 5s is generous for a PATH lookup; a real hang means the - // shim itself is broken and the next resolver deserves a turn. - probeTimeoutMs = 5000, - log = (message) => console.error(message), + timeoutMs = 3000, extraSystemPaths = [ - '/usr/local/cargo/bin/cargo', - '/opt/rust/bin/cargo', + '/usr/local/bin/cargo', + '/opt/homebrew/bin/cargo', '/root/.cargo/bin/cargo', '/home/daytona/.cargo/bin/cargo', ], } = options; - const attempts = []; + const candidates = []; + const seen = new Set(); + const add = (candidate) => { + if (!candidate || seen.has(candidate)) return; + seen.add(candidate); + candidates.push(candidate); + }; - // 1) A direct non-shim cargo on PATH. Rustup's symlink proxy is rejected by - // the basename check (realpath's basename is "rustup"); hard-linked or - // scripted shims (mise/asdf/volta/rustup) are rejected by isShimPath. - for (const entry of pathEntries) { - const candidate = path.join(entry, 'cargo'); - if (!(await isExec(candidate))) continue; - const resolved = await realpathFn(candidate); - if (path.basename(resolved) !== 'cargo') { - attempts.push(`${candidate} -> ${resolved} (rejected: proxy, not named cargo)`); - continue; + if (env.CARGO_HOME) add(path.join(env.CARGO_HOME, 'bin', 'cargo')); + if (env.HOME) add(path.join(env.HOME, '.cargo', 'bin', 'cargo')); + if (env.HOME) { + const toolchainsDir = path.join(env.HOME, '.rustup', 'toolchains'); + let entries = []; + try { + entries = readdirSync(toolchainsDir); + } catch { + // no toolchains directory } - if (isShimPath(candidate) || isShimPath(resolved)) { - attempts.push(`${resolved} (rejected: shim path)`); - continue; + for (const entry of entries.slice().sort().reverse()) { + add(path.join(toolchainsDir, entry, 'bin', 'cargo')); } - return resolved; } + for (const entry of pathEntries) add(path.join(entry, 'cargo')); + for (const candidate of extraSystemPaths) add(candidate); - // 2) Ask an installed version manager for the toolchain-selected cargo. - // `rustup which cargo` (and mise/asdf equivalents) prints the resolved - // real binary. This is the reliable path in Cloud sandboxes where only - // shims sit on PATH. - for (const [tool, args] of [ - ['rustup', ['which', 'cargo']], - ['mise', ['which', 'cargo']], - ['asdf', ['which', 'cargo']], - ]) { - let binary = null; - for (const entry of pathEntries) { - const candidate = path.join(entry, tool); - if (await isExec(candidate)) { - binary = candidate; - break; - } - } - if (!binary) { - attempts.push(`${tool}: not found on PATH`); + const attempts = []; + for (const candidate of candidates) { + try { + accessSync(candidate, fsConstants.X_OK); + } catch { + attempts.push({ path: candidate, reason: 'missing or not executable' }); continue; } - let result; + let stdout; try { - const timeoutSentinel = Symbol('probe-timeout'); - let timer = null; - const timeoutPromise = new Promise((resolve) => { - timer = setTimeout(() => resolve(timeoutSentinel), probeTimeoutMs); - if (typeof timer.unref === 'function') timer.unref(); + stdout = execFileSync(candidate, ['--version'], { + timeout: timeoutMs, + killSignal: 'SIGKILL', + stdio: ['ignore', 'pipe', 'pipe'], + encoding: 'utf8', }); - try { - const raced = await Promise.race([ - runOnce(binary, args, { timeoutMs: probeTimeoutMs }), - timeoutPromise, - ]); - if (raced === timeoutSentinel) { - log(`[resolveCargo] ${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms; treating as failed probe and trying next resolver`); - attempts.push(`${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms`); - continue; - } - result = raced; - } finally { - if (timer) clearTimeout(timer); - } } catch (error) { - attempts.push(`${tool} ${args.join(' ')}: threw ${error?.message ?? error}`); - continue; - } - if (result?.timedOut) { - log(`[resolveCargo] ${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms; treating as failed probe and trying next resolver`); - attempts.push(`${tool} ${args.join(' ')}: timed out after ${probeTimeoutMs}ms`); - continue; - } - if (!result || result.code !== 0) { - attempts.push(`${tool} ${args.join(' ')}: exit ${result?.code ?? 'unknown'}`); - continue; - } - const printed = (result.stdout || '') - .split(/\r?\n/) - .map((line) => line.trim()) - .find((line) => line.length > 0); - if (!printed) { - attempts.push(`${tool} ${args.join(' ')}: no output`); + const signal = error?.signal; + const status = error?.status; + const reason = signal + ? `killed by ${signal} after ${timeoutMs}ms` + : `exit ${status ?? 'unknown'}${error?.code ? ` (${error.code})` : ''}`; + attempts.push({ path: candidate, reason }); continue; } - if (!(await isExec(printed))) { - attempts.push(`${tool} ${args.join(' ')} -> ${printed} (not executable)`); + if (!/^cargo \d+\.\d+\.\d+/.test(stdout)) { + const preview = (stdout || '').slice(0, 80).replace(/\n/g, '\\n'); + attempts.push({ path: candidate, reason: `unexpected --version output: ${preview}` }); continue; } - if (path.basename(printed) !== 'cargo' || isShimPath(printed)) { - attempts.push(`${tool} ${args.join(' ')} -> ${printed} (rejected: still a shim)`); - continue; - } - return printed; - } - - // 3) Enumerate rustup toolchains under any home we can plausibly infer. - const homes = new Set(); - for (const entry of pathEntries) { - const normalized = entry.replaceAll('\\', '/'); - for (const suffix of ['/.local/share/mise/shims', '/.cargo/bin', '/.asdf/shims']) { - if (normalized.endsWith(suffix)) homes.add(normalized.slice(0, -suffix.length)); - } - } - if (env.HOME) homes.add(env.HOME); - if (env.CARGO_HOME) homes.add(path.dirname(path.resolve(env.CARGO_HOME))); - if (env.RUSTUP_HOME) homes.add(path.dirname(path.resolve(env.RUSTUP_HOME))); - - for (const home of homes) { - const toolchains = path.join(home, '.rustup', 'toolchains'); - const entries = await readdirFn(toolchains); - for (const entry of entries.slice().sort().reverse()) { - const candidate = path.join(toolchains, entry, 'bin', 'cargo'); - if (await isExec(candidate)) return candidate; - attempts.push(`${candidate} (missing/not executable)`); - } - } - - // 4) System installs. - for (const candidate of extraSystemPaths) { - if (!(await isExec(candidate))) continue; - const resolved = await realpathFn(candidate); - if (path.basename(resolved) === 'cargo' && !isShimPath(resolved)) return resolved; - attempts.push(`${candidate} -> ${resolved} (rejected)`); + return candidate; } - const detail = attempts.length ? `; attempts: ${attempts.join('; ')}` : ''; - throw new Error(`could not resolve a real Cargo executable outside a version-manager shim${detail}`); + throw new CargoNotResolvableError(attempts); } function run(command, args, options = {}) {