Merge pull request #1322 from KrauseFx/fix/cron-edit-preserve-delivery-on-message

Fix(cli): Preserve delivery settings when updating message via cron edit

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.

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);
+  });
 });

src/cli/cron-cli/register.cron-edit.ts

@@ -10,6 +10,15 @@ import {
   warnIfCronSchedulerDisabled,
 } from "./shared.js";
 
+const assignIf = (
+  target: Record<string, unknown>,
+  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<string, unknown> = { 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;
           }