Skip to content

feat(agent-core): add session-scoped cron tools with persistence - #157

Merged
sailist merged 24 commits into
MoonshotAI:mainfrom
sailist:feat/cron-tools-with-persistance
May 28, 2026
Merged

feat(agent-core): add session-scoped cron tools with persistence#157
sailist merged 24 commits into
MoonshotAI:mainfrom
sailist:feat/cron-tools-with-persistance

Conversation

@sailist

@sailist sailist commented May 28, 2026

Copy link
Copy Markdown
Collaborator

Summary

This PR introduces session-scoped scheduled prompts ("cron tools") that let the assistant register, list, and cancel recurring or one-shot reminders that fire later in the same session. It also adds persistence so scheduled tasks survive across Kimi resume/restart, and integrates the subsystem into the default agent profile.


1. Cron scheduler engine & expression parsing

Problem: Kimi Code had no way to schedule future or recurring work within a session.

What was done:

  • Added a 5-field cron expression parser (cron-expr.ts) supporting standard wildcards, ranges, steps, and named months/days.
  • Implemented CronScheduler with deterministic jitter to avoid thundering-herd on aligned schedules.
  • Added pluggable ClockSources (system, manual-tick for tests, SIGUSR1 bench hook).
  • Added SessionCronStore for in-memory task storage keyed by session ID.

2. Cron tools (Create / List / Delete)

Problem: The agent needed callable tools to manage its own scheduled tasks.

What was done:

  • Added CronCreate, CronList, and CronDelete tools with markdown prompts and telemetry events.
  • Wired tools into the agent’s builtin tool registry and default profile so they are available out-of-the-box.
  • Added CronManager as the Agent integration layer that bridges the scheduler, store, and tool calls.

3. Persistence across resume

Problem: Scheduled tasks were lost when the Kimi session resumed or restarted.

What was done:

  • Added per-id-json-store utility for lightweight per-session JSON persistence.
  • Implemented cron persist.ts to save and restore scheduled tasks on session open/close.
  • Added resume tests verifying tasks survive session teardown and continue firing after restart.

4. Review fixes & test coverage

Problem: Earlier iterations had type issues, missing edge-case tests, and review feedback from PR #136.

What was done:

  • Addressed deep review + Codex review comments (type safety, error messages, edge cases).
  • Removed the durable flag and env-based clock source in favor of a simpler session-only model.
  • Added comprehensive tests: e2e smoke, manual-tick, subagent skip, plan-mode hard block, inline snapshots.

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked a related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.
  • Ran gen-changesets skill, or this PR needs no changeset.
  • Ran gen-docs skill, or this PR needs no doc update.

@changeset-bot

changeset-bot Bot commented May 28, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 9944df3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@moonshot-ai/agent-core Minor
@moonshot-ai/kimi-code Minor
@moonshot-ai/migration-legacy Patch

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

@pkg-pr-new

pkg-pr-new Bot commented May 28, 2026

Copy link
Copy Markdown
pnpm dlx https://pkg.pr.new/@moonshot-ai/kimi-code@9944df3
npx https://pkg.pr.new/@moonshot-ai/kimi-code@9944df3

commit: 9944df3

@sailist
sailist force-pushed the feat/cron-tools-with-persistance branch from 45bdb32 to 37a6381 Compare May 28, 2026 10:27

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 31752466b4

ℹ️ 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".

Comment on lines +278 to +280
const seen = lastSeenAt.get(task.id);
const baseFromMs =
seen !== undefined && seen > task.createdAt ? seen : task.createdAt;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Persist the cron fire cursor across resumes

When a persisted recurring job is reloaded, lastSeenAt is empty, so the scheduler falls back to task.createdAt and recomputes the first occurrence after the job was originally created. For any recurring task that already fired before shutdown (for example, a daily 09:00 job created yesterday and resumed today after 09:00), the first post-resume tick sees that old occurrence as due and fires it again, duplicating completed scheduled work on every restart/resume. Persisting the last delivered/seen fire time, or otherwise advancing the baseline on load, is needed to avoid replaying old fires.

Useful? React with 👍 / 👎.

Comment on lines +227 to +233
const ideal = computeNextCronRun(parsed, nowMs);
const nextFireAt =
ideal === null
? null
: recurring
? jitteredNextCronRunMs(task, parsed, ideal)
: oneShotJitteredNextCronRunMs(task, ideal);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent one-shot dates from rolling into next year

For recurring: false reminders that follow the tool docs and pin day/month for “today” (for example 0 12 <today_dom> <today_month> *), computeNextCronRun(parsed, nowMs) returns the next annual match once execution is even seconds past the target minute. If manual approval or tool execution crosses 12:00, the task is accepted because next year is within the 5-year window, and the user’s one-shot reminder fires a year late instead of being rejected or treated as missed. One-shot pinned calendar dates need an additional guard before accepting the next cron match.

Useful? React with 👍 / 👎.

description: recurring
? `Scheduling cron ${normalizedCron}`
: `Scheduling one-shot ${normalizedCron}`,
approvalRule: this.name,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scope CronCreate approvals to each cron payload

When a user approves one CronCreate call with scope: session, the permission layer caches this approvalRule; because the rule is only the tool name, every later CronCreate with a different cron expression or prompt is auto-approved. In manual/ask modes this lets one harmless reminder approval authorize arbitrary future scheduled prompts for the rest of the session, unlike Bash/Write which include the requested payload in their approval rule.

Useful? React with 👍 / 👎.

@sailist
sailist force-pushed the feat/cron-tools-with-persistance branch from 37a6381 to e78bd3a Compare May 28, 2026 10:30
sailist added 22 commits May 28, 2026 18:31
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.
…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.
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
@sailist
sailist force-pushed the feat/cron-tools-with-persistance branch from e78bd3a to 9e61457 Compare May 28, 2026 10:31
@sailist
sailist force-pushed the feat/cron-tools-with-persistance branch from 9e61457 to 84d1061 Compare May 28, 2026 10:37

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 84d1061b4e

ℹ️ 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".


return {
description: `Deleting cron ${args.id}`,
approvalRule: this.name,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scope CronDelete approvals to the target id

When the permission mode asks for approval and the user approves a single CronDelete with session scope, the cached rule is just CronDelete, so SessionApprovalHistoryPermissionPolicy will auto-approve every later delete in the session regardless of which job id is being removed. For a mutating/destructive tool, one approval to delete job a should not authorize deleting unrelated scheduled jobs b and c; include the id in the approval rule (and matching logic) the way path/command tools scope their cached approvals.

Useful? React with 👍 / 👎.

// WHY: cron fires are not user turns (see isReplayUserTurnRecord); skip
// visual render and turn advance so the raw <cron-fire ...> envelope never
// surfaces in the resumed transcript.
if (message.origin?.kind === 'cron_job' || message.origin?.kind === 'cron_missed') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Advance or suppress the whole cron replay turn

When a resumed transcript contains a cron fire that produced assistant/tool output, this branch skips the cron user message without advancing the replay turn or suppressing the rest of that turn. The following assistant/tool records are still rendered, but with the previous (or undefined) currentTurnId, so scheduled-job output can be attached to the prior user prompt or replayed as an orphan. Either render a cron marker/advance the turn here, or skip the entire cron turn consistently.

Useful? React with 👍 / 👎.

async close(): Promise<void> {
try {
await this.stopBackgroundTasksOnExit();
await this.stopCronOnExit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop cron before waiting for background shutdown

When background.keepAliveOnExit is false, stopBackgroundTasksOnExit() can spend time waiting for background workers to stop while the cron scheduler is still running. Any cron job that becomes due during that shutdown window can call turn.steer() and start/append a new turn while close() is already in progress, before metadata and records are flushed. Stop the cron managers before awaiting background task termination so shutdown cannot enqueue new scheduled work.

Useful? React with 👍 / 👎.

@sailist

sailist commented May 28, 2026

Copy link
Copy Markdown
Collaborator Author

@codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 84d1061b4e

ℹ️ 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".

Comment on lines +279 to +280
const baseFromMs =
seen !== undefined && seen > task.createdAt ? seen : task.createdAt;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Persist the cron baseline before resume

When a recurring task has already fired in a previous CLI process, a resumed manager starts with an empty lastSeenAt and this fallback reuses task.createdAt. For example, an hourly job created at 12:30, fired at 13:00, then exited and resumed at 13:10 will compute the 13:00 occurrence as due again, injecting a duplicate cron fire and inflating coalescedCount from the original creation time on every resume. Persist/restore the last delivered/seen timestamp, or otherwise advance the baseline on load, so resume only coalesces work missed since the last actual delivery.

Useful? React with 👍 / 👎.

};
}

const task = this.manager.addTask({

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Anchor new tasks to the sampled execution time

When a schedule is created right on a minute boundary, CronCreate samples nowMs before this call but addTask() samples the wall clock again for createdAt. If those two reads straddle the target minute (for example scheduling a one-shot 0 9 * * * at 08:59:59.999 and inserting at 09:00:00.001), the response can report the 09:00 fire while the scheduler uses the later createdAt and skips to the next matching day/year. Pass the already-sampled execution timestamp into the store so validation, response, and scheduler baseline agree.

Useful? React with 👍 / 👎.

}

export interface CronJobOrigin {
readonly kind: 'cron_job';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Hide cron fire records from markdown export

Adding this internal origin means cron fire messages now enter ContextMessage history, but the markdown exporter only filters injection, system_trigger, compaction_summary, and hook_result in apps/kimi-code/src/tui/utils/export-markdown.ts. After any cron job fires, exporting the session will therefore create a user turn containing the raw <cron-fire ...> envelope that replay explicitly hides, leaking implementation XML into a user-facing export; add the cron origins to the exporter’s internal-message filter or render them in a sanitized form.

Useful? React with 👍 / 👎.

@sailist

sailist commented May 28, 2026

Copy link
Copy Markdown
Collaborator Author

@codex

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ab23528ce6

ℹ️ 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".

Comment on lines +273 to +277
const task = this.manager.addTask({
cron: normalizedCron,
prompt: args.prompt,
recurring,
});

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Recheck one-shot rollover at execution time

When CronCreate waits for manual approval, this adds the task using the execution-time clock but only ran the one-shot “rolled to next year” guard against the prepare-time clock. If the model schedules a one-shot for today at 10:00 and the user approves at 10:01, computeNextCronRun now rolls the pinned day/month to next year and the reminder is persisted a year late instead of being rejected. Recompute the first fire and the same max-future check inside execute before calling addTask.

Useful? React with 👍 / 👎.

Comment on lines +91 to +94
return {
description: `Deleting cron ${args.id}`,
approvalRule: this.name,
execute: async () => {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Scope CronDelete session approvals to the id

Using the bare tool name here means a user who approves deleting one cron job with session scope grants all later CronDelete calls in the session, because permission patterns without arguments match by tool name only. In manual mode that lets the model delete unrelated scheduled work without another prompt after a single approved deletion; make the approval rule id-specific (and provide a matching matchesRule) so the cached approval only applies to the job the user saw.

Useful? React with 👍 / 👎.

Comment on lines +244 to +251
approvalRule: literalRulePattern(
this.name,
JSON.stringify({
cron: normalizedCron,
prompt: args.prompt,
recurring,
}),
),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add a matcher for CronCreate session approvals

This records an argument-scoped approval rule, but the execution does not provide matchesRule, so the session-approval policy can never match the cached CronCreate(...) pattern for a later identical schedule. In manual permission mode, selecting “approve for session” on CronCreate therefore behaves like a one-time approval, contrary to the payload-scoping comments and the behavior of other literal-rule tools such as Bash/Read; add a matcher against the same normalized payload string.

Useful? React with 👍 / 👎.

Comment on lines +184 to +185
await this.stopBackgroundTasksOnExit();
await this.stopCronOnExit();

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop cron before awaiting other shutdown work

Because the cron scheduler is still running while stopBackgroundTasksOnExit() is awaited, a due cron can fire during session shutdown (especially with background.keepAliveOnExit=false, where stopping tasks may take time) and call turn.steer after close has begun. Stop cron before any awaited background cleanup so shutdown cannot start new agent turns while metadata/session-end cleanup is in progress.

Useful? React with 👍 / 👎.

Comment on lines +75 to +76
| CronJobOrigin
| CronMissedOrigin

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Hide cron-origin messages from markdown export

Adding cron_job/cron_missed as user-role origins also requires treating them as internal in the TUI markdown export path; apps/kimi-code/src/tui/utils/export-markdown.ts currently only filters injection/system/compaction/hook origins. After any scheduled job fires, exporting the session will include the raw <cron-fire ...> envelope as a user turn and may make it the overview topic, even though replay explicitly hides these same records.

Useful? React with 👍 / 👎.

Comment on lines +234 to +237
return {
description: recurring
? `Scheduling cron ${normalizedCron}`
: `Scheduling one-shot ${normalizedCron}`,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Show the scheduled prompt in CronCreate approvals

In manual permission mode this falls back to the generic approval display: the TUI ignores non-string detail for generic displays and renders only this description, so the user is asked to approve “Scheduling cron …” without seeing the prompt that will be injected later. For CronCreate, the prompt is the sensitive part of the scheduled work; include it in a display/description so approval is based on the full payload.

Useful? React with 👍 / 👎.

- 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

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sailist
sailist force-pushed the feat/cron-tools-with-persistance branch from ab23528 to 9944df3 Compare May 28, 2026 12:26

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9944df3637

ℹ️ 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 firstFire = computeNextCronRun(parsed, nowAtPrepare);
if (
firstFire !== null &&
firstFire - nowAtPrepare > ONE_SHOT_MAX_FUTURE_MS

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Allow valid year-end one-shot schedules

For one-shot jobs created early in the year, this fixed 350-day cutoff rejects legitimate same-year dates near year end. For example, on January 1 a recurring: false reminder for December 31 has computeNextCronRun return the next matching date in the current year, but it is ~364 days away so this branch reports it as a rollover error even though the schedule is valid and within the existing 5-year validation window. Detect the actual “already rolled into next year” case instead of capping all one-shots below a year.

Useful? React with 👍 / 👎.

Comment on lines +104 to +105
'cron_job',
'cron_missed',

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep cron responses out of the previous export turn

When a fired cron job has assistant/tool output in history, filtering only the cron-origin user message leaves the following assistant/tool records in the current export turn. For example, [user A, assistant A, cron_job user, assistant cron] exports assistant cron under Turn 1 with no scheduled-job marker, which misattributes autonomous scheduled work to the prior user prompt. Either render a sanitized cron turn boundary or suppress the whole cron-triggered turn consistently.

Useful? React with 👍 / 👎.

Comment on lines +349 to +350
onFire(task, { coalescedCount });
delivered = true;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Re-check idleness after delivering one cron fire

When two jobs are due in the same tick, this first onFire starts an agent turn, but the loop continues delivering later due tasks without consulting isIdle() again. In the real manager those later fires call turn.steer while the first cron turn is active, so they are buffered into that turn and their cursors/one-shot removals are still advanced instead of waiting for a separate idle tick. Break or re-check idleness after each delivery so simultaneous cron jobs are not injected mid-turn.

Useful? React with 👍 / 👎.

@sailist
sailist merged commit 971fce6 into MoonshotAI:main May 28, 2026
6 checks passed
@github-actions github-actions Bot mentioned this pull request May 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants