feat(runtime): add session-internal CronJob scheduling (Issue #15 Primitive 4) - #545
feat(runtime): add session-internal CronJob scheduling (Issue #15 Primitive 4)#545hqhq1025 wants to merge 2 commits into
Conversation
… Primitive 4) Implements CronCreate/CronDelete/CronList tools aligned with Claude Code's pattern. The agent can schedule prompts to fire at future times within the same session, enabling polling, monitoring, and periodic check workflows. New files: - wakeup-scheduler.ts: core scheduler with timer management, cron parser, auto-expire (7d for recurring), jitter, idle-gate, and race-safe fire() - wakeup-tools.ts: CronCreate/CronDelete/CronList tool wrappers - wakeup-scheduler.test.ts: 28 unit tests covering scheduling, cron parsing, recurring reschedule, auto-expire, jitter bounds, idle-gate backoff, and cancel-during-canFire race condition Integration: - runtime/index.ts: export WakeupScheduler and cron tool builders - desktop/main.ts: instantiate scheduler, register tools, wire session stop/archive/delete cleanup, dispose on before-quit - ui/tool-activity.tsx: CronJob result preview cards with Lucide icons (Clock for one-shot, Repeat for recurring, Check for cancelled) Closes apache#15
Recurring cron jobs were accumulating ALL historical fired records
indefinitely. When CronList was called, it returned all records
(800+ observed), triggering context budget archiving and creating
hundreds of tool-result-active-* files.
Three fixes applied:
1. Non-recurring jobs: remove from records after successful fire
(no reason to keep them; they'll never fire again)
2. Recurring jobs: reuse the same record in-place instead of
creating a new record on each fire. The record stays in the
map with its original id, just gets updated firesAt/status.
3. Safety cap: MAX_RECORDS_PER_SESSION = 50. pruneSession()
drops oldest terminal (fired/expired/cancelled) records when
exceeded.
Additionally, CronList now only returns active (pending) jobs by
default via listForSession({ activeOnly: true }), preventing the
full history from being serialized into tool results.
Converting to a discussion comment; not requesting changes.
Astro-Han
left a comment
There was a problem hiding this comment.
Verified locally first: I resolved the rebase conflicts against current main by hand (see P1 #3), then build / typecheck / tests are all green (wakeup 32/32, runtime 894/0, desktop 2040/0). So the items below are about behavior and shape, not compile or test breakage.
P0
None, once the conflicts below are resolved.
P1
1. Recurring monitoring jobs silently expire after ~10s of session-busy
fire() backs off when canFire is false (session mid-turn): BACKOFF_MS = 5000, expire when retryCount >= MAX_FIRE_RETRIES(3). The job tries at T, T+5s, T+10s, and expires on the third attempt. So if the previous injected turn is still running ~10s past the next scheduled fire, the recurring job is marked expired and never fires again, with no event anywhere. The expires after max retries test bakes this in as expected behavior.
For the headline use case ("check deploy status every 30 seconds"), any check turn that runs long kills the loop silently. Idle deferral should not count against a failure budget; keep the job pending until the 7-day TTL, user cancel, or session end.
2. Idle gate is not atomic; two turns can start in the same session
injectTurn bypasses ensureSessionCanSend (the guard the human send and bot-incoming paths use) and rolls its own split gate: canFire does await store.readHeader and checks status === 'active', then injectTurn calls runtime.sendMessage. The runtime side does not close the gap: startTurn / ensureActive / registerRun never reject a second concurrent run, and registerRun just adds to the map.
Between canFire resolving and the status being written back to running, two wakeups firing in the same minute, or a wakeup colliding with a user send, can both read active and both start an agent turn. Either move the gate into the runtime as tryStartScheduledTurnIfIdle (check active runs, register, start, in one action), or reuse ensureSessionCanSend and accept its existing TOCTOU window rather than adding a second one.
3. Merge conflict with current main (rebase needed)
GitHub already shows CONFLICTING. Two spots:
packages/ui/src/tool-activity.tsx: main extractedToolActivityCard(Collapsible-based) and removed the inline<details>block this PR patches. The cron preview branch has to move intoToolActivityCard's result block. I wired it there locally and the suite passes.apps/desktop/src/main/main.ts:before-quitneeds bothwakeupScheduler.dispose()and main's newconfigWatcher?.stop().
P2
4. Injected turn has no attribution
The wakeup injects [Scheduled wakeup: ${reason}]\n\n${message} as a plain user message through runtime.sendMessage. bot-incoming uses formatBotMessageForSession to attribute bot-injected turns; the wakeup path has no equivalent. A scheduled turn is indistinguishable from real user input in the transcript and data model. Either reuse the bot-incoming attribution format or add a source marker on the turn.
5. Injected-turn errors are swallowed
injectTurn does void streamEvents(...). If the injected turn rejects while streaming, nothing catches it. bot-incoming collects and surfaces errors via collectBotReply. A failed scheduled turn should not vanish.
P3
6. CronDelete has no session owner check
buildCronDeleteTool calls scheduler.cancel(job_id) with no ctx.sessionId, and cancel looks the record up by global id. A session that somehow obtains another session's job id can cancel it. Low practical risk (ids are server-generated UUIDs, not enumerable cross-session), but the fix is cheap: cancelForSession(sessionId, jobId), return cancelled: false on mismatch.
7. One-shot jitter checks delay alignment, not the firesAt timestamp
computeJitter comment says it applies early jitter when firesAt lands on :00 or :30, but the code checks delayMs % (30*60*1000) === 0. A 30-minute delay scheduled at 12:07 fires at 12:37 (not a round minute) yet gets jittered. Simplest fix is to drop one-shot jitter entirely; if kept, judge it against the actual firesAt.
8. WakeupScheduler forward-references runtime and streamEvents
The scheduler is constructed at line 386, but its closures reference runtime (line 684) and streamEvents (line 1477). It works because the closures only run after module init, and a comment says so, but it is fragile. Move construction after runtime, or resolve lazily.
9. as Required<WakeupSchedulerDeps> cast is unsound
The constructor resolves optional deps then spreads ...deps last with a cast. It only works because omitted optional keys are not own properties. Resolve each field explicitly instead of relying on that.
10. No integration test for the wired gate
Unit tests mock both canFire and injectTurn. There is no test exercising the real status === 'active' && !archivedAt check or the runtime.sendMessage injection, only manual GUI verification.
11. Minor doc nits
PR body says 28 unit tests; there are 32. CronDelete returns ok: true even when nothing was cancelled; consider ok: cancelled.
Separate alignment question: where should scheduled work live?
Not a merge gate, but worth deciding before this grows. I want to flag it as a design question rather than a list of must-fixes.
This PR sits between two shapes. It is not light enough to be a throwaway in-session primitive (+1154 lines: a hand-rolled cron parser, two jitter heuristics, recurring, CronCreate/CronDelete/CronList as public agent tools, auto-expire, retry, UI cards). It is also not heavy enough to honor the contract that surface implies: in-memory, its own WakeupRecord state, no runtime events for fire/expire/cancel, gone on app quit. The P1 and P2 bugs above are mostly symptoms of running a durable-looking scheduler through a volatile side channel.
Two coherent directions:
- Path A, ephemeral primitive. Strip to a one-shot, same-session, short-delay
schedule_wakeupwith no cron, no recurring, no list/delete, documented as non-durable, with a trace breadcrumb. Roughly the ~50-line version. It can coexist with a future durable system without competing with it. - Path B, durable model. Per #544's Cronjob track ("fold scheduled work into the same task/run/trace/memory model instead of creating an unobservable timer subsystem"; "Cronjob should produce logs, tasks, results, failure attribution, and next-schedule state, not just a local timer"), the recurring/cron/list surface belongs on task/run/trace. PawWork's
automationmodel (one tool, durable Definition+Run,context: continuefor in-session vscontext: freshfor new-session, writer-key concurrency lock) is a worked example and maps onto what we already have.
A rough boundary rule: if it is recurring, user-visible, named cron/job/automation, shown in UI, expected after restart, or has a failure policy, it belongs in the durable model. If it is "ping this conversation once in 30s while it is alive," the ephemeral primitive is fine.
My worry about landing #545 as-is is that it ships the CronCreate/Delete/List + recurring + cron-parser contract through a volatile side channel, which is the one combination both paths want to avoid. Happy to help write the RFC for B, or review a stripped-down A. Either way, the P1/P2 bugs above stand regardless of which direction we pick.
… scheduling Replaces the fragmented CronCreate/CronDelete/CronList approach (apache#545) with a single `Automation` tool using a `mode` parameter (create/delete/list/pause/resume) and a `kind` parameter (heartbeat vs cron), following Codex Desktop's pattern. Key design decisions: - One tool, one concept: model only decides heartbeat (continue session) vs cron (fresh session) - Schedule supports: cron 5-field expressions, interval seconds, or one-shot delay - Optional durable persistence (JSON file, atomic writes) for cross-restart survival - 7-day auto-expiry, max_fires cap, consecutive-failure auto-pause (5 strikes) - Scheduler tick every 5s, defers when session busy (max 120s then skip) - Desktop + CLI/TUI both integrated with full persistence and turn-tail injection Addresses PR apache#545 reviewer feedback (Astro-Han): - Not in "unhappy middle": ephemeral heartbeats are honestly session-scoped, durable automations persist to disk. Each kind delivers what its API promises. - Trace is free: injectTurn → sendMessage → AgentRun → full RuntimeEvent trace without new event types or data model changes. - No dependency on apache#544 task/run/trace infrastructure. 3 rounds of adversarial review, 26 bugs found and fixed.
…es) (#639) * feat(runtime): add session-internal CronJob scheduling (Issue #15 Primitive 4) Implements CronCreate/CronDelete/CronList tools aligned with Claude Code's pattern. The agent can schedule prompts to fire at future times within the same session, enabling polling, monitoring, and periodic check workflows. New files: - wakeup-scheduler.ts: core scheduler with timer management, cron parser, auto-expire (7d for recurring), jitter, idle-gate, and race-safe fire() - wakeup-tools.ts: CronCreate/CronDelete/CronList tool wrappers - wakeup-scheduler.test.ts: 28 unit tests covering scheduling, cron parsing, recurring reschedule, auto-expire, jitter bounds, idle-gate backoff, and cancel-during-canFire race condition Integration: - runtime/index.ts: export WakeupScheduler and cron tool builders - desktop/main.ts: instantiate scheduler, register tools, wire session stop/archive/delete cleanup, dispose on before-quit - ui/tool-activity.tsx: CronJob result preview cards with Lucide icons (Clock for one-shot, Repeat for recurring, Check for cancelled) Closes #15 * fix(runtime): prevent unbounded CronJob record accumulation Recurring cron jobs were accumulating ALL historical fired records indefinitely. When CronList was called, it returned all records (800+ observed), triggering context budget archiving and creating hundreds of tool-result-active-* files. Three fixes applied: 1. Non-recurring jobs: remove from records after successful fire (no reason to keep them; they'll never fire again) 2. Recurring jobs: reuse the same record in-place instead of creating a new record on each fire. The record stays in the map with its original id, just gets updated firesAt/status. 3. Safety cap: MAX_RECORDS_PER_SESSION = 50. pruneSession() drops oldest terminal (fired/expired/cancelled) records when exceeded. Additionally, CronList now only returns active (pending) jobs by default via listForSession({ activeOnly: true }), preventing the full history from being serialized into tool results. * review: wakeup-scheduler first-principles fixes - Idle-gate backoff: 3×5s silently dropped any wakeup landing mid-turn (agent turns run minutes) — the feature's core use case. Now exponential 5s→5min cap × 12 attempts (~45min window). - canFire: waiting_for_user is the wakeup's home scenario (fire in place of the user); only running/terminal/archived defers. - Observability: fired one-shots stayed unreachable ('fired' status was deleted on the spot, making pruneSession's terminal-history design dead code). Terminal records now persist under the per-session cap. - Jitter semantics: the :00/:30 early-jitter keyed off delayMs%30min, but a 30-minute delay from 10:07 fires at 10:37 — the round-mark property belongs to the fire timestamp. computeJitter now takes firesAtMs. --------- Co-authored-by: hqhq1025 <1506751656@qq.com>
|
感谢贡献!核心状态机(竞态双检、依赖注入、测试覆盖)质量很好。maintainer review 后以 #639 收编合入(rebase 到最新 main + 4 项修复),本 PR 关闭。 Review 发现的 4 个设计盲点,供参考:
另外分支落后 main 较多(task-ledger / tool-activity / before-quit 都已漂移),后续 PR 建议先 rebase。 |
Summary
Implements session-internal CronJob scheduling aligned with Claude Code's CronCreate/CronDelete/CronList pattern. This is Issue #15 Primitive 4 (Loop / automation) — the agent can schedule prompts to fire at future times within the same session, enabling polling, monitoring, and periodic check workflows.
What changed
New files
packages/runtime/src/wakeup-scheduler.tspackages/runtime/src/wakeup-tools.tspackages/runtime/src/__tests__/wakeup-scheduler.test.tsModified files
packages/runtime/src/index.tsapps/desktop/src/main/main.tspackages/ui/src/tool-activity.tsxHow it works
The agent calls
CronCreatewith eitherdelay_secondsor a 5-fieldcronexpression. TheWakeupSchedulerregisters a timer. When it fires, it checkscanFire()(session exists, not archived, not mid-turn), then injects a synthetic user message viaruntime.sendMessage()— the same path used by bot-incoming and OpenGateway. The agent sees the full conversation context and continues working.Features aligned with CC
Self-review and bug fixes
After initial implementation, we ran a 4-dimension adversarial review (correctness, integration, UI, test coverage) with verification. This caught and fixed 4 confirmed bugs before submission:
Race condition in cancel-during-fire:
fire()did not re-checkrecord.statusafter the asynccanFire()await. Ifcancel()was called whilecanFire()was in-flight, the cancelled wakeup would still fire. Fixed: added status re-check after await.Cron wildcard flag for
*/Npatterns:*/5in day-of-month incorrectly setwildcard=true, breaking the day-of-month OR day-of-week matching logic. Fixed: only set wildcard for bare*without step.canFire idle-gate incomplete: Only checked
!archivedbut notrunningorwaiting_for_usersession status. Wakeup could inject a turn while the model was mid-turn. Fixed: added status checks.dispose() did not prevent in-flight fire: An async
fire()could still execute afterdispose(). Fixed: addeddisposedflag checked at fire entry.Testing
Unit tests (28/28 pass)
Desktop GUI testing
Launched Maka desktop (
npm run dev), verified end-to-end in a real session:Headless testing (6 runs, all passed)
Used
maka-headless harbor runwith coproxy (Anthropic via B200 Tailscale):*/10 * * * *scheduling (correctly parsed and registered)Known limitations (not in scope for this PR)
durable: truefor cross-restart persistence. Planned for v2.sendMessage()which creates arole: 'user'message. A dedicated message type for system-triggered turns is a broader architectural change.Verification