diff --git a/packages/coding-agent/src/core/extensions/types.ts b/packages/coding-agent/src/core/extensions/types.ts index 07d8d949..e41e4a0b 100644 --- a/packages/coding-agent/src/core/extensions/types.ts +++ b/packages/coding-agent/src/core/extensions/types.ts @@ -114,6 +114,15 @@ export interface ExtensionUIDialogOptions { * drift is visible because no output follows the dialog to fill the gap back in. */ overlay?: boolean; + /** + * Ask about one action and forget the answer. + * + * A host grants standing permissions by tool name ("allow for this session"). + * A dialog that manages a durable object — a scheduled task, say — must not + * join that grant: one approval would otherwise silently cover every later + * change of the same kind. + */ + once?: boolean; /** * Reference lines for an input dialog, rendered under the title and above the * editable row. Use them to show the accepted spellings of an answer that has diff --git a/packages/coding-agent/src/core/session-manager.ts b/packages/coding-agent/src/core/session-manager.ts index e20136a8..4b1cfb89 100644 --- a/packages/coding-agent/src/core/session-manager.ts +++ b/packages/coding-agent/src/core/session-manager.ts @@ -981,6 +981,25 @@ export class SessionManager { return this.persist; } + /** + * Persist a newly created session before its first turn. Host-created task + * sessions need a file that can be reopened after a restart. + */ + persistNewSessionHeader(): string | undefined { + if (!this.persist || !this.sessionFile || this.flushed) return this.sessionFile; + const fd = openSync(this.sessionFile, "wx"); + try { + for (const entry of this.fileEntries) { + writeFileSync(fd, `${JSON.stringify(entry)}\n`); + } + } finally { + closeSync(fd); + } + this.flushed = true; + this._notifyFilePersisted(this.sessionFile, "create"); + return this.sessionFile; + } + getCwd(): string { return this.cwd; } diff --git a/packages/coding-agent/src/features/step-cron.ts b/packages/coding-agent/src/features/step-cron.ts index 10c9239a..b549fedf 100644 --- a/packages/coding-agent/src/features/step-cron.ts +++ b/packages/coding-agent/src/features/step-cron.ts @@ -14,7 +14,7 @@ import { } from "node:fs"; import path from "node:path"; import type { AgentToolResult } from "@step-harness/agent-core"; -import { Type } from "typebox"; +import { type Static, Type } from "typebox"; import type { ExtensionAPI, ExtensionCommandContext, @@ -26,26 +26,125 @@ import type { StepTelemetryProperties, StepTelemetryReporter } from "../step/tel const TICK_INTERVAL_MS = 1_000; /** Cross-process pickup cadence; firing precision comes from the in-memory view. */ const DURABLE_REFRESH_INTERVAL_MS = 30_000; -const MAX_JOBS = 50; +const DEFAULT_MAX_JOBS = 50; const AUTO_EXPIRE_MS = 7 * 24 * 60 * 60 * 1000; const MAX_SEARCH_MINUTES = 366 * 24 * 60; -const MAX_PROMPT_LENGTH = 16_000; +/** + * Kernel ceilings, not a product policy. A host that shows a form may apply a + * stricter cap of its own; it must not narrow these, because the + * persisted-record reader shares them — a prompt written by an earlier build + * that exceeded a narrowed value would be treated as an unsupported record and + * silently stop being scheduled. + * + * The two behave differently on purpose, matching the original records: an + * oversized prompt is rejected, while an oversized title is truncated. A title + * is presentation; a prompt is what the task will do, so silently shortening it + * would change the work. + */ +export const CRON_MAX_PROMPT_LENGTH = 16_000; +export const CRON_MAX_TITLE_LENGTH = 200; +const MAX_PROMPT_LENGTH = CRON_MAX_PROMPT_LENGTH; const MAX_CRON_LENGTH = 200; const MAX_LOCK_AGE_MS = 60_000; type TimerHandle = ReturnType; +export type CronJobStatus = "enabled" | "paused" | "expired" | "completed"; +/** + * Why a task is in its current status when the status alone is ambiguous. + * + * `session_missing` is the one the host sets: the bound session file was + * deleted, so the task can never deliver and is paused rather than retried. + */ +export type CronStatusReason = "session_missing"; +export type CronRunStatus = "delivered" | "failed" | "missed" | "skipped" | "blocked"; + +/** + * A scheduled job. `schemaVersion: 1` is the original CLI record; v2 adds the + * optional desktop fields below. The CLI ignores fields it does not use. + */ export interface CronJob { - schemaVersion: 1; + schemaVersion: 1 | 2; id: string; cron: string; prompt: string; recurring: boolean; + /** Explicit interval schedule; when set, cron is only a validation/fallback expression. */ + intervalHours?: number; + /** One-shot absolute fire time. */ + runAt?: number; durable: boolean; createdAt: number; nextFireAt: number; lastFiredAt?: number; autoExpireAt?: number; + workspace?: string; + sessionId?: string; + /** + * Set when the host created the bound session for this task rather than + * binding a session the user already had. The sidebar uses it to place the + * session under scheduled tasks instead of guessing from its cwd. + */ + sessionCreated?: boolean; + title?: string; + /** Why the task is not running, when its status alone does not say it. */ + statusReason?: CronStatusReason; + status?: CronJobStatus; + enabled?: boolean; + inputs?: { skills?: string[] }; + model?: { provider: string; modelId?: string; thinkingLevel?: string }; + permissionPreset?: string; + fullAccessAcknowledged?: boolean; + delivery?: { lark?: { target: string; format?: "markdown" | "text" } }; + expiresAt?: number; + runCount?: number; + lastRunId?: string; + lastStatus?: CronRunStatus; + lastError?: string; + origin?: "agent" | "desktop" | "legacy"; + jitter?: boolean; +} + +export interface CronCreateOptions { + id?: string; + workspace?: string; + sessionId?: string; + sessionCreated?: boolean; + title?: string; + status?: CronJobStatus; + statusReason?: CronStatusReason; + enabled?: boolean; + inputs?: { skills?: string[] }; + model?: { provider: string; modelId?: string; thinkingLevel?: string }; + permissionPreset?: string; + fullAccessAcknowledged?: boolean; + intervalHours?: number; + runAt?: number; + delivery?: { lark?: { target: string; format?: "markdown" | "text" } }; + expiresAt?: number; + origin?: "agent" | "desktop" | "legacy"; + /** null keeps the record indefinitely; undefined keeps the legacy seven-day guard for recurring jobs. */ + autoExpireMs?: number | null; + jitter?: boolean; +} + +export interface CronUpdatePatch { + cron?: string; + prompt?: string; + title?: string; + inputs?: { skills?: string[] }; + model?: { provider: string; modelId?: string; thinkingLevel?: string }; + permissionPreset?: string | null; + fullAccessAcknowledged?: boolean | null; + delivery?: { lark?: { target: string; format?: "markdown" | "text" } }; + expiresAt?: number | null; + status?: CronJobStatus; + /** Host note for why a task was paused; null clears it. */ + statusReason?: CronStatusReason | null; + recurring?: boolean; + intervalHours?: number | null; + runAt?: number | null; + runCount?: number; } export interface CronCreateResult { @@ -291,7 +390,7 @@ function isStaleLock(lockPath: string, maxAgeMs: number): boolean { function parsePersistedJob(value: unknown): CronJob | undefined { if (!value || typeof value !== "object" || Array.isArray(value)) return undefined; const candidate = value as Record; - if (candidate.schemaVersion !== 1 || candidate.durable !== true) return undefined; + if ((candidate.schemaVersion !== 1 && candidate.schemaVersion !== 2) || candidate.durable !== true) return undefined; if ( typeof candidate.id !== "string" || !candidate.id.trim() || @@ -306,14 +405,75 @@ function parsePersistedJob(value: unknown): CronJob | undefined { return undefined; } if ( - ![ - candidate.createdAt, - candidate.nextFireAt, - candidate.lastFiredAt === undefined ? 0 : candidate.lastFiredAt, - candidate.autoExpireAt === undefined ? 0 : candidate.autoExpireAt, - ].every((value) => typeof value === "number" && Number.isFinite(value) && Math.abs(value) <= 8.64e15) + candidate.intervalHours !== undefined && + (typeof candidate.intervalHours !== "number" || + !Number.isInteger(candidate.intervalHours) || + candidate.intervalHours < 1 || + candidate.intervalHours > 168) ) return undefined; + const numbers = [ + candidate.createdAt, + candidate.nextFireAt, + candidate.lastFiredAt === undefined ? 0 : candidate.lastFiredAt, + candidate.autoExpireAt === undefined ? 0 : candidate.autoExpireAt, + candidate.expiresAt === undefined ? 0 : candidate.expiresAt, + ]; + if (!numbers.every((value) => typeof value === "number" && Number.isFinite(value) && Math.abs(value) <= 8.64e15)) + return undefined; + if (candidate.workspace !== undefined && (typeof candidate.workspace !== "string" || !candidate.workspace.trim())) + return undefined; + if (candidate.sessionId !== undefined && (typeof candidate.sessionId !== "string" || !candidate.sessionId.trim())) + return undefined; + if ( + candidate.title !== undefined && + (typeof candidate.title !== "string" || candidate.title.length > CRON_MAX_TITLE_LENGTH) + ) + return undefined; + if (candidate.sessionCreated !== undefined && typeof candidate.sessionCreated !== "boolean") return undefined; + if (candidate.statusReason !== undefined && candidate.statusReason !== "session_missing") return undefined; + if ( + candidate.status !== undefined && + !["enabled", "paused", "expired", "completed"].includes(candidate.status as string) + ) + return undefined; + if (candidate.inputs !== undefined) { + const inputs = candidate.inputs as Record; + if (!inputs || typeof inputs !== "object" || Array.isArray(inputs)) return undefined; + if (inputs.skills !== undefined) { + if (!Array.isArray(inputs.skills) || inputs.skills.some((skill) => typeof skill !== "string")) + return undefined; + } + } + if (candidate.model !== undefined) { + const model = candidate.model as Record; + if (!model || typeof model !== "object" || typeof model.provider !== "string" || !model.provider.trim()) + return undefined; + } + if (candidate.permissionPreset !== undefined && typeof candidate.permissionPreset !== "string") return undefined; + if (candidate.fullAccessAcknowledged !== undefined && typeof candidate.fullAccessAcknowledged !== "boolean") + return undefined; + if (candidate.delivery !== undefined) { + const delivery = candidate.delivery as Record; + if (!delivery || typeof delivery !== "object" || Array.isArray(delivery)) return undefined; + if (delivery.lark !== undefined) { + const lark = delivery.lark as Record; + if (!lark || typeof lark !== "object" || typeof lark.target !== "string" || !lark.target.trim()) + return undefined; + } + } + if (candidate.runCount !== undefined && (typeof candidate.runCount !== "number" || candidate.runCount < 0)) + return undefined; + if (candidate.lastRunId !== undefined && typeof candidate.lastRunId !== "string") return undefined; + if ( + candidate.lastStatus !== undefined && + !["delivered", "failed", "missed", "skipped", "blocked"].includes(candidate.lastStatus as string) + ) + return undefined; + if (candidate.lastError !== undefined && typeof candidate.lastError !== "string") return undefined; + if (candidate.origin !== undefined && !["agent", "desktop", "legacy"].includes(candidate.origin as string)) + return undefined; + if (candidate.jitter !== undefined && typeof candidate.jitter !== "boolean") return undefined; try { SimpleCronExpression.parse(candidate.cron); } catch { @@ -321,7 +481,7 @@ function parsePersistedJob(value: unknown): CronJob | undefined { } return { ...candidate, - schemaVersion: 1, + schemaVersion: candidate.schemaVersion, id: candidate.id, cron: candidate.cron, prompt: candidate.prompt, @@ -331,7 +491,7 @@ function parsePersistedJob(value: unknown): CronJob | undefined { nextFireAt: candidate.nextFireAt, ...(typeof candidate.lastFiredAt === "number" ? { lastFiredAt: candidate.lastFiredAt } : {}), ...(typeof candidate.autoExpireAt === "number" ? { autoExpireAt: candidate.autoExpireAt } : {}), - }; + } as CronJob; } export interface StepCronRuntimeOptions { @@ -346,15 +506,51 @@ export interface StepCronRuntimeOptions { storagePath?: string; storeFactory?: (filePath: string) => CronFileStore; isIdle?: () => boolean; + /** Per-job delivery gate; defaults to `isIdle`. */ + canDeliver?: (job: CronJob) => boolean; + /** Batch missed-notice gate; defaults to `isIdle`. */ + canDeliverMissed?: () => boolean; + maxJobs?: number; sendMessage?: (message: CronDelivery) => void; telemetry?: StepTelemetryReporter; warn?: (message: string) => void; + /** + * Which records this host delivers and may change. + * + * Durable records are shared by file path, so a project folder can hold a + * record another host owns. Unowned records are still read and written back + * unchanged; they are only never fired, listed or mutated here. + */ + ownsJob?: (job: CronJob) => boolean; +} + +/** The narrow surface a host facade needs to expose to the extension. */ +export interface CronRuntimeLike { + start(cwd: string, trusted: boolean): void; + setContext(cwd: string): void; + create( + cron: string, + prompt: string, + recurring?: boolean, + durable?: boolean, + trusted?: boolean, + options?: CronCreateOptions, + ): CronCreateResult; + list(): CronJob[]; + delete(id: string): boolean; + update?(id: string, patch: CronUpdatePatch): CronJob; + setEnabled?(id: string, enabled: boolean): boolean; + runNow?(id: string): CronJob; + onTurnEnd(): void; + stop(): void; } export interface CronDelivery { kind: "fire" | "missed"; job?: CronJob; jobs?: CronJob[]; + /** A host-initiated run (`runNow`) leaves the schedule untouched. */ + manual?: boolean; } export class StepCronRuntime { @@ -367,6 +563,10 @@ export class StepCronRuntime { private readonly storagePath?: string; private readonly storeFactory: (filePath: string) => CronFileStore; private readonly isIdle: () => boolean; + private readonly canDeliver?: (job: CronJob) => boolean; + private readonly canDeliverMissed?: () => boolean; + private readonly maxJobs: number; + private readonly ownsJob: (job: CronJob) => boolean; private readonly sendMessage: (message: CronDelivery) => void; private readonly telemetry?: StepTelemetryReporter; private readonly warn: (message: string) => void; @@ -392,6 +592,10 @@ export class StepCronRuntime { this.storagePath = options.storagePath; this.storeFactory = options.storeFactory ?? ((filePath) => new CronFileStore(filePath, { warn: options.warn })); this.isIdle = options.isIdle ?? (() => true); + this.canDeliver = options.canDeliver; + this.canDeliverMissed = options.canDeliverMissed; + this.maxJobs = options.maxJobs ?? DEFAULT_MAX_JOBS; + this.ownsJob = options.ownsJob ?? (() => true); this.sendMessage = options.sendMessage ?? (() => {}); this.telemetry = options.telemetry; this.warn = options.warn ?? ((message) => console.warn(`[step-cron] ${message}`)); @@ -418,11 +622,20 @@ export class StepCronRuntime { this.currentCwd = path.resolve(cwd); } - create(cron: string, prompt: string, recurring = true, durable = false, trusted = true): CronCreateResult { + create( + cron: string, + prompt: string, + recurring = true, + durable = false, + trusted = true, + options: CronCreateOptions = {}, + ): CronCreateResult { const normalizedCron = cron.trim(); const expression = SimpleCronExpression.parse(normalizedCron); const normalizedPrompt = prompt.trim(); if (!normalizedPrompt) throw new Error("cron_create requires a non-empty prompt"); + if (options.permissionPreset === "full-access" && options.fullAccessAcknowledged !== true) + throw new Error("Full access cron jobs require an explicit risk acknowledgment"); if (normalizedPrompt.length > MAX_PROMPT_LENGTH) throw new Error(`Cron prompt exceeds ${MAX_PROMPT_LENGTH} characters`); if (durable && !trusted) throw new Error("Durable cron jobs require a trusted project"); @@ -432,21 +645,77 @@ export class StepCronRuntime { this.currentStore = this.storeFactory(filePath); } const createdAt = this.readNow(); - const nominalNext = expression.next(createdAt); + if ( + options.intervalHours !== undefined && + (!Number.isInteger(options.intervalHours) || options.intervalHours < 1 || options.intervalHours > 168) + ) + throw new Error("Cron interval must be a whole number between 1 and 168 hours"); + if (options.intervalHours !== undefined && !recurring) + throw new Error("Cron interval jobs repeat; set recurring to true or omit intervalHours"); + if (options.runAt !== undefined) { + if (!Number.isFinite(options.runAt)) throw new Error("Cron runAt must be a finite timestamp"); + if (recurring) throw new Error("Only a one-shot cron job takes a runAt time"); + // A one-shot time in the past would fire on the next tick, which reads as + // "ran immediately" rather than "scheduled". + if (options.runAt <= createdAt) throw new Error("A one-shot cron job must be scheduled in the future"); + } + const nominalNext = + (!recurring ? options.runAt : undefined) ?? + (options.intervalHours ? createdAt + options.intervalHours * 60 * 60 * 1000 : expression.next(createdAt)); + const desktopFields = + options.workspace !== undefined || + options.sessionId !== undefined || + options.title !== undefined || + options.status !== undefined || + options.inputs !== undefined || + options.model !== undefined || + options.permissionPreset !== undefined || + options.fullAccessAcknowledged !== undefined || + options.delivery !== undefined || + options.expiresAt !== undefined || + options.origin !== undefined || + options.jitter === false || + options.intervalHours !== undefined || + options.runAt !== undefined; + const autoExpireAt = + options.autoExpireMs === null + ? undefined + : recurring + ? createdAt + (options.autoExpireMs ?? AUTO_EXPIRE_MS) + : undefined; const job = this.withLatestJobs(() => { - if (this.jobs.size >= MAX_JOBS) - throw new Error(`At most ${MAX_JOBS} cron jobs can be scheduled; delete an existing job first`); - const id = this.uniqueId(); + if (this.jobs.size >= this.maxJobs) + throw new Error(`At most ${this.maxJobs} cron jobs can be scheduled; delete an existing job first`); + const requestedId = options.id?.trim(); + if (requestedId !== undefined && !/^[A-Za-z0-9_-]{1,64}$/u.test(requestedId)) + throw new Error("Cron job id may only contain letters, digits, underscore, and hyphen"); + const id = requestedId || this.uniqueId(); + if (this.jobs.has(id)) throw new Error(`Cron job id ${id} already exists`); const next: CronJob = { - schemaVersion: 1, + schemaVersion: desktopFields ? 2 : 1, id, cron: normalizedCron, prompt: normalizedPrompt, recurring, + ...(options.intervalHours ? { intervalHours: options.intervalHours } : {}), + ...(!recurring && options.runAt !== undefined ? { runAt: options.runAt } : {}), durable, createdAt, - nextFireAt: this.jitter(nominalNext, createdAt, id, recurring, expression), - ...(recurring ? { autoExpireAt: createdAt + AUTO_EXPIRE_MS } : {}), + nextFireAt: this.jitter(nominalNext, createdAt, id, recurring, expression, options.jitter !== false), + ...(options.workspace ? { workspace: path.resolve(options.workspace) } : {}), + ...(options.sessionId ? { sessionId: options.sessionId } : {}), + ...(options.sessionCreated ? { sessionCreated: true } : {}), + ...(options.title ? { title: options.title.slice(0, CRON_MAX_TITLE_LENGTH) } : {}), + ...(options.status ? { status: options.status } : {}), + ...(options.inputs ? { inputs: { skills: [...(options.inputs.skills ?? [])] } } : {}), + ...(options.model ? { model: { ...options.model } } : {}), + ...(options.permissionPreset ? { permissionPreset: options.permissionPreset } : {}), + ...(options.fullAccessAcknowledged ? { fullAccessAcknowledged: true } : {}), + ...(options.delivery ? { delivery: structuredClone(options.delivery) } : {}), + ...(options.expiresAt !== undefined ? { expiresAt: options.expiresAt } : {}), + ...(options.origin ? { origin: options.origin } : {}), + ...(options.jitter === false ? { jitter: false } : {}), + ...(autoExpireAt !== undefined ? { autoExpireAt } : {}), }; this.jobs.set(id, next); return { ...next }; @@ -455,16 +724,166 @@ export class StepCronRuntime { return { job, nextFireAt: job.nextFireAt }; } + update(id: string, patch: CronUpdatePatch): CronJob { + const job = this.withLatestJobs(() => { + const current = this.readableJob(id, "update"); + const next: CronJob = { ...current }; + if (patch.cron !== undefined) { + next.cron = patch.cron.trim(); + SimpleCronExpression.parse(next.cron); + } + if (patch.prompt !== undefined) { + const prompt = patch.prompt.trim(); + if (!prompt) throw new Error("cron_update requires a non-empty prompt"); + if (prompt.length > MAX_PROMPT_LENGTH) + throw new Error(`Cron prompt exceeds ${MAX_PROMPT_LENGTH} characters`); + next.prompt = prompt; + } + if (patch.title !== undefined) next.title = patch.title.slice(0, CRON_MAX_TITLE_LENGTH); + if (patch.inputs !== undefined) next.inputs = { skills: [...(patch.inputs.skills ?? [])] }; + if (patch.model !== undefined) next.model = { ...patch.model }; + if (patch.permissionPreset !== undefined) { + if (patch.permissionPreset === null) delete next.permissionPreset; + else next.permissionPreset = patch.permissionPreset; + } + if (patch.fullAccessAcknowledged !== undefined) { + if (patch.fullAccessAcknowledged === null) delete next.fullAccessAcknowledged; + else next.fullAccessAcknowledged = patch.fullAccessAcknowledged || undefined; + } + if (patch.delivery !== undefined) next.delivery = structuredClone(patch.delivery); + if ("expiresAt" in patch) { + if (patch.expiresAt === null) delete next.expiresAt; + else if (patch.expiresAt !== undefined) next.expiresAt = patch.expiresAt; + } + if (patch.status !== undefined) { + if (!["enabled", "paused", "expired", "completed"].includes(patch.status)) + throw new Error(`Unknown cron job status: ${String(patch.status)}`); + next.status = patch.status; + next.enabled = patch.status === "enabled"; + // A status the user set themselves replaces whatever the host recorded. + if (patch.status !== "paused") delete next.statusReason; + } + if (patch.statusReason !== undefined) { + if (patch.statusReason === null) delete next.statusReason; + else next.statusReason = patch.statusReason; + } + if (patch.recurring !== undefined) next.recurring = patch.recurring; + if (patch.intervalHours !== undefined) { + if (patch.intervalHours === null) delete next.intervalHours; + else next.intervalHours = patch.intervalHours; + } + if (patch.runAt !== undefined) { + if (patch.runAt === null) delete next.runAt; + else next.runAt = patch.runAt; + } + if (next.recurring && patch.runAt !== undefined && patch.runAt !== null) + throw new Error("A repeating cron job takes a schedule, not a one-shot runAt"); + // One schedule shape per task. `runAt` is a one-shot time and + // `intervalHours` is a repeating interval, so converting between the two + // kinds has to drop the field that no longer applies: a stale `runAt` + // otherwise wins the `nextFireAt` calculation below, and a converted + // `once -> daily` task fires at its old absolute time or immediately. + if (next.recurring) delete next.runAt; + else delete next.intervalHours; + if (patch.runCount !== undefined) next.runCount = Math.max(0, Math.floor(patch.runCount)); + if ( + (patch.permissionPreset !== undefined || patch.fullAccessAcknowledged !== undefined) && + next.permissionPreset === "full-access" && + next.fullAccessAcknowledged !== true + ) + throw new Error("Full access cron jobs require an explicit risk acknowledgment"); + if (next.schemaVersion === 1) next.schemaVersion = 2; + const now = this.readNow(); + const expression = SimpleCronExpression.parse(next.cron); + if (next.intervalHours !== undefined) { + if (!Number.isInteger(next.intervalHours) || next.intervalHours < 1 || next.intervalHours > 168) + throw new Error("Cron interval must be a whole number between 1 and 168 hours"); + } + if (patch.runAt !== undefined && patch.runAt !== null) { + if (!Number.isFinite(patch.runAt)) throw new Error("Cron runAt must be a finite timestamp"); + if (patch.runAt <= now) throw new Error("A one-shot cron job must be scheduled in the future"); + } + if (patch.expiresAt !== undefined && patch.expiresAt !== null && !Number.isFinite(patch.expiresAt)) + throw new Error("Cron expiry must be a finite timestamp or null"); + + // Reopening: a task that used up its schedule comes back to life only when + // the edit gives it a future schedule again. An explicit status in the same + // patch wins over this, so "pause" stays pause. + const extendedExpiry = + next.expiresAt !== undefined && patch.expiresAt !== undefined && patch.expiresAt !== null; + const scheduledOneShot = + !next.recurring && patch.runAt !== undefined && patch.runAt !== null && patch.runAt > now; + if (patch.status === undefined && (next.status === "expired" || next.status === "completed")) { + const reopened = + (next.status === "expired" && extendedExpiry && next.expiresAt !== undefined && next.expiresAt > now) || + (next.status === "completed" && scheduledOneShot); + if (reopened) { + next.status = "enabled"; + next.enabled = true; + delete next.statusReason; + } + } + + // A one-shot task fires at its `runAt`. Re-enabling one whose time has + // already passed would deliver on the next tick, which reads as an + // immediate run; the user has to pick a new time instead. + if (!next.recurring && patch.status === "enabled" && patch.runAt === undefined) { + if (next.runAt === undefined || next.runAt <= now) + throw new Error("A one-shot task needs a future time before it can run again"); + } + + if ( + patch.cron !== undefined || + patch.recurring !== undefined || + patch.intervalHours !== undefined || + patch.runAt !== undefined || + patch.expiresAt !== undefined || + patch.status === "enabled" + ) { + if (!next.recurring) + next.nextFireAt = + next.runAt ?? this.jitter(expression.next(now), now, id, false, expression, next.jitter !== false); + else + next.nextFireAt = + next.intervalHours !== undefined + ? now + next.intervalHours * 60 * 60 * 1000 + : this.jitter(expression.next(now), now, id, true, expression, next.jitter !== false); + } + this.jobs.set(id, next); + return { ...next }; + }); + return job; + } + + setEnabled(id: string, enabled: boolean): boolean { + try { + this.update(id, { status: enabled ? "enabled" : "paused" }); + return true; + } catch { + return false; + } + } + + runNow(id: string): CronJob { + const job = this.withLatestJobs(() => { + const current = this.readableJob(id, "run"); + this.fire(current, this.readNow(), true); + return { ...current }; + }); + return job; + } + list(): CronJob[] { this.refreshDurableNow(); return [...this.jobs.values()] + .filter((job) => this.ownsJob(job)) .sort((left, right) => left.nextFireAt - right.nextFireAt || left.id.localeCompare(right.id)) .map((job) => ({ ...job })); } delete(id: string): boolean { const found = this.withLatestJobs(() => { - if (!this.jobs.has(id)) return false; + if (!this.jobs.has(id) || !this.ownsJob(this.jobs.get(id)!)) return false; this.removeJob(id); return true; }); @@ -517,21 +936,27 @@ export class StepCronRuntime { this.refreshDurable(); const now = this.readNow(); if ( - [...this.jobs.values()].some( - (job) => - job.durable && (job.nextFireAt <= now || (job.autoExpireAt !== undefined && job.autoExpireAt <= now)), - ) + [...this.jobs.values()].some((job) => { + if (!job.durable) return false; + const expires = this.expirationOf(job); + return job.nextFireAt <= now || (expires !== undefined && expires <= now); + }) ) { this.withLatestJobs(() => { // Import the complete file before any delivery can change persisted state. for (const job of [...this.jobs.values()]) { if (!job.durable) continue; - if (job.autoExpireAt !== undefined && job.autoExpireAt <= now) { + if (job.status !== undefined && job.status !== "enabled") continue; + const expires = this.expirationOf(job); + if (expires !== undefined && expires <= now) { if (job.nextFireAt <= now) { - if (this.isIdle()) this.fire(job, now); + if (this.deliveryAllowed(job)) this.fire(job, now); else this.defer(job); } else { - this.removeJob(job.id); + if (job.status !== undefined) { + job.status = "expired"; + job.nextFireAt = now; + } else this.removeJob(job.id); this.emit("cron_expired", { id: job.id, recurring: job.recurring }); } continue; @@ -542,7 +967,10 @@ export class StepCronRuntime { } else { try { const expression = SimpleCronExpression.parse(job.cron); - job.nextFireAt = this.jitter(expression.next(now), now, job.id, true, expression); + job.nextFireAt = + job.intervalHours !== undefined + ? now + job.intervalHours * 60 * 60 * 1000 + : this.jitter(expression.next(now), now, job.id, true, expression, job.jitter !== false); } catch (error) { // Keep the record, but do not let one bad schedule block project recovery. this.reportFailure( @@ -567,13 +995,18 @@ export class StepCronRuntime { private drain(now: number): void { this.deliverMissed(); for (const job of [...this.jobs.values()]) { + if (!this.ownsJob(job)) continue; + if (job.status !== undefined && job.status !== "enabled") continue; if (this.isExpiredBeforeDelivery(job, now)) { - this.removeJob(job.id); + if (job.status !== undefined) { + job.status = "expired"; + job.nextFireAt = now; + } else this.removeJob(job.id); this.emit("cron_expired", { id: job.id, recurring: job.recurring }); continue; } if (!this.isDue(job, now)) continue; - if (!this.isIdle()) { + if (!this.deliveryAllowed(job)) { this.defer(job); continue; } @@ -586,9 +1019,23 @@ export class StepCronRuntime { return job.nextFireAt <= now && !this.pendingMissed.has(job.id); } + private expirationOf(job: CronJob): number | undefined { + return job.expiresAt ?? job.autoExpireAt; + } + /** Reached its expiry before the next occurrence, so it leaves without a final delivery. */ private isExpiredBeforeDelivery(job: CronJob, now: number): boolean { - return job.autoExpireAt !== undefined && now >= job.autoExpireAt && job.nextFireAt > now; + const expires = this.expirationOf(job); + return expires !== undefined && now >= expires && job.nextFireAt > now; + } + + private deliveryAllowed(job: CronJob): boolean { + if (job.status !== undefined && job.status !== "enabled") return false; + return this.canDeliver ? this.canDeliver(job) : this.isIdle(); + } + + private missedAllowed(): boolean { + return this.canDeliverMissed ? this.canDeliverMissed() : this.isIdle(); } /** @@ -597,25 +1044,30 @@ export class StepCronRuntime { * every one-second tick just to re-mark the same jobs deferred. */ private needsLockedDrain(now: number): boolean { - const idle = this.isIdle(); - if (idle && this.pendingMissed.size > 0) return true; + if (this.missedAllowed() && this.pendingMissed.size > 0) return true; for (const job of this.jobs.values()) { + if (!this.ownsJob(job)) continue; + if (job.status !== undefined && job.status !== "enabled") continue; if (this.isExpiredBeforeDelivery(job, now)) return true; - if (idle && this.isDue(job, now)) return true; + if (this.deliveryAllowed(job) && this.isDue(job, now)) return true; } return false; } /** `drain`'s busy-host branch, which is in-memory bookkeeping only. */ private deferDue(now: number): void { - for (const job of this.jobs.values()) if (this.isDue(job, now)) this.defer(job); + for (const job of this.jobs.values()) { + if (!this.ownsJob(job)) continue; + if (job.status !== undefined && job.status !== "enabled") continue; + if (this.isDue(job, now)) this.defer(job); + } } private deliverMissed(): void { - if (this.pendingMissed.size === 0 || !this.isIdle()) return; + if (this.pendingMissed.size === 0 || !this.missedAllowed()) return; const jobs = [...this.pendingMissed].flatMap((id) => { const job = this.jobs.get(id); - return job ? [{ ...job }] : []; + return job && this.ownsJob(job) ? [{ ...job }] : []; }); if (jobs.length === 0) return; try { @@ -627,35 +1079,68 @@ export class StepCronRuntime { ); return; } - for (const job of jobs) this.removeJob(job.id); + for (const job of jobs) { + if (job.status !== undefined) { + const current = this.jobs.get(job.id); + if (current) { + current.status = "completed"; + current.nextFireAt = this.readNow(); + current.lastStatus = "missed"; + } + } else this.removeJob(job.id); + } this.failures.delete("missed"); } - private fire(job: CronJob, now: number): void { - const expired = job.autoExpireAt !== undefined && now >= job.autoExpireAt; + private fire(job: CronJob, now: number, manual = false): void { + const expires = this.expirationOf(job); + const expired = expires !== undefined && now >= expires; let nextFireAt: number | undefined; try { - if (job.recurring && !expired) { + if (job.recurring && !expired && !manual) { const expression = SimpleCronExpression.parse(job.cron); - nextFireAt = this.jitter(expression.next(now), now, job.id, true, expression); + nextFireAt = + job.intervalHours !== undefined + ? now + job.intervalHours * 60 * 60 * 1000 + : this.jitter(expression.next(now), now, job.id, true, expression, job.jitter !== false); } - this.sendMessage({ kind: "fire", job: { ...job } }); + this.sendMessage({ kind: "fire", job: { ...job }, ...(manual ? { manual: true } : {}) }); } catch (error) { - this.reportFailure( - `delivery:${job.id}`, - `Unable to deliver cron ${job.id}: ${error instanceof Error ? error.message : String(error)}`, - ); + const message = error instanceof Error ? error.message : String(error); + if (job.schemaVersion === 2 || job.status !== undefined) { + job.lastStatus = "failed"; + job.lastError = message; + } + this.reportFailure(`delivery:${job.id}`, `Unable to deliver cron ${job.id}: ${message}`); return; } this.failures.delete(`delivery:${job.id}`); this.deferred.delete(job.id); - if (nextFireAt === undefined) this.removeJob(job.id); - else { - job.lastFiredAt = now; + job.lastFiredAt = now; + if (job.schemaVersion === 2 || job.status !== undefined) { + job.runCount = (job.runCount ?? 0) + 1; + job.lastStatus = "delivered"; + delete job.lastError; + } + if (manual) { + this.emit("cron_fired", { recurring: job.recurring }); + return; + } + if (job.recurring && expired) { + if (job.status !== undefined) { + job.status = "expired"; + job.nextFireAt = now; + } else this.removeJob(job.id); + this.emit("cron_expired", { id: job.id, recurring: true }); + } else if (nextFireAt === undefined) { + if (job.status !== undefined) { + job.status = "completed"; + job.nextFireAt = now; + } else this.removeJob(job.id); + } else { job.nextFireAt = nextFireAt; } this.emit("cron_fired", { recurring: job.recurring }); - if (job.recurring && expired) this.emit("cron_expired", { id: job.id, recurring: true }); } private defer(job: CronJob): void { @@ -671,6 +1156,14 @@ export class StepCronRuntime { this.failures.delete(`delivery:${id}`); } + /** A job this host may change, or an error saying whose it is. */ + private readableJob(id: string, action: "run" | "update"): CronJob { + const job = this.jobs.get(id); + if (!job) throw new Error(`No cron job with id ${id}`); + if (!this.ownsJob(job)) throw new Error(`Cron job ${id} belongs to another host and cannot be ${action}`); + return job; + } + /** * Adopt an existing project store. Creating one belongs to `create`, so a * session that never schedules durable work performs no storage I/O. @@ -764,8 +1257,9 @@ export class StepCronRuntime { id: string, recurring: boolean, expression: SimpleCronExpression, + enabled = true, ): number { - if (this.jitterRatio === 0) return timestamp; + if (!enabled || this.jitterRatio === 0) return timestamp; const sample = this.random?.() ?? createHash("sha256").update(id).digest().readUInt32BE(0) / 0xffff_ffff; const fraction = Number.isFinite(sample) ? Math.max(0, Math.min(1, sample)) : 0; if (recurring) { @@ -813,10 +1307,32 @@ export const CronCreateParams = Type.Object({ export const CronDeleteParams = Type.Object({ id: Type.String({ description: "Cron job id" }) }); +export const CronUpdateParams = Type.Object({ + id: Type.String({ description: "Cron job id" }), + cron: Type.Optional(Type.String({ description: "Replacement five-field local-time cron expression" })), + prompt: Type.Optional(Type.String({ description: "Replacement prompt" })), + title: Type.Optional(Type.String({ description: "Task title" })), + skills: Type.Optional(Type.Array(Type.String(), { description: "Skills loaded when the task runs" })), + enabled: Type.Optional(Type.Boolean({ description: "Enable or pause the task" })), + expiresAt: Type.Optional(Type.Union([Type.Number(), Type.Null()], { description: "Expiry epoch ms, or null" })), +}); + export interface StepCronExtensionOptions { telemetry?: StepTelemetryReporter; enabled?: boolean; - runtime?: StepCronRuntime; + readOnly?: boolean; + /** + * Ask the user before an agent creates, updates or deletes a task. + * + * This is a host capability, not a product default: the desktop host has a + * page that can show the dialog and wants the gate, while a CLI session (and + * any print / RPC host with no dialog surface) would otherwise lose the + * ability to schedule work altogether. + */ + requireApproval?: boolean; + /** Register `cron_update`; off for the CLI so its tool surface stays unchanged. */ + allowUpdate?: boolean; + runtime?: CronRuntimeLike; } function envFlagEnabled(value: string | undefined): boolean { @@ -837,25 +1353,104 @@ function formatCronStatus(jobs: readonly CronJob[]): string { .join("\n"); } +/** + * Describe a pending `cron_update` by what the task is now, not by its id. + * + * The approval dialog is the only place a person sees what they are agreeing + * to, and an id alone says nothing about which scheduled work changes. + */ +function describeUpdateTarget(runtime: CronRuntimeLike, params: Static): string { + const job = runtime.list().find((candidate) => candidate.id === params.id); + const current = job ? `${job.title ?? job.prompt.slice(0, 60)} (${job.cron})` : `unknown task ${params.id}`; + const changes = [ + params.cron === undefined ? undefined : `schedule -> ${params.cron}`, + params.prompt === undefined ? undefined : "prompt", + params.title === undefined ? undefined : `title -> ${params.title}`, + params.skills === undefined ? undefined : "skills", + params.enabled === undefined ? undefined : params.enabled ? "resume" : "pause", + params.expiresAt === undefined ? undefined : "expiry", + ].filter((value): value is string => value !== undefined); + return `${current}\n${changes.length > 0 ? `changes: ${changes.join(", ")}` : "no changes"}`; +} + +/** + * Read from a captured extension context, tolerating a stale one. + * + * A ctx goes stale the moment its session is replaced — fork, newSession, + * switchSession or reload — and touching a stale ctx throws. Timer callbacks can + * land in that window, and an uncaught throw there takes down the whole host + * rather than just skipping one notification, so treat it as "nothing to read". + * + * The plan-mode gate is read through the same helper: it asks the session + * whether a human decision is pending, and a stale ctx cannot answer. + */ +function withLiveContext(context: ExtensionContext | undefined, read: (ctx: ExtensionContext) => T): T | undefined { + if (!context) return undefined; + try { + return read(context); + } catch { + return undefined; + } +} + +async function requireCronApproval( + ctx: ExtensionContext, + action: "create" | "update" | "delete", + detail: string, +): Promise { + // `once` marks the dialog as a decision about one scheduled task, so a + // standing "allow for this session" cannot silently cover every later + // scheduled-task change in the conversation. + const approved = await ctx.ui.confirm( + `Approve scheduled task ${action}?`, + `${detail}\n\nAgent-created scheduled-task changes require your approval.`, + { once: true }, + ); + if (!approved) throw new Error(`Cron ${action} was not approved`); +} + /** Register expression-based Cron tools; this extension has no Loop dependency. */ export function createStepCronExtension(options: StepCronExtensionOptions = {}): ExtensionFactory { return (pi: ExtensionAPI): void => { if (options.enabled === false || envFlagEnabled(process.env.STEP_DISABLE_CRON)) return; + /** + * The approval gate is a host capability. A host that asks has a dialog + * surface; a host that does not must not lose the ability to schedule, and a + * host whose context cannot show a dialog at all is refused rather than + * silently allowed. + */ + const approve = (ctx: ExtensionContext, action: "create" | "update" | "delete", detail: string) => { + if (!options.requireApproval) return; + if (!ctx.hasUI) throw new Error(`Cron ${action} requires user approval, but this host has no dialog surface`); + return requireCronApproval(ctx, action, detail); + }; let currentContext: ExtensionContext | undefined; const runtime = options.runtime ?? new StepCronRuntime({ telemetry: options.telemetry, + // A durable record bound to a session belongs to the host that owns + // that session. A CLI session loading the same project folder must not + // fire, list, or delete it: it has no way to see that session, and + // firing it would deliver a desktop task's prompt into this terminal + // conversation instead. + ownsJob: (job) => job.sessionId === undefined, isIdle: () => Boolean( - currentContext?.isIdle() && - !currentContext.hasPendingMessages() && - // A job whose delivery would start a turn stays due instead of - // being fired into a queue, so an approval does not replay work the - // user may reject. - currentContext.hasPendingUserDecision?.() !== true, + withLiveContext( + currentContext, + (ctx) => + ctx.isIdle() && + !ctx.hasPendingMessages() && + // A job whose delivery would start a turn stays due instead of + // being fired into a queue, so an approval does not replay work + // the user may reject. + ctx.hasPendingUserDecision?.() !== true, + ), ), - warn: (message) => currentContext?.ui.notify(message, "warning"), + warn: (message) => { + withLiveContext(currentContext, (ctx) => ctx.ui.notify(message, "warning")); + }, sendMessage: (delivery) => { if (delivery.kind === "missed") { const jobs = delivery.jobs ?? []; @@ -922,6 +1517,8 @@ Scheduling: recurring jobs get a stable task-ID offset of up to 30 minutes late, promptSnippet: "Schedule recurring or one-shot calendar work", parameters: CronCreateParams, execute: async (_id, params, _signal, _onUpdate, ctx) => { + if (options.readOnly) throw new Error("This task session cannot create cron jobs"); + await approve(ctx, "create", `${params.cron} - ${params.prompt.slice(0, 120)}`); currentContext = ctx; runtime.setContext(ctx.cwd); const recurring = params.recurring !== false; @@ -949,13 +1546,39 @@ Scheduling: recurring jobs get a stable task-ID offset of up to 30 minutes late, promptSnippet: "Delete a scheduled cron job", parameters: CronDeleteParams, execute: async (_id, params, _signal, _onUpdate, ctx) => { + if (options.readOnly) throw new Error("This task session cannot delete cron jobs"); + await approve(ctx, "delete", params.id); currentContext = ctx; runtime.setContext(ctx.cwd); const found = runtime.delete(params.id); return jsonResult({ id: params.id, found }); }, }); - + if (options.allowUpdate) { + pi.registerTool({ + name: "cron_update", + label: "Update cron job", + description: "Update a scheduled cron job: title, expression, prompt, skills, enabled/paused, or expiry.", + promptSnippet: "Update a scheduled cron job", + parameters: CronUpdateParams, + execute: async (_id, params, _signal, _onUpdate, ctx) => { + if (options.readOnly) throw new Error("This task session cannot update cron jobs"); + await approve(ctx, "update", describeUpdateTarget(runtime, params)); + if (!runtime.update) throw new Error("This host does not support updating cron jobs"); + currentContext = ctx; + runtime.setContext(ctx.cwd); + const patch: CronUpdatePatch = {}; + if (params.cron !== undefined) patch.cron = params.cron; + if (params.prompt !== undefined) patch.prompt = params.prompt; + if (params.title !== undefined) patch.title = params.title; + if (params.skills !== undefined) patch.inputs = { skills: params.skills }; + if (params.enabled !== undefined) patch.status = params.enabled ? "enabled" : "paused"; + if (params.expiresAt !== undefined) patch.expiresAt = params.expiresAt; + const job = runtime.update(params.id, patch); + return jsonResult(job); + }, + }); + } pi.registerCommand("cron", { description: "Inspect or delete cron jobs", handler: async (args: string, ctx: ExtensionCommandContext) => { @@ -964,6 +1587,10 @@ Scheduling: recurring jobs get a stable task-ID offset of up to 30 minutes late, const tokens = args.trim().split(/\s+/u).filter(Boolean); try { if (tokens[0] === "delete" || tokens[0] === "remove") { + if (options.readOnly) { + ctx.ui.notify("This task session cannot delete cron jobs", "warning"); + return; + } const id = tokens[1]; if (!id || tokens.length !== 2) { ctx.ui.notify("Usage: /cron delete ", "warning"); diff --git a/packages/coding-agent/src/features/step.ts b/packages/coding-agent/src/features/step.ts index c947bb0d..c6c75a4b 100644 --- a/packages/coding-agent/src/features/step.ts +++ b/packages/coding-agent/src/features/step.ts @@ -174,11 +174,13 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi * grant options ("always" persists the acknowledgment) and false when the * dialog is dismissed, so callers keep the previous tier. */ - const confirmFullAccessRisk = async (ui: ExtensionUIContext): Promise => { + const confirmFullAccessRisk = async (ui: ExtensionUIContext, allowAlways = true): Promise => { const generation = permissionSessionGeneration; if (typeof ui.select !== "function") return false; const selected = await ui.select(`${FULL_ACCESS_RISK_TITLE}\n\n${FULL_ACCESS_RISK_MESSAGE}`, [ - ...FULL_ACCESS_RISK_OPTIONS, + FULL_ACCESS_RISK_OPTIONS[0], + ...(allowAlways ? [FULL_ACCESS_RISK_OPTIONS[1]] : []), + FULL_ACCESS_RISK_OPTIONS[2], ]); if (generation !== permissionSessionGeneration) return false; const choice = parseFullAccessRiskChoice(selected); @@ -459,6 +461,7 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi const handlePermissionCommand = async (args: string, ctx: ExtensionCommandContext): Promise => { const generation = permissionSessionGeneration; const requested = args.trim().toLowerCase(); + const sessionOnly = /(^|\s)--session(?:\s|$)/u.test(requested); if (requested === "--cycle" || requested === "cycle") { // Full Access is gated by the risk acknowledgment; a canceled dialog // keeps the current tier instead of moving. @@ -483,14 +486,14 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi return; } - let selected = requested; + let selected = requested.replace(/(^|\s)--session(?:\s|$)/gu, " ").trim(); if (!selected) { const options = permissionsPresetsForSelector(); selected = (await ctx.ui.select("Permission mode", options))?.trim().toLowerCase() ?? ""; if (!selected || generation !== permissionSessionGeneration) return; } if (normalizeStepPermissionPresetId(selected) === "full-access" && !isFullAccessAcknowledged()) { - const granted = await confirmFullAccessRisk(ctx.ui); + const granted = await confirmFullAccessRisk(ctx.ui, !sessionOnly); if (generation !== permissionSessionGeneration) return; if (!granted) { ctx.ui.notify("Full access was not enabled.", "warning"); @@ -506,8 +509,9 @@ export function createStepExtension(options: StepExtensionOptions = {}): Extensi applyPermissionState(ctx, state, { source: "command", persist: - state.preset !== "full-access" || - options.stepSettings?.()?.getStepSettings().fullAccessAcknowledged === true, + !sessionOnly && + (state.preset !== "full-access" || + options.stepSettings?.()?.getStepSettings().fullAccessAcknowledged === true), }); }; diff --git a/packages/coding-agent/src/index.ts b/packages/coding-agent/src/index.ts index a58ac742..89bda053 100644 --- a/packages/coding-agent/src/index.ts +++ b/packages/coding-agent/src/index.ts @@ -508,11 +508,19 @@ export { } from "./features/step.ts"; export { createStepCapabilitiesExtensionInline } from "./features/step-capabilities.ts"; export { + CRON_MAX_PROMPT_LENGTH, + CRON_MAX_TITLE_LENGTH, + type CronCreateOptions, type CronCreateResult, type CronDelivery, CronFileStore, type CronFileStoreOptions, type CronJob, + type CronJobStatus, + type CronRunStatus, + type CronRuntimeLike, + type CronStatusReason, + type CronUpdatePatch, createStepCronExtension, SimpleCronExpression, type StepCronExtensionOptions, diff --git a/packages/coding-agent/test/step-cron.test.ts b/packages/coding-agent/test/step-cron.test.ts index b46fad98..bc6bd81c 100644 --- a/packages/coding-agent/test/step-cron.test.ts +++ b/packages/coding-agent/test/step-cron.test.ts @@ -4,6 +4,8 @@ import path from "node:path"; import { afterEach, describe, expect, test, vi } from "vitest"; import type { ExtensionAPI, ExtensionContext, ToolDefinition } from "../src/core/extensions/types.ts"; import { + CRON_MAX_PROMPT_LENGTH, + CRON_MAX_TITLE_LENGTH, type CronDelivery, CronFileStore, type CronJob, @@ -175,13 +177,13 @@ function createHarness(): Harness { const sent: Array<{ message: Record; options?: Record }> = []; const ctx = { mode: "rpc", - hasUI: false, + hasUI: true, cwd: makeDirectory(), isIdle: () => true, hasPendingMessages: () => false, hasPendingUserDecision: () => false, isProjectTrusted: () => true, - ui: { notify: vi.fn() }, + ui: { notify: vi.fn(), confirm: vi.fn().mockResolvedValue(true) }, sessionManager: { getSessionId: () => "cron-session", getEntries: () => [] }, } as unknown as ExtensionContext; const api = { @@ -208,11 +210,11 @@ function createHarness(): Harness { return { api, tools, handlers, sent, ctx, commands }; } -test("cron extension exposes three tools and steer delivery", async () => { +test("cron extension exposes create/list/update/delete and steer delivery", async () => { vi.useFakeTimers(); const harness = createHarness(); - createStepCronExtension({ enabled: true })(harness.api); - expect([...harness.tools.keys()].sort()).toEqual(["cron_create", "cron_delete", "cron_list"]); + createStepCronExtension({ enabled: true, allowUpdate: true })(harness.api); + expect([...harness.tools.keys()].sort()).toEqual(["cron_create", "cron_delete", "cron_list", "cron_update"]); for (const handler of harness.handlers.get("session_start") ?? []) await handler({ type: "session_start" } as never, harness.ctx); const create = harness.tools.get("cron_create")!; @@ -233,6 +235,81 @@ test("cron extension exposes three tools and steer delivery", async () => { await handler({ type: "session_shutdown" } as never, harness.ctx); }); +test("a task session cannot create, update, or delete cron jobs", async () => { + const harness = createHarness(); + createStepCronExtension({ enabled: true, readOnly: true, allowUpdate: true })(harness.api); + await emitCron(harness, "session_start"); + const create = harness.tools.get("cron_create")!; + await expect( + create.execute("create", { cron: "* * * * *", prompt: "nested" } as never, undefined, undefined, harness.ctx), + ).rejects.toThrow("cannot create"); + await expect( + harness.tools + .get("cron_update")! + .execute("update", { id: "x", title: "y" } as never, undefined, undefined, harness.ctx), + ).rejects.toThrow("cannot update"); + await expect( + harness.tools.get("cron_delete")!.execute("delete", { id: "x" } as never, undefined, undefined, harness.ctx), + ).rejects.toThrow("cannot delete"); + const listed = (await harness.tools + .get("cron_list")! + .execute("list", {} as never, undefined, undefined, harness.ctx)) as { content: Array<{ text: string }> }; + expect(JSON.parse(listed.content[0]!.text)).toEqual([]); + await emitCron(harness, "session_shutdown"); +}); + +test("the CLI cron extension keeps its original tool surface", () => { + const harness = createHarness(); + createStepCronExtension({ enabled: true })(harness.api); + expect([...harness.tools.keys()].sort()).toEqual(["cron_create", "cron_delete", "cron_list"]); +}); + +test("the approval gate is a host choice, and a host that asks gets one dialog per change", async () => { + // The CLI host does not pass `requireApproval`: scheduling work is its own + // user-driven command surface, and a print / RPC session has no page to show + // a dialog on. A host that opts in does. + const cli = createHarness(); + createStepCronExtension({ enabled: true })(cli.api); + await emitCron(cli, "session_start"); + const cliConfirm = cli.ctx.ui.confirm as ReturnType; + await cli.tools + .get("cron_create")! + .execute("create", { cron: "* * * * *", prompt: "no gate here" } as never, undefined, undefined, cli.ctx); + expect(cliConfirm).not.toHaveBeenCalled(); + await emitCron(cli, "session_shutdown"); + + const desktop = createHarness(); + createStepCronExtension({ enabled: true, requireApproval: true })(desktop.api); + await emitCron(desktop, "session_start"); + const confirm = desktop.ctx.ui.confirm as ReturnType; + confirm.mockResolvedValueOnce(false); + await expect( + desktop.tools + .get("cron_create")! + .execute( + "create", + { cron: "* * * * *", prompt: "needs approval" } as never, + undefined, + undefined, + desktop.ctx, + ), + ).rejects.toThrow("not approved"); + // The dialog is per scheduled task, never part of a session-wide grant. + expect(confirm.mock.calls[0]?.[2]).toMatchObject({ once: true }); + await emitCron(desktop, "session_shutdown"); +}); + +test("a host without a dialog surface refuses rather than silently scheduling", async () => { + const harness = createHarness(); + Object.assign(harness.ctx, { hasUI: false }); + createStepCronExtension({ enabled: true, requireApproval: true })(harness.api); + await expect( + harness.tools + .get("cron_create")! + .execute("create", { cron: "* * * * *", prompt: "no dialog" } as never, undefined, undefined, harness.ctx), + ).rejects.toThrow("no dialog surface"); +}); + test("durable creation is rejected in an untrusted project", async () => { const harness = createHarness(); Object.assign(harness.ctx, { isProjectTrusted: () => false }); @@ -746,3 +823,295 @@ test("an unschedulable persisted recurrence does not block other jobs on restart runtime.stop(); } }); + +describe("desktop cron task fields", () => { + test("creates a v2 task with task session metadata and updates it", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const directory = makeDirectory(); + const runtime = new StepCronRuntime({ + now: () => now, + jitterRatio: 0, + storagePath: path.join(directory, "tasks.json"), + }); + const created = runtime.create("0 9 * * *", "desktop task", true, true, true, { + workspace: directory, + sessionId: "cron-task-1", + status: "enabled", + title: "Task", + inputs: { skills: ["skill-a"] }, + permissionPreset: "full-access", + fullAccessAcknowledged: true, + autoExpireMs: null, + jitter: false, + origin: "desktop", + }).job; + expect(created.schemaVersion).toBe(2); + expect(created.sessionId).toBe("cron-task-1"); + expect(created.status).toBe("enabled"); + expect(created.fullAccessAcknowledged).toBe(true); + expect(created.autoExpireAt).toBeUndefined(); + const updated = runtime.update(created.id, { + title: "Task 2", + inputs: { skills: ["skill-b"] }, + status: "paused", + }); + expect(updated.title).toBe("Task 2"); + expect(updated.inputs?.skills).toEqual(["skill-b"]); + expect(updated.status).toBe("paused"); + expect(runtime.setEnabled(created.id, true)).toBe(true); + expect(runtime.list()[0]?.status).toBe("enabled"); + }); + + test("runNow hands off without advancing the schedule or completing a one-shot", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const deliveries: CronDelivery[] = []; + const runtime = new StepCronRuntime({ + now: () => now, + jitterRatio: 0, + sendMessage: (delivery) => deliveries.push(delivery), + }); + const job = runtime.create("0 9 * * *", "manual", false, false, true, { + status: "enabled", + autoExpireMs: null, + jitter: false, + }).job; + const next = job.nextFireAt; + runtime.runNow(job.id); + expect(deliveries).toHaveLength(1); + expect(runtime.list()[0]?.nextFireAt).toBe(next); + expect(runtime.list()[0]?.status).toBe("enabled"); + }); + + test("accepts a caller-provided id and a bound session", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const runtime = new StepCronRuntime({ now: () => now, jitterRatio: 0 }); + const job = runtime.create("0 9 * * *", "named", true, false, true, { + id: "task-a", + sessionId: "session-a", + status: "enabled", + autoExpireMs: null, + jitter: false, + }).job; + expect(job.id).toBe("task-a"); + expect(runtime.list()[0]?.sessionId).toBe("session-a"); + expect(() => + runtime.create("0 9 * * *", "duplicate", true, false, true, { + id: "task-a", + status: "enabled", + autoExpireMs: null, + jitter: false, + }), + ).toThrow(/already exists/); + }); + + test("full access requires an explicit acknowledgment on create and update", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const runtime = new StepCronRuntime({ now: () => now, jitterRatio: 0 }); + expect(() => + runtime.create("* * * * *", "unsafe", true, false, true, { + permissionPreset: "full-access", + }), + ).toThrow(/risk acknowledgment/); + const created = runtime.create("* * * * *", "safe", true, false, true, { + permissionPreset: "full-access", + fullAccessAcknowledged: true, + }).job; + expect(() => runtime.update(created.id, { fullAccessAcknowledged: false })).toThrow(/risk acknowledgment/); + }); + + test("one-shot and repeating schedules cannot leak into each other", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const runtime = new StepCronRuntime({ now: () => now, jitterRatio: 0 }); + const runAt = now + 2 * 60 * 60 * 1000; + const once = runtime.create("0 9 * * *", "once", false, false, true, { + runAt, + status: "enabled", + autoExpireMs: null, + jitter: false, + }).job; + expect(once.runAt).toBe(runAt); + expect(once.nextFireAt).toBe(runAt); + + // `once -> daily`: the old absolute time must not decide the next fire. + const daily = runtime.update(once.id, { recurring: true, cron: "0 9 * * *" }); + expect(daily.recurring).toBe(true); + expect(daily.runAt).toBeUndefined(); + expect(daily.nextFireAt).toBe(SimpleCronExpression.parse("0 9 * * *").next(now)); + + // …and back: `daily -> once` carries an explicit time. + const backToOnce = runtime.update(once.id, { recurring: false, runAt: now + 60 * 60 * 1000 }); + expect(backToOnce.intervalHours).toBeUndefined(); + expect(backToOnce.nextFireAt).toBe(now + 60 * 60 * 1000); + + // A repeating task cannot take a one-shot time. + expect(() => + runtime.update(once.id, { + recurring: true, + runAt: now + 60 * 60 * 1000, + }), + ).toThrow(/one-shot/); + }); + + test("a one-shot time must be in the future on create and update", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const runtime = new StepCronRuntime({ now: () => now, jitterRatio: 0 }); + expect(() => + runtime.create("0 9 * * *", "past", false, false, true, { + runAt: now - 60_000, + status: "enabled", + autoExpireMs: null, + jitter: false, + }), + ).toThrow(/future/); + expect(() => + runtime.create("0 9 * * *", "nan", false, false, true, { + runAt: Number.NaN, + status: "enabled", + autoExpireMs: null, + jitter: false, + }), + ).toThrow(/finite/); + const once = runtime.create("0 9 * * *", "ok", false, false, true, { + runAt: now + 60_000, + status: "enabled", + autoExpireMs: null, + jitter: false, + }).job; + expect(() => runtime.update(once.id, { runAt: now - 1 })).toThrow(/future/); + expect(() => runtime.update(once.id, { runAt: Number.POSITIVE_INFINITY })).toThrow(/finite/); + expect(() => runtime.update(once.id, { expiresAt: Number.NaN })).toThrow(/finite/); + }); + + test("editing an expiry or a one-shot time reopens a finished task", () => { + let now = Date.UTC(2026, 0, 1, 12, 0); + const runtime = new StepCronRuntime({ now: () => now, jitterRatio: 0 }); + const expired = runtime.create("0 9 * * *", "expiring", true, false, true, { + status: "expired", + expiresAt: now - 60_000, + autoExpireMs: null, + jitter: false, + }).job; + const renewed = runtime.update(expired.id, { expiresAt: now + 24 * 60 * 60 * 1000 }); + expect(renewed.status).toBe("enabled"); + expect(renewed.enabled).toBe(true); + + const completed = runtime.create("0 9 * * *", "done", false, false, true, { + runAt: now + 60_000, + status: "completed", + autoExpireMs: null, + jitter: false, + }).job; + expect(runtime.update(completed.id, { runAt: now + 60 * 60 * 1000 }).status).toBe("enabled"); + + // An edit that does not restore a future schedule must not reopen it. + const stillExpired = runtime.update(expired.id, { status: "expired" }); + expect(runtime.update(stillExpired.id, { title: "renamed" }).status).toBe("expired"); + + // Re-enabling a one-shot whose time already passed would fire it at once on + // the next tick, so it needs a new time instead. + const stale = runtime.create("0 9 * * *", "stale", false, false, true, { + runAt: now + 60_000, + status: "paused", + autoExpireMs: null, + jitter: false, + }).job; + now += 10 * 60_000; + expect(() => runtime.update(stale.id, { status: "enabled" })).toThrow(/future time/); + expect(runtime.update(stale.id, { status: "enabled", runAt: now + 60_000 }).status).toBe("enabled"); + }); + + test("a host only sees and fires the records it owns", () => { + let now = Date.UTC(2026, 0, 1, 12, 0); + const deliveries: CronDelivery[] = []; + const runtime = new StepCronRuntime({ + now: () => now, + jitterRatio: 0, + ownsJob: (job) => job.sessionId === undefined, + sendMessage: (delivery) => deliveries.push(delivery), + }); + const mine = runtime.create("* * * * *", "mine", false, false, true, { + status: "enabled", + // One-shot at the next tick, so the drain loop has real work to do. + runAt: now + 60_000, + autoExpireMs: null, + jitter: false, + }).job; + const foreign = runtime.create("* * * * *", "desktop", true, false, true, { + sessionId: "session-desktop", + status: "enabled", + autoExpireMs: null, + jitter: false, + }).job; + expect(runtime.list().map((job) => job.id)).toEqual([mine.id]); + expect(runtime.delete(foreign.id)).toBe(false); + expect(() => runtime.runNow(foreign.id)).toThrow(/another host/); + expect(() => runtime.update(foreign.id, { title: "hijack" })).toThrow(/another host/); + now += 60_000; + runtime.tick(); + expect(deliveries.map((delivery) => delivery.job?.id)).toEqual([mine.id]); + }); + + test("the kernel keeps generic ceilings the persisted reader shares", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const runtime = new StepCronRuntime({ now: () => now, jitterRatio: 0 }); + const created = runtime.create("0 9 * * *", "x", true, false, true, { status: "enabled" }).job; + // 16,000 is the kernel ceiling; a host form may be stricter, but narrowing + // this value would make an earlier record with a longer prompt unreadable. + expect(CRON_MAX_PROMPT_LENGTH).toBe(16_000); + expect(CRON_MAX_TITLE_LENGTH).toBe(200); + expect(() => runtime.create("0 9 * * *", "y".repeat(CRON_MAX_PROMPT_LENGTH + 1), true, false, true)).toThrow( + /exceeds 16000/, + ); + expect(() => runtime.update(created.id, { prompt: "y".repeat(CRON_MAX_PROMPT_LENGTH + 1) })).toThrow( + /exceeds 16000/, + ); + // A title over the ceiling is truncated rather than rejected, as before. + const longTitle = runtime.create("0 9 * * *", "z", true, false, true, { + title: "t".repeat(CRON_MAX_TITLE_LENGTH + 10), + status: "enabled", + }).job; + expect(longTitle.title?.length).toBe(CRON_MAX_TITLE_LENGTH); + }); + + test("supports explicit hour intervals and absolute one-shot times", () => { + const now = Date.UTC(2026, 0, 1, 12, 0); + const runtime = new StepCronRuntime({ now: () => now, jitterRatio: 0 }); + const interval = runtime.create("* * * * *", "interval", true, false, true, { + intervalHours: 5, + status: "enabled", + autoExpireMs: null, + jitter: false, + }).job; + expect(interval.intervalHours).toBe(5); + expect(interval.nextFireAt).toBe(now + 5 * 60 * 60 * 1000); + const runAt = now + 2 * 60 * 60 * 1000; + const once = runtime.create("* * * * *", "once", false, false, true, { + runAt, + status: "enabled", + autoExpireMs: null, + jitter: false, + }).job; + expect(once.nextFireAt).toBe(runAt); + }); + + test("canDeliver gates each job and maxJobs is configurable", () => { + let now = Date.UTC(2026, 0, 1, 12, 0); + let allow = false; + const deliveries: CronDelivery[] = []; + const runtime = new StepCronRuntime({ + now: () => now, + jitterRatio: 0, + maxJobs: 1, + canDeliver: () => allow, + sendMessage: (delivery) => deliveries.push(delivery), + }); + runtime.create("* * * * *", "gated", false, false); + expect(() => runtime.create("* * * * *", "second", false, false)).toThrow(/At most 1/); + now += 60_000; + runtime.tick(); + expect(deliveries).toHaveLength(0); + allow = true; + runtime.tick(); + expect(deliveries).toHaveLength(1); + }); +}); diff --git a/packages/coding-agent/test/subagent-child-env.test.ts b/packages/coding-agent/test/subagent-child-env.test.ts index d99513bf..20a0397a 100644 --- a/packages/coding-agent/test/subagent-child-env.test.ts +++ b/packages/coding-agent/test/subagent-child-env.test.ts @@ -88,14 +88,19 @@ test("every rpc child is stripped of scheduling authority", () => { vi.stubEnv("STEP_DISABLE_CRON", ""); vi.stubEnv("STEP_DISABLE_GOAL", ""); vi.stubEnv("STEP_DISABLE_SCHEDULE", ""); - expect(registeredTools(createStepCronExtension())).toEqual(["cron_create", "cron_delete", "cron_list"]); + expect(registeredTools(createStepCronExtension({ allowUpdate: true }))).toEqual([ + "cron_create", + "cron_delete", + "cron_list", + "cron_update", + ]); expect(registeredTools(createStepGoalExtension()).length).toBeGreaterThan(0); const childEnv = buildSubagentChildEnv(runInput()); vi.stubEnv("STEP_DISABLE_CRON", String(childEnv.STEP_DISABLE_CRON)); vi.stubEnv("STEP_DISABLE_GOAL", String(childEnv.STEP_DISABLE_GOAL)); - expect(registeredTools(createStepCronExtension())).toEqual([]); + expect(registeredTools(createStepCronExtension({ allowUpdate: true }))).toEqual([]); expect(registeredTools(createStepGoalExtension())).toEqual([]); });