diff --git a/scripts/e2e/cron-cli-docker.sh b/scripts/e2e/cron-cli-docker.sh index e2272385a1d4..eb49f2a078f7 100755 --- a/scripts/e2e/cron-cli-docker.sh +++ b/scripts/e2e/cron-cli-docker.sh @@ -63,6 +63,16 @@ dump_logs_on_error() { /tmp/cron-cli-edit-exact.json \ /tmp/cron-cli-edit-timeout.json \ /tmp/cron-cli-get-after-edit.json \ + /tmp/cron-cli-script-add.json \ + /tmp/cron-cli-script-before.json \ + /tmp/cron-cli-script-generic-timeout.log \ + /tmp/cron-cli-script-after-generic.json \ + /tmp/cron-cli-script-edit-timeout.json \ + /tmp/cron-cli-script-after-specific.json \ + /tmp/cron-cli-event-add.json \ + /tmp/cron-cli-event-before.json \ + /tmp/cron-cli-event-generic-timeout.log \ + /tmp/cron-cli-event-after.json \ /tmp/cron-cli-list.json \ /tmp/cron-cli-show.json \ /tmp/cron-cli-disable.json \ @@ -389,6 +399,80 @@ node --input-type=module -e ' } ' +cat > /tmp/cron-cli-script.js <<'SCRIPT' +return { notify: "cron script timeout proof" }; +SCRIPT +cron_cli add \ + "script timeout smoke" \ + --every 1h \ + --session isolated \ + --script /tmp/cron-cli-script.js \ + --script-timeout-seconds 15 \ + --no-deliver \ + --json > /tmp/cron-cli-script-add.json +script_job_id="$(read_json_field /tmp/cron-cli-script-add.json id)" +cron_cli get "$script_job_id" > /tmp/cron-cli-script-before.json +if cron_cli edit "$script_job_id" --timeout-seconds 30 > /tmp/cron-cli-script-generic-timeout.log 2>&1; then + echo "generic timeout unexpectedly succeeded for script job" >&2 + exit 1 +fi +grep -q -- "Use --script-timeout-seconds for script jobs" /tmp/cron-cli-script-generic-timeout.log +cron_cli get "$script_job_id" > /tmp/cron-cli-script-after-generic.json +cron_cli edit "$script_job_id" --script-timeout-seconds 30 > /tmp/cron-cli-script-edit-timeout.json +cron_cli get "$script_job_id" > /tmp/cron-cli-script-after-specific.json +node --input-type=module -e ' + const fs = await import("node:fs/promises"); + const before = JSON.parse(await fs.readFile("/tmp/cron-cli-script-before.json", "utf8")); + const afterGeneric = JSON.parse( + await fs.readFile("/tmp/cron-cli-script-after-generic.json", "utf8"), + ); + const afterSpecific = JSON.parse( + await fs.readFile("/tmp/cron-cli-script-after-specific.json", "utf8"), + ); + if (before.payload?.kind !== "script" || before.payload.timeoutSeconds !== 15) { + throw new Error(`script setup mismatch: ${JSON.stringify(before.payload)}`); + } + if (JSON.stringify(afterGeneric.payload) !== JSON.stringify(before.payload)) { + throw new Error( + `rejected generic timeout changed script payload: ${JSON.stringify(afterGeneric.payload)}`, + ); + } + if (afterSpecific.payload?.kind !== "script" || afterSpecific.payload.timeoutSeconds !== 30) { + throw new Error( + `script-specific timeout did not persist: ${JSON.stringify(afterSpecific.payload)}`, + ); + } +' +cron_cli rm "$script_job_id" --json >/dev/null + +cron_cli add \ + "event timeout smoke" \ + --every 1h \ + --session main \ + --system-event "cron event timeout proof" \ + --json > /tmp/cron-cli-event-add.json +event_job_id="$(read_json_field /tmp/cron-cli-event-add.json id)" +cron_cli get "$event_job_id" > /tmp/cron-cli-event-before.json +if cron_cli edit "$event_job_id" --timeout-seconds 30 > /tmp/cron-cli-event-generic-timeout.log 2>&1; then + echo "generic timeout unexpectedly succeeded for systemEvent job" >&2 + exit 1 +fi +grep -q -- "--timeout-seconds is not supported for systemEvent jobs" \ + /tmp/cron-cli-event-generic-timeout.log +cron_cli get "$event_job_id" > /tmp/cron-cli-event-after.json +node --input-type=module -e ' + const fs = await import("node:fs/promises"); + const before = JSON.parse(await fs.readFile("/tmp/cron-cli-event-before.json", "utf8")); + const after = JSON.parse(await fs.readFile("/tmp/cron-cli-event-after.json", "utf8")); + if (before.payload?.kind !== "systemEvent") { + throw new Error(`event setup mismatch: ${JSON.stringify(before.payload)}`); + } + if (JSON.stringify(after.payload) !== JSON.stringify(before.payload)) { + throw new Error(`rejected generic timeout changed event payload: ${JSON.stringify(after.payload)}`); + } +' +cron_cli rm "$event_job_id" --json >/dev/null + cron_cli list --all --json > /tmp/cron-cli-list.json node --input-type=module -e ' const fs = await import("node:fs/promises"); diff --git a/src/cli/cron-cli/register.cron-edit-options.ts b/src/cli/cron-cli/register.cron-edit-options.ts index fb952a7225a9..fa17a6de596a 100644 --- a/src/cli/cron-cli/register.cron-edit-options.ts +++ b/src/cli/cron-cli/register.cron-edit-options.ts @@ -137,10 +137,22 @@ export async function resolveCronEditPayloadDeliveryPatch( typeof opts.lightContext === "boolean"; const hasScriptSpecificPayloadField = Boolean(scriptPath) || scriptTimeoutSeconds !== undefined || scriptToolBudget !== undefined; + if (hasTimeoutSeconds && hasScriptSpecificPayloadField) { + throw new Error("Use --script-timeout-seconds for script jobs, not --timeout-seconds."); + } + if (hasTimeoutSeconds && hasSystemEventPatch) { + throw new Error("--timeout-seconds is not supported for systemEvent jobs."); + } let timeoutOnlyPayloadKind: "agentTurn" | "command" | undefined; if (hasTimeoutSeconds && !hasCommandSpecificPayloadField && !hasAgentTurnSpecificPayloadField) { - const existing = await loadExistingJob(); - timeoutOnlyPayloadKind = existing.payload.kind === "command" ? "command" : "agentTurn"; + const existingKind = (await loadExistingJob()).payload.kind; + if (existingKind === "script") { + throw new Error("Use --script-timeout-seconds for script jobs, not --timeout-seconds."); + } + if (existingKind === "systemEvent" || existingKind === "heartbeat") { + throw new Error(`--timeout-seconds is not supported for ${existingKind} jobs.`); + } + timeoutOnlyPayloadKind = existingKind; } let toolsOnlyPayloadKind: CronJob["payload"]["kind"] | undefined; if ( @@ -157,14 +169,11 @@ export async function resolveCronEditPayloadDeliveryPatch( } const hasAgentTurnPayloadField = hasAgentTurnSpecificPayloadField || - (hasTimeoutSeconds && - !hasCommandSpecificPayloadField && - timeoutOnlyPayloadKind !== "command") || + timeoutOnlyPayloadKind === "agentTurn" || (hasToolsAllowPatch && toolsOnlyPayloadKind === "agentTurn"); const hasCommandPayloadField = hasCommandSpecificPayloadField || - (hasTimeoutSeconds && - (hasCommandSpecificPayloadField || timeoutOnlyPayloadKind === "command")) || + timeoutOnlyPayloadKind === "command" || toolsOnlyPayloadKind === "command"; const hasAgentTurnPatch = hasAgentTurnPayloadField; const hasCommandPatch = hasCommandPayloadField; diff --git a/src/cli/cron-cli/register.cron-edit.test.ts b/src/cli/cron-cli/register.cron-edit.test.ts index 0dc0df54ef4b..2001ed72f1e8 100644 --- a/src/cli/cron-cli/register.cron-edit.test.ts +++ b/src/cli/cron-cli/register.cron-edit.test.ts @@ -260,13 +260,19 @@ describe("cron edit command", () => { }); }); - it("preserves command payload kind for timeout-only edits", async () => { + it.each([ + { + kind: "agentTurn", + payload: { kind: "agentTurn", message: "hello" }, + }, + { + kind: "command", + payload: { kind: "command", argv: ["sh", "-lc", "echo ok"] }, + }, + ])("preserves $kind payload kind for timeout-only edits", async ({ kind, payload }) => { callGatewayFromCli.mockImplementation(async (method: string) => { if (method === "cron.get") { - return { - id: "job-1", - payload: { kind: "command", argv: ["sh", "-lc", "echo ok"] }, - }; + return { id: "job-1", payload }; } return { ok: true }; }); @@ -285,7 +291,7 @@ describe("cron edit command", () => { id: "job-1", patch: { payload: { - kind: "command", + kind, timeoutSeconds: 12, }, }, @@ -293,6 +299,128 @@ describe("cron edit command", () => { ); }); + it.each([ + { + kind: "script", + payload: { kind: "script", script: "return { notify: 'hello' }", timeoutSeconds: 5 }, + error: "Use --script-timeout-seconds for script jobs", + }, + { + kind: "systemEvent", + payload: { kind: "systemEvent", text: "hello" }, + error: "--timeout-seconds is not supported for systemEvent jobs", + }, + { + kind: "heartbeat", + payload: { kind: "heartbeat" }, + error: "--timeout-seconds is not supported for heartbeat jobs", + }, + ])( + "rejects timeout-only edits for stored $kind payloads before cron.update", + async ({ payload, error }) => { + callGatewayFromCli.mockImplementation(async (method: string) => { + if (method === "cron.get") { + return { id: "job-1", payload }; + } + return { ok: true }; + }); + const errorSpy = vi.spyOn(defaultRuntime, "error").mockImplementation(() => {}); + const exitSpy = vi + .spyOn(defaultRuntime, "exit") + .mockImplementation((() => undefined) as never); + + try { + await createCronProgram().parseAsync(["edit", "job-1", "--timeout-seconds", "12"], { + from: "user", + }); + + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining(error)); + expect(callGatewayFromCli).toHaveBeenCalledWith("cron.get", expect.anything(), { + id: "job-1", + }); + expect(callGatewayFromCli.mock.calls.some(([method]) => method === "cron.update")).toBe( + false, + ); + } finally { + errorSpy.mockRestore(); + exitSpy.mockRestore(); + } + }, + ); + + it("rejects generic timeout combined with an explicit systemEvent before cron.update", async () => { + const errorSpy = vi.spyOn(defaultRuntime, "error").mockImplementation(() => {}); + const exitSpy = vi.spyOn(defaultRuntime, "exit").mockImplementation((() => undefined) as never); + + try { + await createCronProgram().parseAsync( + ["edit", "job-1", "--system-event", "hello", "--timeout-seconds", "12"], + { from: "user" }, + ); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining("--timeout-seconds is not supported for systemEvent jobs"), + ); + expect(callGatewayFromCli.mock.calls.some(([method]) => method === "cron.update")).toBe( + false, + ); + } finally { + errorSpy.mockRestore(); + exitSpy.mockRestore(); + } + }); + + it.each([ + ["--script", "missing-script.js"], + ["--script-tool-budget", "3"], + ["--script-timeout-seconds", "20"], + ])( + "rejects generic timeout combined with script option %s before cron.update", + async (flag, value) => { + const errorSpy = vi.spyOn(defaultRuntime, "error").mockImplementation(() => {}); + const exitSpy = vi + .spyOn(defaultRuntime, "exit") + .mockImplementation((() => undefined) as never); + + try { + await createCronProgram().parseAsync( + ["edit", "job-1", "--timeout-seconds", "12", flag, value], + { from: "user" }, + ); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining("Use --script-timeout-seconds for script jobs"), + ); + expect(callGatewayFromCli.mock.calls.some(([method]) => method === "cron.update")).toBe( + false, + ); + } finally { + errorSpy.mockRestore(); + exitSpy.mockRestore(); + } + }, + ); + + it("updates script timeout with the script-specific option", async () => { + await createCronProgram().parseAsync(["edit", "job-1", "--script-timeout-seconds", "20"], { + from: "user", + }); + + expect(callGatewayFromCli).toHaveBeenCalledWith( + "cron.update", + expect.objectContaining({ scriptTimeoutSeconds: "20" }), + { + id: "job-1", + patch: { + payload: { + kind: "script", + timeoutSeconds: 20, + }, + }, + }, + ); + }); + it("falls back to cron.list when an older Gateway does not support cron.get", async () => { const unknownMethodError = Object.assign(new Error("unknown method: cron.get"), { name: "GatewayClientRequestError",