fix(cron): move session reaper to finally block so it runs reliably by scoootscooob · Pull Request #31996 · openclaw/openclaw

scoootscooob · 2026-03-02T17:53:15Z

Summary

Cron session reaper was inside the try block of onTimer(), after job execution — if the locked persist section threw, the reaper was skipped entirely
Isolated cron run sessions accumulated indefinitely in sessions.json, growing to 1000+ entries with massive token waste
Move the reaper into the finally block so it always executes after a timer tick, regardless of job execution outcome
The reaper is already self-throttled (MIN_SWEEP_INTERVAL_MS = 5 min) so calling it more reliably has no performance impact

Test plan

New test: session reaper runs even when job execution throws
New test: resolveSessionStorePath is invoked to build reaper store paths
Existing timer re-arm tests still pass
TypeScript compilation clean (tsgo)

Closes #31946

🤖 Generated with Claude Code

chatgpt-codex-connector

💡 Codex Review

openclaw/src/cron/service/timer.ts

Line 658 in 00c10d5

storePaths.add(state.deps.resolveSessionStorePath(agentId));

Keep timer cleanup outside reaper error path

Placing resolveSessionStorePath calls inside the outer finally means an exception there can abort the same finally block before state.running = false and armTimer(state) run. In that scenario (for example, a throwing resolver implementation), the cron service remains stuck in running state and later ticks only hit the early-return branch, so scheduled jobs stop executing. Before this change, resolver/reaper failures happened in the try block and cleanup still ran in finally.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

Open a pull request for review
Mark a draft as ready
Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

greptile-apps · 2026-03-02T17:58:05Z

Greptile Summary

Moves the cron session reaper into the finally block of onTimer() to ensure it runs reliably after every timer tick, regardless of whether job execution or persistence throws an error. Previously, if persist(state) threw an exception, the reaper would be skipped, causing isolated cron run sessions to accumulate indefinitely in sessions.json.

Key changes:

Session reaper now runs in the finally block before state.running = false and timer re-arming
Reaper executes even when job execution fails or persist() throws
Self-throttled design (5 min intervals) prevents performance impact from more reliable execution
Comprehensive test coverage for both error scenarios and multi-agent path resolution

Confidence Score: 5/5

This PR is safe to merge with minimal risk
The fix correctly addresses a real bug (session accumulation) with a simple, well-tested solution. The session reaper already has robust error handling, is self-throttled, and the placement in the finally block ensures reliable execution without introducing new failure modes. The tests verify both error scenarios and path resolution.
No files require special attention

_{Last reviewed commit: 00c10d5}

@scoootscooob

…claw#31996) (thanks @scoootscooob)

The cron session reaper was placed inside the try block of onTimer(), after job execution and state updates. If the locked persist section threw, the reaper was skipped — causing isolated cron run sessions to accumulate indefinitely in sessions.json. Move the reaper into the finally block so it always executes after a timer tick, regardless of whether job execution succeeded. The reaper is already self-throttled (MIN_SWEEP_INTERVAL_MS = 5 min) so calling it more reliably has no performance impact. Closes openclaw#31946 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@scoootscooob

…claw#31996) (thanks @scoootscooob)

steipete · 2026-03-02T18:39:02Z

Landed via temp rebase onto main.

Gate: bunx vitest run src/cron/service.session-reaper-in-finally.test.ts
Land commit: acdd13e
Merge commit: 4030de6

Thanks @scoootscooob!

@scoootscooob

…penclaw#31996) * fix(cron): move session reaper to finally block so it runs reliably The cron session reaper was placed inside the try block of onTimer(), after job execution and state updates. If the locked persist section threw, the reaper was skipped — causing isolated cron run sessions to accumulate indefinitely in sessions.json. Move the reaper into the finally block so it always executes after a timer tick, regardless of whether job execution succeeded. The reaper is already self-throttled (MIN_SWEEP_INTERVAL_MS = 5 min) so calling it more reliably has no performance impact. Closes openclaw#31946 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: strengthen cron reaper failure-path coverage and changelog (openclaw#31996) (thanks @scoootscooob) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>

