diff --git a/docs/automation/cron-jobs.md b/docs/automation/cron-jobs.md index 1fd50dae2583..2e14a545b122 100644 --- a/docs/automation/cron-jobs.md +++ b/docs/automation/cron-jobs.md @@ -169,6 +169,9 @@ If stdout is non-empty, that text is the delivered result. If stdout is empty an Thinking level override. + + On `cron edit`, removes the per-job thinking override so the job follows normal cron thinking precedence. Cannot be combined with `--thinking`. + Skip workspace bootstrap file injection. diff --git a/docs/cli/cron.md b/docs/cli/cron.md index 115c7b5be2b3..cc192e1caf94 100644 --- a/docs/cli/cron.md +++ b/docs/cli/cron.md @@ -170,7 +170,7 @@ Use `--due` when you want the manual command to run only if the job is currently ## Models -`cron add|edit --model ` selects an allowed model for the job. `cron add|edit --fallbacks ` sets per-job fallback models, for example `--fallbacks openrouter/gpt-4.1-mini,openai/gpt-5`; pass `--fallbacks ""` for a strict run with no fallbacks. `cron edit --clear-fallbacks` removes the per-job fallback override. `cron edit --clear-model` removes the per-job model override so the job follows normal cron model-selection precedence (a stored cron-session override if present, otherwise the agent/default model); it cannot be combined with `--model`. +`cron add|edit --model ` selects an allowed model for the job. `cron add|edit --fallbacks ` sets per-job fallback models, for example `--fallbacks openrouter/gpt-4.1-mini,openai/gpt-5`; pass `--fallbacks ""` for a strict run with no fallbacks. `cron edit --clear-fallbacks` removes the per-job fallback override. `cron edit --clear-model` removes the per-job model override so the job follows normal cron model-selection precedence (a stored cron-session override if present, otherwise the agent/default model); it cannot be combined with `--model`. `cron add|edit --thinking ` sets a per-job thinking override; `cron edit --clear-thinking` removes it so the job follows normal cron thinking precedence, and it cannot be combined with `--thinking`. If the model is not allowed or cannot be resolved, cron fails the run with an explicit validation error instead of falling back to the job's agent or default model selection. diff --git a/packages/gateway-protocol/src/schema/cron.ts b/packages/gateway-protocol/src/schema/cron.ts index 9296d1860f3f..4f191e027079 100644 --- a/packages/gateway-protocol/src/schema/cron.ts +++ b/packages/gateway-protocol/src/schema/cron.ts @@ -15,6 +15,7 @@ function cronAgentTurnPayloadSchema(params: { model: TSchema; fallbacks: TSchema; toolsAllow: TSchema; + thinking: TSchema; }) { return Type.Object( { @@ -22,7 +23,7 @@ function cronAgentTurnPayloadSchema(params: { message: params.message, model: Type.Optional(params.model), fallbacks: Type.Optional(params.fallbacks), - thinking: Type.Optional(Type.String()), + thinking: Type.Optional(params.thinking), timeoutSeconds: Type.Optional(Type.Number({ minimum: 0 })), allowUnsafeExternalContent: Type.Optional(Type.Boolean()), lightContext: Type.Optional(Type.Boolean()), @@ -238,6 +239,7 @@ export const CronPayloadSchema = Type.Union([ model: Type.String(), fallbacks: Type.Array(Type.String()), toolsAllow: Type.Array(Type.String()), + thinking: Type.String(), }), cronCommandPayloadSchema({ argv: Type.Array(NonEmptyString, { minItems: 1 }), @@ -258,6 +260,7 @@ export const CronPayloadPatchSchema = Type.Union([ model: Type.Union([Type.String(), Type.Null()]), fallbacks: Type.Union([Type.Array(Type.String()), Type.Null()]), toolsAllow: Type.Union([Type.Array(Type.String()), Type.Null()]), + thinking: Type.Union([Type.String(), Type.Null()]), }), cronCommandPayloadSchema({ argv: Type.Optional(Type.Array(NonEmptyString, { minItems: 1 })), diff --git a/src/cli/cron-cli/register.cron-edit.test.ts b/src/cli/cron-cli/register.cron-edit.test.ts index ebcd48a612a4..56a9b48b99c0 100644 --- a/src/cli/cron-cli/register.cron-edit.test.ts +++ b/src/cli/cron-cli/register.cron-edit.test.ts @@ -222,11 +222,50 @@ describe("cron edit command", () => { ); }); + it("clears the thinking override with --clear-thinking (CLI parity with cron.update thinking:null)", async () => { + const program = createCronProgram(); + + await program.parseAsync(["edit", "job-1", "--clear-thinking"], { from: "user" }); + + expect(callGatewayFromCli).toHaveBeenCalledWith( + "cron.update", + expect.objectContaining({ clearThinking: true }), + { + id: "job-1", + patch: { + payload: { + kind: "agentTurn", + thinking: null, + }, + }, + }, + ); + }); + + it("rejects combining --thinking with --clear-thinking", async () => { + const errorSpy = vi.spyOn(defaultRuntime, "error").mockImplementation(() => {}); + const exitSpy = vi.spyOn(defaultRuntime, "exit").mockImplementation((() => undefined) as never); + const program = createCronProgram(); + + await program.parseAsync(["edit", "job-1", "--thinking", "high", "--clear-thinking"], { + from: "user", + }); + + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining("Use --thinking or --clear-thinking, not both"), + ); + expect(callGatewayFromCli).not.toHaveBeenCalled(); + + errorSpy.mockRestore(); + exitSpy.mockRestore(); + }); + it("documents the --clear-model flag alongside the sibling --clear-tools", () => { const editCommand = createCronProgram().commands.find((command) => command.name() === "edit"); const help = editCommand?.helpInformation() ?? ""; expect(help).toContain("--clear-model"); + expect(help).toContain("--clear-thinking"); expect(help).toContain("--clear-tools"); }); diff --git a/src/cli/cron-cli/register.cron-edit.ts b/src/cli/cron-cli/register.cron-edit.ts index 2e7a34592c41..79298720d5f9 100644 --- a/src/cli/cron-cli/register.cron-edit.ts +++ b/src/cli/cron-cli/register.cron-edit.ts @@ -112,6 +112,11 @@ export function registerCronEditCommand(cron: Command) { "--thinking ", "Thinking level for agent jobs (off|minimal|low|medium|high|xhigh)", ) + .option( + "--clear-thinking", + "Remove the per-job thinking override (restore normal cron thinking precedence)", + false, + ) .option("--model ", "Model override for agent jobs") .option("--fallbacks ", "Fallback model list for agent jobs") .option("--clear-fallbacks", "Remove per-job fallback override", false) @@ -292,6 +297,9 @@ export function registerCronEditCommand(cron: Command) { throw new Error("Use --model or --clear-model, not both"); } const thinking = normalizeOptionalString(opts.thinking); + if (thinking && opts.clearThinking) { + throw new Error("Use --thinking or --clear-thinking, not both"); + } const fallbacks = parseCronFallbacks(opts.fallbacks); if (typeof opts.fallbacks === "string" && opts.clearFallbacks) { throw new Error("Use --fallbacks or --clear-fallbacks, not both"); @@ -370,6 +378,7 @@ export function registerCronEditCommand(cron: Command) { typeof opts.fallbacks !== "string" && !opts.clearFallbacks && !thinking && + !opts.clearThinking && typeof opts.lightContext !== "boolean" && typeof opts.tools !== "string" && !Array.isArray(opts.tools) && @@ -385,6 +394,7 @@ export function registerCronEditCommand(cron: Command) { typeof opts.fallbacks === "string" || Boolean(opts.clearFallbacks) || Boolean(thinking) || + Boolean(opts.clearThinking) || (hasTimeoutSeconds && !hasCommandSpecificPayloadField && timeoutOnlyPayloadKind !== "command") || @@ -418,7 +428,11 @@ export function registerCronEditCommand(cron: Command) { } assignIf(payload, "fallbacks", fallbacks, typeof opts.fallbacks === "string"); assignIf(payload, "fallbacks", null, Boolean(opts.clearFallbacks)); - assignIf(payload, "thinking", thinking, Boolean(thinking)); + if (opts.clearThinking) { + payload.thinking = null; + } else { + assignIf(payload, "thinking", thinking, Boolean(thinking)); + } assignIf(payload, "timeoutSeconds", timeoutSeconds, hasTimeoutSeconds); assignIf( payload, diff --git a/src/cron/normalize.test.ts b/src/cron/normalize.test.ts index d7cabd3233ef..1e419b817f76 100644 --- a/src/cron/normalize.test.ts +++ b/src/cron/normalize.test.ts @@ -154,6 +154,20 @@ describe("normalizeCronJobCreate", () => { expect(normalized.payload?.model).toBeNull(); }); + it("preserves explicit null thinking clear in payload patches", () => { + const normalized = normalizeCronJobPatch({ + payload: { + kind: "agentTurn", + thinking: null, + }, + }) as unknown as Record; + + const payload = normalized.payload as Record; + expect(payload.kind).toBe("agentTurn"); + expect(payload.thinking).toBeNull(); + expect(validateCronUpdateParams({ id: "job-1", patch: normalized })).toBe(true); + }); + it("coerces ISO schedule.at to normalized ISO (UTC)", () => { expectNormalizedAtSchedule({ kind: "at", at: "2026-01-12T18:00:00" }); }); diff --git a/src/cron/normalize.ts b/src/cron/normalize.ts index 98ab1a72f107..63210dc7b5d1 100644 --- a/src/cron/normalize.ts +++ b/src/cron/normalize.ts @@ -194,11 +194,17 @@ function coercePayload(payload: UnknownRecord) { } } if ("thinking" in next) { - const thinking = parseOptionalField(TrimmedNonEmptyStringFieldSchema, next.thinking); - if (thinking !== undefined) { - next.thinking = thinking; + // Preserve an explicit null so patches can clear a stored thinking override, + // matching the model/fallbacks/toolsAllow clear paths. + if (next.thinking === null) { + next.thinking = null; } else { - delete next.thinking; + const thinking = parseOptionalField(TrimmedNonEmptyStringFieldSchema, next.thinking); + if (thinking !== undefined) { + next.thinking = thinking; + } else { + delete next.thinking; + } } } if ("timeoutSeconds" in next) { diff --git a/src/cron/service.jobs.test.ts b/src/cron/service.jobs.test.ts index 257f7f0251d7..ce5017092762 100644 --- a/src/cron/service.jobs.test.ts +++ b/src/cron/service.jobs.test.ts @@ -520,6 +520,75 @@ describe("applyJobPatch", () => { } }); + it("persists agentTurn payload.thinking updates when editing existing jobs", () => { + const job = createIsolatedAgentTurnJob("job-thinking", { + mode: "announce", + channel: "telegram", + }); + job.payload = { + kind: "agentTurn", + message: "do it", + thinking: "high", + }; + + applyJobPatch(job, { + payload: { + kind: "agentTurn", + message: "do it", + thinking: "low", + }, + }); + + expect(job.payload.kind).toBe("agentTurn"); + if (job.payload.kind === "agentTurn") { + expect(job.payload.thinking).toBe("low"); + } + }); + + it("clears agentTurn payload.thinking when patch requests null", () => { + const job = createIsolatedAgentTurnJob("job-thinking-clear", { + mode: "announce", + channel: "telegram", + }); + job.payload = { + kind: "agentTurn", + message: "do it", + thinking: "high", + }; + + applyJobPatch(job, { + payload: { + kind: "agentTurn", + thinking: null, + }, + }); + + expect(job.payload.kind).toBe("agentTurn"); + if (job.payload.kind === "agentTurn") { + expect(job.payload.message).toBe("do it"); + expect(job.payload.thinking).toBeUndefined(); + } + }); + + it("omits null thinking when patch builds a replacement agentTurn payload", () => { + const job = createMainSystemEventJob("job-thinking-replace", { mode: "none" }); + + applyJobPatch(job, { + sessionTarget: "isolated", + payload: { + kind: "agentTurn", + message: "do it", + thinking: null, + }, + }); + + expect(job.payload.kind).toBe("agentTurn"); + if (job.payload.kind === "agentTurn") { + expect(job.payload.message).toBe("do it"); + expect(job.payload.thinking).toBeUndefined(); + } + }); + it("applies payload.lightContext when replacing payload kind via patch", () => { const job = createIsolatedAgentTurnJob("job-light-context-switch", { mode: "announce", diff --git a/src/cron/service/jobs.ts b/src/cron/service/jobs.ts index 0de5d1e47209..509e8d19f3c4 100644 --- a/src/cron/service/jobs.ts +++ b/src/cron/service/jobs.ts @@ -1005,6 +1005,8 @@ function mergeCronPayload(existing: CronPayload, patch: CronPayloadPatch): CronP applyAgentTurnToolsAllowPatch(next, patch, existing); if (typeof patch.thinking === "string") { next.thinking = patch.thinking; + } else if (patch.thinking === null) { + delete next.thinking; } if (typeof patch.timeoutSeconds === "number") { next.timeoutSeconds = patch.timeoutSeconds; @@ -1051,7 +1053,7 @@ function buildPayloadFromPatch(patch: CronPayloadPatch): CronPayload { message: patch.message, model: typeof patch.model === "string" ? patch.model : undefined, fallbacks: Array.isArray(patch.fallbacks) ? patch.fallbacks : undefined, - thinking: patch.thinking, + thinking: typeof patch.thinking === "string" ? patch.thinking : undefined, timeoutSeconds: patch.timeoutSeconds, lightContext: patch.lightContext, allowUnsafeExternalContent: patch.allowUnsafeExternalContent, diff --git a/src/cron/types.ts b/src/cron/types.ts index df132805a6cb..b61a167b7b7f 100644 --- a/src/cron/types.ts +++ b/src/cron/types.ts @@ -258,10 +258,11 @@ type CronAgentTurnPayload = { type CronAgentTurnPayloadPatch = { kind: "agentTurn"; -} & Partial> & { +} & Partial> & { model?: string | null; fallbacks?: string[] | null; toolsAllow?: string[] | null; + thinking?: string | null; }; type CronCommandPayloadFields = { diff --git a/ui/src/ui/controllers/cron.test.ts b/ui/src/ui/controllers/cron.test.ts index 37017e7eca83..83d4d5f8a28f 100644 --- a/ui/src/ui/controllers/cron.test.ts +++ b/ui/src/ui/controllers/cron.test.ts @@ -405,6 +405,108 @@ describe("cron controller", () => { }); }); + it("sends explicit null model/thinking clears when blanking stored overrides on edit", async () => { + const request = vi.fn(async (method: string, _payload?: unknown) => { + if (method === "cron.update") { + return { id: "job-clear-overrides" }; + } + if (method === "cron.list") { + return { jobs: [{ id: "job-clear-overrides" }] }; + } + if (method === "cron.status") { + return { enabled: true, jobs: 1, nextWakeAtMs: null }; + } + return {}; + }); + + const state = createState({ + client: { + request, + } as unknown as CronState["client"], + cronEditingJobId: "job-clear-overrides", + cronJobs: [ + { + id: "job-clear-overrides", + payload: { + kind: "agentTurn", + message: "do work", + model: "openai/gpt-5.5", + thinking: "high", + }, + } as unknown as CronState["cronJobs"][number], + ], + cronForm: { + ...DEFAULT_CRON_FORM, + name: "clear overrides", + scheduleKind: "every", + everyAmount: "30", + everyUnit: "minutes", + sessionTarget: "isolated", + wakeMode: "next-heartbeat", + payloadKind: "agentTurn", + payloadText: "do work", + payloadModel: "", + payloadThinking: "", + }, + }); + + await addCronJob(state); + + const updateCall = findRequestCall(request.mock.calls, "cron.update"); + expectNestedRecordFields(requestPatch(updateCall), "payload", { + kind: "agentTurn", + message: "do work", + model: null, + thinking: null, + }); + }); + + it("does not send null model/thinking for a new job with blank fields", async () => { + const request = vi.fn(async (method: string, _payload?: unknown) => { + if (method === "cron.add") { + return { id: "job-new-blank" }; + } + if (method === "cron.list") { + return { jobs: [{ id: "job-new-blank" }] }; + } + if (method === "cron.status") { + return { enabled: true, jobs: 1, nextWakeAtMs: null }; + } + return {}; + }); + + const state = createState({ + client: { + request, + } as unknown as CronState["client"], + cronForm: { + ...DEFAULT_CRON_FORM, + name: "new blank", + scheduleKind: "every", + everyAmount: "30", + everyUnit: "minutes", + sessionTarget: "isolated", + wakeMode: "next-heartbeat", + payloadKind: "agentTurn", + payloadText: "do work", + payloadModel: "", + payloadThinking: "", + }, + }); + + await addCronJob(state); + + const addCall = findRequestCall(request.mock.calls, "cron.add"); + // A new job never had a stored override, so a blank field stays omitted + // (no explicit null clear) rather than being mistaken for a cleared value. + expectNestedRecordFields(requestPayload(addCall), "payload", { + kind: "agentTurn", + message: "do work", + model: undefined, + thinking: undefined, + }); + }); + it("does not submit stale announce delivery when unsupported", async () => { const request = vi.fn(async (method: string, _payload?: unknown) => { if (method === "cron.add") { diff --git a/ui/src/ui/controllers/cron.ts b/ui/src/ui/controllers/cron.ts index 56da1d4970f5..a7935ca96c12 100644 --- a/ui/src/ui/controllers/cron.ts +++ b/ui/src/ui/controllers/cron.ts @@ -631,8 +631,8 @@ function buildCronPayload(form: CronFormState) { const payload: { kind: "agentTurn"; message: string; - model?: string; - thinking?: string; + model?: string | null; + thinking?: string | null; timeoutSeconds?: number; lightContext?: boolean; } = { kind: "agentTurn", message }; @@ -721,14 +721,22 @@ export async function addCronJob(state: CronState): Promise { state.cronEditingJobId && form.payloadLocked && editingPayload?.kind === "command", ); const payload = preserveLockedPayload ? undefined : buildCronPayload(form); - if (payload?.kind === "agentTurn") { - const existingLightContext = - editingPayload?.kind === "agentTurn" ? editingPayload.lightContext : undefined; - if ( - !form.payloadLightContext && - state.cronEditingJobId && - existingLightContext !== undefined - ) { + if ( + payload?.kind === "agentTurn" && + state.cronEditingJobId && + editingPayload?.kind === "agentTurn" + ) { + // When editing, a blanked field that previously held a stored override must + // send an explicit clear; an omitted key means "leave unchanged" on merge. + // The form only shows stored overrides (not inherited defaults), so a blank + // input with a stored value is an intentional clear. + if (!form.payloadModel.trim() && editingPayload.model !== undefined) { + payload.model = null; + } + if (!form.payloadThinking.trim() && editingPayload.thinking !== undefined) { + payload.thinking = null; + } + if (!form.payloadLightContext && editingPayload.lightContext !== undefined) { payload.lightContext = false; } }