feat(cron): require job name
This commit is contained in:
@@ -132,7 +132,8 @@ export function registerCronCli(program: Command) {
|
||||
cron
|
||||
.command("add")
|
||||
.description("Add a cron job")
|
||||
.option("--name <name>", "Optional name")
|
||||
.requiredOption("--name <name>", "Job name")
|
||||
.option("--description <text>", "Optional description")
|
||||
.option("--disabled", "Create job disabled", false)
|
||||
.option("--session <target>", "Session target (main|isolated)", "main")
|
||||
.option(
|
||||
@@ -278,11 +279,17 @@ export function registerCronCli(program: Command) {
|
||||
}
|
||||
: undefined;
|
||||
|
||||
const name = String(opts.name ?? "").trim();
|
||||
if (!name) throw new Error("--name is required");
|
||||
|
||||
const description =
|
||||
typeof opts.description === "string" && opts.description.trim()
|
||||
? opts.description.trim()
|
||||
: undefined;
|
||||
|
||||
const params = {
|
||||
name:
|
||||
typeof opts.name === "string" && opts.name.trim()
|
||||
? opts.name.trim()
|
||||
: undefined,
|
||||
name,
|
||||
description,
|
||||
enabled: !opts.disabled,
|
||||
schedule,
|
||||
sessionTarget,
|
||||
@@ -388,6 +395,7 @@ export function registerCronCli(program: Command) {
|
||||
.description("Edit a cron job (patch fields)")
|
||||
.argument("<id>", "Job id")
|
||||
.option("--name <name>", "Set name")
|
||||
.option("--description <text>", "Set description")
|
||||
.option("--enable", "Enable job", false)
|
||||
.option("--disable", "Disable job", false)
|
||||
.option("--session <target>", "Session target (main|isolated)")
|
||||
@@ -430,6 +438,8 @@ export function registerCronCli(program: Command) {
|
||||
|
||||
const patch: Record<string, unknown> = {};
|
||||
if (typeof opts.name === "string") patch.name = opts.name;
|
||||
if (typeof opts.description === "string")
|
||||
patch.description = opts.description;
|
||||
if (opts.enable && opts.disable)
|
||||
throw new Error("Choose --enable or --disable, not both");
|
||||
if (opts.enable) patch.enabled = true;
|
||||
|
||||
@@ -201,7 +201,7 @@ export async function runCronIsolatedAgentTurn(params: {
|
||||
});
|
||||
|
||||
const base =
|
||||
`[cron:${params.job.id}${params.job.name ? ` ${params.job.name}` : ""}] ${params.message}`.trim();
|
||||
`[cron:${params.job.id} ${params.job.name}] ${params.message}`.trim();
|
||||
|
||||
const commandBody = base;
|
||||
|
||||
|
||||
@@ -54,6 +54,7 @@ describe("CronService", () => {
|
||||
await cron.start();
|
||||
const atMs = Date.parse("2025-12-13T00:00:02.000Z");
|
||||
const job = await cron.add({
|
||||
name: "one-shot hello",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "main",
|
||||
@@ -139,6 +140,7 @@ describe("CronService", () => {
|
||||
await cron.start();
|
||||
const atMs = Date.parse("2025-12-13T00:00:01.000Z");
|
||||
await cron.add({
|
||||
name: "isolated error test",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "isolated",
|
||||
@@ -174,6 +176,7 @@ describe("CronService", () => {
|
||||
|
||||
await expect(
|
||||
cron.add({
|
||||
name: "bad combo (main/agentTurn)",
|
||||
enabled: true,
|
||||
schedule: { kind: "every", everyMs: 1000 },
|
||||
sessionTarget: "main",
|
||||
@@ -184,6 +187,7 @@ describe("CronService", () => {
|
||||
|
||||
await expect(
|
||||
cron.add({
|
||||
name: "bad combo (isolated/systemEvent)",
|
||||
enabled: true,
|
||||
schedule: { kind: "every", everyMs: 1000 },
|
||||
sessionTarget: "isolated",
|
||||
@@ -265,6 +269,7 @@ describe("CronService", () => {
|
||||
await cron.start();
|
||||
const atMs = Date.parse("2025-12-13T00:00:01.000Z");
|
||||
await cron.add({
|
||||
name: "empty systemEvent test",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "main",
|
||||
@@ -303,6 +308,7 @@ describe("CronService", () => {
|
||||
await cron.start();
|
||||
const atMs = Date.parse("2025-12-13T00:00:01.000Z");
|
||||
await cron.add({
|
||||
name: "disabled cron job",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "main",
|
||||
@@ -342,6 +348,7 @@ describe("CronService", () => {
|
||||
await cron.start();
|
||||
const atMs = Date.parse("2025-12-13T00:00:05.000Z");
|
||||
await cron.add({
|
||||
name: "status next wake",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "main",
|
||||
|
||||
@@ -45,8 +45,49 @@ export type CronServiceDeps = {
|
||||
|
||||
const STUCK_RUN_MS = 2 * 60 * 60 * 1000;
|
||||
|
||||
function isNonEmptyString(value: unknown): value is string {
|
||||
return typeof value === "string" && value.trim().length > 0;
|
||||
function normalizeRequiredName(raw: unknown) {
|
||||
if (typeof raw !== "string") throw new Error("cron job name is required");
|
||||
const name = raw.trim();
|
||||
if (!name) throw new Error("cron job name is required");
|
||||
return name;
|
||||
}
|
||||
|
||||
function normalizeOptionalText(raw: unknown) {
|
||||
if (typeof raw !== "string") return undefined;
|
||||
const trimmed = raw.trim();
|
||||
return trimmed ? trimmed : undefined;
|
||||
}
|
||||
|
||||
function truncateText(input: string, maxLen: number) {
|
||||
if (input.length <= maxLen) return input;
|
||||
return `${input.slice(0, Math.max(0, maxLen - 1)).trimEnd()}…`;
|
||||
}
|
||||
|
||||
function inferLegacyName(job: {
|
||||
schedule?: { kind?: unknown; everyMs?: unknown; expr?: unknown };
|
||||
payload?: { kind?: unknown; text?: unknown; message?: unknown };
|
||||
}) {
|
||||
const text =
|
||||
job?.payload?.kind === "systemEvent" && typeof job.payload.text === "string"
|
||||
? job.payload.text
|
||||
: job?.payload?.kind === "agentTurn" &&
|
||||
typeof job.payload.message === "string"
|
||||
? job.payload.message
|
||||
: "";
|
||||
const firstLine =
|
||||
text
|
||||
.split("\n")
|
||||
.map((l) => l.trim())
|
||||
.find(Boolean) ?? "";
|
||||
if (firstLine) return truncateText(firstLine, 60);
|
||||
|
||||
const kind = typeof job?.schedule?.kind === "string" ? job.schedule.kind : "";
|
||||
if (kind === "cron" && typeof job?.schedule?.expr === "string")
|
||||
return `Cron: ${truncateText(job.schedule.expr, 52)}`;
|
||||
if (kind === "every" && typeof job?.schedule?.everyMs === "number")
|
||||
return `Every: ${job.schedule.everyMs}ms`;
|
||||
if (kind === "at") return "One-shot";
|
||||
return "Cron job";
|
||||
}
|
||||
|
||||
function normalizePayloadToSystemText(payload: CronPayload) {
|
||||
@@ -131,7 +172,8 @@ export class CronService {
|
||||
const id = crypto.randomUUID();
|
||||
const job: CronJob = {
|
||||
id,
|
||||
name: input.name?.trim() || undefined,
|
||||
name: normalizeRequiredName(input.name),
|
||||
description: normalizeOptionalText(input.description),
|
||||
enabled: input.enabled !== false,
|
||||
createdAtMs: now,
|
||||
updatedAtMs: now,
|
||||
@@ -165,8 +207,9 @@ export class CronService {
|
||||
const job = this.findJobOrThrow(id);
|
||||
const now = this.deps.nowMs();
|
||||
|
||||
if (isNonEmptyString(patch.name)) job.name = patch.name.trim();
|
||||
if (patch.name === null || patch.name === "") job.name = undefined;
|
||||
if ("name" in patch) job.name = normalizeRequiredName(patch.name);
|
||||
if ("description" in patch)
|
||||
job.description = normalizeOptionalText(patch.description);
|
||||
if (typeof patch.enabled === "boolean") job.enabled = patch.enabled;
|
||||
if (patch.schedule) job.schedule = patch.schedule;
|
||||
if (patch.sessionTarget) job.sessionTarget = patch.sessionTarget;
|
||||
@@ -251,7 +294,30 @@ export class CronService {
|
||||
private async ensureLoaded() {
|
||||
if (this.store) return;
|
||||
const loaded = await loadCronStore(this.deps.storePath);
|
||||
this.store = { version: 1, jobs: loaded.jobs ?? [] };
|
||||
const jobs = (loaded.jobs ?? []) as unknown as Array<
|
||||
Record<string, unknown>
|
||||
>;
|
||||
let mutated = false;
|
||||
for (const raw of jobs) {
|
||||
const nameRaw = raw.name;
|
||||
if (typeof nameRaw !== "string" || nameRaw.trim().length === 0) {
|
||||
raw.name = inferLegacyName({
|
||||
schedule: raw.schedule as never,
|
||||
payload: raw.payload as never,
|
||||
});
|
||||
mutated = true;
|
||||
} else {
|
||||
raw.name = nameRaw.trim();
|
||||
}
|
||||
|
||||
const desc = normalizeOptionalText(raw.description);
|
||||
if (raw.description !== desc) {
|
||||
raw.description = desc;
|
||||
mutated = true;
|
||||
}
|
||||
}
|
||||
this.store = { version: 1, jobs: jobs as unknown as CronJob[] };
|
||||
if (mutated) await this.persist();
|
||||
}
|
||||
|
||||
private warnIfDisabled(action: string) {
|
||||
|
||||
@@ -34,7 +34,8 @@ export type CronJobState = {
|
||||
|
||||
export type CronJob = {
|
||||
id: string;
|
||||
name?: string;
|
||||
name: string;
|
||||
description?: string;
|
||||
enabled: boolean;
|
||||
createdAtMs: number;
|
||||
updatedAtMs: number;
|
||||
|
||||
@@ -412,7 +412,8 @@ export const CronJobStateSchema = Type.Object(
|
||||
export const CronJobSchema = Type.Object(
|
||||
{
|
||||
id: NonEmptyString,
|
||||
name: Type.Optional(Type.String()),
|
||||
name: NonEmptyString,
|
||||
description: Type.Optional(Type.String()),
|
||||
enabled: Type.Boolean(),
|
||||
createdAtMs: Type.Integer({ minimum: 0 }),
|
||||
updatedAtMs: Type.Integer({ minimum: 0 }),
|
||||
@@ -440,7 +441,8 @@ export const CronStatusParamsSchema = Type.Object(
|
||||
|
||||
export const CronAddParamsSchema = Type.Object(
|
||||
{
|
||||
name: Type.Optional(Type.String()),
|
||||
name: NonEmptyString,
|
||||
description: Type.Optional(Type.String()),
|
||||
enabled: Type.Optional(Type.Boolean()),
|
||||
schedule: CronScheduleSchema,
|
||||
sessionTarget: Type.Union([Type.Literal("main"), Type.Literal("isolated")]),
|
||||
|
||||
@@ -968,6 +968,7 @@ describe("gateway server", () => {
|
||||
id: "cron-add-log-1",
|
||||
method: "cron.add",
|
||||
params: {
|
||||
name: "log test",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "main",
|
||||
@@ -1077,6 +1078,7 @@ describe("gateway server", () => {
|
||||
id: "cron-add-log-2",
|
||||
method: "cron.add",
|
||||
params: {
|
||||
name: "log test (jobs.json)",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "main",
|
||||
@@ -1207,6 +1209,7 @@ describe("gateway server", () => {
|
||||
id: "cron-add-auto-1",
|
||||
method: "cron.add",
|
||||
params: {
|
||||
name: "auto run test",
|
||||
enabled: true,
|
||||
schedule: { kind: "at", atMs },
|
||||
sessionTarget: "main",
|
||||
|
||||
Reference in New Issue
Block a user