Skip to content

Commit 5a275d5

Browse files
kagura-agentclaude
andcommitted
fix(scheduler): add duplicate-name and task-size checks to updateJob
Addresses PR ghostwright#87 feedback: updateJob now validates duplicate names (skipping when unchanged for idempotency) and enforces the 32KB MAX_TASK_BYTES cap at the service layer, matching createJob behavior. Co-Authored-By: Claude Opus 4 (1M context) <noreply@anthropic.com>
1 parent 453e29a commit 5a275d5

2 files changed

Lines changed: 63 additions & 1 deletion

File tree

src/scheduler/__tests__/service.test.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -578,4 +578,48 @@ describe("Scheduler", () => {
578578
scheduler.updateJob(job.id, { delivery: { channel: "slack", target: "invalid" } });
579579
}).toThrow("invalid delivery.target");
580580
});
581+
582+
test("updateJob rejects duplicate name belonging to another job", () => {
583+
const scheduler = new Scheduler({ db, runtime: mockRuntime as never });
584+
scheduler.createJob({
585+
name: "Existing",
586+
schedule: { kind: "every", intervalMs: 60_000 },
587+
task: "Task A",
588+
});
589+
const jobB = scheduler.createJob({
590+
name: "Other",
591+
schedule: { kind: "every", intervalMs: 60_000 },
592+
task: "Task B",
593+
});
594+
595+
expect(() => {
596+
scheduler.updateJob(jobB.id, { name: "Existing" });
597+
}).toThrow('job with name "Existing" already exists');
598+
});
599+
600+
test("updateJob allows renaming to current name (idempotent)", () => {
601+
const scheduler = new Scheduler({ db, runtime: mockRuntime as never });
602+
const job = scheduler.createJob({
603+
name: "SameName",
604+
schedule: { kind: "every", intervalMs: 60_000 },
605+
task: "Task",
606+
});
607+
608+
const updated = scheduler.updateJob(job.id, { name: "SameName" });
609+
expect(updated?.name).toBe("SameName");
610+
});
611+
612+
test("updateJob rejects task exceeding MAX_TASK_BYTES", () => {
613+
const scheduler = new Scheduler({ db, runtime: mockRuntime as never });
614+
const job = scheduler.createJob({
615+
name: "BigTask",
616+
schedule: { kind: "every", intervalMs: 60_000 },
617+
task: "small",
618+
});
619+
620+
const huge = "x".repeat(33 * 1024);
621+
expect(() => {
622+
scheduler.updateJob(job.id, { task: huge });
623+
}).toThrow("byte limit");
624+
});
581625
});

src/scheduler/service.ts

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import type { Database } from "bun:sqlite";
22
import { randomUUID } from "node:crypto";
33
import type { AgentRuntime } from "../agent/runtime.ts";
44
import type { SlackTransport } from "../channels/slack-transport.ts";
5-
import { validateCreateInput } from "./create-validation.ts";
5+
import { MAX_TASK_BYTES, validateCreateInput } from "./create-validation.ts";
66
import { executeJob } from "./executor.ts";
77
import { type SchedulerHealthSummary, computeHealthSummary } from "./health.ts";
88
import { cleanupOldTerminalJobs, staggerMissedJobs } from "./recovery.ts";
@@ -151,6 +151,24 @@ export class Scheduler {
151151
}
152152
}
153153

154+
// Duplicate name detection: skip if unchanged (idempotent rename).
155+
if (input.name !== undefined && input.name.toLowerCase() !== job.name.toLowerCase()) {
156+
const dupe = this.db
157+
.query("SELECT id FROM scheduled_jobs WHERE lower(name) = lower(?)")
158+
.get(input.name) as { id: string } | null;
159+
if (dupe) {
160+
throw new Error(`job with name "${input.name}" already exists (id: ${dupe.id})`);
161+
}
162+
}
163+
164+
// Task size cap (mirrors validateCreateInput).
165+
if (input.task !== undefined) {
166+
const taskBytes = Buffer.byteLength(input.task, "utf8");
167+
if (taskBytes > MAX_TASK_BYTES) {
168+
throw new Error(`task text is ${taskBytes} bytes, exceeds ${MAX_TASK_BYTES} byte limit`);
169+
}
170+
}
171+
154172
// Build dynamic UPDATE statement
155173
const updates: string[] = [];
156174
const values: (string | number | null)[] = [];

0 commit comments

Comments
 (0)