From 0bf35ec3912753c26af7a90f4e6492d302d2a3b0 Mon Sep 17 00:00:00 2001 From: truffle Date: Sun, 26 Apr 2026 04:08:51 +0000 Subject: [PATCH] scheduler: add phantom_schedule update action so editing a job preserves run history (#86) Adds `action: "update"` to phantom_schedule. The caller can change `task`, `description`, `schedule`, `delivery`, or `enabled` on an existing job by jobId or name. The history columns (last_run_at, last_run_status, last_run_duration_ms, last_run_error, run_count, consecutive_errors, created_at) and the stable jobId are preserved. If schedule changes, next_run_at is recomputed via the same computeNextRunAt path resumeJob uses, then armTimer() so an earlier next-fire wakes the timer in time. Closes #86. --- src/scheduler/__tests__/service.test.ts | 91 +++++++++++++++++++++++++ src/scheduler/__tests__/tool.test.ts | 25 +++++++ src/scheduler/service.ts | 81 +++++++++++++++++++++- src/scheduler/tool-schema.ts | 31 +++++++++ src/scheduler/tool.ts | 55 ++++++++++++--- 5 files changed, 273 insertions(+), 10 deletions(-) diff --git a/src/scheduler/__tests__/service.test.ts b/src/scheduler/__tests__/service.test.ts index d73f06f5..2e9070cd 100644 --- a/src/scheduler/__tests__/service.test.ts +++ b/src/scheduler/__tests__/service.test.ts @@ -444,6 +444,97 @@ describe("Scheduler", () => { expect(row.enabled).toBe(0); }); + test("updateJob changes task in place and preserves run history", async () => { + const scheduler = new Scheduler({ db, runtime: mockRuntime as never }); + const job = scheduler.createJob({ + name: "TaskEditable", + schedule: { kind: "every", intervalMs: 60_000 }, + task: "Original task text", + }); + // Run once so run_count and last_run_at are populated. The point of + // the update path is that this history survives a task edit. + await scheduler.runJobNow(job.id); + const before = scheduler.getJob(job.id); + expect(before?.runCount).toBe(1); + expect(before?.lastRunAt).toBeTruthy(); + + const updated = scheduler.updateJob(job.id, { task: "New task text" }); + expect(updated?.task).toBe("New task text"); + expect(updated?.runCount).toBe(1); + expect(updated?.lastRunAt).toBe(before?.lastRunAt ?? null); + expect(updated?.id).toBe(job.id); + }); + + test("updateJob recomputes next_run_at when schedule changes", () => { + const scheduler = new Scheduler({ db, runtime: mockRuntime as never }); + const job = scheduler.createJob({ + name: "ScheduleEditable", + schedule: { kind: "every", intervalMs: 24 * 60 * 60 * 1000 }, + task: "Daily", + }); + const originalNext = job.nextRunAt ? new Date(job.nextRunAt).getTime() : 0; + + const updated = scheduler.updateJob(job.id, { + schedule: { kind: "every", intervalMs: 60_000 }, + }); + expect(updated?.schedule).toEqual({ kind: "every", intervalMs: 60_000 }); + const newNext = updated?.nextRunAt ? new Date(updated.nextRunAt).getTime() : 0; + expect(newNext).toBeLessThan(originalNext); + expect(newNext).toBeGreaterThan(Date.now() - 5_000); + expect(newNext).toBeLessThan(Date.now() + 120_000); + }); + + test("updateJob can flip enabled to false and back to true", () => { + const scheduler = new Scheduler({ db, runtime: mockRuntime as never }); + const job = scheduler.createJob({ + name: "EnabledToggle", + schedule: { kind: "every", intervalMs: 60_000 }, + task: "Toggle me", + }); + expect(job.enabled).toBe(true); + + const disabled = scheduler.updateJob(job.id, { enabled: false }); + expect(disabled?.enabled).toBe(false); + + const enabled = scheduler.updateJob(job.id, { enabled: true }); + expect(enabled?.enabled).toBe(true); + }); + + test("updateJob can change delivery target", () => { + const scheduler = new Scheduler({ db, runtime: mockRuntime as never }); + const job = scheduler.createJob({ + name: "DeliveryEditable", + schedule: { kind: "every", intervalMs: 60_000 }, + task: "Deliver", + }); + expect(job.delivery).toEqual({ channel: "slack", target: "owner" }); + + const updated = scheduler.updateJob(job.id, { + delivery: { channel: "slack", target: "C04ABC123" }, + }); + expect(updated?.delivery).toEqual({ channel: "slack", target: "C04ABC123" }); + }); + + test("updateJob rejects an invalid slack target", () => { + const scheduler = new Scheduler({ db, runtime: mockRuntime as never }); + const job = scheduler.createJob({ + name: "BadTargetReject", + schedule: { kind: "every", intervalMs: 60_000 }, + task: "Try bad target", + }); + expect(() => + scheduler.updateJob(job.id, { + delivery: { channel: "slack", target: "#general" }, + }), + ).toThrow(/invalid delivery.target/); + }); + + test("updateJob returns null for unknown id", () => { + const scheduler = new Scheduler({ db, runtime: mockRuntime as never }); + const result = scheduler.updateJob("does-not-exist", { task: "anything" }); + expect(result).toBeNull(); + }); + test("paused job is excluded from the armTimer MIN query", async () => { const scheduler = new Scheduler({ db, runtime: mockRuntime as never }); await scheduler.start(); diff --git a/src/scheduler/__tests__/tool.test.ts b/src/scheduler/__tests__/tool.test.ts index 3d40fa4d..21b862ad 100644 --- a/src/scheduler/__tests__/tool.test.ts +++ b/src/scheduler/__tests__/tool.test.ts @@ -2,6 +2,7 @@ import { Database } from "bun:sqlite"; import { afterAll, beforeAll, beforeEach, describe, expect, mock, test } from "bun:test"; import { runMigrations } from "../../db/migrate.ts"; import { Scheduler } from "../service.ts"; +import { JobUpdateInputSchema } from "../tool-schema.ts"; import { createSchedulerToolServer } from "../tool.ts"; function createMockRuntime() { @@ -106,4 +107,28 @@ describe("createSchedulerToolServer", () => { expect(mcpServers["phantom-scheduler"].type).toBe("sdk"); expect(mcpServers["phantom-scheduler"].name).toBe("phantom-scheduler"); }); + + test("update action via scheduler edits a job by name", () => { + const job = scheduler.createJob({ + name: "Editable", + schedule: { kind: "every", intervalMs: 60_000 }, + task: "Original", + }); + const targetId = scheduler.findJobIdByName("Editable"); + expect(targetId).toBe(job.id); + + const updated = scheduler.updateJob(targetId as string, { task: "Edited" }); + expect(updated?.task).toBe("Edited"); + expect(updated?.id).toBe(job.id); + }); + + test("JobUpdateInputSchema rejects an empty partial", () => { + const result = JobUpdateInputSchema.safeParse({}); + expect(result.success).toBe(false); + }); + + test("JobUpdateInputSchema accepts a single-field partial", () => { + const result = JobUpdateInputSchema.safeParse({ task: "Just the task" }); + expect(result.success).toBe(true); + }); }); diff --git a/src/scheduler/service.ts b/src/scheduler/service.ts index 807fd231..ebca6eb7 100644 --- a/src/scheduler/service.ts +++ b/src/scheduler/service.ts @@ -7,8 +7,9 @@ import { executeJob } from "./executor.ts"; import { type SchedulerHealthSummary, computeHealthSummary } from "./health.ts"; import { cleanupOldTerminalJobs, staggerMissedJobs } from "./recovery.ts"; import { rowToJob } from "./row-mapper.ts"; -import { computeNextRunAt, serializeScheduleValue } from "./schedule.ts"; -import type { JobCreateInput, JobRow, ScheduledJob } from "./types.ts"; +import { computeNextRunAt, serializeScheduleValue, validateSchedule } from "./schedule.ts"; +import type { JobUpdateInputParsed } from "./tool-schema.ts"; +import { type JobCreateInput, type JobRow, type ScheduledJob, isValidSlackTarget } from "./types.ts"; // Upper bound on the setTimeout delay we pass when arming the next wake-up. // Both Node and Bun use a 32-bit signed integer for the setTimeout delay, so @@ -180,6 +181,82 @@ export class Scheduler { return this.getJob(id); } + /** + * Edit a job's user-authored columns in place. History is preserved: + * last_run_at, last_run_status, run_count, consecutive_errors, created_at, + * and the stable `id` are never touched. The caller chooses which subset + * of {task, description, schedule, delivery, enabled} to change; the + * refine() on JobUpdateInputSchema enforces that at least one is present, + * so an empty partial here is treated as a no-op (defense in depth). + * + * When `schedule` changes we recompute next_run_at via the same + * computeNextRunAt path resumeJob uses, then armTimer() so a schedule + * that pulls the next fire earlier wakes the timer in time. Returns the + * fresh row, or null if `id` does not exist. + */ + updateJob(id: string, partial: JobUpdateInputParsed): ScheduledJob | null { + const job = this.getJob(id); + if (!job) return null; + + const sets: string[] = []; + const params: Array = []; + + if (partial.task !== undefined) { + sets.push("task = ?"); + params.push(partial.task); + } + + if (partial.description !== undefined) { + sets.push("description = ?"); + params.push(partial.description); + } + + if (partial.delivery !== undefined) { + // Mirror create-validation's slack-target check so an update cannot + // install a malformed target the create path would have rejected. + if (partial.delivery.channel === "slack" && !isValidSlackTarget(partial.delivery.target)) { + throw new Error( + `invalid delivery.target '${partial.delivery.target}': must be "owner", a Slack channel id (C...), or a Slack user id (U...)`, + ); + } + sets.push("delivery_channel = ?"); + params.push(partial.delivery.channel); + sets.push("delivery_target = ?"); + params.push(partial.delivery.target); + } + + if (partial.enabled !== undefined) { + sets.push("enabled = ?"); + params.push(partial.enabled ? 1 : 0); + } + + if (partial.schedule !== undefined) { + const scheduleError = validateSchedule(partial.schedule); + if (scheduleError) { + throw new Error(`invalid schedule: ${scheduleError}`); + } + const nextRun = computeNextRunAt(partial.schedule); + const nextRunIso = nextRun ? nextRun.toISOString() : null; + sets.push("schedule_kind = ?"); + params.push(partial.schedule.kind); + sets.push("schedule_value = ?"); + params.push(serializeScheduleValue(partial.schedule)); + sets.push("next_run_at = ?"); + params.push(nextRunIso); + } + + if (sets.length === 0) { + return job; + } + + sets.push("updated_at = datetime('now')"); + params.push(id); + + this.db.run(`UPDATE scheduled_jobs SET ${sets.join(", ")} WHERE id = ?`, params); + this.armTimer(); + return this.getJob(id); + } + /** * Defensive read: one corrupt row (a future kind, a truncated write) must * not brick the whole list. Bad rows are logged and skipped. See M8. diff --git a/src/scheduler/tool-schema.ts b/src/scheduler/tool-schema.ts index 6d92c383..00ca9b58 100644 --- a/src/scheduler/tool-schema.ts +++ b/src/scheduler/tool-schema.ts @@ -29,3 +29,34 @@ export const JobCreateInputSchema = z.object({ }); export type JobCreateInputParsed = z.infer; + +// Partial-update shape for Scheduler.updateJob. All fields optional with a +// refine() that rejects an empty object so the caller cannot silently no-op. +// Excludes identity columns (name, createdBy) and history columns +// (last_run_*, run_count, consecutive_errors, created_at): name is the +// human-readable lookup handle and renaming mid-life breaks cross-references, +// history is owned by the executor and would erase the run trail an update +// is supposed to preserve. +export const JobUpdateInputSchema = z + .object({ + description: z.string().max(1000).optional(), + schedule: ScheduleInputSchema.optional(), + task: z + .string() + .min(1) + .max(32 * 1024) + .optional(), + delivery: JobDeliverySchema.optional(), + enabled: z.boolean().optional(), + }) + .refine( + (v) => + v.description !== undefined || + v.schedule !== undefined || + v.task !== undefined || + v.delivery !== undefined || + v.enabled !== undefined, + { message: "update requires at least one of: description, schedule, task, delivery, enabled" }, + ); + +export type JobUpdateInputParsed = z.infer; diff --git a/src/scheduler/tool.ts b/src/scheduler/tool.ts index 2ec15998..4de2213d 100644 --- a/src/scheduler/tool.ts +++ b/src/scheduler/tool.ts @@ -2,7 +2,7 @@ import { createSdkMcpServer, tool } from "@anthropic-ai/claude-agent-sdk"; import type { McpSdkServerConfigWithInstance } from "@anthropic-ai/claude-agent-sdk"; import { z } from "zod"; import type { Scheduler } from "./service.ts"; -import { ScheduleInputSchema } from "./tool-schema.ts"; +import { JobUpdateInputSchema, ScheduleInputSchema } from "./tool-schema.ts"; import { JobDeliverySchema } from "./types.ts"; function ok(data: Record): { content: Array<{ type: "text"; text: string }> } { @@ -13,11 +13,12 @@ function err(message: string): { content: Array<{ type: "text"; text: string }>; return { content: [{ type: "text" as const, text: JSON.stringify({ error: message }) }], isError: true }; } -const TOOL_DESCRIPTION = `Create, list, delete, or trigger scheduled tasks. Lets you set up recurring jobs, one-shot reminders, and automated reports. +const TOOL_DESCRIPTION = `Create, list, update, delete, or trigger scheduled tasks. Lets you set up recurring jobs, one-shot reminders, and automated reports. Actions: - create: Create a new scheduled task. Returns the job id and next run time. Rejects invalid schedules, past timestamps, duplicate names, task text over 32 KB, and delivery targets that are not "owner", a channel id (C...), or a user id (U...). - list: List all scheduled tasks with status and next run time. Corrupt rows are logged and skipped. +- update: Edit task, description, schedule, delivery, or enabled on an existing job by jobId or name. Run history (last_run_at, run_count, consecutive_errors) and the stable jobId are preserved. If schedule changes, next_run_at is recomputed. Requires at least one field; name is not editable. - delete: Remove a scheduled task by jobId or by name (case insensitive). - run: Trigger a task immediately. Only runs when status is active and no other job is currently executing. Returns the task output. @@ -60,19 +61,25 @@ export function createSchedulerToolServer(scheduler: Scheduler): McpSdkServerCon TOOL_DESCRIPTION, { action: z - .enum(["create", "list", "delete", "run"]) + .enum(["create", "list", "update", "delete", "run"]) .describe( - "create: new scheduled task. list: enumerate tasks. delete: remove by jobId or name. run: trigger immediately (only when status=active and scheduler is idle).", + "create: new scheduled task. list: enumerate tasks. update: edit user-authored fields by jobId or name (preserves run history). delete: remove by jobId or name. run: trigger immediately (only when status=active and scheduler is idle).", ), - name: z.string().optional().describe("Job name (required for create)"), + name: z.string().optional().describe("Job name (required for create; lookup key for update, delete, run)"), description: z.string().optional().describe("Job description"), - schedule: ScheduleInputSchema.optional().describe("Schedule definition (required for create)"), + schedule: ScheduleInputSchema.optional().describe( + "Schedule definition (required for create, optional for update)", + ), task: z .string() .optional() - .describe("The prompt for the agent when the job fires (required for create, 32 KB max)"), + .describe("The prompt for the agent when the job fires (required for create, optional for update, 32 KB max)"), delivery: JobDeliverySchema.optional().describe("Where to deliver results"), - jobId: z.string().optional().describe("Job ID (for delete or run)"), + enabled: z + .boolean() + .optional() + .describe("Whether the job fires (optional for update; pause/resume manage status separately)"), + jobId: z.string().optional().describe("Job ID (for update, delete, or run)"), }, async (input) => { try { @@ -123,6 +130,38 @@ export function createSchedulerToolServer(scheduler: Scheduler): McpSdkServerCon }); } + case "update": { + const targetId = input.jobId ?? scheduler.findJobIdByName(input.name); + if (!targetId) return err("Provide jobId or name to update"); + + const partialResult = JobUpdateInputSchema.safeParse({ + description: input.description, + schedule: input.schedule, + task: input.task, + delivery: input.delivery, + enabled: input.enabled, + }); + if (!partialResult.success) { + return err(partialResult.error.issues.map((i) => i.message).join("; ")); + } + + const updated = scheduler.updateJob(targetId, partialResult.data); + if (!updated) return err(`Job not found: ${targetId}`); + + return ok({ + updated: true, + id: updated.id, + name: updated.name, + schedule: updated.schedule, + task: updated.task, + description: updated.description, + delivery: updated.delivery, + enabled: updated.enabled, + nextRunAt: updated.nextRunAt, + runCount: updated.runCount, + }); + } + case "delete": { const targetId = input.jobId ?? scheduler.findJobIdByName(input.name); if (!targetId) return err("Provide jobId or name to delete");