fix(cron): cancel timed-out runs before side effects by Takhoffman · Pull Request #22411 · openclaw/openclaw

Takhoffman · 2026-02-21T04:15:28Z

Summary

Describe the problem and fix in 2–5 bullets:

Problem: Cron timer timeouts marked runs as failed, but isolated execution could still finish and emit downstream side effects (announce/main-session summary).
Why it matters: This can produce inconsistent outcomes (error state + late delivery) and duplicate/confusing notifications.
What changed: Added abort-signal plumbing from timer timeout into cron isolated execution path and guard checks before post-run side effects; wired signal through gateway cron service.
What did NOT change (scope boundary): No scheduler concurrency behavior changes; no CLI/API surface changes beyond internal optional signal threading.

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Timed-out cron isolated runs are now prevented from emitting late follow-up side effects after timeout.
Reduces cases where users see timeout/error while delayed follow-up delivery still appears later.

Security Impact (required)

New permissions/capabilities? (Yes/No) No
Secrets/tokens handling changed? (Yes/No) No
New/changed network calls? (Yes/No) No
Command/tool execution surface changed? (Yes/No) No
Data access scope changed? (Yes/No) No
If any Yes, explain risk + mitigation:

Repro + Verification

Environment

OS: macOS (darwin-arm64)
Runtime/container: Node + Vitest
Model/provider: N/A (unit/integration tests)
Integration/channel (if any): Cron service internals
Relevant config (redacted): default test config

Steps

Create an isolated cron job with a very small timeout and delayed runner completion.
Trigger scheduler tick.
Confirm timed-out status and absence of follow-up enqueue side effects.

Expected

Run is marked timed out/error and does not emit late follow-up side effects.

Actual

Matches expected after fix.

Evidence

Attach at least one:

Failing test/log before + passing after
Trace/log snippets
Screenshot/recording
Perf numbers (if relevant)

Human Verification (required)

What you personally verified (not just CI), and how:

Verified scenarios:
pnpm vitest run src/cron/service.issue-regressions.test.ts src/cron/service.rearm-timer-when-running.test.ts src/cron/service.restart-catchup.test.ts --maxWorkers=1
pnpm vitest run src/cron/service.delivery-plan.test.ts src/cron/service.runs-one-shot-main-job-disables-it.test.ts --maxWorkers=1
Edge cases checked:
timeoutSeconds=0 semantics remain intact
existing timer rearm/restart catch-up behavior still passes
What you did not verify:
Full live provider/channel end-to-end with real network delivery

Compatibility / Migration

Backward compatible? (Yes/No) Yes
Config/env changes? (Yes/No) No
Migration needed? (Yes/No) No
If yes, exact upgrade steps:

Failure Recovery (if this breaks)

How to disable/revert this change quickly:
Revert this PR commit from cron timeout-cancellation branch.
Files/config to restore:
src/cron/service/state.ts, src/cron/service/timer.ts, src/cron/isolated-agent/run.ts, src/gateway/server-cron.ts, src/cron/service.issue-regressions.test.ts
Known bad symptoms reviewers should watch for:
Timed-out jobs still emitting late announce/main-session summaries.

Risks and Mitigations

List only real risks for this PR. Add/remove entries as needed. If none, write None.

Risk:
- Over-cancellation could suppress legitimate post-run side effects in borderline timeout races.
- Mitigation:
- Abort checks are limited to timeout path and covered with a dedicated regression test for side-effect suppression after timeout.

Greptile Summary

Adds abort signal plumbing to cancel timed-out cron jobs before they emit downstream side effects (announce flows and main-session summaries). The implementation threads an AbortSignal from the timer timeout through executeJobCore, runIsolatedAgentJob, runCronIsolatedAgentTurn, and into deliverOutboundPayloads and runSubagentAnnounceFlow. Guard checks (if (signal?.aborted)) are placed before all side-effect operations to prevent late deliveries after timeout.

Key changes:

timer.ts: Creates AbortController on timeout, passes signal to executeJobCore, adds abort checks in main-session wake loop and after isolated job completion
isolated-agent/run.ts: Threads signal through execution, adds abort guards before structured delivery and announce flow
subagent-announce.ts: Adds abort checks at the start of delivery functions to short-circuit when already timed out
state.ts and server-cron.ts: Updates type signatures to accept optional signal parameter

The PR includes comprehensive regression tests covering timeout side-effect suppression for both isolated execution and structured outbound delivery paths.

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The implementation is well-designed with proper abort signal plumbing throughout the execution chain, strategic guard checks before all side effects, thorough test coverage including regression tests for the specific bug, and backwards compatibility (signal parameter is optional everywhere). The changes are scoped to the timeout cancellation path only and include proper error normalization.
No files require special attention

_{Last reviewed commit: e4d9061}

greptile-apps

