diff --git a/CHANGELOG.md b/CHANGELOG.md index a32e7de4f..fbc5c50ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,7 @@ Docs: https://docs.clawd.bot - Agents: avoid treating timeout errors with "aborted" messages as user aborts, so model fallback still runs. - Diagnostics: export OTLP logs, correct queue depth tracking, and document message-flow telemetry. - Diagnostics: emit message-flow diagnostics across channels via shared dispatch; gate heartbeat/webhook logging. (#1244) — thanks @oscargavin. +- CLI: preserve cron delivery settings when editing message payloads. (#1322) — thanks @KrauseFx. - Model catalog: avoid caching import failures, log transient discovery errors, and keep partial results. (#1332) — thanks @dougvk. - Doctor: clarify plugin auto-enable hint text in the startup banner. - Gateway: clarify unauthorized handshake responses with token/password mismatch guidance. diff --git a/src/cli/cron-cli.test.ts b/src/cli/cron-cli.test.ts index 2bb5d9fc6..cefc030b1 100644 --- a/src/cli/cron-cli.test.ts +++ b/src/cli/cron-cli.test.ts @@ -246,4 +246,128 @@ describe("cron cli", () => { expect(patch?.patch?.payload?.kind).toBe("agentTurn"); expect(patch?.patch?.payload?.deliver).toBe(false); }); + + it("does not include undefined delivery fields when updating message", async () => { + callGatewayFromCli.mockClear(); + + const { registerCronCli } = await import("./cron-cli.js"); + const program = new Command(); + program.exitOverride(); + registerCronCli(program); + + // Update message without delivery flags - should NOT include undefined delivery fields + await program.parseAsync(["cron", "edit", "job-1", "--message", "Updated message"], { + from: "user", + }); + + const updateCall = callGatewayFromCli.mock.calls.find((call) => call[0] === "cron.update"); + const patch = updateCall?.[2] as { + patch?: { + payload?: { + message?: string; + deliver?: boolean; + channel?: string; + to?: string; + bestEffortDeliver?: boolean; + }; + }; + }; + + // Should include the new message + expect(patch?.patch?.payload?.message).toBe("Updated message"); + + // Should NOT include delivery fields at all (to preserve existing values) + expect(patch?.patch?.payload).not.toHaveProperty("deliver"); + expect(patch?.patch?.payload).not.toHaveProperty("channel"); + expect(patch?.patch?.payload).not.toHaveProperty("to"); + expect(patch?.patch?.payload).not.toHaveProperty("bestEffortDeliver"); + }); + + it("includes delivery fields when explicitly provided with message", async () => { + callGatewayFromCli.mockClear(); + + const { registerCronCli } = await import("./cron-cli.js"); + const program = new Command(); + program.exitOverride(); + registerCronCli(program); + + // Update message AND delivery - should include both + await program.parseAsync( + [ + "cron", + "edit", + "job-1", + "--message", + "Updated message", + "--deliver", + "--channel", + "telegram", + "--to", + "19098680", + ], + { from: "user" }, + ); + + const updateCall = callGatewayFromCli.mock.calls.find((call) => call[0] === "cron.update"); + const patch = updateCall?.[2] as { + patch?: { + payload?: { + message?: string; + deliver?: boolean; + channel?: string; + to?: string; + }; + }; + }; + + // Should include everything + expect(patch?.patch?.payload?.message).toBe("Updated message"); + expect(patch?.patch?.payload?.deliver).toBe(true); + expect(patch?.patch?.payload?.channel).toBe("telegram"); + expect(patch?.patch?.payload?.to).toBe("19098680"); + }); + + it("includes best-effort delivery when provided with message", async () => { + callGatewayFromCli.mockClear(); + + const { registerCronCli } = await import("./cron-cli.js"); + const program = new Command(); + program.exitOverride(); + registerCronCli(program); + + await program.parseAsync( + ["cron", "edit", "job-1", "--message", "Updated message", "--best-effort-deliver"], + { from: "user" }, + ); + + const updateCall = callGatewayFromCli.mock.calls.find((call) => call[0] === "cron.update"); + const patch = updateCall?.[2] as { + patch?: { payload?: { message?: string; bestEffortDeliver?: boolean } }; + }; + + expect(patch?.patch?.payload?.message).toBe("Updated message"); + expect(patch?.patch?.payload?.bestEffortDeliver).toBe(true); + }); + + it("includes no-best-effort delivery when provided with message", async () => { + callGatewayFromCli.mockClear(); + + const { registerCronCli } = await import("./cron-cli.js"); + const program = new Command(); + program.exitOverride(); + registerCronCli(program); + + await program.parseAsync( + ["cron", "edit", "job-1", "--message", "Updated message", "--no-best-effort-deliver"], + { from: "user" }, + ); + + const updateCall = callGatewayFromCli.mock.calls.find((call) => call[0] === "cron.update"); + const patch = updateCall?.[2] as { + patch?: { payload?: { message?: string; bestEffortDeliver?: boolean } }; + }; + + expect(patch?.patch?.payload?.message).toBe("Updated message"); + expect(patch?.patch?.payload?.bestEffortDeliver).toBe(false); + }); }); diff --git a/src/cli/cron-cli/register.cron-edit.ts b/src/cli/cron-cli/register.cron-edit.ts index d051ff2e8..3b50fc3f5 100644 --- a/src/cli/cron-cli/register.cron-edit.ts +++ b/src/cli/cron-cli/register.cron-edit.ts @@ -10,6 +10,15 @@ import { warnIfCronSchedulerDisabled, } from "./shared.js"; +const assignIf = ( + target: Record, + key: string, + value: unknown, + shouldAssign: boolean, +) => { + if (shouldAssign) target[key] = value; +}; + export function registerCronEditCommand(cron: Command) { addGatewayClientOptions( cron @@ -136,18 +145,19 @@ export function registerCronEditCommand(cron: Command) { }; } else if (hasAgentTurnPatch) { const payload: Record = { kind: "agentTurn" }; - if (typeof opts.message === "string") payload.message = String(opts.message); - if (model) payload.model = model; - if (thinking) payload.thinking = thinking; - if (hasTimeoutSeconds) { - payload.timeoutSeconds = timeoutSeconds; - } - if (typeof opts.deliver === "boolean") payload.deliver = opts.deliver; - if (typeof opts.channel === "string") payload.channel = opts.channel; - if (typeof opts.to === "string") payload.to = opts.to; - if (typeof opts.bestEffortDeliver === "boolean") { - payload.bestEffortDeliver = opts.bestEffortDeliver; - } + assignIf(payload, "message", String(opts.message), typeof opts.message === "string"); + assignIf(payload, "model", model, Boolean(model)); + assignIf(payload, "thinking", thinking, Boolean(thinking)); + assignIf(payload, "timeoutSeconds", timeoutSeconds, hasTimeoutSeconds); + assignIf(payload, "deliver", opts.deliver, typeof opts.deliver === "boolean"); + assignIf(payload, "channel", opts.channel, typeof opts.channel === "string"); + assignIf(payload, "to", opts.to, typeof opts.to === "string"); + assignIf( + payload, + "bestEffortDeliver", + opts.bestEffortDeliver, + typeof opts.bestEffortDeliver === "boolean", + ); patch.payload = payload; }