fix(cron): spawn scheduler loop at startup so scheduled jobs auto-fire#873
Conversation
The cron scheduler polling loop (`cron::scheduler::run()`) was never spawned at startup — only "Run Now" worked via direct RPC. Add `tokio::spawn` for the scheduler in `src/core/jsonrpc.rs`, gated on `config.cron.enabled`, following the same pattern as the update scheduler. Also install missing deps from upstream PR tinyhumansai#745 (command palette). Closes tinyhumansai#830
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughAdds startup spawning of the cron scheduler (gated by Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Server as JsonRPC Server
participant Config as OpenHuman Config
participant Cron as Cron Scheduler
rect rgba(200,230,255,0.5)
Server->>Config: load config on startup
Config-->>Server: config (cron.enabled true/false)
alt cron.enabled == true
Server->>Cron: tokio::spawn(cron::scheduler::run(config))
Cron-->>Server: started (background)
else cron.enabled == false
Server-->>Server: log "cron disabled"
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/core/jsonrpc.rs (1)
749-759: Consider: Graceful shutdown for the scheduler loop.The
cron::scheduler::run()function (per context snippet fromscheduler.rs:19-54) runs an infinite loop with no cancellation token. When the server receives a shutdown signal (Line 799), this spawned task will be forcibly aborted rather than gracefully stopped.This matches the existing pattern used by other background tasks (update scheduler, channel listeners), so it's not a regression. However, for future improvement, consider passing a
CancellationTokento allow the scheduler to complete any in-flight job execution before shutdown.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/core/jsonrpc.rs` around lines 749 - 759, The spawned cron scheduler task currently calls crate::openhuman::cron::scheduler::run(config).await without any cancellation token, so the task is force-aborted on server shutdown; modify the spawn logic to create a CancellationToken (or equivalent used elsewhere in the project), clone/propagate it into the task, and change the call to run(config, token) so the scheduler's run function can observe cancellation and gracefully finish in-flight jobs; also wire the token to the server shutdown notifier used elsewhere so it is triggered when the server begins shutdown.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/package.json`:
- Line 68: In package.json remove the duplicate "@testing-library/user-event"
entry from the dependencies block so it only appears under devDependencies;
specifically delete the dependencies entry for "@testing-library/user-event":
"^14.6.1" and keep the existing devDependencies entry intact to ensure testing
utilities are not shipped to production.
---
Nitpick comments:
In `@src/core/jsonrpc.rs`:
- Around line 749-759: The spawned cron scheduler task currently calls
crate::openhuman::cron::scheduler::run(config).await without any cancellation
token, so the task is force-aborted on server shutdown; modify the spawn logic
to create a CancellationToken (or equivalent used elsewhere in the project),
clone/propagate it into the task, and change the call to run(config, token) so
the scheduler's run function can observe cancellation and gracefully finish
in-flight jobs; also wire the token to the server shutdown notifier used
elsewhere so it is triggered when the server begins shutdown.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 49bd23bf-5b94-4174-b5b9-b897bfb29918
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
.claude/memory.mdapp/package.jsonsrc/core/jsonrpc.rs
Testing utilities should not be shipped to production.
|
Re: CodeRabbit nitpick on graceful shutdown via |
tinyhumansai#873) Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
Summary
cron::scheduler::run(config)at startup insrc/core/jsonrpc.rs, gated onconfig.cron.enableddue_jobs()every ~5s) was never started — only "Run Now" via direct RPC workedtokio::spawn+Config::load_or_init()pattern as the update schedulerTest plan
*/1 * * * *schedulecron.enabled = falseprevents the scheduler from startingCloses #830
Summary by CodeRabbit
Bug Fixes
Dependencies
Documentation