mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-12 21:53:00 -06:00
fix(cron): clear agentTurn thinking override by blanking the field (#96293)
* fix(cron): clear agentTurn thinking override when patched with null Cron agentTurn patches could clear model/fallbacks/toolsAllow overrides by sending an explicit null, but thinking had no clear path: the patch schema and normalizer dropped thinking:null before it reached the merge logic, and the payload merge only handled string values. Blanking the Thinking/Effort field in the Cron Control UI therefore silently preserved the old value. Add thinking:null support across the patch schema, exported type, normalizer, and payload merge (mirroring model). The Control UI now sends an explicit clear for model/thinking when an edited job blanks a previously stored override, and the CLI gains --clear-thinking for parity with --clear-model. * docs(cron): document --clear-thinking beside sibling clear flags
This commit is contained in:
@@ -169,6 +169,9 @@ If stdout is non-empty, that text is the delivered result. If stdout is empty an
|
||||
<ParamField path="--thinking" type="string">
|
||||
Thinking level override.
|
||||
</ParamField>
|
||||
<ParamField path="--clear-thinking" type="boolean">
|
||||
On `cron edit`, removes the per-job thinking override so the job follows normal cron thinking precedence. Cannot be combined with `--thinking`.
|
||||
</ParamField>
|
||||
<ParamField path="--light-context" type="boolean">
|
||||
Skip workspace bootstrap file injection.
|
||||
</ParamField>
|
||||
|
||||
+1
-1
@@ -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 <ref>` selects an allowed model for the job. `cron add|edit --fallbacks <list>` 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 <job-id> --clear-fallbacks` removes the per-job fallback override. `cron edit <job-id> --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 <ref>` selects an allowed model for the job. `cron add|edit --fallbacks <list>` 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 <job-id> --clear-fallbacks` removes the per-job fallback override. `cron edit <job-id> --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 <level>` sets a per-job thinking override; `cron edit <job-id> --clear-thinking` removes it so the job follows normal cron thinking precedence, and it cannot be combined with `--thinking`.
|
||||
|
||||
<Warning>
|
||||
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.
|
||||
|
||||
@@ -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 })),
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
|
||||
|
||||
@@ -112,6 +112,11 @@ export function registerCronEditCommand(cron: Command) {
|
||||
"--thinking <level>",
|
||||
"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>", "Model override for agent jobs")
|
||||
.option("--fallbacks <list>", "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,
|
||||
|
||||
@@ -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<string, unknown>;
|
||||
|
||||
const payload = normalized.payload as Record<string, unknown>;
|
||||
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" });
|
||||
});
|
||||
|
||||
+10
-4
@@ -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) {
|
||||
|
||||
@@ -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",
|
||||
|
||||
@@ -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,
|
||||
|
||||
+2
-1
@@ -258,10 +258,11 @@ type CronAgentTurnPayload = {
|
||||
|
||||
type CronAgentTurnPayloadPatch = {
|
||||
kind: "agentTurn";
|
||||
} & Partial<Omit<CronAgentTurnPayloadFields, "model" | "fallbacks" | "toolsAllow">> & {
|
||||
} & Partial<Omit<CronAgentTurnPayloadFields, "model" | "fallbacks" | "toolsAllow" | "thinking">> & {
|
||||
model?: string | null;
|
||||
fallbacks?: string[] | null;
|
||||
toolsAllow?: string[] | null;
|
||||
thinking?: string | null;
|
||||
};
|
||||
|
||||
type CronCommandPayloadFields = {
|
||||
|
||||
@@ -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") {
|
||||
|
||||
@@ -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<boolean> {
|
||||
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;
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user