From 1d2c8b16488d0b9d5d89025db0726aaf1380730d Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 26 May 2026 20:00:47 +0700 Subject: [PATCH] Fix MCP schedule cadence placeholder handling Treat undefined, blank, whitespace-only, and /__omit__ cadence values as absent at the MCP boundary for schedule create and update. That keeps placeholder inputs from tripping cadence validation while preserving the real invalid cases for conflicting or missing cadence values. --- .../src/server/agent/mcp-server.test.ts | 219 ++++++++++++++++++ .../server/src/server/agent/mcp-server.ts | 38 ++- 2 files changed, 248 insertions(+), 9 deletions(-) diff --git a/packages/server/src/server/agent/mcp-server.test.ts b/packages/server/src/server/agent/mcp-server.test.ts index 416cef14a..e12060d70 100644 --- a/packages/server/src/server/agent/mcp-server.test.ts +++ b/packages/server/src/server/agent/mcp-server.test.ts @@ -94,6 +94,15 @@ function registeredTool( return { ...tool, handler }; } +async function invokeToolWithParsedInput( + tool: RegisteredMcpToolWithHandler, + input: Record, +) { + const parsed = await tool.inputSchema.safeParseAsync(input); + expect(parsed.success).toBe(true); + return tool.handler(parsed.data); +} + function agentsOf(response: { structuredContent: LooseStructuredContent; }): Array> { @@ -1718,6 +1727,146 @@ describe("create_schedule MCP tool", () => { }), ); }); + + it("accepts blank cron placeholder when every is provided", async () => { + const { agentManager, agentStorage } = createTestDeps(); + const create = vi.fn(async (scheduleInput: CreateScheduleInput) => + createStoredSchedule(scheduleInput), + ); + const server = await createAgentMcpServer({ + agentManager, + agentStorage, + scheduleService: { create } as unknown as ScheduleService, + logger, + }); + const tool = registeredTool(server, "create_schedule"); + + await invokeToolWithParsedInput(tool, { + prompt: "say hello", + every: "10m", + cron: "", + provider: "codex", + }); + + expect(create).toHaveBeenCalledWith( + expect.objectContaining({ + cadence: { type: "every", everyMs: 600000 }, + }), + ); + }); + + it.each([ + { + label: "omit cron placeholder", + input: { prompt: "say hello", every: "10m", cron: "/__omit__", provider: "codex" }, + cadence: { type: "every", everyMs: 600000 }, + }, + { + label: "whitespace cron placeholder", + input: { prompt: "say hello", every: "10m", cron: " ", provider: "codex" }, + cadence: { type: "every", everyMs: 600000 }, + }, + { + label: "blank every placeholder for cron cadence", + input: { + prompt: "say hello", + every: "", + cron: "*/10 * * * *", + provider: "codex", + }, + cadence: { type: "cron", expression: "*/10 * * * *" }, + }, + { + label: "omit every placeholder for cron cadence", + input: { + prompt: "say hello", + every: "/__omit__", + cron: "*/10 * * * *", + provider: "codex", + }, + cadence: { type: "cron", expression: "*/10 * * * *" }, + }, + { + label: "whitespace every placeholder for cron cadence", + input: { + prompt: "say hello", + every: " ", + cron: "*/10 * * * *", + provider: "codex", + }, + cadence: { type: "cron", expression: "*/10 * * * *" }, + }, + ])("normalizes create_schedule cadence placeholder for $label", async ({ input, cadence }) => { + const { agentManager, agentStorage } = createTestDeps(); + const create = vi.fn(async (scheduleInput: CreateScheduleInput) => + createStoredSchedule(scheduleInput), + ); + const server = await createAgentMcpServer({ + agentManager, + agentStorage, + scheduleService: { create } as unknown as ScheduleService, + logger, + }); + const tool = registeredTool(server, "create_schedule"); + + await invokeToolWithParsedInput(tool, input); + + expect(create).toHaveBeenCalledWith( + expect.objectContaining({ + cadence, + }), + ); + }); + + it("still rejects both real every and cron inputs", async () => { + const { agentManager, agentStorage } = createTestDeps(); + const create = vi.fn(); + const server = await createAgentMcpServer({ + agentManager, + agentStorage, + scheduleService: { create } as unknown as ScheduleService, + logger, + }); + const tool = registeredTool(server, "create_schedule"); + + await expect( + invokeToolWithParsedInput(tool, { + prompt: "say hello", + every: "10m", + cron: "*/10 * * * *", + provider: "codex", + }), + ).rejects.toThrow("Specify exactly one of every or cron"); + + expect(create).not.toHaveBeenCalled(); + }); + + it.each([ + { + label: "missing both cadence fields", + input: { prompt: "say hello", provider: "codex" }, + }, + { + label: "placeholder-only cadence fields", + input: { prompt: "say hello", every: " ", cron: "/__omit__", provider: "codex" }, + }, + ])("still rejects create_schedule when $label", async ({ input }) => { + const { agentManager, agentStorage } = createTestDeps(); + const create = vi.fn(); + const server = await createAgentMcpServer({ + agentManager, + agentStorage, + scheduleService: { create } as unknown as ScheduleService, + logger, + }); + const tool = registeredTool(server, "create_schedule"); + + await expect(invokeToolWithParsedInput(tool, input)).rejects.toThrow( + "Specify exactly one of every or cron", + ); + + expect(create).not.toHaveBeenCalled(); + }); }); describe("update_schedule MCP tool", () => { @@ -1795,6 +1944,76 @@ describe("update_schedule MCP tool", () => { }); }); + it("accepts blank cron placeholder when updating every cadence", async () => { + const { agentManager, agentStorage } = createTestDeps(); + const stored = makeStoredSchedule(); + const update = vi.fn(async (_input: UpdateScheduleInput) => stored); + const server = await createAgentMcpServer({ + agentManager, + agentStorage, + scheduleService: { update } as unknown as ScheduleService, + logger, + }); + const tool = registeredTool(server, "update_schedule"); + + await invokeToolWithParsedInput(tool, { + id: "schedule-1", + every: "10m", + cron: "", + }); + + expect(update).toHaveBeenCalledWith({ + id: "schedule-1", + cadence: { type: "every", everyMs: 600000 }, + }); + }); + + it.each([ + { + label: "omit cron placeholder", + input: { id: "schedule-1", every: "10m", cron: "/__omit__" }, + cadence: { type: "every", everyMs: 600000 }, + }, + { + label: "whitespace cron placeholder", + input: { id: "schedule-1", every: "10m", cron: " " }, + cadence: { type: "every", everyMs: 600000 }, + }, + { + label: "blank every placeholder for cron cadence", + input: { id: "schedule-1", every: "", cron: "*/10 * * * *" }, + cadence: { type: "cron", expression: "*/10 * * * *" }, + }, + { + label: "omit every placeholder for cron cadence", + input: { id: "schedule-1", every: "/__omit__", cron: "*/10 * * * *" }, + cadence: { type: "cron", expression: "*/10 * * * *" }, + }, + { + label: "whitespace every placeholder for cron cadence", + input: { id: "schedule-1", every: " ", cron: "*/10 * * * *" }, + cadence: { type: "cron", expression: "*/10 * * * *" }, + }, + ])("normalizes update_schedule cadence placeholder for $label", async ({ input, cadence }) => { + const { agentManager, agentStorage } = createTestDeps(); + const stored = makeStoredSchedule(); + const update = vi.fn(async (_input: UpdateScheduleInput) => stored); + const server = await createAgentMcpServer({ + agentManager, + agentStorage, + scheduleService: { update } as unknown as ScheduleService, + logger, + }); + const tool = registeredTool(server, "update_schedule"); + + await invokeToolWithParsedInput(tool, input); + + expect(update).toHaveBeenCalledWith({ + id: "schedule-1", + cadence, + }); + }); + it("rejects both every and cron", async () => { const { agentManager, agentStorage } = createTestDeps(); const update = vi.fn(); diff --git a/packages/server/src/server/agent/mcp-server.ts b/packages/server/src/server/agent/mcp-server.ts index aab4bd8f0..5c97d0ff8 100644 --- a/packages/server/src/server/agent/mcp-server.ts +++ b/packages/server/src/server/agent/mcp-server.ts @@ -370,15 +370,31 @@ interface ScheduleUpdateToolInput { clearExpires?: boolean; } +function normalizeScheduleCadenceArg(value: string | undefined): string | undefined { + if (value === undefined) { + return undefined; + } + + const trimmed = value.trim(); + if (!trimmed || trimmed === "/__omit__") { + return undefined; + } + + return trimmed; +} + function resolveScheduleUpdateCadence(input: ScheduleUpdateToolInput): ScheduleCadence | undefined { - if (input.every !== undefined && input.cron !== undefined) { + const every = normalizeScheduleCadenceArg(input.every); + const cron = normalizeScheduleCadenceArg(input.cron); + + if (every !== undefined && cron !== undefined) { throw new Error("Specify at most one of every or cron"); } - if (input.every !== undefined) { - return { type: "every", everyMs: parseDurationString(input.every) }; + if (every !== undefined) { + return { type: "every", everyMs: parseDurationString(every) }; } - if (input.cron !== undefined) { - return { type: "cron", expression: input.cron.trim() }; + if (cron !== undefined) { + return { type: "cron", expression: cron }; } return undefined; } @@ -1779,7 +1795,10 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom throw new Error("Schedule service is not configured"); } - const cadenceCount = Number(every !== undefined) + Number(cron !== undefined); + const normalizedEvery = normalizeScheduleCadenceArg(every); + const normalizedCron = normalizeScheduleCadenceArg(cron); + const cadenceCount = + Number(normalizedEvery !== undefined) + Number(normalizedCron !== undefined); if (cadenceCount !== 1) { throw new Error("Specify exactly one of every or cron"); } @@ -1817,9 +1836,10 @@ export async function createAgentMcpServer(options: AgentMcpServerOptions): Prom const schedule = await scheduleService.create({ prompt: prompt.trim(), - cadence: every - ? { type: "every" as const, everyMs: parseDurationString(every) } - : { type: "cron" as const, expression: cron!.trim() }, + cadence: + normalizedEvery !== undefined + ? { type: "every" as const, everyMs: parseDurationString(normalizedEvery) } + : { type: "cron" as const, expression: normalizedCron! }, target: scheduleTarget, ...(name?.trim() ? { name: name.trim() } : {}), ...(maxRuns === undefined ? {} : { maxRuns }),