Files
Jesse Merhi ee6e0251b4 fix(scripts): clean up tsgo process trees on timeout or signal (#123975)
* fix(scripts): bound tsgo runs with the managed-command watchdog

run-tsgo bypassed the repo's managed-command seam and called spawnSync
directly, so a wedged tsgo blocked its caller indefinitely: no timeout, no
process-group cleanup, and no SIGKILL escalation.

Observed in the wild as a tsgo holding 2.85 GB for 90+ minutes on 41s of
total CPU with RSS frozen to the byte, ignoring SIGTERM, with its wrapper
reparented to init. Because shouldReclaimLock() treats a live PID as a valid
lock owner, that orphan also held the heavy-check lock until every other
invocation hit the 10-minute lock timeout and threw.

Route the run through runManagedCommand, which already owns process-group
termination and SIGKILL escalation on timeout, and bound it with
OPENCLAW_TSGO_TIMEOUT_MS (default 45m) through the shared readPositiveEnvInt
helper, mirroring OPENCLAW_CLI_STARTUP_BUILD_TIMEOUT_MS in
ensure-cli-startup-build.mts.

* fix(scripts): saturate the tsgo watchdog at Node's timer ceiling

An OPENCLAW_TSGO_TIMEOUT_MS above 2147483647 reached setTimeout unchanged,
where Node collapses it to a 1ms delay, so raising the override killed
healthy typechecks immediately instead of loosening the bound.

* fix(scripts): make the tsgo watchdog opt-in and stop the harness leaking

ClawSweeper review on 6ba02c9d0a raised two findings.

[P1] The 45-minute default applied an unproven deadline to every tsgo
invocation. No supported duration contract covers every host and project, and
CI already bounds its own tsgo jobs at 15-20 minutes, so the default could only
ever fire outside CI where it was least validated. Drop it: an unset
OPENCLAW_TSGO_TIMEOUT_MS keeps the pre-existing unbounded wait, so no existing
run changes behavior, and operators opt in per host. Documented in
docs/help/testing.md beside the sibling Vitest watchdog.

[P2] The regression harness could leak its wedged child. The fake compiler
ignores SIGTERM by design, so a pre-fix or otherwise failing run left the tree
running after spawnSync gave up. Bound the fixture's loop as a backstop.

* fix(scripts): set the tsgo watchdog default from measured lane duration

ClawSweeper on c7a699ee82 reversed its earlier guidance: the opt-in default
adopted last iteration "deliberately preserves the indefinite tsgo hang that
this PR is meant to fix". Its objection was never that a default existed, only
that 45 minutes was unmeasured.

Measured instead of guessed: hosted tsgo lanes (check-test-types, and its core
stripes) complete in 1-2 minutes across recent successful main runs, against CI
job caps of 15-20 minutes. 30 minutes is 15-30x the observed duration, leaves
room for a far slower local host, and still bounds the 90-minute and multi-hour
wedges that motivated this PR. OPENCLAW_TSGO_TIMEOUT_MS remains the documented
override for hosts that need longer.

* test(scripts): reap the wedged fake tsgo tree on the harness outer timeout

ClawSweeper on 0d9f3604e8 flagged that the harness can still leave a detached
pre-fix process tree alive after its outer timeout. The bounded fixture loop
added earlier only capped the leak; it did not terminate the tree.

spawnSync's killSignal reaches the direct child only. runManagedCommand spawns
the compiler detached into its own process group, so the fake tsgo is a
grandchild that never receives that signal. The fixture now records its pid and
the harness reaps that group in a finally, with the bounded loop kept as a
last-resort backstop.

Verified: pid file written with the live pid, and killing that group terminates
the tree; focused suite 16/16 with no surviving fake-tsgo processes.

* fix(scripts): harden the tsgo watchdog after two-phase code review

Review fixes on top of the watchdog change, from one native pass and six cold
passes:

- A rejected OPENCLAW_TSGO_TIMEOUT_MS escaped main() as a raw module rejection.
  It now reports one actionable line and exits 1. Strict validation was kept
  rather than switching to coercion, so a typo cannot silently fall back to the
  30-minute default.
- The rejection message named a numeric range while the parser enforces plain
  decimal digits, so 1e5 and 007 were refused by a message saying they
  qualified. It now names the real format and states that the watchdog cannot
  be disabled.
- The timer ceiling is declared locally rather than imported from packages/.
  A static import there resolves before the sparse-checkout guard runs, which
  turned a clean sparse skip into ERR_MODULE_NOT_FOUND and flipped
  check-changed's typecheck lane from exit 0 to exit 1.
- The wedge test asserted the kill message but not the outcome; it now captures
  the wedged pid and asserts the process group is gone.
- Three near-duplicate "not killed" cases are table-driven.
- Doc bullet corrected: values ABOVE the ceiling saturate at it, and the
  rejected-value list now includes non-decimal input.

Deferred follow-up, not fixed here: scripts/lib/tsx-cli-shim.mjs shares a
5000ms force-kill delay with managed-child-process, so Ctrl-C can still orphan
a wedged compiler about one run in three. Measured base 4/4 orphaned versus
4/10 here, so this change improves it; the fix is out of diff and shared with
four other wrappers.

* fix(scripts): close tsgo signal cleanup race

* fix(scripts): make tsgo watchdog opt-in

---------

Co-authored-by: ClawSweeper <steipete+clawsweeper@gmail.com>
2026-08-23 14:45:40 +00:00

157 lines
4.7 KiB
JavaScript

// Bootstraps documented JavaScript entrypoints before the TypeScript loader is active.
import { spawn, spawnSync } from "node:child_process";
import { createRequire } from "node:module";
import { constants as osConstants } from "node:os";
import path from "node:path";
import { fileURLToPath, pathToFileURL } from "node:url";
const FORWARDED_SIGNALS = ["SIGINT", "SIGTERM", "SIGHUP"];
const DEFAULT_FORCE_KILL_DELAY_MS = 5_000;
const SHIM_CHECKOUT_ROOT = path.resolve(path.dirname(fileURLToPath(import.meta.url)), "..", "..");
function resolvePrimaryRoot(checkoutRoot) {
const result = spawnSync("git", ["rev-parse", "--git-common-dir"], {
cwd: checkoutRoot,
encoding: "utf8",
stdio: ["ignore", "pipe", "ignore"],
});
if (result.status !== 0) {
return null;
}
const commonDir = result.stdout.trim();
if (!commonDir) {
return null;
}
const resolved = path.resolve(checkoutRoot, commonDir);
return path.basename(resolved) === ".git" ? path.dirname(resolved) : null;
}
function resolveTsxImport(checkoutRoot) {
const modulesDir =
process.env.PNPM_CONFIG_MODULES_DIR?.trim() || process.env.npm_config_modules_dir?.trim();
const hydratedTsxRoot = modulesDir
? path.join(path.resolve(checkoutRoot, modulesDir), "tsx")
: null;
let resolutionError;
for (const candidateRoot of [
hydratedTsxRoot,
checkoutRoot,
resolvePrimaryRoot(checkoutRoot),
].filter(Boolean)) {
try {
const require = createRequire(path.join(candidateRoot, "package.json"));
return pathToFileURL(require.resolve("tsx")).href;
} catch (error) {
resolutionError = error;
}
}
throw resolutionError;
}
function signalExitCode(signal) {
const signalNumber = osConstants.signals[signal];
return typeof signalNumber === "number" ? 128 + signalNumber : 1;
}
function writeFailureTrailer(tool, exitCode) {
if (tool && exitCode !== 0) {
console.error(`[${tool}] FAILED (exit ${exitCode})`);
}
}
function signalChild(child, signal, detached) {
if (!child?.pid) {
return;
}
try {
if (detached && process.platform !== "win32") {
process.kill(-child.pid, signal);
} else {
child.kill(signal);
}
} catch (error) {
if (!error || typeof error !== "object" || !("code" in error) || error.code !== "ESRCH") {
console.error(error);
}
}
}
async function runTsxCliShimInner(moduleUrl, options) {
const detached = options.detached ?? (process.platform !== "win32" && !process.stdin.isTTY);
const forceKillDelayMs = options.forceKillDelayMs ?? DEFAULT_FORCE_KILL_DELAY_MS;
let child = null;
let forceKillTimer = null;
const signalHandlers = new Map();
const cleanup = () => {
if (forceKillTimer) {
clearTimeout(forceKillTimer);
forceKillTimer = null;
}
for (const [signal, handler] of signalHandlers) {
process.off(signal, handler);
}
process.off("exit", exitHandler);
};
const exitHandler = () => signalChild(child, "SIGTERM", detached);
for (const signal of FORWARDED_SIGNALS) {
const handler = () => {
signalChild(child, signal, detached);
forceKillTimer ??= setTimeout(
() => signalChild(child, "SIGKILL", detached),
forceKillDelayMs,
);
forceKillTimer.unref();
};
signalHandlers.set(signal, handler);
process.on(signal, handler);
}
process.on("exit", exitHandler);
try {
const implementationUrl = new URL(options.implementation, moduleUrl);
const implementationPath = fileURLToPath(implementationUrl);
const tsxImport = resolveTsxImport(SHIM_CHECKOUT_ROOT);
const nodeExecutable = process.versions.bun ? "node" : process.execPath;
child = spawn(
nodeExecutable,
["--import", tsxImport, implementationPath, ...process.argv.slice(2)],
{
cwd: process.cwd(),
detached,
env: process.env,
stdio: "inherit",
},
);
const result = await new Promise((resolve, reject) => {
child.once("error", reject);
child.once("close", (code, signal) => resolve({ code, signal }));
});
cleanup();
if (result.signal) {
writeFailureTrailer(options.failureTool, signalExitCode(result.signal));
process.kill(process.pid, result.signal);
return;
}
const exitCode = result.code ?? 1;
writeFailureTrailer(options.failureTool, exitCode);
process.exitCode = exitCode;
} catch (error) {
cleanup();
console.error(error);
writeFailureTrailer(options.failureTool, 1);
process.exitCode = 1;
}
}
export async function runTsxCliShim(moduleUrl, options = {}) {
try {
await runTsxCliShimInner(moduleUrl, options);
} catch (error) {
console.error(error);
writeFailureTrailer(options.failureTool, 1);
process.exitCode = 1;
}
}