@scoootscooob

…penclaw#31996) * fix(cron): move session reaper to finally block so it runs reliably The cron session reaper was placed inside the try block of onTimer(), after job execution and state updates. If the locked persist section threw, the reaper was skipped — causing isolated cron run sessions to accumulate indefinitely in sessions.json. Move the reaper into the finally block so it always executes after a timer tick, regardless of whether job execution succeeded. The reaper is already self-throttled (MIN_SWEEP_INTERVAL_MS = 5 min) so calling it more reliably has no performance impact. Closes openclaw#31946 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: strengthen cron reaper failure-path coverage and changelog (openclaw#31996) (thanks @scoootscooob) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>

@scoootscooob

…penclaw#31996) * fix(cron): move session reaper to finally block so it runs reliably The cron session reaper was placed inside the try block of onTimer(), after job execution and state updates. If the locked persist section threw, the reaper was skipped — causing isolated cron run sessions to accumulate indefinitely in sessions.json. Move the reaper into the finally block so it always executes after a timer tick, regardless of whether job execution succeeded. The reaper is already self-throttled (MIN_SWEEP_INTERVAL_MS = 5 min) so calling it more reliably has no performance impact. Closes openclaw#31946 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * fix: strengthen cron reaper failure-path coverage and changelog (openclaw#31996) (thanks @scoootscooob) --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Peter Steinberger <steipete@gmail.com>

@scoootscooob

…penclaw#31996) * fix(cron): move session reaper to finally block so it runs reliably The cron session reaper was placed inside the try block of onTimer(), after job execution and state updates. If the locked persist section threw, the reaper was skipped — causing isolated cron run sessions to accumulate indefinitely in sessions.json. Move the reaper into the finally block so it always executes after a timer tick, regardless of whether job execution succeeded. The reaper is already self-throttled (MIN_SWEEP_INTERVAL_MS = 5 min) so calling it more reliably has no performance impact. Closes openclaw#31946 * fix: strengthen cron reaper failure-path coverage and changelog (openclaw#31996) (thanks @scoootscooob) --------- Co-authored-by: Peter Steinberger <steipete@gmail.com>

openclaw-barnacle bot added the size: S label Mar 2, 2026

chatgpt-codex-connector bot reviewed Mar 2, 2026

View reviewed changes

steipete force-pushed the fix/cron-session-reaper-unreachable branch from 00c10d5 to fbe2150 Compare March 2, 2026 18:37

steipete added a commit to scoootscooob/openclaw that referenced this pull request Mar 2, 2026

fix: strengthen cron reaper failure-path coverage and changelog (open…

fbe2150

…claw#31996) (thanks @scoootscooob)

scoootscooob and others added 2 commits March 2, 2026 18:38

fix: strengthen cron reaper failure-path coverage and changelog (open…

acdd13e

…claw#31996) (thanks @scoootscooob)

steipete force-pushed the fix/cron-session-reaper-unreachable branch from fbe2150 to acdd13e Compare March 2, 2026 18:38

steipete merged commit 4030de6 into openclaw:main Mar 2, 2026
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(cron): move session reaper to finally block so it runs reliably#31996

fix(cron): move session reaper to finally block so it runs reliably#31996
steipete merged 2 commits intoopenclaw:mainfrom
scoootscooob:fix/cron-session-reaper-unreachable

scoootscooob commented Mar 2, 2026

Uh oh!

chatgpt-codex-connector bot left a comment

Uh oh!

greptile-apps bot commented Mar 2, 2026

Uh oh!

Uh oh!

steipete commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

scoootscooob commented Mar 2, 2026

Summary

Test plan

Uh oh!

chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

greptile-apps bot commented Mar 2, 2026

Greptile Summary

Confidence Score: 5/5

Uh oh!

Uh oh!

steipete commented Mar 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants