mirror of
https://github.com/openclaw/openclaw.git
synced 2026-10-03 01:29:56 +00:00
6eeb120d69
* fix(gateway): derive the darwin stop budget from the launchd job resolveGatewayShutdownBudget derives the stop deadline from restart ownership, so a darwin host running with OPENCLAW_SUPERVISOR_MODE=external resolves drain=315000ms whatever launchd actually enforces. The linux-gated systemd probe becomes a platform dispatch, and a new readLaunchdStopTimeout mirrors readSystemdStopTimeout: it reads the running job's effective exit timeout and accepts it only when the printed pid is this process. run-loop.ts is untouched because the reader detects launchd from the environment itself. Closes #156968 * fix(gateway): accept the launchd launcher parent and cap its deadline The darwin reader accepted the printed job only when its pid was this process. The installed service can keep a launcher parent while the serving Gateway runs as its child, so the job prints the launcher's pid, the enforcing job was rejected, and the Gateway fell back to 20 seconds in the one layout where an operator's ExitTimeOut was meant to apply. Accepting that job at face value would be worse than rejecting it. node-runtime-recovery.mjs builds the launcher's own reap timer from the compile-time LAUNCH_AGENT_EXIT_TIMEOUT_SECONDS rather than from the job, so on a forwarded stop it re-sends SIGTERM to its child at 18000ms and SIGKILLs it at 19000ms. Adopting a 90 second job deadline in that layout would plan a 75000ms drain and then lose it to its own parent, which is the truncated drain this change exists to prevent. The reader now accepts process.pid or process.ppid and takes min(jobExitTimeout, 20000) in the launcher case, naming the cap in the source string only when it binds. A shorter job deadline still applies in both layouts, because launchd reaps the whole job regardless of the launcher. The docs claimed the deadline is read at startup and when accepting shutdown. run-loop.ts gates the budget refresh on linux at both consumption sites, so on darwin it is read once, at startup, and nativeStopBudget has no shutdown-time consumer there. The page now says startup-only and drops the sentence about a failed shutdown reread retaining the startup budget, which described linux behaviour only. * fix(gateway): scope the darwin stop budget to launchd-driven stops Address the two review findings on the previous revision. The launcher parent is now identified by an explicit OPENCLAW_LAUNCHER_STOP_TIMEOUT_MS declaration rather than a bare ppid match, so an unknown parent keeps the job's full deadline instead of being capped on a guess. The darwin reader inspects the launchd job only while a stop is under way, and adopts that job's deadline only when the job reports the SIGTERMed state. An external SIGTERM keeps the platform-neutral drain, because launchd's ExitTimeOut bounds only a stop launchd itself is running. The job state is read with an anchored single-tab pattern. launchctl print emits further "state = active" lines inside the resource and jetsam coalition blocks, and the shared key/value parser keeps the last occurrence, so the shared parser could never observe SIGTERMed. A test fixture reproduces the nested coalition shape. * fix(gateway): keep a failed launchd inspection off the native stop budget `readLaunchdStopTimeout` answered a failed job inspection with the Gateway's own 330000ms stop policy wrapped in a non-null result. `resolveGatewayShutdownBudget` classifies every non-null result as a native stop budget, so a probe that established nothing still set `nativeStopBudget`. On an externally supervised darwin Gateway that caps a longer requested restart drain and can arm a forced exit against a deadline no supervisor was confirmed to be enforcing. The reader now reports its two answers separately. `stop` carries a deadline only while launchd is confirmed to be stopping the job; `warning` reaches the operator either way. A failed inspection reports `stop: null` and leaves the caller on the platform-neutral policy it had already resolved, which is the same number as before, now classified honestly. The reader no longer imports GATEWAY_SERVICE_STOP_TIMEOUT_MS, because choosing the Gateway's policy number was never its job. Tests assert the flag and the downstream restart drain in the failure case, paired with a confirmed-deadline case so deleting the read outright cannot pass both. Also fixes TS2532 on the job-state regex: the named group is optional under `noUncheckedIndexedAccess`, so `.trim()` needed the optional chain. * test(infra): isolate the hoisted spawn mock and ratchet the OPENCLAW_* count `spawn` is hoisted once per file, so its call log survived across cases and `toHaveBeenCalledExactlyOnceWith` could only hold for the first one. Clear it in `beforeEach`. CI reports `OPENCLAW_* count 488 exceeds budget 487; update config/env-var-count-budget.txt` as a warning. The new name is the launcher's OPENCLAW_LAUNCHER_STOP_TIMEOUT_MS declaration, so the count is correct and the ratchet moves to 488 with the reason recorded in the file. * style(infra): apply oxfmt to the new launchd reader assertions Run by the repo's own `pnpm run format`; joins one over-wrapped assertion line. `pnpm run format:check` then reports "All matched files use the correct format." * test(cli): mock the launchd reader and split the darwin run-loop cases out Activating the darwin stop-budget refresh made `run-loop.test.ts` exercise the real `readLaunchdStopTimeout`, which spawns three `launchctl print` calls against live subprocess I/O while the suite holds `vi.useFakeTimers()`. One case ran 120039ms before failing and the abandoned loop leaked into later cases, for eight failures in total. All eight clear by mocking the reader; no existing expectation was wrong. The mock could not simply be added. `run-loop.test.ts` is 3813 lines against a 1000 line cap where `check-line-cap-ratchet` rejects any growth, and an earlier attempt failed with `3501 -> 3510 counted lines`. Taking the remedy that ratchet names, the darwin cases move to `run-loop.launchd.test.ts` and the original file shrinks by 82 lines. It keeps the mock too, because four of those cases are registered into it by shared `register*` helpers whose `it.each` arrays interleave systemd and launchd variants and cannot be split without touching those support files. The new file adds one case the default mock would otherwise hide: a job reporting a 30000ms deadline bounds the stop at 25000ms and arms the force exit there. `node-runtime-recovery.test.ts` asserts the replacement child's spawn env exactly, so it gains the launcher's own OPENCLAW_LAUNCHER_STOP_TIMEOUT_MS declaration rather than a loosened matcher. That argv is a non-foreground `doctor --fix`, so the value is the 1s signal exit grace plus the 1s force-kill grace. * fix(gateway): only retain a startup budget when the probe was inconclusive Extending the stop-budget refresh to darwin gave the retained-budget safety net a new and wrong trigger. On a launchd-owned Gateway the startup budget is already native, and an in-process restart signals the process without launchd running the stop, so the job still prints `running` and the reader reports `stop: null`. The old condition treated any null as unconfirmed, so every in-process restart of a default macOS install logged "Retaining the startup shutdown budget of 15000ms because the current supervisor stop timeout could not be confirmed." The timeout was confirmed; it was confirmed not to apply. `readNativeStopTimeout` now reports `inconclusive` alongside the deadline, and only an inconclusive probe retains. Linux keeps its exact previous condition, an absent unit or a warned read. The budget number was already unaffected, so this is the message and the wasted `launchctl print` calls, not the deadline. Two cases pin it: a confirmed not-stopping read warns nothing, and a failed inspection still retains and still says so. Also fixes two wording defects found in review. `unresolved()` no longer takes a label it only ever interpolated as "the launchd job the configured label"; each failure already names its own target. And the `readJobState` comment said `state` appears four times in `launchctl print` output when its own next clause counts three, which is what a live LaunchDaemon prints. * style(cli): apply oxfmt to the new retained-budget assertions Run by the repo's own `pnpm run format`; joins one over-wrapped assertion line. Whitespace only, in a test file, so it cannot change runtime behaviour. * fix(gateway): a defaulted deadline is not an inconclusive probe `inconclusive` keyed off the warning alone, so the defaulted-value case counted as a failed probe. There launchd is confirmed to be stopping the job and only its deadline had to be guessed, so a clock is genuinely running and there is nothing to retain. Under launchd ownership that combination logged both "using 20000ms default" and "could not be confirmed" for the same stop, the second being false. Now only a warning with no deadline at all counts as inconclusive. A third case pins it, asserting the defaulted read warns exactly once and about the deadline rather than about confirmation. Also corrects a test fixture comment that still said `launchctl print` emits `state` four times; the arrangement it documents, and a live LaunchDaemon, both show three. * test(cli): correct the regression guard's account of the reported drain The comment said active work "had time to finish" during the reported 315 second drain. The reporting host's log shows the opposite: that drain hit its own timeout with five tasks still active, and the Sep 20 stop with six. The drain was being used, which is a stronger reason for the guard, not a weaker one. * fix(infra): stop exporting a type nothing outside the module reads Knip's all-exports gate reads LaunchdStopTimeout as dead: the only public surface is readLaunchdStopTimeout's LaunchdStopRead return, and nothing imports the nested shape by name. Keep it module-local. * fix(gateway): keep a proportional drain and cap an undeclared launcher The fixed 10s reserve and 5s exit margin were sized against the 315s policy drain. Subtracting them outright from a launchd job's own ExitTimeOut spent a short deadline entirely on overhead: 5 seconds funded the margin alone and 15 funded margin plus reserve, so active work drained for zero milliseconds in both. Each allowance now takes at most a share of what it is carved from, so a deadline long enough to fund them is unchanged and a shorter one keeps a proportional drain. Drain is now positive for every positive deadline and never decreases as ExitTimeOut grows. The shutdown log also subtracted the unscaled reserve constant when reporting drain, which understated a short budget by the amount the scaled reserve gives back, so it now reports the margin and drain actually spent. An already-running launcher published before OPENCLAW_LAUNCHER_STOP_TIMEOUT_MS existed arms the same reap timer and declares nothing, which is the upgrade shape: replacing files cannot change a launcher that is already running. Reading that silence as "no deadline" let a long custom ExitTimeOut be budgeted past a force-kill the parent was already counting down. The launcher's arithmetic moves into the shared budget module so the serving Gateway reconstructs the same timer, used only where the printed job carries OpenClaw's own label and its pid is this process's immediate parent. In that position the parent is the process launchd started for OpenClaw's job and the recovery launcher is the only path that puts a Gateway underneath it, so an unrelated process manager holding the parent slot still caps nothing. * fix(infra): gate the launcher reconstruction on the recovery respawn marker Holding the launchd job's pid is not evidence of a reap timer. An operator wrapper can keep that pid and start the Gateway itself while running none, so reconstructing a cap from the parent relation alone would cut a drain nothing was going to interrupt. The recovery launcher has stamped OPENCLAW_NODE_UPDATE_RESPAWNED on every child it respawns since long before it declared a stop timer, so that marker is present in exactly the upgrade case and absent for any other parent. * fix(infra): declare the new shared budget helpers for the type checker The root module is typed by a hand-written declaration file rather than being compiled, so new exports are invisible to src without being declared there. * docs(gateway): scope the positive-drain claim to the allocation The elapsed cost of resolving the budget is debited after the allocation, so a deadline shorter than that cost can still leave nothing to spend. Claiming every positive deadline yields a positive drain overstated it. * test(cli): accept the retained-budget attribution on a synthetic launchd label The fixture declares a launchd label with no matching job, so the per-stop re-inspection cannot read one and the startup budget is retained. The deadline is unchanged at 15000ms; only the source attribution differs. The assertion still fails if the budget comes from the platform-neutral policy instead. * fix(gateway): derive the launcher's reap deadline instead of declaring it Measurement showed the declared value and the derived value are the same number: a published launcher and a candidate launcher both arm an 18000ms exit grace, and a candidate Gateway resolved an identical 19000ms cap and 14231ms budget under each. The declaration was therefore carrying no information the child could not compute, while adding an OPENCLAW_* name and leaving the upgrade path conditional on which build of the launcher happened to be running. Both sides now read one expression in gateway-shutdown-budget.mjs, the launcher to arm its escalation and the Gateway to bound its budget, so they cannot disagree and there is no published-versus-candidate launcher distinction left to prove. The cap stays gated on OPENCLAW_NODE_UPDATE_RESPAWNED so a parent that did not respawn this process still caps nothing. Drops the env-var count back to 486, so the name ratchet no longer needs an owner waiver, and takes config/env-var-count-budget.txt out of the diff entirely. Test deadlines move off 90 seconds onto 55, because launchd clamps ExitTimeOut at 60 and a 90-second case cannot occur on macOS 27. * test(infra): restore the recovery spawn-env cases to their base form The launcher no longer declares a deadline, so the assertions these cases grew have nothing left to check and their explanatory comment described behavior that is gone. * fix(gateway): cap every respawning launcher, not just the Node-recovery one All three runRespawnedChild call sites reach the same launcher through the same function and arm the same escalation, but only the Node-recovery one sets OPENCLAW_NODE_UPDATE_RESPAWNED. Gating on that marker alone left the two compile-cache respawns uncapped, and the packaged one can wrap a foreground gateway run on an installed service: the job's deadline would then be budgeted past a force-kill the parent had already armed, which is the hazard this cap exists to prevent. The marker set moves next to the deadline it authorises, so adding a respawn path cannot silently escape the cap. Keeping the literals in the root module also keeps them out of the env-count ratchet's scope: reading the marker from counted production source promoted a previously test-only name and held the count at 487 even after the declared-timer variable was removed. Also applies oxfmt's own wrap to the launcher-relation expression. * fix(infra): re-export the respawn marker set through the infra barrel The constant existed only on the root module, so the test importing it from the barrel got undefined and it.each(undefined) threw during collection: the suite reported zero tests rather than a failure, and typecheck flagged the missing member. Re-exporting the binding adds no OPENCLAW_* literal under src, so the env-var count stays at 486. * fix(gateway): address the pre-review findings on the launchd stop budget Docs were stale or wrong in four places. The systemd section still promised a fixed 10 second reserve and 5 second margin, but the share caps are platform-neutral and do change a Linux unit whose TimeoutStopSec is under 25 seconds; that radius is now stated with the 20 second and 15 second cases spelled out. The direct-SIGTERM paragraph claimed the platform-neutral drain "is the deadline that actually governs that stop", which the branch's own kill -TERM capture contradicts: no supervisor deadline governs that stop at all and the fallback is the template value. The updated-launcher requirement no longer applies to the stop budget and says so. The marker sentence named one marker when three are honoured, and one measured figure was stale. Two claims in the code were too strong. The marker list asserted every setter arms the shared escalation, which is false for the compile-cache name: entry.compile-cache sets it for a runner that reaps on a fixed short grace. That runner refuses a foreground Gateway run off Windows and this deadline is only read during a darwin stop, so it cannot be the parent here, and the comment now says that instead of implying exclusivity. The sync note next to the escalation now records that the other runner keeps its own copies of the graces. An absent job state was indistinguishable from a job launchd is not stopping, so a macOS that printed the block differently would silently revert to the platform-neutral policy. When a deadline parsed but the state did not, that now warns and marks the read inconclusive rather than passing as a positive answer. Tests: launchd cases move off 90 and 315 second deadlines, which launchd clamps to 60 and cannot produce; the marker list is restated locally and tied back to the implementation so dropping a marker fails; and the real-process case now asserts the darwin probe actually ran, which it could not before. Retires five exports with no production consumer that the dead-export scan flagged. * test(infra): move the empty job state onto the missing-state warning An empty value yields no state to recognise, so it reaches the same branch as a missing line and now carries the same warning. Keeping it in the unrecognised-state table asserted the absence of a field the reader deliberately reports. * fix(infra): treat a whitespace-only job state as missing, and sharpen two docs claims The state value is trimmed after matching, so a line carrying only whitespace yielded an empty string rather than undefined and skipped the warning while behaving exactly like a missing line. The systemd section now says the exit margin shrinks below a 20-second deadline, not just the reserve. The direct-signal paragraph distinguished only the launchd-supervised budget; external supervisor mode keeps the platform-neutral policy and arms no force-exit timer, and a signal delivered to the job pid does reach a resident recovery launcher, which arms its own reap timer even though launchd is not stopping the job. * docs(gateway): correct the loaded launchd job deadline guidance launchctl print reports the job launchd has loaded, not the plist on disk, so editing a loaded job's ExitTimeOut changes nothing until the job is reloaded. Measured on macOS 27 against a scratch job: the plist moves from 20 to 47 while the loaded job keeps reporting 20, and it still reports 20 after launchctl kickstart -k, which restarts the process without reloading the job. Only bootout then bootstrap makes it report 47, and that restarts the Gateway with it. State what reading per stop actually buys instead: the Gateway never plans against a deadline it cached at its own startup. * fix(gateway): keep the full cleanup reserve wherever a deadline funds it The reserve was capped at half the shutdown budget whenever the budget could not fund it outright. That kept a drain on short deadlines, but it also reallocated deadlines that already worked: a 20 second ExitTimeOut, which is both the shipped LaunchAgent template and launchd's own default, moved from a 10 second reserve and a 5 second drain to 7.5 seconds of each. Cleanup costing between 7.5 and 10 seconds finished before and would have been cut off. Bound the reserve by what keeping a drain actually requires instead. The floor is 5 seconds, which is the drain the 20 second template already yields once the fixed margin and reserve are subtracted, so funding the full reserve alongside it takes 15 seconds of budget: exactly what that template resolves to. Every deadline from 20 seconds up therefore keeps the allocation it had, and only a deadline the fixed subtraction had already driven under a 5 second drain gives any reserve up, never below half the budget. Both allocations stay monotone in the deadline, and the 5 second case is unchanged at 1875ms of each. * fix(gateway): correct shutdown-budget probe-debit overclaims - gateway-shutdown-budget.mjs: replace the floor-invariant claim with the real bound (reserve unchanged only at a >=15000ms post-margin budget, i.e. a >=20s deadline once the launchd probe's own cost, up to three launchctl print calls at 2000ms each, is subtracted); name the 8-20s sub-band reserve loss the old comment denied, including 16-19s where a flat subtraction already left a positive drain while still funding the reserve in full - docs/gateway/restart-recovery.md: drop the systemd section's "down to the millisecond" promise and the launchd section's "exact allocation ... before this change" claim; both now state the inspection cost that comes off instead - run-loop-shutdown-budget.test.ts: add a real 13ms elapsed-debit case at a 20s exit timeout (asserts reserve=9987, drain=5000, contrasting with the existing acceptedAtMs=MAX_SAFE_INTEGER zero-debit fixtures) and mirror darwin's 19/16/15/10s sub-20s it.each against a systemd stub, which previously had no sub-20s coverage * test(cli): wrap the flat-subtraction assertion to satisfy oxfmt The sub-20-second systemd cases added in 60e5eb509b6d left the flat-subtraction comparison on a single line past the formatter's width, which failed oxfmt --check and took check-lint and check-docs down with it. Lint itself reported zero warnings and zero errors. Formatting only, no assertion or value changed. * docs(gateway): bound the 20-second allocation claim by the inspection debit The previous wording paired 'gives up exactly what the probe cost' with a probe bounded near 6 seconds, and those two cannot both hold. Which allowance pays the debit turns on a 5 second threshold: at or under it the reserve absorbs the whole cost and the 5 second drain floor is untouched, so a 20 second deadline resolves 10000 minus the debit. Past it the floor is share-bounded as well and the two converge on half the remainder, so a 6 second debit splits 9000 into 4500/4500 rather than retaining the old reserve. Documents the threshold in the docs and the root-module comment, and pins the over-threshold case with a test so the boundary is measured rather than reasoned. * docs(gateway): qualify the full-reserve claim by the probe debit The drain-floor comment asserted that a whole 10 second reserve follows for every ExitTimeOut of 20 seconds or more once the probe that reads the deadline is subtracted. The probe cost is charged before the budget is split, so a 20 second ExitTimeOut clears the 15 second budget that a whole reserve needs only when that probe costs nothing. At the 13ms measured on this rig the reserve is 9987, and a whole reserve at that cost needs 20013. The same comment already states this correctly six lines down, where it resolves a 20 second deadline to 10000 - cost. This removes the contradiction by qualifying the claim at its first statement instead of restating the arithmetic twice. Comment-only: no non-comment line changes and the export list is unchanged. * fix(gateway): honor unlimited launchd stops Preserve the observed launcher cap only for launchd-driven stops, and shorten redundant budget documentation without changing the measured split. Co-authored-by: Patrick-Erichsen <20157849+Patrick-Erichsen@users.noreply.github.com> * test(gateway): use action-based drain budget Co-authored-by: Patrick-Erichsen <20157849+Patrick-Erichsen@users.noreply.github.com> * style(gateway): format launchd timeout assertions Co-authored-by: Patrick-Erichsen <20157849+Patrick-Erichsen@users.noreply.github.com> * fix(ci): complete frozen Docker planner import closures --------- Co-authored-by: roboclaw-bot <309084314+roboclaw-bot@users.noreply.github.com> Co-authored-by: Patrick-Erichsen <20157849+Patrick-Erichsen@users.noreply.github.com> Co-authored-by: Patrick Erichsen <patrick.a.erichsen@gmail.com>
16 lines
808 B
TypeScript
16 lines
808 B
TypeScript
export const GATEWAY_SHUTDOWN_RESERVE_MS: number;
|
|
export const GATEWAY_SUPERVISOR_EXIT_MARGIN_MS: number;
|
|
export const GATEWAY_SHUTDOWN_TIMEOUT_MS: number;
|
|
export const GATEWAY_SERVICE_STOP_TIMEOUT_MS: number;
|
|
export const LAUNCH_AGENT_EXIT_TIMEOUT_SECONDS: 20;
|
|
export const GATEWAY_RESTART_REPLACEMENT_TIMEOUT_MS: number;
|
|
export const RESPAWN_SIGNAL_FORCE_KILL_GRACE_MS: number;
|
|
export const RESPAWN_SIGNAL_HARD_EXIT_GRACE_MS: number;
|
|
export function resolveSupervisorExitMarginMs(stopTimeoutMs: number): number;
|
|
export function resolveShutdownReserveMs(shutdownTimeoutMs: number): number;
|
|
export function isRespawnedByLauncher(env: NodeJS.ProcessEnv): boolean;
|
|
export function resolveLauncherStopTimeoutMs(params: {
|
|
env: NodeJS.ProcessEnv;
|
|
platform: NodeJS.Platform;
|
|
foreground: boolean;
|
|
}): number;
|