fix(scripts): capture check/shard rc before watchdog teardown in no-timeout fallback (#2864)

On machines with neither gtimeout nor timeout on PATH, run-verify-parallel.sh
and run-unit-parallel.sh fall back to a bg-pid + sleep-watchdog cap. Both
read $? only after tearing the watchdog down (kill + wait on cap_pid), so the
sentinel .exit files recorded the killed watchdog's status — 143 — instead of
the check/shard's own exit code. Every run reported total failure (verify:
pass=0 fail=31; unit: rc=143 per shard) while every per-check/shard log
showed success.

Capture rc immediately after `wait $pid` in both scripts, and reap the
watchdog's sleep child (pkill -P, children-first — the same orphan quirk the
heartbeat cleanup documents) so the fallback stops leaking one sleep per
check/shard.

Regression tests force the fallback branch hermetically on any host via a
curated PATH with no timeout binaries: the verify dispatcher runs from a
tempdir copy with a stubbed `bun`, pinning exit 0 + all-zero sentinels when
checks pass and the check's own rc (not 143) when one fails; the unit wrapper
runs real two-shard fixture passes, pinning rc=0 sentinels and a real
failure's rc=1.

Co-authored-by: YMYD <53603073+OJ-OnJourney@users.noreply.github.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
paul-0320
2026-07-23 11:38:15 -07:00
committed by GitHub
co-authored by YMYD Claude Fable 5
parent 054badbe60
commit e9a4fee97f
4 changed files with 217 additions and 4 deletions
+12 -1
View File
@@ -133,6 +133,7 @@ for i in $(seq 1 "$N"); do
env SHARD="$i/$N" \
bash scripts/run-unit-shard.sh --max-concurrency="$INTRA_CONC" \
> "$SHARD_LOG" 2>&1
rc=$?
else
env SHARD="$i/$N" \
bash scripts/run-unit-shard.sh --max-concurrency="$INTRA_CONC" \
@@ -142,10 +143,20 @@ for i in $(seq 1 "$N"); do
sleep 5 && kill -KILL "$pid" 2>/dev/null ) &
cap_pid=$!
wait "$pid" 2>/dev/null
# Capture the shard's exit code from ITS `wait`, before any watchdog
# teardown runs. The teardown commands below overwrite $? — the killed
# watchdog reports 143 — which used to get stamped into every shard's
# sentinel on machines with no gtimeout/timeout: every run "failed"
# with rc=143 summaries even when all tests passed.
rc=$?
# Reap the watchdog's `sleep` child too (pkill -P), then the watchdog.
# Killing only the subshell leaves the sleep orphaned until
# $SHARD_TIMEOUT elapses — same quirk the heartbeat cleanup below works
# around; CI's orphan-process sweep flags those.
pkill -P "$cap_pid" 2>/dev/null
kill "$cap_pid" 2>/dev/null
wait "$cap_pid" 2>/dev/null
fi
rc=$?
echo "$rc" > "$LOG_DIR/shard-$i.exit"
[ "$rc" = "124" ] && echo "WEDGED" > "$LOG_DIR/shard-$i.wedged"
) &
+12 -1
View File
@@ -126,6 +126,7 @@ for c in "${CHECKS[@]}"; do
(
if [ -n "$TIMEOUT_BIN" ]; then
"$TIMEOUT_BIN" "${TIMEOUT}s" bun run "$c" > "$LOG_FILE" 2>&1
rc=$?
else
bun run "$c" > "$LOG_FILE" 2>&1 &
pid=$!
@@ -133,10 +134,20 @@ for c in "${CHECKS[@]}"; do
sleep 5 && kill -KILL "$pid" 2>/dev/null ) &
cap_pid=$!
wait "$pid" 2>/dev/null
# Capture the check's exit code from ITS `wait`, before any watchdog
# teardown runs. The teardown commands below overwrite $? — the killed
# watchdog reports 143 — which used to get stamped into every sentinel
# on machines with no gtimeout/timeout: verify reported pass=0
# fail=<all> while every per-check log said OK.
rc=$?
# Reap the watchdog's `sleep` child too (pkill -P), then the watchdog.
# Killing only the subshell leaves the sleep orphaned until $TIMEOUT
# elapses — same quirk the heartbeat cleanup in run-unit-parallel.sh
# works around; CI's orphan-process sweep flags those.
pkill -P "$cap_pid" 2>/dev/null
kill "$cap_pid" 2>/dev/null
wait "$cap_pid" 2>/dev/null
fi
rc=$?
echo "$rc" > "$EXIT_FILE"
) &
PIDS+=($!)
+91 -1
View File
@@ -22,7 +22,7 @@
import { describe, it, expect, beforeAll, afterAll } from 'bun:test';
import { execFileSync, spawnSync } from 'child_process';
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync, copyFileSync, chmodSync } from 'fs';
import { mkdtempSync, mkdirSync, writeFileSync, readFileSync, existsSync, rmSync, copyFileSync, chmodSync, symlinkSync } from 'fs';
import { tmpdir } from 'os';
import { join, resolve } from 'path';
@@ -154,3 +154,93 @@ describe('failing-on-purpose', () => {
expect(summary).toMatch(/shard 2\/2: pass=\d+ fail=\d+ skip=\d+ rc=\d+/);
});
});
describe('run-unit-parallel.sh no-timeout-binary fallback (rc from shard wait, not watchdog teardown)', () => {
// Forces the no-gtimeout/no-timeout branch by running the wrapper under a
// curated PATH that has every tool the scripts call EXCEPT timeout
// binaries (real `bun` symlinked in), so the fallback executes even on
// hosts with coreutils installed.
//
// Regression pinned here: the shard's sentinel .exit file must record the
// exit code read right after `wait $pid` (the shard's own rc). The
// watchdog subshell is killed with SIGTERM and reports 143; reading `$?`
// after that teardown stamped rc=143 into every shard's sentinel — the
// wrapper exited non-zero with rc=143 summaries even when every test
// passed.
let FROOT: string;
let FENV: Record<string, string>;
beforeAll(() => {
FROOT = mkdtempSync(join(tmpdir(), 'gbrain-parallel-fallback-'));
mkdirSync(join(FROOT, 'scripts'), { recursive: true });
mkdirSync(join(FROOT, 'test'), { recursive: true });
for (const s of ['run-unit-parallel.sh', 'run-unit-shard.sh', 'run-serial-tests.sh']) {
copyFileSync(resolve(REPO_ROOT, 'scripts', s), join(FROOT, 'scripts', s));
chmodSync(join(FROOT, 'scripts', s), 0o755);
}
const passing = `import { describe, it, expect } from 'bun:test';
describe('passing', () => {
it('arithmetic works', () => { expect(1 + 1).toBe(2); });
});`;
writeFileSync(join(FROOT, 'test', 'a-pass.test.ts'), passing);
writeFileSync(join(FROOT, 'test', 'b-pass.test.ts'), passing);
const bin = join(FROOT, 'bin');
mkdirSync(bin);
for (const tool of ['bash', 'sh', 'env', 'dirname', 'basename', 'mktemp', 'date', 'sleep', 'cat', 'tail', 'head', 'rm', 'mkdir', 'pkill', 'grep', 'sed', 'awk', 'wc', 'tr', 'seq', 'find', 'sort', 'bun']) {
const p = Bun.which(tool);
if (p) symlinkSync(p, join(bin, tool));
}
FENV = {
PATH: bin,
HOME: process.env.HOME ?? FROOT,
TMPDIR: process.env.TMPDIR ?? '/tmp',
GBRAIN_TEST_SHARD_TIMEOUT: '300',
};
});
afterAll(() => {
if (FROOT) rmSync(FROOT, { recursive: true, force: true });
});
function runFallbackWrapper(): { code: number; stdout: string; stderr: string } {
const result = spawnSync(
'bash',
[join(FROOT, 'scripts', 'run-unit-parallel.sh'), '--shards', '2'],
{ cwd: FROOT, encoding: 'utf-8', env: FENV },
);
return {
code: result.status ?? -1,
stdout: result.stdout || '',
stderr: result.stderr || '',
};
}
it('exits zero with rc=0 shard sentinels when all shards pass', () => {
const r = runFallbackWrapper();
const summary = readFileSync(join(FROOT, '.context', 'test-summary.txt'), 'utf-8');
expect(summary).toMatch(/shard 1\/2: pass=\d+ fail=0 skip=0 rc=0/);
expect(summary).toMatch(/shard 2\/2: pass=\d+ fail=0 skip=0 rc=0/);
expect(summary).not.toContain('rc=143');
expect(r.code).toBe(0);
});
it('propagates a failing shard rc as the test runner rc (1), not the watchdog 143', () => {
const failing = `import { describe, it, expect } from 'bun:test';
describe('failing-on-purpose', () => {
it('expects 1 to equal 2', () => { expect(1).toBe(2); });
});`;
writeFileSync(join(FROOT, 'test', 'z-fail.test.ts'), failing);
try {
const r = runFallbackWrapper();
expect(r.code).not.toBe(0);
const summary = readFileSync(join(FROOT, '.context', 'test-summary.txt'), 'utf-8');
expect(summary).toMatch(/shard \d\/2: pass=\d+ fail=1 skip=0 rc=1/);
expect(summary).not.toContain('rc=143');
const failureLog = readFileSync(join(FROOT, '.context', 'test-failures.log'), 'utf-8');
expect(failureLog).toContain('failing-on-purpose');
} finally {
rmSync(join(FROOT, 'test', 'z-fail.test.ts'), { force: true });
}
});
});
+102 -1
View File
@@ -15,7 +15,16 @@
import { describe, expect, it } from "bun:test";
import { spawnSync } from "node:child_process";
import { writeFileSync, mkdtempSync, rmSync } from "node:fs";
import {
copyFileSync,
mkdirSync,
mkdtempSync,
readFileSync,
readdirSync,
rmSync,
symlinkSync,
writeFileSync,
} from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
@@ -173,3 +182,95 @@ exit 0
}
});
});
describe("run-verify-parallel.sh — no-timeout-binary fallback rc capture (regression)", () => {
// macOS ships no `timeout`; without brew coreutils (`gtimeout`) — stock
// machines, minimal containers, restricted/sandboxed PATHs — the dispatcher
// degrades to the bg-pid + sleep-watchdog branch.
//
// Regression pinned here: each check's sentinel .exit file must record the
// exit code of the CHECK (read right after `wait $pid`), not of the
// watchdog teardown. The watchdog subshell is killed with SIGTERM and so
// reports 143; reading `$?` after the teardown stamped 143 into every
// sentinel — verify reported pass=0 fail=<all> while every per-check log
// said OK.
//
// Hermetic on any host: the script runs from a tempdir copy with `bun`
// stubbed (checks complete instantly, no repo needed) and PATH set to a
// curated symlink dir containing everything the script calls EXCEPT
// gtimeout/timeout — forcing the fallback branch even where coreutils is
// installed.
function makeFallbackHarness(): { root: string; env: Record<string, string> } {
const root = mkdtempSync(join(tmpdir(), "verify-fallback-"));
mkdirSync(join(root, "scripts"), { recursive: true });
copyFileSync(SCRIPT, join(root, "scripts", "run-verify-parallel.sh"));
const bin = join(root, "bin");
mkdirSync(bin);
// Everything the dispatcher and its subshells invoke, minus timeout bins.
for (const tool of ["bash", "sh", "env", "dirname", "mktemp", "date", "sleep", "cat", "tail", "head", "rm", "mkdir", "pkill", "grep", "sed", "awk"]) {
const p = Bun.which(tool);
if (p) symlinkSync(p, join(bin, tool));
}
// `bun run <name>` stand-in: instant, prints OK, exits 7 for the check
// named in $STUB_FAIL_CHECK (if any).
writeFileSync(
join(bin, "bun"),
`#!/usr/bin/env bash
name="\${2:-}"
echo "stub check OK: $name"
if [ -n "\${STUB_FAIL_CHECK:-}" ] && [ "$name" = "\${STUB_FAIL_CHECK}" ]; then
echo "stub check failing: $name" >&2
exit 7
fi
exit 0
`,
{ mode: 0o755 },
);
return {
root,
env: {
PATH: bin,
HOME: process.env.HOME ?? root,
TMPDIR: process.env.TMPDIR ?? "/tmp",
GBRAIN_VERIFY_TIMEOUT: "30",
GBRAIN_VERIFY_LOG_DIR: join(root, "logs"),
},
};
}
it("all checks passing → exit 0, every sentinel records 0 (not the watchdog's 143)", () => {
const { root, env } = makeFallbackHarness();
try {
const r = spawnSync("bash", [join(root, "scripts", "run-verify-parallel.sh")], { encoding: "utf8", env });
expect(r.stderr).toMatch(/pass=\d+ fail=0/);
expect(r.status).toBe(0);
const exits = readdirSync(join(root, "logs")).filter((f) => f.endsWith(".exit"));
expect(exits.length).toBeGreaterThan(10);
for (const f of exits) {
expect(readFileSync(join(root, "logs", f), "utf8").trim()).toBe("0");
}
} finally {
rmSync(root, { recursive: true, force: true });
}
});
it("one check failing → exit 1, sentinel records the check's own rc (7), not 143", () => {
const { root, env } = makeFallbackHarness();
try {
const r = spawnSync("bash", [join(root, "scripts", "run-verify-parallel.sh")], {
encoding: "utf8",
env: { ...env, STUB_FAIL_CHECK: "check:jsonb" },
});
expect(r.status).toBe(1);
expect(r.stderr).toContain("--- check:jsonb (rc=7)");
expect(r.stderr).toContain("stub check failing: check:jsonb");
expect(r.stderr).toMatch(/fail=1\b/);
expect(readFileSync(join(root, "logs", "check_jsonb.exit"), "utf8").trim()).toBe("7");
} finally {
rmSync(root, { recursive: true, force: true });
}
});
});