mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-23 10:55:31 -06:00
Jesse Merhi
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>