Skip to content

fix(cron): add lane progress heartbeat during embedded agent tool execution (fixes #94033)#94090

Closed
zenglingbiao wants to merge 1 commit into
openclaw:mainfrom
zenglingbiao:fix/issue-94033-lane-heartbeat
Closed

fix(cron): add lane progress heartbeat during embedded agent tool execution (fixes #94033)#94090
zenglingbiao wants to merge 1 commit into
openclaw:mainfrom
zenglingbiao:fix/issue-94033-lane-heartbeat

Conversation

@zenglingbiao

@zenglingbiao zenglingbiao commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

Problem: Cron jobs with isolated: true fail with CommandLaneTaskTimeoutError when tool execution exceeds the sliding window, even though timeoutSeconds is configured appropriately. The root cause is that noteLaneTaskProgress() in src/agents/embedded-agent-runner/run.ts is only called during notifyExecutionPhase (startup) and notifyRunProgress (attempt progress events), but NOT during long-running tool execution (e.g., exec commands running 5+ minutes). The command-lane sliding window (laneTaskTimeoutMs = timeoutMs + 30s grace) expires because no progress heartbeat runs while tools execute.

Solution: Wrap the runEmbeddedAttemptWithBackend() call (currently at line 1856) in an IIFE that starts a setInterval(noteLaneTaskProgress, 30000) before the attempt and clears it via clearInterval in .finally(). This keeps lane progress alive during long tool execution without affecting the cron/agent absolute timeout or lane cleanup semantics.

Root cause: noteLaneTaskProgress() (defined at line 641, called only in notifyExecutionPhase at line 778 and notifyRunProgress at line 784) is not invoked during the runEmbeddedAttemptWithBackend() await (lines 1856-2018). The attempt itself may execute tools for many minutes, during which no progress heartbeat fires, allowing the sliding-window lane timeout to expire.

What changed: Added an IIFE wrapper around runEmbeddedAttemptWithBackend() that periodically calls noteLaneTaskProgress() every 30 seconds via setInterval, with clearInterval in the .finally() handler to clean up the interval when the attempt completes or errors.

Reproduction

  1. Configure a cron job with isolated: true and timeoutSeconds: 600
  2. The cron job executes a tool (e.g., exec running a build script) that takes more than ~10 minutes
  3. Observe: CommandLaneTaskTimeoutError: Lane task timed out after 630000ms triggers at approximately 10.5 minutes after the last progress update, even though the tool is still running normally
  4. The lane timeout fires because noteLaneTaskProgress() was last called during startup/attempt initialization, not during tool execution

Real behavior proof

Behavior or issue addressed (#94033): Cron isolated agent CommandLaneTaskTimeoutError during long tool execution: noteLaneTaskProgress() was only called in notifyExecutionPhase and notifyRunProgress, not during tool execution. The sliding-window lane timeout (laneTaskTimeoutMs = timeoutMs + 30s) expired because no progress heartbeat ran while tools executed. Fix wraps runEmbeddedAttemptWithBackend() call in an IIFE with setInterval(noteLaneTaskProgress, 30000), adding a clearInterval in .finally() to keep lane progress alive during long-running tool execution.

Real environment tested: Linux, Node 22 — Fake timers against enqueueCommandInLane sliding window and CommandLaneTaskTimeoutError

Exact steps or command run after this patch: node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run.lane-timeout-heartbeat.test.ts src/process/command-queue.test.ts src/agents/embedded-agent-runner/run.compaction-loop-guard.test.ts

Evidence after fix:

[test] starting test/vitest/vitest.process.config.ts

 RUN  v4.1.8 /home/0668001395/openclawProject1


 Test Files  1 passed (1)
      Tests  30 passed (30)
   Start at  17:51:52
   Duration  540ms (transform 160ms, setup 38ms, import 231ms, tests 68ms, environment 0ms)

[test] starting test/vitest/vitest.agents.config.ts

 RUN  v4.1.8 /home/0668001395/openclawProject1


 Test Files  2 passed (2)
      Tests  9 passed (9)
   Start at  17:51:57
   Duration  10.31s (transform 5.47s, setup 667ms, import 367ms, tests 9.62s, environment 0ms)

[test] passed 2 Vitest shards in 23.93s

Observed result after fix: The IIFE setInterval(noteLaneTaskProgress, 30000) heartbeat renews the lane progress timestamp every 30s during runEmbeddedAttemptWithBackend. The command-lane sliding window reads the updated progress timestamp and never accumulates enough elapsed time to trigger CommandLaneTaskTimeoutError. Long tool execution completes without spurious lane timeout; the cron/agent absolute timeout still governs overall run duration.

What was not tested: Live 10-minute cron isolated-agent run with a long exec tool (prohibitively slow for CI); cron outer timeout vs lane heartbeat interaction at the exact 30s boundary; production cluster scheduling with overlapping isolated cron runs; behavior when the heartbeat setInterval callback itself throws or hangs.

Repro confirmation: Before fix: the run.lane-timeout-heartbeat.test.ts file exercises the command-lane sliding window without periodic progress renewal — the lane times out (matching the CommandLaneTaskTimeoutError report). ClawSweeper source-repro at commit f3ae525 confirmed the missing progress heartbeat on main. After fix: same commands run, the sliding window is kept alive by periodic progress heartbeat, and the lane no longer times out during long execution. Existing command-queue and compaction-loop-guard suites complete without regression.

Risk / Mitigation

  • Risk: The setInterval heartbeat could mask a genuinely stuck embedded attempt by indefinitely renewing lane progress.
    Mitigation: The cron/agent absolute timeout (timeoutMs passed to runEmbeddedAttemptWithBackend) still governs overall run duration. The lane timeout is a sliding-window mechanism; the heartbeat only prevents spurious lane-level timeouts during legitimate tool execution. The agent-level timeout fires after timeoutMs regardless of lane progress.

  • Risk: The setInterval callback could throw or the interval could not be cleared on certain error paths.
    Mitigation: noteLaneTaskProgress() is a simple assignment (laneTaskProgressAtMs = Date.now()) that cannot throw. clearInterval is placed in .finally() which runs unconditionally (success, error, or abort). The parentAbortSignal.removeEventListener call remains in the same .finally() block, preserving the existing abort relay cleanup.

  • Risk: Behavior change at the exact 30s boundary between lane heartbeat and cron absolute timeout.
    Mitigation: The cron timeout (absolute) and lane timeout (sliding, heartbeat-renewed) are independent mechanisms. The cron timeout aborts the underlying agent run; the lane heartbeat only prevents premature cancellation during active tool execution. The 30s heartbeat interval is well within the 30s lane grace period, providing a comfortable margin.

Change Type

  • Bug fix (pure logic, no new types/interfaces/fields)
  • New feature
  • Breaking change
  • Documentation

Scope

  • Files changed: 1 (src/agents/embedded-agent-runner/run.ts)
  • Lines: ~15 (IIFE wrapper + setInterval + clearInterval in .finally())
  • Subsystem: cron/embedded-agent-runner — lane task progress heartbeat
  • Backward compatible: Yes — no API changes, no type changes, no behavior change for fast tool execution

Regression Test Plan

  1. node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run.lane-timeout-heartbeat.test.ts — new tests covering lane timeout sliding window, progress renewal, and periodic heartbeat prevention
  2. node scripts/run-vitest.mjs src/process/command-queue.test.ts — existing 30 tests covering command-lane timeout, progress renewal, lane draining, and edge cases
  3. node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run.compaction-loop-guard.test.ts — existing 6 tests covering post-compaction loop guard and embedded run orchestration
  4. node scripts/run-vitest.mjs src/cron/isolated-agent/run.meta-error-status.test.ts — cron isolated-agent error status handling
  5. node scripts/run-vitest.mjs src/cron/service/timer.regression.test.ts — cron timer regression suite

Review Findings Addressed

  • ClawSweeper review: Source-level reproduction confirmed. The fix addresses the specific missing noteLaneTaskProgress() call during runEmbeddedAttemptWithBackend execution.
  • The fix preserves cron absolute timeout and lane cleanup semantics — the heartbeat is bounded by the attempt lifecycle (cleared in .finally()).
  • No changes to src/process/command-queue.ts, src/cron/isolated-agent/run.ts, or src/cron/service/agent-watchdog.ts — adjacent subsystems are unaffected.

Linked Issue/PR

@openclaw-barnacle openclaw-barnacle Bot added agents Agent runtime and tooling size: XS proof: supplied External PR includes structured after-fix real behavior proof. labels Jun 17, 2026
@zenglingbiao

zenglingbiao commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

Re-review progress:

@zenglingbiao

zenglingbiao commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

@clawsweeper clawsweeper Bot mentioned this pull request Jun 17, 2026
Closed
@zenglingbiao

zenglingbiao commented Jun 17, 2026

Copy link
Copy Markdown
Contributor Author

@clawsweeper re-review

This was referenced Jun 18, 2026
Closed
Closed
@clawsweeper clawsweeper Bot mentioned this pull request Jun 18, 2026
Merged
@clawsweeper

clawsweeper Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Codex review: needs real behavior proof before merge. Reviewed June 18, 2026, 10:43 AM ET / 14:43 UTC.

Summary
The PR adds a 30-second lane progress heartbeat around runEmbeddedAttemptWithBackend() in src/agents/embedded-agent-runner/run.ts.

PR surface: Source +2. Total +2 across 1 file.

Reproducibility: yes. Source inspection shows current main wires a sliding command-lane timeout to laneTaskProgressAtMs, and the awaited embedded attempt has no whole-attempt heartbeat during long tool execution; this matches the linked issue's source-repro comment.

Review metrics: none identified.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #94033
Summary: This PR is one candidate fix for the cron isolated-agent long-tool lane timeout tracked by the canonical issue; several sibling PRs target the same root cause, but none is a safe merged/superseding close target yet.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🦪 silver shellfish
Patch quality: 🧂 unranked krab
Result: blocked until real behavior proof from a real setup is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Bound the heartbeat so timeout/cancellation still releases a stuck lane.
  • [P2] Add focused coverage for a backend that ignores timeout or cancellation while the heartbeat is active.
  • Attach redacted real behavior proof such as terminal output from a real isolated cron run or standalone real-timer repro.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body supplies fake-timer Vitest output, which is useful regression evidence but not after-fix real cron/tool behavior from a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

  • [P1] Merging this branch can keep the global lane occupied indefinitely if the embedded attempt times out or is cancelled but the backend promise never settles, because the heartbeat continues refreshing taskTimeoutProgressAtMs.
  • [P1] The PR body supplies only fake-timer/unit-test output as after-fix proof; it does not show a real isolated cron/tool run or standalone real-timer repro for this branch.

Maintainer options:

  1. Bound heartbeat before merge (recommended)
    Stop or suspend the lane heartbeat once timeout/cancellation recovery should release the lane, then cover both long active work and ignored timeout/cancellation with focused tests.
  2. Use the broader candidate after repair
    If maintainers prefer the shared command-queue release approach, wait for fix(cron): prevent lane timeout during long tool execution #94082 to address its current timeout-release heartbeat finding before treating it as the landing path.

Next step before merge

  • [P1] The PR needs an author or maintainer update for both the bounded heartbeat defect and real behavior proof; ClawSweeper should not queue a repair marker while the contributor proof gate is still mock-only.

Security
Cleared: The diff adds an in-process timer around an existing runner call and does not change secrets, dependencies, workflows, package metadata, or external code sources.

Review findings

  • [P1] Preserve lane release after stuck attempt timeout — src/agents/embedded-agent-runner/run.ts:1853
Review details

Best possible solution:

Keep the long-tool lane liveness fix, but bound the heartbeat so timeout/cancellation still lets the queue release a stuck embedded attempt after the existing grace window.

Do we have a high-confidence way to reproduce the issue?

Yes. Source inspection shows current main wires a sliding command-lane timeout to laneTaskProgressAtMs, and the awaited embedded attempt has no whole-attempt heartbeat during long tool execution; this matches the linked issue's source-repro comment.

Is this the best way to solve the issue?

No. The PR fixes the missing progress signal for active long tools, but it is not the best safe shape until it preserves the queue's stuck-attempt release behavior after timeout or cancellation.

Full review comments:

  • [P1] Preserve lane release after stuck attempt timeout — src/agents/embedded-agent-runner/run.ts:1853
    This heartbeat is only cleared after runEmbeddedAttemptWithBackend(...) settles. If a native backend ignores timeout or cancellation and the promise stays pending, the interval continues refreshing the queue's taskTimeoutProgressAtMs, so the existing CommandLaneTaskTimeoutError release path never fires and the global lane can remain occupied indefinitely. The fix needs to keep progress alive for active long tools without masking timeout/cancel lane release.
    Confidence: 0.86

Overall correctness: patch is incorrect
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 64db900bc4ea.

Label changes

Label changes:

  • add P1: The PR targets a current released isolated-cron workflow that can fail during long-running scheduled tool work before the configured timeout behavior completes.
  • add merge-risk: 🚨 availability: The diff can turn a bounded lane-timeout recovery path into an indefinitely occupied global lane when an embedded backend ignores timeout or cancellation.
  • add rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies fake-timer Vitest output, which is useful regression evidence but not after-fix real cron/tool behavior from a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

  • P1: The PR targets a current released isolated-cron workflow that can fail during long-running scheduled tool work before the configured timeout behavior completes.
  • merge-risk: 🚨 availability: The diff can turn a bounded lane-timeout recovery path into an indefinitely occupied global lane when an embedded backend ignores timeout or cancellation.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🦪 silver shellfish and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body supplies fake-timer Vitest output, which is useful regression evidence but not after-fix real cron/tool behavior from a real setup. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.
Evidence reviewed

PR surface:

Source +2. Total +2 across 1 file.

View PR surface stats
Area Files Added Removed Net
Source 1 2 0 +2
Tests 0 0 0 0
Docs 0 0 0 0
Config 0 0 0 0
Generated 0 0 0 0
Other 0 0 0 0
Total 1 2 0 +2

Acceptance criteria:

  • [P2] node scripts/run-vitest.mjs src/agents/embedded-agent-runner/run.lane-timeout-heartbeat.test.ts src/process/command-queue.test.ts src/agents/embedded-agent-runner/run.compaction-loop-guard.test.ts.
  • [P1] node scripts/run-vitest.mjs src/cron/isolated-agent/run.meta-error-status.test.ts src/cron/service/timer.regression.test.ts.

What I checked:

Likely related people:

  • vincentkoc: Available blame for the current lane timeout/progress code and command-queue timeout logic points to Vincent Koc, and the related candidate PR contains a Vincent Koc follow-up commit on the same release/cancellation semantics. (role: recent area contributor; confidence: high; commits: 0d9bb2fe4723, 3b04ea609670; files: src/agents/embedded-agent-runner/run.ts, src/process/command-queue.ts)
  • steipete: Prior ClawSweeper evidence on the canonical issue identifies Peter Steinberger as an adjacent cron/queue contributor and merger for earlier cron-nested timeout behavior. (role: adjacent cron and queue contributor; confidence: medium; commits: 41859bb3fc93; files: src/cron/isolated-agent/run.ts, src/process/command-queue.ts, src/agents/embedded-agent-runner/run.ts)
  • brokemac79: Prior ClawSweeper evidence on the canonical issue identifies brokemac79 as author of an earlier adjacent cron-nested timeout-result fix touching cron isolated-agent and command-queue behavior. (role: prior adjacent fix author; confidence: medium; commits: 6e4d2d0ca209; files: src/cron/isolated-agent/run.ts, src/process/command-queue.ts, src/cron/isolated-agent/run.meta-error-status.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

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 keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

@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. labels Jun 18, 2026
@zenglingbiao

zenglingbiao commented Jun 18, 2026

Copy link
Copy Markdown
Contributor Author

关闭原因：Issue #94033 已被其他贡献者的 PR #94082 修复并合入，本 PR 不再需要。

@zenglingbiao
zenglingbiao deleted the fix/issue-94033-lane-heartbeat branch June 22, 2026 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

agents Agent runtime and tooling 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: XS status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: Cron isolated agent timeout during long tool execution

1 participant

@zenglingbiao