Skip to content

feat(cron): make isolated-agent setup watchdog configurable#88396

Closed
Kaspnov wants to merge 2 commits into
openclaw:mainfrom
Kaspnov:feat/configurable-cron-setup-watchdog
Closed

feat(cron): make isolated-agent setup watchdog configurable#88396
Kaspnov wants to merge 2 commits into
openclaw:mainfrom
Kaspnov:feat/configurable-cron-setup-watchdog

Conversation

@Kaspnov

@Kaspnov Kaspnov commented May 30, 2026

Copy link
Copy Markdown

Summary

  • The cron setup watchdog that aborts an isolated agent job which has not reached runner started is a hardcoded 60_000 ms (CRON_AGENT_SETUP_WATCHDOG_MS in src/cron/service/timer.ts). It is not overridable via config.
  • When several heavy isolated agent jobs come due in the same window, their setup (runtime init, plugin load, CLI handshake) contends on the single gateway event loop. A healthy agent can then exceed 60s during setup and is aborted with isolated agent setup timed out before runner start — even on idle, high-spec hosts where nothing is actually stuck.
  • This change adds an optional cron.agentSetupWatchdogMs override (default 60000), clamped to a 1000 ms minimum so the safety timer can never be disabled. It mirrors the existing resolveRunConcurrency() 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 timeoutSeconds are unchanged. Default behaviour is unchanged when the new field is unset.

What success looks like: with cron.agentSetupWatchdogMs set 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.agentSetupWatchdogMs is 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-watchdog at 3fb689ac0d, 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 ' and node_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:

✓ |cron| ../../src/cron/service/timer.setup-watchdog.test.ts > resolveCronAgentSetupWatchdogMs > defaults to 60s when no override is provided 10ms
✓ |cron| ../../src/cron/service/timer.setup-watchdog.test.ts > resolveCronAgentSetupWatchdogMs > honours a valid override 0ms
✓ |cron| ../../src/cron/service/timer.setup-watchdog.test.ts > resolveCronAgentSetupWatchdogMs > floors fractional overrides 0ms
✓ |cron| ../../src/cron/service/timer.setup-watchdog.test.ts > resolveCronAgentSetupWatchdogMs > clamps overrides below the safety minimum (1s) so the watchdog cannot be disabled 0ms
✓ |cron| ../../src/cron/service/timer.setup-watchdog.test.ts > resolveCronAgentSetupWatchdogMs > falls back to the default for non-finite values 0ms
      Tests  5 passed (5)

Test Files  13 passed (13)
      Tests  123 passed (123)

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) and Tests 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

  • New unit test src/cron/service/timer.setup-watchdog.test.ts (5 cases: default, valid override, fractional floor, sub-minimum clamp, non-finite fallback) — passing.
  • Full src/cron/service plus src/config/zod-schema.cron-setup-watchdog.test.ts run passes with no regressions.

Linked context

Closes #

Related #

@openclaw-barnacle openclaw-barnacle Bot added size: S triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026
@clawsweeper

clawsweeper Bot commented May 30, 2026

Copy link
Copy Markdown
Contributor

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
Relationship: superseded
Canonical: #96219
Summary: The central isolated cron runner-start timeout problem is now owned by a newer open PR that is clean, proof-positive, and patches the current watchdog owner path without adding a config key.

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 details

Best 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:

  • Repository policy applied: Root review policy marks config/default additions as compatibility-sensitive and requires best-fix comparison across current main, siblings, tests, and docs. (AGENTS.md:31, 08d15ec32db3)
  • Current main owner boundary: Current main defines and arms the isolated cron setup watchdog in agent-watchdog.ts, so the PR's old timer.ts patch is stale against the current owner module. (src/cron/service/agent-watchdog.ts:16, 08d15ec32db3)
  • Current main timer caller: executeJobCoreWithTimeout now calls createCronAgentWatchdog from timer.ts and reports setup-timeout telemetry from the current watchdog path. (src/cron/service/timer.ts:236, 08d15ec32db3)
  • Superseding PR: fix: let isolated cron setup use the job timeout budget #96219 is open, mergeable, labelled proof-sufficient/ready for maintainer look, and changes the current agent-watchdog.ts owner path with a 60s floor, half-job-timeout budget, and 10m cap. (src/cron/service/agent-watchdog.ts:145, ea14857772d2)
  • PR timer-safety defect: This PR's resolver minimum-clamps finite values but does not cap oversized values before they reach setTimeout. (src/cron/service/timer.ts:90, 3fb689ac0d61)
  • Node timer contract check: A local Node probe showed setTimeout(..., 2147483648) gets _idleTimeout: 1 and emits TimeoutOverflowWarning, making unbounded large config values availability-sensitive.

Likely related people:

  • aaroneden: Authored the open superseding bounded-watchdog PR and the earlier merged retry-classifier work for isolated setup timeouts. (role: canonical replacement author and adjacent contributor; confidence: high; commits: ea14857772d2, 73aab309894d; files: src/cron/service/agent-watchdog.ts, src/cron/service/timer.ts, src/cron/retry-hint.ts)
  • ghitafilali: Authored merged setup-timeout restart handling that touched the same cron watchdog, timer, and gateway surfaces. (role: adjacent setup-timeout contributor; confidence: medium; commits: b2c5e790b441; files: src/cron/service/agent-watchdog.ts, src/cron/service/timer.ts, src/gateway/server-cron.ts)

Codex review notes: model internal, reasoning high; reviewed against 08d15ec32db3.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal backlog priority with limited blast radius. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. labels May 30, 2026
caotran added 2 commits May 30, 2026 19:46
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.
@Kaspnov
Kaspnov force-pushed the feat/configurable-cron-setup-watchdog branch from 564f085 to 3fb689a Compare May 30, 2026 18:49
@openclaw-barnacle openclaw-barnacle Bot added proof: supplied External PR includes structured after-fix real behavior proof. and removed triage: needs-real-behavior-proof Candidate: external PR needs after-fix proof from a real setup. labels May 30, 2026
@clawsweeper clawsweeper Bot added rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. labels May 30, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. labels Jun 15, 2026
@clawsweeper

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

@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

clawsweeper Bot commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper applied the proposed close for this PR.

  • Action: closed this PR.
  • Close reason: duplicate or superseded.
  • Evidence: durable ClawSweeper review.
  • Coverage proof: PR B clearly carries the useful runtime fix PR A was trying to deliver and is the canonical current path for reviewing or landing that work. PR A's extra config knob, docs, and proof issues are either avoided by PR B's design or belong in a separate future product-approved config proposal, so no material PR A work still needs independent review here. Covering PR: fix: let isolated cron setup use the job timeout budget #96219.

@clawsweeper clawsweeper Bot closed this Jun 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 availability 🚨 May cause crashes, hangs, restart loops, stalls, or process outages. merge-risk: 🚨 compatibility 🚨 May break existing users, config, migrations, defaults, or upgrade paths. P2 Normal backlog priority with limited blast radius. proof: supplied External PR includes structured after-fix real behavior proof. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. size: S status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants