feat(agent-core): add cron tools and scheduler for session-level recurring tasks - #136
feat(agent-core): add cron tools and scheduler for session-level recurring tasks#136sailist wants to merge 26 commits into
Conversation
🦋 Changeset detectedLatest commit: 9dfd5ed The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1ad2cbb63e
ℹ️ 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".
| const next = computeNextCronRun(parsed, cursor); | ||
| if (next === null) break; | ||
| if (next > nowMs) break; | ||
| count++; |
There was a problem hiding this comment.
With jitter enabled, this coalesces by comparing each ideal cron time to nowMs, but recurring jobs are actually delivered at jitteredNextCronRunMs(...). If the scheduler wakes at e.g. 09:10:01 for a minutely job whose 09:10 occurrence is jittered to 09:10:06, this loop counts the 09:10 ideal fire and the caller then advances lastSeenAt to 09:10:01, so the 09:10 occurrence is never delivered. Count only occurrences whose jittered delivery time is due, or advance the baseline to the last due occurrence instead of wall-clock now.
Useful? React with 👍 / 👎.
| const offset = -config.oneShotMaxMs * fractionFromId(task.id); | ||
| return idealMs + offset; |
There was a problem hiding this comment.
For one-shot reminders on round minutes, this can move nextFireAt before the task's createdAt because the offset can be as large as 90 seconds. If a user schedules a one-shot at 08:59:30 for 0 9 * * *, a high-hash id can jitter the 09:00 ideal time back to 08:58:30; the next scheduler tick then treats the brand-new reminder as overdue and fires it immediately instead of near 09:00. The pull-forward needs a floor at the scheduling/current time (or enough context to avoid returning a past timestamp).
Useful? React with 👍 / 👎.
| const task = this.manager.store.add( | ||
| { | ||
| cron: args.cron, | ||
| prompt: args.prompt, | ||
| recurring, | ||
| }, | ||
| nowMs, | ||
| ); |
There was a problem hiding this comment.
This stores the nowMs captured during resolveExecution, before permission prompts and hooks run, and reuses it inside execute. In manual permission mode a user can approve a CronCreate minutes later; a one-shot for the next minute (or a frequent recurring job) is then inserted with an old createdAt/nextFireAt and can fire immediately with coalesced missed occurrences. Read wallNow() inside execute so the schedule is anchored to the time it is approved and committed.
Useful? React with 👍 / 👎.
| } else { | ||
| // Recurring: advance baseline so the next tick computes | ||
| // the next-after-now ideal fire. | ||
| lastSeenAt.set(task.id, now); |
There was a problem hiding this comment.
Recurring jobs are described to the model as firing until deleted or auto-expired after 7 days, but the recurring branch only advances lastSeenAt and leaves stale tasks in the store forever. Once a session stays up past the stale threshold, an old cron prompt continues to run indefinitely instead of expiring, which can trigger unexpected future agent turns. Delete or skip stale recurring tasks here (or remove the auto-expiry contract from the tool schema).
Useful? React with 👍 / 👎.
| try { | ||
| const parsed = parseCronExpression(task.cron); | ||
| humanSchedule = cronToHuman(parsed); | ||
| const ideal = computeNextCronRun(parsed, nowMs); |
There was a problem hiding this comment.
For one-shot jobs that are already due but could not fire yet because the agent was busy or manual ticking is disabled, CronList recomputes from nowMs, so a daily one-shot scheduled for 09:00 and listed at 09:05 is reported as tomorrow even though the scheduler will still fire today's missed occurrence from task.createdAt. This makes the list output disagree with the actual pending work; compute one-shot nextFireAt from the same baseline the scheduler uses until the job is removed.
Useful? React with 👍 / 👎.
| } | ||
|
|
||
| // 4. Session-level cap. | ||
| if (this.manager.store.list().length >= MAX_CRON_JOBS_PER_SESSION) { |
There was a problem hiding this comment.
This cap is checked while preparing the tool call, but the actual insert happens later in execute. If the session already has 49 jobs and the model issues two CronCreate calls in the same step, both preparations see 49 and pass; the scheduler then runs the executions sequentially and the store ends up with 51 jobs. Recheck the live store length inside execute immediately before store.add(...) so concurrent prepared calls cannot bypass the cap.
Useful? React with 👍 / 👎.
| const coalescedCount = | ||
| ideal === null ? 1 : Math.max(1, countCoalesced(parsed, ideal, now)); |
There was a problem hiding this comment.
This coalescing path also runs for recurring: false tasks, so a one-shot daily reminder whose first fire was missed while the laptop was closed for several days can be delivered with coalescedCount greater than 1. One-shots are removed after a single delivery and should not imply that multiple scheduled occurrences existed; force the coalesced count to 1 for one-shot tasks or skip this counting branch when task.recurring === false.
Useful? React with 👍 / 👎.
… for cron tools Typecheck: - Bracket-access KIMI_CRON_* env reads under TS4111 in cron source/tests. - Add `approvalRule` to CronCreate/CronList/CronDelete tool executions. - Guard `cron-expr.detectStep` against undefined array elements; cast the readonly snapshot in session-store.test through `unknown`; import the missing `ClockSources`/`ContentPart` symbols in manager.test. - Extend `isReplayUserTurnRecord` switch to cover the new `cron_job` and `cron_missed` origins so kimi-code stays exhaustive. Semantics (from Codex review): - CronCreate re-reads `wallNow()` and re-checks the session cap inside `execute()`, so manual-approval delays and concurrent prepared calls can't backdate the schedule or breach the cap. - One-shot jitter floors the pull-forward at `task.createdAt`, so a brand-new `:00`/`:30` reminder can't end up before its scheduling time. - Scheduler coalesce loop reapplies the same jitter as the delivery path and advances `lastSeenAt` to the last actually-delivered ideal fire; a not-yet-due jittered slot is no longer lost. One-shot fires always report `coalescedCount: 1`. - Manager removes recurring tasks after the first stale fire and emits a `cron_deleted` event, matching the 7-day auto-expire contract. - CronList anchors one-shot `nextFireAt` at `createdAt`, so a pending today's slot isn't rendered as tomorrow. Tests cover each of the above and a changeset is added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex review Address typecheck failures and the previous Codex P2 review:
Verified: agent-core (2046 tests pass), full repo tsgo typecheck clean, lint clean, builds OK. Changeset added. |
commit: |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6df93ab9be
ℹ️ 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".
| const baseFromMs = | ||
| task.recurring === false ? task.createdAt : nowMs; |
There was a problem hiding this comment.
For recurring jobs, using nowMs skips any already-ideal but not-yet-delivered jittered occurrence. For example, a daily 0 9 * * * job with a 15-minute forward jitter listed at 09:05 has not fired yet, but computeNextCronRun(parsed, nowMs) returns tomorrow's 09:00, so CronList reports tomorrow even though the scheduler will still deliver today's jittered fire. This contradicts the tool contract that nextFireAt is the value the scheduler compares against; expose/use the scheduler's per-task baseline instead of recomputing from nowMs.
Useful? React with 👍 / 👎.
| - `stale` — `true` when a recurring task is older than 7 days. The | ||
| task is **not** auto-deleted; recurring jobs keep firing. When you |
There was a problem hiding this comment.
This tool prompt now tells the model that stale recurring jobs are not auto-deleted and keep firing, but CronManager.handleFire removes stale recurring tasks immediately after one final delivery. When CronList shows stale: true, the model can therefore promise the user a keep/delete/refresh flow for a task that will be deleted on its next fire, which is user-visible misinformation from the tool description.
Useful? React with 👍 / 👎.
| coalescedCount: ctx.coalescedCount, | ||
| stale, | ||
| }; | ||
| const content: ContentPart[] = [{ type: 'text', text: task.prompt }]; |
There was a problem hiding this comment.
In stale or coalesced fires, the model only receives task.prompt here; CronJobOrigin is kept on the internal context record but ContextMemory.messages projects records into provider Messages and drops origin. That means the model cannot observe coalescedCount or stale even though the tool docs tell it to change behavior based on those fields, so a week-old final fire or a collapsed backlog is handled as an ordinary user prompt.
Useful? React with 👍 / 👎.
| // setInterval so the cron timer never keeps the process alive on its | ||
| // own, and isKilled (reading KIMI_DISABLE_CRON) short-circuits every | ||
| // tick — no need to delay start when the killswitch is set. | ||
| this.cron.start(); |
There was a problem hiding this comment.
Starting every agent's cron loop here leaves it running for the lifetime of the process, but Session.close() only stops background tasks/MCP/log handles and never calls agent.cron.stop(). In long-lived hosts or tests that close a session while other handles keep the process alive, unref() does not cancel the interval, so scheduled tasks can continue polling and fire into a closed session instead of dying with the session-only store.
Useful? React with 👍 / 👎.
| case 'cron_job': | ||
| case 'cron_missed': |
There was a problem hiding this comment.
These cron-origin messages are excluded from limitReplayRecordsByTurn, but renderReplayUserMessage has no cron branch and falls through to advanceReplayTurn() for the same records. In a session with many cron-fired turns and few real user turns, resume can therefore include far more than REPLAY_TURN_LIMIT rendered turns while also showing cron prompts as plain user messages; either count these as turn starts or render/skip them consistently as internal cron events.
Useful? React with 👍 / 👎.
… for cron tools Typecheck: - Bracket-access KIMI_CRON_* env reads under TS4111 in cron source/tests. - Add `approvalRule` to CronCreate/CronList/CronDelete tool executions. - Guard `cron-expr.detectStep` against undefined array elements; cast the readonly snapshot in session-store.test through `unknown`; import the missing `ClockSources`/`ContentPart` symbols in manager.test. - Extend `isReplayUserTurnRecord` switch to cover the new `cron_job` and `cron_missed` origins so kimi-code stays exhaustive. Semantics (from Codex review): - CronCreate re-reads `wallNow()` and re-checks the session cap inside `execute()`, so manual-approval delays and concurrent prepared calls can't backdate the schedule or breach the cap. - One-shot jitter floors the pull-forward at `task.createdAt`, so a brand-new `:00`/`:30` reminder can't end up before its scheduling time. - Scheduler coalesce loop reapplies the same jitter as the delivery path and advances `lastSeenAt` to the last actually-delivered ideal fire; a not-yet-due jittered slot is no longer lost. One-shot fires always report `coalescedCount: 1`. - Manager removes recurring tasks after the first stale fire and emits a `cron_deleted` event, matching the 7-day auto-expire contract. - CronList anchors one-shot `nextFireAt` at `createdAt`, so a pending today's slot isn't rendered as tomorrow. Tests cover each of the above and a changeset is added. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ClockSources splits wall-clock and monotonic time so the cron scheduler can be driven by an injected/simulated clock without breaking the lock heartbeat. resolveClockSources reads KIMI_CRON_CLOCK to switch between system, env-var-backed, and file-backed wall clocks; monotonic time is always process.hrtime.bigint() and never overridable.
parseCronExpression handles the standard 5-field syntax with the
cron-style dom/dow OR rule. computeNextCronRun uses field-by-field
jumping (not minute scanning) so sparse expressions like '0 12 1 1 *'
stay fast. hasFireWithinYears caps the search at 5 years so syntactically
legal but never-firing expressions ('0 0 31 2 *') return null instead of
spinning forever — required by CronCreate validation.
Recurring jobs shift forward by min(10% of period, 15min); one-shot jobs landing on :00/:30 shift earlier by up to 90s. Offset is hashed from task.id so reschedules and restarts stay stable. KIMI_CRON_NO_JITTER disables both branches for reproducible benches.
CronTask matches what gets persisted to tasks.json (with durable stripped). CronJobOrigin carries coalescedCount and stale so the agent can react to collapsed fires without separate channels; CronMissedOrigin tags the boot-time AskUserQuestion path.
oxlint 1.59 does not support no-restricted-syntax, so the ESLint-style guard from the plan is implemented as a vitest scan. The four guarded files (scheduler/persist/lock/jitter) must route every wall-clock read through ClockSources.wallNow(); clock.ts is excluded because that is where the abstraction is defined. Non-existent files are skipped so the guard activates automatically when later commits introduce them.
Four event names emitted by later commits (cron_scheduled, cron_fired, cron_missed, cron_deleted) live with the cron module rather than in the generic telemetry interface so a typo can't drift the metric and the abstraction stays domain-free.
Holds cron tasks scoped to a single CLI session — vanish on exit. Phase 2 will add a file-backed sibling that shares the shape. Ids are 8 hex characters with a collision-retry cap; createdAt is supplied by the caller's wall clock so the store stays clock-pure (and exempt from the Date.now() guard for the same reason).
The scheduler is a pure callback-driven loop: it gets tasks from a source(), gates on isIdle()/isKilled?(), computes next fire via cron-expr + jitter, and invokes onFire with the coalesced count when a task is due. lastSeenAt is in-memory only — coalesce semantics make a restart skip acceptable, but persisting last-fire would silently swallow legitimately-due fires. pollIntervalMs=null lets P1.8 disable the auto-tick timer for bench scenarios.
CronManager owns a SessionCronStore + CronScheduler and wires them to the Agent: scheduler.isIdle reads agent.turn.hasActiveTurn, isKilled reads KIMI_DISABLE_CRON, onFire builds a CronJobOrigin and routes through agent.turn.steer. Stale flag is computed on read (7-day age, recurring only, KIMI_CRON_NO_STALE shorts it) so manager doesn't have to mutate persisted tasks. handleMissed takes a renderer callback so P2.7 can plug in the AskUserQuestion text without bringing render imports into Phase 1.
CronCreate validates the expression, enforces the 5-year fire window (blocking '0 0 31 2 *' typos), caps prompt bytes at 8KB, caps active jobs at 50 per session, and rejects durable=true until Phase 2 adds the file-backed store. Manager exposes emitScheduled/emitDeleted so tools never reach into agent.telemetry directly.
Read-only tool surfacing every scheduled cron job for the session. Each record carries id / cron / humanSchedule / nextFireAt / recurring / durable / ageDays / stale, formatted in the same key: value\n---\n shape as TaskList. nextFireAt is the post-jitter timestamp so the model sees what the scheduler will actually fire on. Malformed cron strings render with null nextFireAt instead of throwing — defends against any future direct store inserts.
Validates the 8-hex id shape up front and routes deletion through the manager so cron_deleted telemetry stays consistent with cron_scheduled. Not-found is reported as an error so the model corrects itself rather than silently believing the delete succeeded — the next CronList would still show whatever id was missed.
Agent gains a public cron field constructed and started in the constructor. The scheduler's setInterval is .unref()'d so the cron timer never keeps the process alive, and isKilled (KIMI_DISABLE_CRON) short-circuits every tick, so eager start is safe. ToolManager registers CronCreate/CronList/CronDelete next to the background tools. initializeBuiltinTools runs lazily after the Agent constructor finishes, so this.agent.cron is already defined when the tools are constructed.
KIMI_CRON_MANUAL_TICK=1 forces the scheduler into manual-drive mode (pollIntervalMs: null), and in the same gate SIGUSR1 binds to a no-throw manager.tick() so bench scripts can advance the scheduler with kill -USR1 <pid> without a custom RPC. SIGUSR1 binding is opt-in (rather than always-on) for two reasons: the auto-tick interval already advances the scheduler, and a CLI with many subagents would otherwise pile up listeners and trip Node's 10-listener default. Tests cover the env gate, signal swallowing, listener cleanup, and the no-bind path when the env is unset. The AgentTestContext harness gains an onTestFinished cleanup so the auto-started cron manager never leaks across test files.
Exercises the full Agent → ToolManager → CronScheduler stack through the production CronCreateTool surface. Local-time anchor + injected ClockSources make coalescedCount=3 deterministic across host timezones. A second case walks the Create → List → Delete tool cycle to confirm the three-tool surface composes round-trip.
Pulls the duplicated createAgentStub + createClocks helpers out of the five cron test files into test/agent/cron/harness/stub.ts. The shared stub keeps the lightweight-Agent shape (only turn + telemetry surfaces need to look real) while letting individual tests opt into the options that mattered locally (hasActiveTurn / steerReturns).
Compresses each cron test header to a couple of lines describing what the file covers. The long rationale blocks were process documentation (why-this-file-exists, why-stub-vs-real-agent, plan-doc references) that didn't help anyone reading the test later. The few details that mattered (local-time anchor, coalescedCount math) stayed inline next to the code that needs them. E2E test also picks up the shared createClocks helper instead of defining its own.
…hots The 21 multi-field toMatch / toContain assertions across CronCreate / CronList / CronDelete tests covered the same ground a snapshot would have but cost more diff churn when a format detail changes. Errors become single-line snapshots; success outputs go through a small scrubCronOutput helper that replaces the random 8-hex id and ISO timestamp with stable placeholders so the snapshot is deterministic across TZ and run.
…n tools in default profile - Remove the unimplemented durable persistence field from the entire cron stack (types, tools, manager, tests, docs) to avoid misleading the model into promising cross-session persistence that does not exist yet. - Drop the env:VAR clock source in favour of the file:path source for test/bench control. - Register CronCreate, CronList, and CronDelete in the default agent profile.
… for cron tools Typecheck: - Bracket-access KIMI_CRON_* env reads under TS4111 in cron source/tests. - Add `approvalRule` to CronCreate/CronList/CronDelete tool executions. - Guard `cron-expr.detectStep` against undefined array elements; cast the readonly snapshot in session-store.test through `unknown`; import the missing `ClockSources`/`ContentPart` symbols in manager.test. - Extend `isReplayUserTurnRecord` switch to cover the new `cron_job` and `cron_missed` origins so kimi-code stays exhaustive. Semantics (from Codex review): - CronCreate re-reads `wallNow()` and re-checks the session cap inside `execute()`, so manual-approval delays and concurrent prepared calls can't backdate the schedule or breach the cap. - One-shot jitter floors the pull-forward at `task.createdAt`, so a brand-new `:00`/`:30` reminder can't end up before its scheduling time. - Scheduler coalesce loop reapplies the same jitter as the delivery path and advances `lastSeenAt` to the last actually-delivered ideal fire; a not-yet-due jittered slot is no longer lost. One-shot fires always report `coalescedCount: 1`. - Manager removes recurring tasks after the first stale fire and emits a `cron_deleted` event, matching the 7-day auto-expire contract. - CronList anchors one-shot `nextFireAt` at `createdAt`, so a pending today's slot isn't rendered as tomorrow. Tests cover each of the above and a changeset is added.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a38791411f
ℹ️ 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".
| // setInterval so the cron timer never keeps the process alive on its | ||
| // own, and isKilled (reading KIMI_DISABLE_CRON) short-circuits every | ||
| // tick — no need to delay start when the killswitch is set. | ||
| this.cron.start(); |
There was a problem hiding this comment.
Starting the scheduler from the Agent constructor leaves it running after CoreImpl.closeSession() calls Session.close() and removes the session; I checked packages/agent-core/src/session/index.ts and it only stops background tasks/MCP/logging, not agent.cron.stop(). In the TUI, switching sessions calls previous.close(), so any scheduled task in the old session can still tick later through this retained interval closure and call turn.steer for a session the SDK has already closed/cleared handlers for. Please wire the cron manager into the session/agent close lifecycle before starting it unconditionally here.
Useful? React with 👍 / 👎.
| if (task.createdAt !== undefined && shifted < task.createdAt) { | ||
| return task.createdAt; |
There was a problem hiding this comment.
When a one-shot is created shortly before a round :00/:30 match and the deterministic pull-forward would land before createdAt, returning createdAt makes the scheduler consider the job due on the very next tick (now >= nextFireAt), before the requested cron match. For example, creating 0 9 * * * at 08:59:30 with a high-hash id returns 08:59:30, so it can fire at 08:59:31 instead of the next 09:00 match; in this case the jitter should be skipped or clamped to a future time rather than to createdAt.
Useful? React with 👍 / 👎.
| return [ | ||
| `id: ${task.id}`, | ||
| `cron: ${task.cron}`, | ||
| `humanSchedule: ${humanSchedule}`, | ||
| `nextFireAt: ${nextFireAtIso}`, |
There was a problem hiding this comment.
In sessions where the original CronCreate output has scrolled away or been compacted, CronList is the only way for the model to inspect scheduled jobs before explaining, deleting, or refreshing them. The record rendered here omits task.prompt, so a stale job cannot be recreated with the same prompt as the tool description instructs, and the user cannot be told what an existing reminder/job will actually do from the list alone. Please include the stored prompt (possibly truncated) in each record.
Useful? React with 👍 / 👎.
| new b.CronCreateTool(this.agent.cron), | ||
| new b.CronListTool(this.agent.cron), | ||
| new b.CronDeleteTool(this.agent.cron), |
There was a problem hiding this comment.
Registering CronCreate and CronDelete exposes session-mutating tools that the existing plan-mode guard never denies; I checked PlanModeGuardDenyPermissionPolicy, and it only hard-blocks Write, Edit, and TaskStop. In auto permission mode that means a model can schedule or delete recurring jobs while plan mode is active, even though the analogous background mutation is explicitly blocked until ExitPlanMode. Please add the mutating cron tools to the plan-mode deny path while leaving CronList read-only.
Useful? React with 👍 / 👎.
| new b.TaskOutputTool(background), | ||
| new b.TaskStopTool(background), | ||
| new b.CronCreateTool(this.agent.cron), | ||
| new b.CronListTool(this.agent.cron), |
There was a problem hiding this comment.
CronList is read-only, and its tool description says it is always safe to call, including in plan mode, but this new tool is not covered by DefaultToolApprovePermissionPolicy (I checked that allowlist). In manual/default permission mode it therefore falls through to a user approval prompt just to inspect scheduled jobs, unlike TaskList/TaskOutput; add CronList to the default-approved read-only tools or give it an equivalent approval policy.
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aea6eceb07
ℹ️ 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".
| const baseFromMs = | ||
| task.recurring === false ? task.createdAt : nowMs; | ||
| const ideal = computeNextCronRun(parsed, baseFromMs); |
There was a problem hiding this comment.
当循环任务的理想触发点已经过去但前向 jitter 还没到达时（例如 0 9 * * * 在 09:05 查询，而该任务实际会在 09:10 触发），这里用 nowMs 作为 recurring 的基准会让 computeNextCronRun 直接跳到下一次理想触发点，所以 CronList 会把 nextFireAt 报成明天/下一周期，尽管当前周期仍在等待触发。这样会误导用户或模型认为本次提醒已经没有待触发；需要让列表逻辑复用调度器的 per-task next-fire 状态，或至少不要在当前 jitter 窗口内从 nowMs 跳过本次 occurrence。
Useful? React with 👍 / 👎.
| let iterations = 0; | ||
| const maxIterations = capMinutes + 10_000; |
There was a problem hiding this comment.
For legal-but-never-firing inputs such as 0 0 31 2 *, the loop does not actually stop after the documented five-year window: month/day jumps each consume only one iteration, so capMinutes + 10000 lets the search walk tens of thousands of years before returning null (I checked this path with that expression). This makes CronCreate validation and any defensive CronList render of a malformed stored task spend hundreds of milliseconds per expression instead of failing within the intended window; compare the candidate date against a real deadline rather than treating loop count as elapsed minutes.
Useful? React with 👍 / 👎.
| try { | ||
| onFire(task, { coalescedCount }); | ||
| } catch (error) { |
There was a problem hiding this comment.
If onFire throws before the prompt is actually enqueued (for example agent.turn.steer can throw when record persistence has latched a write error), this catch swallows the failure and the code below still removes a one-shot or advances lastSeenAt for a recurring task. In that failure mode the reminder is lost and will not be retried; the scheduler should only remove/advance after a successful delivery, or have onFire report success explicitly.
Useful? React with 👍 / 👎.
| const task = this.manager.store.add( | ||
| { | ||
| cron: args.cron, | ||
| prompt: args.prompt, | ||
| recurring, | ||
| }, |
There was a problem hiding this comment.
parseCronExpression accepts any whitespace between the five fields, so a tool call like "*/5\n* * * *" is valid and reaches this insert path, but storing the original string means CronList and CronCreate render a multi-line cron: value and break their own one-record-per-line format. Store a canonical args.cron.trim().split(/\s+/).join(' ') (or equivalent) after validation so accepted tabs/newlines cannot corrupt later tool output or the cron-fire envelope.
Useful? React with 👍 / 👎.
- session: stop each agent's CronManager during Session.close() so the 1-second tick interval (and via closure, the Agent → Session graph) doesn't leak across session lifecycles. Mirrors the parallel-stop pattern used by stopBackgroundTasksOnExit, with no keepAlive gate. - jitter: oneShotJitteredNextCronRunMs now skips jitter (returns idealMs) when shifted < createdAt, instead of clamping to createdAt which made `now >= nextFireAt` fire the task ~29s before ideal on the very next tick. - cron-list: render a JSON-escaped `prompt:` row truncated to ~200 UTF-8 bytes with a `…(truncated)` suffix; UTF-8 boundary walk-back avoids splitting multi-byte chars. Lets the model recall a task's intent after context compaction. - plan-mode: PlanModeGuardDenyPermissionPolicy now denies CronCreate and CronDelete (state-mutating); CronList stays allowed. - default-approve: add CronList to DEFAULT_APPROVE_TOOLS for parity with TaskList — pure read-only listing shouldn't trigger manual approval prompts. Tests: - session: spy-based and SIGUSR1-listener-count assertions verify cron stop actually runs its teardown. - jitter: both budget-insufficient (returns idealMs) and budget-sufficient (still pulls forward within cap) paths covered. - cron-list: 200-byte truncation marker and a separate CJK regression case confirming the UTF-8 walk-back lands on a legal char boundary. - plan-mode: three new cases (CronCreate denied, CronDelete denied, CronList allowed). - default-approve: dedicated policy unit tests covering CronList approval and CronCreate/CronDelete fall-through.
A — cron-fire envelope:
Cron fires now wrap `task.prompt` in a `<cron-fire ...>` XML envelope
carrying `jobId`, `cron`, `recurring`, `coalescedCount`, `stale` as
attributes and the original text inside `<prompt>...</prompt>`. The
envelope is the only path by which the LLM observes those fields — the
previous content was `{type:'text', text: task.prompt}` and the
CronJobOrigin metadata was server-side only, making the coalesce /
stale guidance in cron-create.md un-actionable. Renderer style mirrors
`agent/context/notification-xml.ts`. Adds projector merge-guard so
`<cron-fire ` envelopes aren't smeared into adjacent user messages
(same load-bearing detector that protects <notification>, etc.).
B / C / D — doc/implementation drift on stale + nextFireAt:
cron-list.md previously said recurring stale tasks "are not
auto-deleted; keep firing"; the implementation does the opposite
(final fire, then auto-delete). cron-create.md / cron-delete.md
prescribed a "CronDelete the stale id, then CronCreate" refresh
ritual that always failed step 1 because the id was already gone.
Rewrote the three docs to match the implementation: one final fire,
auto-delete, refresh = just CronCreate again with the prompt from
CronList. Also fixed cron-create.md "nextFireAt (epoch ms or null)"
to ISO timestamp.
E — subagents skip cron init:
Every Agent constructor unconditionally ran cron.start() (1Hz
setInterval + optional SIGUSR1 listener). Subagents don't expose
Cron tools in their default profile, so the work was pure waste,
and N parallel subagents under KIMI_CRON_MANUAL_TICK=1 would
eventually trip MaxListenersExceededWarning. Now gates start() and
the three Cron tool instantiations on `agent.type !== 'sub'`.
`agent.cron` itself stays constructed so `agent.cron.store` /
`.stop()` consumers (Session.close) keep working.
F — jitter UTC modulo comment:
Clarified that `idealMs % MS_PER_MINUTE === 0` is a UTC minute-
boundary check that incidentally coincides with local minute
boundaries because no current timezone has a sub-minute offset.
G — SIGUSR1 tick errors visible under debug:
The SIGUSR1 handler previously swallowed `tick()` exceptions
silently. Now logs to stderr when `KIMI_CRON_DEBUG=1`, matching the
scheduler's existing debug pattern. Production stays silent.
H — KIMI_CRON_CLOCK=file: bounded read:
`wallNow()` previously `readFileSync`'d the entire file on every
call. A hostile or stray-large file could OOM. Replaced with a
64-byte `openSync` + `readSync` read; epoch-ms is < 20 chars in
practice. fd is closed in all error paths.
Tests:
- New `cron-fire-xml.test.ts`: 5 cases (recurring, one-shot, attribute
escaping vs body verbatim, multi-line body, stale=true).
- New `subagent-skip.test.ts`: 3 cases verifying main/independent
init cron and subagent skips both `start()` and the three tools.
- New context-projector test: cron-fire envelope is not merged into
an adjacent real user message.
- manager.test.ts: content assertions updated to expect the envelope,
with an `=== 1` envelope-count check to detect accidental double-wrap.
- manual-tick.test.ts: two SIGUSR1-debug-log cases (emits with
KIMI_CRON_DEBUG=1, silent without).
- clock.test.ts: 64-byte cap + garbage fallback.
- cron.e2e.test.ts: content assertion updated for the envelope.
C3 — Don't mark failed cron fires as delivered:
scheduler.tick() previously swallowed any throw from onFire and
STILL ran the removeOneShot / lastSeenAt.set block, silently
losing reminders (one-shot deleted, recurring lastSeenAt advanced
past an undelivered ideal). Now gates the advance/remove block on
a `delivered` flag: failures leave state untouched so the next
tick re-detects the task as due and retries. inFlight is still
cleared at end-of-tick.
C1 — CronList nextFireAt matches the scheduler:
Cron list used `nowMs` as the recurring computeNextCronRun base.
When the current period's ideal had passed but its jittered
delivery was still pending (e.g. ideal 09:00 + 30s jitter = 09:00:30,
query at 09:00:15), CronList rendered `nextFireAt = tomorrow 09:00`
while the scheduler still planned 09:00:30 today — model and user
saw conflicting state. Scheduler now exposes `getNextFireForTask(id)`
using the same internal `lastSeenAt` baseline; manager delegates;
CronList calls through. The previous compute/jitter dance in
cron-list.ts and its unused imports are removed.
C2 — cron-expr search budget is wall-time, not iteration-count:
nextRunWithinMinutes capped iteration count, but advanceMonth jumps
~43 200 minutes per iteration. A never-firing expression like
`0 0 30 2 *` could spin ~200 000 years before bailing. The deadline
is now `fromMs + capMinutes * MS_PER_MINUTE` checked against the
advancing candidate date. A generous HARD_ITERATION_CAP (10M)
remains as forward-compat insurance against a refactor that
accidentally fails to advance the date.
C4 — Normalize CronCreate input whitespace:
parseCronExpression accepts any \\s+ between fields (including
tabs / newlines), but downstream sinks (CronList one-key-per-line
records, <cron-fire cron="..." ...> envelope attribute) assume a
single-line cron value. Now `args.cron.trim().split(/\\s+/).join(' ')`
before store.add, and the normalized form flows through the
rendered output and the approval description as well.
Tests:
- scheduler.test.ts: +5 tests — recurring/one-shot retry semantics
for C3, and getNextFireForTask for C1 (including the pending
jittered slot regression with id=`ffffffff`).
- cron-list.test.ts: +1 regression test pinning the current-period
jittered slot for a recurring task.
- manager.test.ts: +1 delegation test for getNextFireForTask.
- cron-expr.test.ts: +3 tests — fast-null for the pathological
`0 0 30 2 *` (under a 500ms budget) and a window-edge case for
hasFireWithinYears.
- cron-create.test.ts: +4 tests covering newline / tab / leading-
and-trailing whitespace normalization, plus a rendered-output
assertion that the LLM sees the normalized form.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f2204e51a
ℹ️ 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".
| case 'cron_job': | ||
| case 'cron_missed': |
There was a problem hiding this comment.
When a resumed session contains cron fires, this filter stops counting cron_job/cron_missed records as user turn starts, but SessionReplayController.renderUserMessage has no matching branch for these origins and still falls through to advanceTurn() and renders the raw <cron-fire> envelope as a user message. That can both display internal cron XML as if the user typed it and let cron-generated turns bypass the replay turn limit because they were excluded here; please either render/skip cron origins explicitly in replay or count them consistently.
Useful? React with 👍 / 👎.
| // offending field. | ||
| let parsed: ParsedCronExpression; | ||
| try { | ||
| parsed = parseCronExpression(args.cron); |
There was a problem hiding this comment.
When the cron argument contains tabs or newlines and the expression is not one of cronToHuman's special cases (for example 1\n2\n3\n4\n5), parsing the raw input preserves those separators in parsed.raw; the later cronToHuman(parsed) fallback then emits a multi-line humanSchedule: in the CronCreate output even though the stored cron: field was normalized. This still corrupts the one-key-per-line tool output for valid inputs with whitespace-separated fields, so parse the normalized expression (or normalize parsed.raw) before formatting the response.
Useful? React with 👍 / 👎.
| lo = Number(loStr); | ||
| hi = Number(hiStr); |
There was a problem hiding this comment.
For minute/hour/day-of-week fields, an input such as -5 * * * * reaches this range branch with an empty lower bound, and Number('') becomes 0, so the parser silently schedules minutes 0 through 5 instead of rejecting the malformed cron. The same Number(...) conversions also accept non-cron forms like 1e1 or 0x10; please validate digit-only bounds/values before converting.
Useful? React with 👍 / 👎.
…nd 4 D1 — TUI replay handles cron origins consistently: `isReplayUserTurnRecord` already excluded `cron_job` / `cron_missed` from user-turn counting, but `SessionReplayController.renderUserMessage` had no matching branch and rendered the raw `<cron-fire ...>` XML envelope as if the user had typed it. Added an early-return branch for both origins so cron records are now consistently skipped: not counted toward the replay turn limit, not visually rendered. D2 — Normalize cron whitespace BEFORE parsing: The previous round normalized `args.cron` only AFTER `parseCronExpression`, but `parsed.raw` retained the original tabs and newlines. `cronToHuman`'s raw-fallback branch then emitted a multi-line `humanSchedule:` row for any non-template expression, re-corrupting the one-key-per-line tool output for inputs like `1\\t2\\t3\\t4\\t5`. Moved the normalize step above the parse so `parsed.raw` is single-line and every downstream sink (description, store.add, output, error messages) reads the same canonical form. D3 — Reject malformed numeric cron tokens: The range / step parsers used `Number(str)` which silently accepted `''` (→ 0, so `-5 * * * *` became `0-5 * * * *`), `'1e1'` (→ 10), `'0x10'` (→ 16), and `'+5'`. Added a `DIGIT_ONLY = /^\\d+$/` gate via a `parseCronInt(raw, name, role)` helper at all four call sites (step, single value, range lower bound, range upper bound). Plain digit forms still parse; only typo-shaped non-cron inputs surface a parse error instead of being silently rescheduled. Tests: - message-replay.test.ts: +2 cases pinning cron_job/cron_missed skipping in replay (no XML in transcript, no user-turn entry). - cron-create.test.ts: +2 cases pinning humanSchedule single-line behavior for both the template-recognized path (`*/5\\n* * * *` → `every 5 minutes`) and the raw-fallback path (`1\\t2\\t3\\t4\\t5` → `1 2 3 4 5`). - cron-expr.test.ts: +9 cases covering negative bound, scientific notation, hex, leading-plus across single/step/range-lo/range-hi positions, plus a regression case asserting plain digit forms still parse.
|
@codex review Fourth batch on this PR (
Tests: message-replay +2 (cron_job/cron_missed skip), cron-create +2 (template + raw-fallback humanSchedule), cron-expr +9 (malformed in all numeric positions plus a regression covering plain forms). agent-core suite at 2098 passing, kimi-code app tests green, typecheck clean. |
|
Codex Review: Didn't find any major issues. Nice work! ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
CI's `tsgo -p packages/agent-core/tsconfig.json` (TypeScript native preview 7.0.0-dev.20260421) intermittently fails to resolve the `localKaos` const export from `@moonshot-ai/kaos` in this file, emitting `TS2724 has no exported member named 'localKaos'`. The three other session tests that hit the same import all also import either a `type` from kaos or a symbol from `@moonshot-ai/kosong`, which seems to anchor the workspace resolution; this file was the lone value-only import and was the only one that flaked. Switch to instantiating `LocalKaos` directly. Functionally identical at runtime (the `localKaos` const is itself just a `new LocalKaos()` singleton), and the class export sidesteps whatever resolution path the value export trips on. Local `tsc` and local `tsgo` both passed before and after; the fix only changes the import shape, not the test behavior.
The previous round revealed that CI's `tsgo` (TypeScript native preview) intermittently fails to resolve `localKaos` from `@moonshot-ai/kaos` in this file, emitting TS2724. Swapping to `new LocalKaos()` traded that for TS2673 (constructor is private) — `LocalKaos` is exported but its constructor is package-internal; only the `localKaos` singleton is meant to be consumed. The three other session tests that hit the same `localKaos` value import all also pull a `type` from kaos (`KaosProcess`, etc.) — and they pass tsgo cleanly. Naming `type Kaos` here adds the same workspace-type anchor and binds `localKaos` through a typed const, which keeps tsgo's resolver stable while preserving the runtime behaviour (kaos field is still the same singleton). Local `tsc` and `tsgo` both pass before and after the change.
… for cron tools Typecheck: - Bracket-access KIMI_CRON_* env reads under TS4111 in cron source/tests. - Add `approvalRule` to CronCreate/CronList/CronDelete tool executions. - Guard `cron-expr.detectStep` against undefined array elements; cast the readonly snapshot in session-store.test through `unknown`; import the missing `ClockSources`/`ContentPart` symbols in manager.test. - Extend `isReplayUserTurnRecord` switch to cover the new `cron_job` and `cron_missed` origins so kimi-code stays exhaustive. Semantics (from Codex review): - CronCreate re-reads `wallNow()` and re-checks the session cap inside `execute()`, so manual-approval delays and concurrent prepared calls can't backdate the schedule or breach the cap. - One-shot jitter floors the pull-forward at `task.createdAt`, so a brand-new `:00`/`:30` reminder can't end up before its scheduling time. - Scheduler coalesce loop reapplies the same jitter as the delivery path and advances `lastSeenAt` to the last actually-delivered ideal fire; a not-yet-due jittered slot is no longer lost. One-shot fires always report `coalescedCount: 1`. - Manager removes recurring tasks after the first stale fire and emits a `cron_deleted` event, matching the 7-day auto-expire contract. - CronList anchors one-shot `nextFireAt` at `createdAt`, so a pending today's slot isn't rendered as tomorrow. Tests cover each of the above and a changeset is added.
…tAI#136 Closes the deep-review pass and the four Codex review rounds that followed on the cron tools feature. Consolidated rather than landed as a series so the PR history reads as one fix wave on top of the original Phase-1 cron implementation. ## Lifecycle + structural - Session.close() now stops every agent's CronManager via `Promise.allSettled` (mirroring `stopBackgroundTasksOnExit`). Without this the 1s setInterval and its closure-captured Agent/Session graph leaked on every closed session. - Agent constructor gates `cron.start()` and the three Cron tool instantiations on `type !== 'sub'`. Subagents no longer pile up empty 1Hz timers or duplicate SIGUSR1 listeners under `KIMI_CRON_MANUAL_TICK=1`. ## Permission + plan-mode parity - `PlanModeGuardDenyPermissionPolicy` denies CronCreate and CronDelete during plan mode (CronList stays allowed); matches the TaskStop precedent. - `DEFAULT_APPROVE_TOOLS` includes CronList for parity with TaskList / TaskOutput so manual-approval mode doesn't prompt on read-only listings. ## Cron-fire envelope + projector - Cron fires wrap `task.prompt` in a `<cron-fire jobId=… cron=… recurring=… coalescedCount=… stale=…><prompt>…</prompt></cron-fire>` XML envelope (mirrors `notification-xml.ts`). Without this the `coalescedCount` and `stale` cues documented in cron-create.md were invisible to the LLM. - Projector `isInjectionUserMessage` recognises `<cron-fire ` so the envelope isn't merged into adjacent real user messages. - TUI `SessionReplayController.renderUserMessage` skips cron_job / cron_missed origins (matches `isReplayUserTurnRecord`'s exclusion); resumed sessions no longer render the raw envelope as user text or miscount cron records toward the replay turn limit. ## Scheduler invariants - `tick()` only advances `lastSeenAt` / removes one-shots after a successful `onFire`. A throw in `agent.turn.steer` previously silently lost the fire; now the next tick re-detects and retries. - New `getNextFireForTask(id)` on the scheduler (delegated through the manager) lets CronList render the same instant the scheduler will fire. Previously CronList computed from `nowMs` and could report tomorrow's slot while a current-period jittered delivery was still pending. ## Cron parser + validation - `parseCronExpression` now rejects non-cron numeric forms via a `parseCronInt` helper guarded by `^\d+$` — `''` / `'1e1'` / `'0x10'` / `'+5'` / `'-5'` no longer silently become 0, 10, 16, 5, or `0-5`. - `nextRunWithinMinutes` bounds search by a wall-time deadline instead of iteration count. Each iteration can skip a month, so the old `capMinutes + 10_000` cap let `0 0 30 2 *` walk ~200 000 years before bailing; the new path returns null in microseconds. - `oneShotJitteredNextCronRunMs` returns `idealMs` (not `createdAt`) when the pulled-forward time would precede `createdAt`. The old clamp made an 08:59:30-scheduled `0 9 * * *` fire on the very next tick — ~29 s before ideal — instead of at 09:00. ## CronList + CronCreate output - CronList output adds a `prompt:` row, JSON-encoded so newlines stay on one line, truncated to ~200 UTF-8 bytes on a char boundary. The model can recall a task's intent after compaction and use it as the source for the documented refresh ritual. - CronCreate normalizes `args.cron` whitespace BEFORE `parseCronExpression` so `parsed.raw` is single-line. Otherwise inputs like `"1\n2\n3\n4\n5"` (legal — parser accepts any \s+) produced a multi-line `humanSchedule:` row via the cronToHuman raw-fallback branch. ## Misc hardening - `KIMI_CRON_CLOCK=file:<path>` reads at most 64 bytes via `openSync` + `readSync` so a stray-large file can't OOM. - SIGUSR1 handler logs swallowed `tick()` exceptions to stderr when `KIMI_CRON_DEBUG=1` (matches scheduler's debug pattern); silent in production. - Documentation rewrite across cron-list.md / cron-create.md / cron-delete.md so the documented stale + nextFireAt behaviour matches the implementation (recurring stale tasks auto-delete after the final fire; `nextFireAt` is an ISO timestamp; refresh ritual is "just CronCreate again — the old id is already gone"). Tests cover each of the above. Suite at 2090+ passing across agent-core, kosong, and the kimi-code app; typecheck clean across all workspace packages.
* feat(agent-core): add cron ClockSources abstraction
ClockSources splits wall-clock and monotonic time so the cron scheduler
can be driven by an injected/simulated clock without breaking the lock
heartbeat. resolveClockSources reads KIMI_CRON_CLOCK to switch between
system, env-var-backed, and file-backed wall clocks; monotonic time is
always process.hrtime.bigint() and never overridable.
* feat(agent-core): add 5-field cron expression parser
parseCronExpression handles the standard 5-field syntax with the
cron-style dom/dow OR rule. computeNextCronRun uses field-by-field
jumping (not minute scanning) so sparse expressions like '0 12 1 1 *'
stay fast. hasFireWithinYears caps the search at 5 years so syntactically
legal but never-firing expressions ('0 0 31 2 *') return null instead of
spinning forever — required by CronCreate validation.
* feat(agent-core): add deterministic cron jitter
Recurring jobs shift forward by min(10% of period, 15min); one-shot
jobs landing on :00/:30 shift earlier by up to 90s. Offset is hashed
from task.id so reschedules and restarts stay stable. KIMI_CRON_NO_JITTER
disables both branches for reproducible benches.
* feat(agent-core): add CronTask type and cron prompt origins
CronTask matches what gets persisted to tasks.json (with durable stripped).
CronJobOrigin carries coalescedCount and stale so the agent can react to
collapsed fires without separate channels; CronMissedOrigin tags the
boot-time AskUserQuestion path.
* test(agent-core): guard against Date.now() in cron scheduler files
oxlint 1.59 does not support no-restricted-syntax, so the ESLint-style
guard from the plan is implemented as a vitest scan. The four guarded
files (scheduler/persist/lock/jitter) must route every wall-clock read
through ClockSources.wallNow(); clock.ts is excluded because that is
where the abstraction is defined. Non-existent files are skipped so the
guard activates automatically when later commits introduce them.
* feat(agent-core): add cron telemetry event-name constants
Four event names emitted by later commits (cron_scheduled, cron_fired,
cron_missed, cron_deleted) live with the cron module rather than in the
generic telemetry interface so a typo can't drift the metric and the
abstraction stays domain-free.
* feat(agent-core): add in-memory SessionCronStore
Holds cron tasks scoped to a single CLI session — vanish on exit. Phase 2
will add a file-backed sibling that shares the shape. Ids are 8 hex
characters with a collision-retry cap; createdAt is supplied by the
caller's wall clock so the store stays clock-pure (and exempt from the
Date.now() guard for the same reason).
* feat(agent-core): add session-only CronScheduler engine
The scheduler is a pure callback-driven loop: it gets tasks from a
source(), gates on isIdle()/isKilled?(), computes next fire via
cron-expr + jitter, and invokes onFire with the coalesced count when a
task is due. lastSeenAt is in-memory only — coalesce semantics make a
restart skip acceptable, but persisting last-fire would silently swallow
legitimately-due fires. pollIntervalMs=null lets P1.8 disable the
auto-tick timer for bench scenarios.
* feat(agent-core): add CronManager Agent integration layer
CronManager owns a SessionCronStore + CronScheduler and wires them to
the Agent: scheduler.isIdle reads agent.turn.hasActiveTurn, isKilled
reads KIMI_DISABLE_CRON, onFire builds a CronJobOrigin and routes
through agent.turn.steer. Stale flag is computed on read (7-day age,
recurring only, KIMI_CRON_NO_STALE shorts it) so manager doesn't have
to mutate persisted tasks. handleMissed takes a renderer callback so
P2.7 can plug in the AskUserQuestion text without bringing render
imports into Phase 1.
* feat(agent-core): add CronCreate tool (session-only path)
CronCreate validates the expression, enforces the 5-year fire window
(blocking '0 0 31 2 *' typos), caps prompt bytes at 8KB, caps active
jobs at 50 per session, and rejects durable=true until Phase 2 adds the
file-backed store. Manager exposes emitScheduled/emitDeleted so tools
never reach into agent.telemetry directly.
* feat(agent-core): add CronList tool
Read-only tool surfacing every scheduled cron job for the session. Each
record carries id / cron / humanSchedule / nextFireAt / recurring /
durable / ageDays / stale, formatted in the same key: value\n---\n
shape as TaskList. nextFireAt is the post-jitter timestamp so the model
sees what the scheduler will actually fire on. Malformed cron strings
render with null nextFireAt instead of throwing — defends against any
future direct store inserts.
* feat(agent-core): add CronDelete tool
Validates the 8-hex id shape up front and routes deletion through the
manager so cron_deleted telemetry stays consistent with cron_scheduled.
Not-found is reported as an error so the model corrects itself rather
than silently believing the delete succeeded — the next CronList would
still show whatever id was missed.
* feat(agent-core): wire CronManager + cron tools into Agent
Agent gains a public cron field constructed and started in the
constructor. The scheduler's setInterval is .unref()'d so the cron
timer never keeps the process alive, and isKilled (KIMI_DISABLE_CRON)
short-circuits every tick, so eager start is safe.
ToolManager registers CronCreate/CronList/CronDelete next to the
background tools. initializeBuiltinTools runs lazily after the Agent
constructor finishes, so this.agent.cron is already defined when the
tools are constructed.
* feat(agent-core): add manual-tick env + SIGUSR1 bench hook
KIMI_CRON_MANUAL_TICK=1 forces the scheduler into manual-drive mode
(pollIntervalMs: null), and in the same gate SIGUSR1 binds to a
no-throw manager.tick() so bench scripts can advance the scheduler
with kill -USR1 <pid> without a custom RPC.
SIGUSR1 binding is opt-in (rather than always-on) for two reasons:
the auto-tick interval already advances the scheduler, and a CLI with
many subagents would otherwise pile up listeners and trip Node's
10-listener default. Tests cover the env gate, signal swallowing,
listener cleanup, and the no-bind path when the env is unset. The
AgentTestContext harness gains an onTestFinished cleanup so the
auto-started cron manager never leaks across test files.
* test(agent-core): add end-to-end session cron smoke
Exercises the full Agent → ToolManager → CronScheduler stack through
the production CronCreateTool surface. Local-time anchor + injected
ClockSources make coalescedCount=3 deterministic across host
timezones. A second case walks the Create → List → Delete tool cycle
to confirm the three-tool surface composes round-trip.
* test(agent-core): extract shared cron test harness
Pulls the duplicated createAgentStub + createClocks helpers out of the
five cron test files into test/agent/cron/harness/stub.ts. The shared
stub keeps the lightweight-Agent shape (only turn + telemetry surfaces
need to look real) while letting individual tests opt into the
options that mattered locally (hasActiveTurn / steerReturns).
* test(agent-core): trim cron test file headers
Compresses each cron test header to a couple of lines describing what
the file covers. The long rationale blocks were process documentation
(why-this-file-exists, why-stub-vs-real-agent, plan-doc references)
that didn't help anyone reading the test later. The few details that
mattered (local-time anchor, coalescedCount math) stayed inline next
to the code that needs them. E2E test also picks up the shared
createClocks helper instead of defining its own.
* test(agent-core): convert cron tool output assertions to inline snapshots
The 21 multi-field toMatch / toContain assertions across CronCreate /
CronList / CronDelete tests covered the same ground a snapshot would
have but cost more diff churn when a format detail changes. Errors
become single-line snapshots; success outputs go through a small
scrubCronOutput helper that replaces the random 8-hex id and ISO
timestamp with stable placeholders so the snapshot is deterministic
across TZ and run.
* refactor(cron): remove durable flag, env clock source, and enable cron tools in default profile
- Remove the unimplemented durable persistence field from the entire cron stack (types, tools, manager, tests, docs) to avoid misleading the model into promising cross-session persistence that does not exist yet.
- Drop the env:VAR clock source in favour of the file:path source for test/bench control.
- Register CronCreate, CronList, and CronDelete in the default agent profile.
* fix(agent-core): address PR #136 typecheck and Codex review for cron tools
Typecheck:
- Bracket-access KIMI_CRON_* env reads under TS4111 in cron source/tests.
- Add `approvalRule` to CronCreate/CronList/CronDelete tool executions.
- Guard `cron-expr.detectStep` against undefined array elements; cast the
readonly snapshot in session-store.test through `unknown`; import the
missing `ClockSources`/`ContentPart` symbols in manager.test.
- Extend `isReplayUserTurnRecord` switch to cover the new `cron_job` and
`cron_missed` origins so kimi-code stays exhaustive.
Semantics (from Codex review):
- CronCreate re-reads `wallNow()` and re-checks the session cap inside
`execute()`, so manual-approval delays and concurrent prepared calls
can't backdate the schedule or breach the cap.
- One-shot jitter floors the pull-forward at `task.createdAt`, so a
brand-new `:00`/`:30` reminder can't end up before its scheduling time.
- Scheduler coalesce loop reapplies the same jitter as the delivery path
and advances `lastSeenAt` to the last actually-delivered ideal fire;
a not-yet-due jittered slot is no longer lost. One-shot fires always
report `coalescedCount: 1`.
- Manager removes recurring tasks after the first stale fire and emits a
`cron_deleted` event, matching the 7-day auto-expire contract.
- CronList anchors one-shot `nextFireAt` at `createdAt`, so a pending
today's slot isn't rendered as tomorrow.
Tests cover each of the above and a changeset is added.
* fix(agent-core,tui): address deep review + Codex review on PR #136
Closes the deep-review pass and the four Codex review rounds that
followed on the cron tools feature. Consolidated rather than landed
as a series so the PR history reads as one fix wave on top of the
original Phase-1 cron implementation.
## Lifecycle + structural
- Session.close() now stops every agent's CronManager via
`Promise.allSettled` (mirroring `stopBackgroundTasksOnExit`).
Without this the 1s setInterval and its closure-captured
Agent/Session graph leaked on every closed session.
- Agent constructor gates `cron.start()` and the three Cron tool
instantiations on `type !== 'sub'`. Subagents no longer pile up
empty 1Hz timers or duplicate SIGUSR1 listeners under
`KIMI_CRON_MANUAL_TICK=1`.
## Permission + plan-mode parity
- `PlanModeGuardDenyPermissionPolicy` denies CronCreate and
CronDelete during plan mode (CronList stays allowed); matches the
TaskStop precedent.
- `DEFAULT_APPROVE_TOOLS` includes CronList for parity with TaskList
/ TaskOutput so manual-approval mode doesn't prompt on read-only
listings.
## Cron-fire envelope + projector
- Cron fires wrap `task.prompt` in a `<cron-fire jobId=… cron=…
recurring=… coalescedCount=… stale=…><prompt>…</prompt></cron-fire>`
XML envelope (mirrors `notification-xml.ts`). Without this the
`coalescedCount` and `stale` cues documented in cron-create.md
were invisible to the LLM.
- Projector `isInjectionUserMessage` recognises `<cron-fire ` so the
envelope isn't merged into adjacent real user messages.
- TUI `SessionReplayController.renderUserMessage` skips cron_job /
cron_missed origins (matches `isReplayUserTurnRecord`'s exclusion);
resumed sessions no longer render the raw envelope as user text or
miscount cron records toward the replay turn limit.
## Scheduler invariants
- `tick()` only advances `lastSeenAt` / removes one-shots after a
successful `onFire`. A throw in `agent.turn.steer` previously
silently lost the fire; now the next tick re-detects and retries.
- New `getNextFireForTask(id)` on the scheduler (delegated through
the manager) lets CronList render the same instant the scheduler
will fire. Previously CronList computed from `nowMs` and could
report tomorrow's slot while a current-period jittered delivery
was still pending.
## Cron parser + validation
- `parseCronExpression` now rejects non-cron numeric forms via a
`parseCronInt` helper guarded by `^\d+$` — `''` / `'1e1'` /
`'0x10'` / `'+5'` / `'-5'` no longer silently become 0, 10, 16,
5, or `0-5`.
- `nextRunWithinMinutes` bounds search by a wall-time deadline
instead of iteration count. Each iteration can skip a month, so
the old `capMinutes + 10_000` cap let `0 0 30 2 *` walk ~200 000
years before bailing; the new path returns null in microseconds.
- `oneShotJitteredNextCronRunMs` returns `idealMs` (not `createdAt`)
when the pulled-forward time would precede `createdAt`. The old
clamp made an 08:59:30-scheduled `0 9 * * *` fire on the very next
tick — ~29 s before ideal — instead of at 09:00.
## CronList + CronCreate output
- CronList output adds a `prompt:` row, JSON-encoded so newlines
stay on one line, truncated to ~200 UTF-8 bytes on a char
boundary. The model can recall a task's intent after compaction
and use it as the source for the documented refresh ritual.
- CronCreate normalizes `args.cron` whitespace BEFORE
`parseCronExpression` so `parsed.raw` is single-line. Otherwise
inputs like `"1\n2\n3\n4\n5"` (legal — parser accepts any \s+)
produced a multi-line `humanSchedule:` row via the cronToHuman
raw-fallback branch.
## Misc hardening
- `KIMI_CRON_CLOCK=file:<path>` reads at most 64 bytes via
`openSync` + `readSync` so a stray-large file can't OOM.
- SIGUSR1 handler logs swallowed `tick()` exceptions to stderr when
`KIMI_CRON_DEBUG=1` (matches scheduler's debug pattern); silent in
production.
- Documentation rewrite across cron-list.md / cron-create.md /
cron-delete.md so the documented stale + nextFireAt behaviour
matches the implementation (recurring stale tasks auto-delete after
the final fire; `nextFireAt` is an ISO timestamp; refresh ritual
is "just CronCreate again — the old id is already gone").
Tests cover each of the above. Suite at 2090+ passing across
agent-core, kosong, and the kimi-code app; typecheck clean across
all workspace packages.
* feat(cron): persist scheduled tasks across kimi resume
Add per-id JSON persistence so cron tasks survive a kimi resume of the same session.
Core changes:
- CronManager: addTask / removeTasks mirror mutations to <sessionDir>/cron/<id>.json
- CronManager.loadFromDisk() rehydrates the in-memory store on resume
- CronManager.flushPersist() drains pending writes for graceful shutdown
- SessionCronStore.adopt() inserts persisted tasks with original id and createdAt
- Extract shared createPerIdJsonStore utility from background/persist.ts
- Refactor background/persist.ts to use createPerIdJsonStore (no behavior change)
- New tests: resume.test.ts, persist.test.ts, per-id-json-store.test.ts
- Update cron-create / cron-list / cron-delete docs to reflect session lifetime
* fix(agent-core): fix cron-stop-on-close test import for tsgo compat
* feat(cron): persist lastFiredAt cursor and scope approval rules
- persist lastFiredAt across resume so recurring tasks don't replay
- add one-shot pinned-date guard to reject >1-year-out first fires
- scope CronCreate approvalRule to exact payload (cron, prompt, recurring)
- add resume replay tests and corrupt-cursor fallback test
- stop cron before awaiting background shutdown so due ticks cannot
start a fresh turn while session.close() is mid-flight
- filter cron_job / cron_missed origins from markdown export so the
internal <cron-fire ...> envelope no longer leaks into user-facing
exports
1 participant
Summary
This PR introduces a complete cron subsystem in
agent-corethat enables agents to schedule, list, and delete recurring tasks within a session. It includes a 5-field cron expression parser, deterministic jitter, pluggable clock sources, an in-memory scheduler, three new builtin tools (cron_create,cron_list,cron_delete), and full Agent integration viaCronManager. The cron tools are now enabled in the default agent profile.1. Cron expression parsing and scheduling engine
Problem: There was no way for agents to manage recurring tasks within a session.
What was done:
cron-expr.ts) supporting standard cron syntax.ClockSources(clock.ts) for testability and time abstraction.jitter.ts) to prevent thundering-herd patterns.CronScheduler(scheduler.ts) with session-scoped task execution.SessionCronStore(session-store.ts) for persisting cron tasks in memory per session.2. Cron tools for agent interaction
Problem: Agents needed a programmatic interface to manage cron tasks.
What was done:
CronCreatetool (cron-create.ts) with markdown documentation for scheduling new tasks.CronListtool (cron-list.ts) for querying active and historical cron jobs.CronDeletetool (cron-delete.ts) for canceling scheduled tasks.telemetry-events.ts) for cron operations.3. Agent integration and lifecycle management
Problem: The cron subsystem needed to be wired into the Agent lifecycle and profile system.
What was done:
CronManager(manager.ts) as the integration layer between the Agent and the cron scheduler.CronManagerand cron tools into the Agent constructor and tool registry.default/agent.yaml).SIGUSR1bench hook for testing.durableflag and environment-based clock source from the final refactor for simplicity.4. Comprehensive test coverage
Problem: The new cron subsystem required validation across all layers.
What was done:
CronManagerand Agent-level cron behavior.harness/stub.ts) for reusable test fixtures.Date.now()guards in cron scheduler files to ensure clock abstraction compliance.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.