mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-24 19:35:28 -06:00
ee6e0251b4
* 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 on6ba02c9d0araised 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 onc7a699ee82reversed 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 on0d9f3604e8flagged 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>
101 lines
3.5 KiB
TypeScript
101 lines
3.5 KiB
TypeScript
// Runs tsgo through local resource policy and sparse-checkout guards.
|
|
import fs from "node:fs";
|
|
import os from "node:os";
|
|
import path from "node:path";
|
|
import { readFlagValue } from "./lib/arg-utils.mts";
|
|
import {
|
|
applyLocalTsgoPolicy,
|
|
ensureRepoToolNodeModulesLink,
|
|
resolveLocalCheckEnv,
|
|
resolveRepoToolBinPath,
|
|
} from "./lib/local-check-runtime.mts";
|
|
import { runManagedCommand } from "./lib/managed-child-process.mts";
|
|
import { readPositiveEnvInt } from "./lib/numeric-options.mjs";
|
|
import {
|
|
getSparseTsgoGuardError,
|
|
shouldSkipSparseTsgoGuardError,
|
|
} from "./lib/tsgo-sparse-guard.mts";
|
|
|
|
// Declared locally, as sibling scripts do, rather than imported from packages/:
|
|
// a static import there resolves before the sparse-checkout guard can report a
|
|
// missing project, turning a clean skip into ERR_MODULE_NOT_FOUND. Mirrors
|
|
// normalization-core's MAX_TIMER_TIMEOUT_MS.
|
|
const MAX_TIMER_TIMEOUT_MS = 2_147_000_000;
|
|
|
|
export function resolveTsgoTimeoutMs(env: NodeJS.ProcessEnv): number | undefined {
|
|
if (!env.OPENCLAW_TSGO_TIMEOUT_MS?.trim()) {
|
|
return undefined;
|
|
}
|
|
return Math.min(
|
|
readPositiveEnvInt("OPENCLAW_TSGO_TIMEOUT_MS", env, MAX_TIMER_TIMEOUT_MS),
|
|
MAX_TIMER_TIMEOUT_MS,
|
|
);
|
|
}
|
|
|
|
async function main(): Promise<void> {
|
|
const hostResources = {
|
|
logicalCpuCount:
|
|
typeof os.availableParallelism === "function" ? os.availableParallelism() : os.cpus().length,
|
|
totalMemoryBytes: os.totalmem(),
|
|
};
|
|
const { args: finalArgs, env } = applyLocalTsgoPolicy(
|
|
process.argv.slice(2),
|
|
resolveLocalCheckEnv(process.env),
|
|
hostResources,
|
|
);
|
|
|
|
const tsgoPath = resolveRepoToolBinPath("tsgo");
|
|
const tsBuildInfoFile = readFlagValue(finalArgs, "--tsBuildInfoFile");
|
|
if (tsBuildInfoFile) {
|
|
fs.mkdirSync(path.dirname(path.resolve(tsBuildInfoFile)), { recursive: true });
|
|
}
|
|
const sparseGuardError = getSparseTsgoGuardError(finalArgs, { cwd: process.cwd() });
|
|
if (sparseGuardError) {
|
|
console.error(sparseGuardError);
|
|
if (shouldSkipSparseTsgoGuardError(env)) {
|
|
console.error("[tsgo] skipping sparse-missing project because OPENCLAW_TSGO_SPARSE_SKIP=1");
|
|
process.exitCode = 0;
|
|
} else {
|
|
process.exitCode = 1;
|
|
}
|
|
return;
|
|
}
|
|
|
|
ensureRepoToolNodeModulesLink(tsgoPath);
|
|
let timeoutMs: number | undefined;
|
|
try {
|
|
timeoutMs = resolveTsgoTimeoutMs(env);
|
|
} catch {
|
|
// main() is top-level awaited, so an escaping parse error would surface as a raw
|
|
// module rejection with no guidance about the variable that caused it.
|
|
console.error(
|
|
`[tsgo] OPENCLAW_TSGO_TIMEOUT_MS must be plain decimal digits with no leading zero, sign, exponent, or decimal point, between 1 and ${Number.MAX_SAFE_INTEGER}; got ${env.OPENCLAW_TSGO_TIMEOUT_MS}. Unset it to disable the watchdog.`,
|
|
);
|
|
process.exitCode = 1;
|
|
return;
|
|
}
|
|
try {
|
|
// Managed run owns the whole tsgo process tree: on timeout it SIGKILLs the
|
|
// process group, because a wedged checker ignores SIGTERM and would otherwise
|
|
// block the caller forever on a compiler that will never report.
|
|
process.exitCode = await runManagedCommand({
|
|
bin: tsgoPath,
|
|
args: finalArgs,
|
|
env,
|
|
timeoutMs,
|
|
});
|
|
} catch (error) {
|
|
if ((error as { code?: string } | undefined)?.code !== "ETIMEDOUT") {
|
|
throw error;
|
|
}
|
|
console.error(
|
|
`[tsgo] no completion after ${timeoutMs}ms; killed the tsgo process tree. Raise OPENCLAW_TSGO_TIMEOUT_MS for intentionally longer builds, or unset it to disable the watchdog.`,
|
|
);
|
|
process.exitCode = 1;
|
|
}
|
|
}
|
|
|
|
if (import.meta.main) {
|
|
await main();
|
|
}
|