_{5 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-21T04:26:15Z

Additional Comments (2)

src/cron/isolated-agent/run.ts
Potential race: if timeout fires while deliverOutboundPayloads is executing, delivery completes despite timeout. Add abort check before this call:

        if (payloadsForDelivery.length > 0) {
          if (isAborted()) {
            return withRunSession({ status: "error", error: abortReason(), ...telemetry });
          }
          const deliveryResults = await deliverOutboundPayloads({

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cron/isolated-agent/run.ts
Line: 654

Comment:
Potential race: if timeout fires while `deliverOutboundPayloads` is executing, delivery completes despite timeout. Add abort check before this call:

```suggestion
        if (payloadsForDelivery.length > 0) {
          if (isAborted()) {
            return withRunSession({ status: "error", error: abortReason(), ...telemetry });
          }
          const deliveryResults = await deliverOutboundPayloads({
```

How can I resolve this? If you propose a fix, please make it concise.

src/cron/isolated-agent/run.ts
Potential race: if timeout fires while runSubagentAnnounceFlow is executing, announce completes despite timeout. Add abort check before this call:

      try {
        if (isAborted()) {
          return withRunSession({ status: "error", error: abortReason(), ...telemetry });
        }
        const didAnnounce = await runSubagentAnnounceFlow({

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/cron/isolated-agent/run.ts
Line: 747

Comment:
Potential race: if timeout fires while `runSubagentAnnounceFlow` is executing, announce completes despite timeout. Add abort check before this call:

```suggestion
      try {
        if (isAborted()) {
          return withRunSession({ status: "error", error: abortReason(), ...telemetry });
        }
        const didAnnounce = await runSubagentAnnounceFlow({
```

How can I resolve this? If you propose a fix, please make it concise.

Takhoffman · 2026-02-22T21:45:42Z

PR #22411 - fix(cron): cancel timed-out runs before side effects (#22411)

Merged via squash.

Merge commit: 556af3f
Verified: pnpm check
Verified: pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1
Changes made:
- Rebased PR branch multiple times to latest main and resolved conflicts in cron timeout cancellation paths.
- Preserved abort propagation through isolated cron execution (abortSignal compatibility + timeout error semantics).
- Added/kept regression coverage for timeout side-effect suppression and manual timeout runs.
- Fixed unrelated CI type regressions in tests so pnpm check passes (src/agents/pi-tools.read.workspace-root-guard.test.ts, src/infra/outbound/outbound-send-service.test.ts, src/node-host/invoke-system-run.test.ts).
Why these changes were made:
- Keep this PR mergeable and behavior-safe while main moved quickly, and remove non-PR typecheck blockers preventing CI green.

Thanks @Takhoffman!

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

@Takhoffman

… thanks @Takhoffman Verified: - pnpm check - pnpm vitest run src/memory/qmd-manager.test.ts src/cron/service.issue-regressions.test.ts src/cron/isolated-agent.delivers-response-has-heartbeat-ok-but-includes.test.ts --maxWorkers=1 Co-authored-by: Takhoffman <781889+Takhoffman@users.noreply.github.com> Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>

openclaw-barnacle Bot added gateway Gateway runtime size: S maintainer Maintainer-authored PR labels Feb 21, 2026

Takhoffman marked this pull request as ready for review February 21, 2026 04:21

greptile-apps Bot reviewed Feb 21, 2026

View reviewed changes

Takhoffman marked this pull request as draft February 21, 2026 04:35

Takhoffman force-pushed the fix-cron-timeout-side-effects branch from 76fd2f8 to e4d9061 Compare February 21, 2026 04:40

openclaw-barnacle Bot added agents Agent runtime and tooling size: M and removed size: S labels Feb 21, 2026

Valadon mentioned this pull request Feb 21, 2026

fix(agents): make subagent announce timeout configurable (restore 60s default) #22719

Closed

18 tasks

graysurf mentioned this pull request Feb 22, 2026

fix(cron): increase default timeout for cron run tool action #22024

Closed

Takhoffman marked this pull request as ready for review February 22, 2026 21:28

Takhoffman force-pushed the fix-cron-timeout-side-effects branch from e4d9061 to 000d28c Compare February 22, 2026 21:34

openclaw-barnacle Bot removed the gateway Gateway runtime label Feb 22, 2026

Takhoffman force-pushed the fix-cron-timeout-side-effects branch from 000d28c to 0b922aa Compare February 22, 2026 21:36

Takhoffman added 2 commits February 22, 2026 15:40

fix(cron): cancel timed-out runs before side effects

1cb3801

fix(cron): preserve timeout abort checks after rebase

7cc1442

Takhoffman force-pushed the fix-cron-timeout-side-effects branch from 88e6a50 to 7cc1442 Compare February 22, 2026 21:42

Takhoffman merged commit 556af3f into main Feb 22, 2026
24 of 25 checks passed

Takhoffman deleted the fix-cron-timeout-side-effects branch February 22, 2026 21:45

This was referenced Feb 22, 2026

fix(cron): respect per-job timeoutSeconds in executeJob path (#16841) #16880

Closed

[UX]: Cron run timeouts display as errors even when job succeeds #20217

Closed

github-actions Bot mentioned this pull request Feb 24, 2026

上游更新: v2026.2.23, v2026.2.22 — 114 P0 + 122 P1 待合并 jiulingyun/openclaw-cn#328

Closed

jiulingyun mentioned this pull request Feb 24, 2026

upstream(cron): 移植 5 个冲突 commit (P1) — v2026.2.21→v2026.2.23 jiulingyun/openclaw-cn#429

Closed

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cron): cancel timed-out runs before side effects#22411

fix(cron): cancel timed-out runs before side effects#22411
Takhoffman merged 2 commits intomainfrom
fix-cron-timeout-side-effects

Takhoffman commented Feb 21, 2026 •

edited by greptile-apps Bot

Loading

Uh oh!

greptile-apps Bot left a comment

Uh oh!

greptile-apps Bot commented Feb 21, 2026

Uh oh!

Uh oh!

Takhoffman commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

Takhoffman commented Feb 21, 2026 • edited by greptile-apps Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Change Type (select all)

Scope (select all touched areas)

Linked Issue/PR

User-visible / Behavior Changes

Security Impact (required)

Repro + Verification

Environment

Steps

Expected

Actual

Evidence

Human Verification (required)

Compatibility / Migration

Failure Recovery (if this breaks)

Risks and Mitigations

Greptile Summary

Confidence Score: 5/5

Uh oh!

greptile-apps Bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Feb 21, 2026

Uh oh!

Uh oh!

Takhoffman commented Feb 22, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Takhoffman commented Feb 21, 2026 •

edited by greptile-apps Bot

Loading