fix: cron stream stalls fail over before job timeout#96096
Conversation
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper review |
|
Codex review: needs maintainer review before merge. Reviewed June 25, 2026, 12:49 PM ET / 16:49 UTC. Summary PR surface: Source +108, Tests +146, Docs 0. Total +254 across 4 files. Reproducibility: yes. at source level: current main returns the full explicit cron run timeout from resolveLlmIdleTimeoutMs, so an opened-but-silent stream can use the same budget as the cron watchdog. I did not run a live provider that opens a stream and sends no chunks. Review metrics: 1 noteworthy metric.
Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Land the focused timeout-policy change only if maintainers accept the 60-second cloud cron idle cap, while keeping broader cron budget exhaustion work separate. Do we have a high-confidence way to reproduce the issue? Yes, at source level: current main returns the full explicit cron run timeout from resolveLlmIdleTimeoutMs, so an opened-but-silent stream can use the same budget as the cron watchdog. I did not run a live provider that opens a stream and sends no chunks. Is this the best way to solve the issue? Yes for the narrow stream-stall fallback bug: the patch changes the embedded stream-idle timeout at the owner boundary and relies on the existing timeout-classified fallback loop. It is not a complete answer to broader cron remaining-budget exhaustion, which should stay separate. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 19707cce1d73. Label changesLabel justifications:
Evidence reviewedPR surface: Source +108, Tests +146, Docs 0. Total +254 across 4 files. View PR surface stats
Acceptance criteria:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
velvet-shark
left a comment
There was a problem hiding this comment.
Thanks for tightening the explicit cron timeout path here. I agree this is a real follow-up to #94445 rather than a duplicate: #94445 covered the no-explicit-timeout cron cloud case, while this PR handles the long explicit cron timeout that lets #85900 wait for the outer job deadline.
I think one correctness issue still needs a small ownership/narrowing pass before this lands:
src/agents/embedded-agent-runner/run/llm-idle-timeout.ts:99 classifies any single-label hostname as self-hosted, and src/agents/embedded-agent-runner/run/llm-idle-timeout.ts:207 then preserves the full explicit cron timeout for that provider. That means a cloud/custom provider routed through an internal alias like http://api:8080/v1 or http://gateway:4000/v1 still bypasses the 60s cron idle cap, so a silent stream can block model fallback until the outer cron timeout again. Please tie this exception to a real local/self-hosted provider signal, or narrow the hostname exception to explicit local/private names that cannot reasonably be cloud aliases, and add a regression test for a cloud/custom provider using a single-label base URL.
Also please update docs/concepts/agent-loop.md:170; the current docs still say run-specific timeouts bound/extend the model idle watchdog, but after this change explicit cron cloud runs can fail over after 60s while local/self-hosted runs keep the configured explicit timeout.
Evidence I checked:
src/cron/isolated-agent/run.tspreserves explicit cron timeout intent.src/cron/isolated-agent/run-executor.tspassestrigger: "cron"into embedded runs and wraps them in model fallback.src/agents/model-fallback.tscontinues after timeout-classified failover errors.src/agents/embedded-agent-runner/run/assistant-failover.tsroutes idle timeouts into profile/model fallback.
Targeted review tests passed:
scripts/pr review-tests 96096 src/agents/embedded-agent-runner/run/llm-idle-timeout.test.ts src/agents/model-fallback.test.ts src/agents/embedded-agent-runner/run/failover-policy.test.ts|
Merged via squash.
|
Dependency GuardThis PR changes dependency-related files. Maintainers should confirm these changes are intentional. Changed files:
Maintainer follow-up:
|
Dependency graph changes are blockedOpenClaw does not accept dependency graph changes through PRs unless a repository admin or security explicitly authorizes the current head SHA. Dependency updates are generated internally by maintainers so external PRs cannot change the resolved graph. Detected dependency graph changes:
Auto-scrub was not attempted because this PR changes package manifest dependency graph fields:
Dependency graph changes must be reviewed by security or handled by maintainers internally. Please remove lockfile changes manually if they are not needed. To remove lockfile changes, restore them from the target branch: git fetch origin
git checkout 'origin/main' -- 'extensions/acpx/npm-shrinkwrap.json' 'extensions/amazon-bedrock-mantle/npm-shrinkwrap.json' 'extensions/amazon-bedrock/npm-shrinkwrap.json' 'extensions/anthropic-vertex/npm-shrinkwrap.json' 'extensions/arcee/npm-shrinkwrap.json' 'extensions/brave/npm-shrinkwrap.json' 'extensions/cerebras/npm-shrinkwrap.json' 'extensions/chutes/npm-shrinkwrap.json' 'extensions/clickclack/npm-shrinkwrap.json' 'extensions/cloudflare-ai-gateway/npm-shrinkwrap.json' 'extensions/codex/npm-shrinkwrap.json' 'extensions/cohere/npm-shrinkwrap.json' 'extensions/copilot/npm-shrinkwrap.json' 'extensions/deepinfra/npm-shrinkwrap.json' 'extensions/deepseek/npm-shrinkwrap.json' 'extensions/diagnostics-otel/npm-shrinkwrap.json' 'extensions/diagnostics-prometheus/npm-shrinkwrap.json' 'extensions/diffs-language-pack/npm-shrinkwrap.json' 'extensions/diffs/npm-shrinkwrap.json' 'extensions/discord/npm-shrinkwrap.json' 'extensions/exa/npm-shrinkwrap.json' 'extensions/feishu/npm-shrinkwrap.json' 'extensions/firecrawl/npm-shrinkwrap.json' 'extensions/fireworks/npm-shrinkwrap.json' 'extensions/gmi/npm-shrinkwrap.json' 'extensions/google-meet/npm-shrinkwrap.json' 'extensions/googlechat/npm-shrinkwrap.json' 'extensions/gradium/npm-shrinkwrap.json' 'extensions/groq/npm-shrinkwrap.json' 'extensions/inworld/npm-shrinkwrap.json' 'extensions/irc/npm-shrinkwrap.json' 'extensions/kilocode/npm-shrinkwrap.json' 'extensions/kimi-coding/npm-shrinkwrap.json' 'extensions/line/npm-shrinkwrap.json' 'extensions/llama-cpp/npm-shrinkwrap.json' 'extensions/lobster/npm-shrinkwrap.json' 'extensions/matrix/npm-shrinkwrap.json' 'extensions/mattermost/npm-shrinkwrap.json' 'extensions/memory-lancedb/npm-shrinkwrap.json' 'extensions/moonshot/npm-shrinkwrap.json' 'extensions/msteams/npm-shrinkwrap.json' 'extensions/nextcloud-talk/npm-shrinkwrap.json' 'extensions/nostr/npm-shrinkwrap.json' 'extensions/openshell/npm-shrinkwrap.json' 'extensions/parallel/npm-shrinkwrap.json' 'extensions/perplexity/npm-shrinkwrap.json' 'extensions/pixverse/npm-shrinkwrap.json' 'extensions/qianfan/npm-shrinkwrap.json' 'extensions/qqbot/npm-shrinkwrap.json' 'extensions/qwen/npm-shrinkwrap.json' 'extensions/raft/npm-shrinkwrap.json' 'extensions/searxng/npm-shrinkwrap.json' 'extensions/signal/npm-shrinkwrap.json' 'extensions/slack/npm-shrinkwrap.json' 'extensions/sms/npm-shrinkwrap.json' 'extensions/stepfun/npm-shrinkwrap.json' 'extensions/synology-chat/npm-shrinkwrap.json' 'extensions/tavily/npm-shrinkwrap.json' 'extensions/tencent/npm-shrinkwrap.json' 'extensions/tlon/npm-shrinkwrap.json' 'extensions/tokenjuice/npm-shrinkwrap.json' 'extensions/twitch/npm-shrinkwrap.json' 'extensions/venice/npm-shrinkwrap.json' 'extensions/vercel-ai-gateway/npm-shrinkwrap.json' 'extensions/voice-call/npm-shrinkwrap.json' 'extensions/whatsapp/npm-shrinkwrap.json' 'extensions/zai/npm-shrinkwrap.json' 'extensions/zalo/npm-shrinkwrap.json' 'extensions/zalouser/npm-shrinkwrap.json' 'npm-shrinkwrap.json' 'pnpm-lock.yaml'
git commit -m 'chore: remove dependency lockfile change'
git pushIf this PR intentionally needs a dependency graph change, ask a repository admin or member of The action will approve the current head SHA ( |
|
Clarification on the post-merge Dependency Guard comments: those warnings were triggered by the prepared PR head compare, not by the landed squash commit. The prepared head The actual landed squash commit is So the dependency warnings are PR-page residue from the bad prepared-head ancestry; no dependency, lockfile, package manifest, workflow, or extension changes landed on |
Closes #85900
What Problem This Solves
Fixes an issue where cron jobs with a pinned cloud model and configured fallbacks could wait until the outer cron timeout when the provider opened a stream but never sent chunks, preventing the existing fallback chain from trying the next model.
Why This Change Was Made
Cron-triggered cloud model calls now cap explicit per-run stream idle waits at 60s so a stalled candidate fails early enough for
runWithModelFallbackto advance. Local/private providers and self-hosted hostname providers keep their previous explicit cron timeout behavior because they can legitimately stay silent during long prompt evaluation, while Ollama Cloud and dotted remote hostnames still use the 60s cron stall cap. Explicit provider request timeouts continue to win for intentionally slow model servers.User Impact
Scheduled jobs with healthy fallback models recover from cloud stream stalls sooner instead of losing the whole cron run to the outer watchdog. Existing slow local/self-hosted cron jobs that set a long
timeoutSecondskeep that longer idle window.Evidence
pnpm docs:listto identify the relevant cron docs surface; readdocs/automation/cron-jobs.md.pnpm exec oxfmt --check --threads=1 src/agents/embedded-agent-runner/run/llm-idle-timeout.ts src/agents/embedded-agent-runner/run/attempt.ts src/agents/embedded-agent-runner/run/llm-idle-timeout.test.ts— pass.git diff --check— pass.node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run/llm-idle-timeout.test.ts src/agents/model-fallback.test.ts— pass, 167 tests..agents/skills/autoreview/scripts/autoreview --mode local --parallel-tests "node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run/llm-idle-timeout.test.ts src/agents/model-fallback.test.ts"— clean; parallel tests passed, 167 tests.Real behavior proof
Behavior addressed: Cron cloud model calls with explicit long
timeoutSecondsno longer let a no-chunk stream stall consume the full cron watchdog before the existing model fallback chain can run, while explicit cron timeouts for local/self-hosted hostname providers keep their previous longer idle window.Real environment tested: Local OpenClaw checkout on branch
fix/85900-cron-stream-stall-fallbackat0c45991c24, macOS host, Node/pnpm workspace dependencies installed bypnpm docs:list.Exact steps or command run after this patch:
Evidence after fix:
{ "cronSelfHostedHostname600s": 600000, "cronHostDocker600s": 600000, "cronCloud300s": 60000, "cronOllamaCloudHostname300s": 60000 }Focused tests also show the fallback loop continues to the next candidate after a provider-side error:
node scripts/run-vitest.mjs src/agents/model-fallback.test.tspassed with the existing unknown-error fallback coverage.Observed result after fix: A cron cloud call with a 300s per-run timeout now uses a 60s stream idle watchdog, leaving the outer cron budget available for configured fallback candidates. Local/self-hosted hostname provider cron calls keep their 600s explicit timeout, and Ollama Cloud remains capped even when the configured base URL is a hostname.
What was not tested: No live provider was forced into a real opened-but-silent network stream; the proof covers the source-level timeout selection and the existing model fallback retry behavior with focused Vitest coverage.