feat(cron): make isolated-agent setup watchdog configurable#88396
feat(cron): make isolated-agent setup watchdog configurable#88396Kaspnov wants to merge 2 commits into
Conversation
|
Thanks for the context here. I swept through the related work, and this is now duplicate or superseded. Close: the runtime problem this branch tries to address is now owned by the newer, clean, proof-positive bounded-watchdog PR, while this branch still adds an unaccepted config surface and has concrete timer-safety and documentation blockers. Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Canonical path: Follow #96219 for the current fix, then open a separate product-approved config-key proposal only if operators still need a manual override after the bounded default ships. So I’m closing this here and keeping the remaining discussion on #96219. Review detailsBest possible solution: Follow #96219 for the current fix, then open a separate product-approved config-key proposal only if operators still need a manual override after the bounded default ships. Do we have a high-confidence way to reproduce the issue? Partly. Current main source clearly still arms a fixed 60s pre-runner setup watchdog, but this PR does not provide a high-confidence real cron-runtime reproduction showing the new config override changes the runner-start path under load. Is this the best way to solve the issue? No. The newer bounded-watchdog PR is the better current fix because it changes the current watchdog owner module, derives from the existing job timeout budget, caps the runtime delay, and avoids a new public config surface. Security review: Security review cleared: No concrete security or supply-chain concern was found; the diff changes cron/config source, tests, and a changeset without new dependencies, workflows, secrets handling, or third-party execution. AGENTS.md: found and applied where relevant. What I checked:
Likely related people:
Codex review notes: model internal, reasoning high; reviewed against 08d15ec32db3. |
The cron setup watchdog (hardcoded 60s) aborts an isolated agent job that does not reach "runner started" in time. When several heavy isolated jobs start in the same window, concurrent setup (runtime init, plugin load, CLI handshake) contends on the single gateway event loop and a healthy agent can exceed 60s during setup, producing false "isolated agent setup timed out before runner start" failures even on idle, high-spec hosts. Add an optional cron.agentSetupWatchdogMs override (default 60000ms), clamped to a 1000ms minimum so the safety timer cannot be disabled. Mirrors the existing resolveRunConcurrency() config pattern in the same module.
OpenClawSchema makes cron strict and did not include agentSetupWatchdogMs, so the new key failed to parse from real config (Unrecognized key), breaking the feature as shipped. Add it to the cron zod schema and a config-parse regression test covering accept / omit / reject cases.
564f085 to
3fb689a
Compare
|
@Kaspnov thanks for the PR. ClawSweeper is still waiting on real behavior proof before this can move forward. Useful proof can be a screenshot, short video, terminal output, copied live output, linked artifact, or redacted logs that show the changed behavior after the fix. Please redact private tokens, phone numbers, private endpoints, customer data, and anything else sensitive. Once proof is added to the PR body or a comment, ClawSweeper or a maintainer can re-check it. |
|
ClawSweeper applied the proposed close for this PR.
|
Summary
runner startedis a hardcoded60_000ms (CRON_AGENT_SETUP_WATCHDOG_MSinsrc/cron/service/timer.ts). It is not overridable via config.isolated agent setup timed out before runner start— even on idle, high-spec hosts where nothing is actually stuck.cron.agentSetupWatchdogMsoverride (default60000), clamped to a1000ms minimum so the safety timer can never be disabled. It mirrors the existingresolveRunConcurrency()config pattern already in the same module (state.deps.cronConfig?.maxConcurrentRuns).Why now: operators running many isolated cron jobs on capable hardware hit false setup timeouts with no supported way to tune the budget; the only current mitigation is manually spreading job schedules.
Out of scope: the pre-execution watchdog and per-job execution
timeoutSecondsare unchanged. Default behaviour is unchanged when the new field is unset.What success looks like: with
cron.agentSetupWatchdogMsset above 60s, concurrently-starting isolated jobs that previously failed setup now reach the runner; unset config behaves exactly as before.Reviewers may want to focus on: the clamp/fallback semantics in
resolveCronAgentSetupWatchdogMs(min 1000 ms; non-finite → default) — the watchdog can never be disabled.Real behavior proof
Behavior addressed:
cron.agentSetupWatchdogMsis accepted by the cron config schema and the cron timer resolver uses the configured setup watchdog budget while preserving the 60s default and 1s safety floor.Real environment tested: Local OpenClaw contributor checkout on macOS, branch
feat/configurable-cron-setup-watchdogat3fb689ac0d, with repository dependencies already installed.Exact steps or command run after this patch:
node_modules/.bin/vitest run src/cron/service/timer.setup-watchdog.test.ts --reporter=verbose 2>&1 | grep -E '✓|Tests 'andnode_modules/.bin/vitest run src/cron/service src/config/zod-schema.cron-setup-watchdog.test.ts 2>&1 | tail -8.Evidence after fix: Terminal capture from this branch after the patch:
Observed result after fix: The resolver proof shows default, valid override, fractional floor, minimum clamp, and non-finite fallback all passing; the broader cron service plus schema run completed with
Test Files 13 passed (13)andTests 123 passed (123).What was not tested: No live production cron jobs were started; this PR only changes local config parsing and timer budget resolution, and the default runtime behaviour remains unchanged when the new option is unset.
Testing
src/cron/service/timer.setup-watchdog.test.ts(5 cases: default, valid override, fractional floor, sub-minimum clamp, non-finite fallback) — passing.src/cron/serviceplussrc/config/zod-schema.cron-setup-watchdog.test.tsrun passes with no regressions.Linked context
Closes #
Related #