fix(desktop): send complete respond_to wire patch on every allowlist transition

desktop/src/features/agents/ui/AgentInstanceEditDialog.tsx

@@ -49,6 +49,7 @@ import {
 } from "./relayMeshModelPicker";
 import {
   computeEditAgentFormValidity,
+  computeRespondToWirePatch,
   envVarsEqual,
   isEditAgentProviderSaveValid,
   resolveAgentCommandUpdate,
@@ -711,18 +712,18 @@ export function AgentInstanceEditDialog({
         envVars: envVarsEqual(submitEnvVars, agent.envVars)
           ? undefined
           : submitEnvVars,
-        respondTo: respondTo !== agent.respondTo ? respondTo : undefined,
-        // The allowlist is preserved across mode toggles in local UI state
-        // (so a user can flip away from allowlist and back without losing
-        // their entries), but we only send it on the wire when (a) it
-        // actually changed, AND (b) the saved mode will need it. Sending
-        // an allowlist while switching to a non-allowlist mode would be
-        // harmless server-side, but it's noise in the persisted record.
-        respondToAllowlist:
-          respondTo === "allowlist" &&
-          respondToAllowlist.join(",") !== agent.respondToAllowlist.join(",")
-            ? respondToAllowlist
-            : undefined,
+        // The respond-to pair is computed via the canonical wire-shape helper
+        // so every transition shape — mode flip, payload edit, or both —
+        // produces the exact patch the harness layer consumes on the local
+        // record (#2501). The helper owns the "flip to allowlist must re-send
+        // the payload even when unchanged" contract; see
+        // `computeRespondToWirePatch` for the full semantics.
+        ...computeRespondToWirePatch({
+          currentMode: agent.respondTo,
+          currentAllowlist: agent.respondToAllowlist,
+          submitMode: respondTo,
+          submitAllowlist: respondToAllowlist,
+        }),
       };
 
       const result = await updateMutation.mutateAsync(input);

desktop/src/features/agents/ui/agentInstanceEditRespondToWire.test.mjs

@@ -0,0 +1,92 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { computeRespondToWirePatch } from "./personaRuntimeModel.ts";
+
+// ── Submit-shape wire contract for the agent-instance edit dialog ────────────
+//
+// Issue #2501: the Desktop edit dialog's allowlist wire condition conflates
+// "mode changed" with "payload changed", so several mode/allowlist transition
+// shapes silently drop the payload the harness layer actually needs. The
+// harness reads from `respond_to` + `respond_to_allowlist` on the local record
+// ( not the definition snapshot projection), and `update_managed_agent` merges
+// `undefined` allowlist as "leave unchanged" — so a mode flip WITHOUT a
+// re-sent payload can leave the record in an unreachable or crash-looping
+// state. These tests pin the exact wire patch each transition must produce.
+
+const PUBKEY_A = "a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2";
+const PUBKEY_B = "b1b2c3d4e5f60718293a4b5c6d7e8f90a1b2c3d4e5f60718293a4b5c6d7e8f90a1b2";
+
+test("owner-only → allowlist with fresh list sends mode AND allowlist", () => {
+  // The reporter's primary flow: pick Allowlist, add a pubkey, Save.
+  const patch = computeRespondToWirePatch({
+    currentMode: "owner-only",
+    currentAllowlist: [],
+    submitMode: "allowlist",
+    submitAllowlist: [PUBKEY_A],
+  });
+  assert.equal(patch.respondTo, "allowlist");
+  assert.deepEqual(patch.respondToAllowlist, [PUBKEY_A]);
+});
+
+test("allowlist → allowlist editing the list sends updated allowlist", () => {
+  // Mode unchanged, list edited — the payload must go on the wire.
+  const patch = computeRespondToWirePatch({
+    currentMode: "allowlist",
+    currentAllowlist: [PUBKEY_A],
+    submitMode: "allowlist",
+    submitAllowlist: [PUBKEY_B],
+  });
+  assert.equal(patch.respondTo, undefined);
+  assert.deepEqual(patch.respondToAllowlist, [PUBKEY_B]);
+});
+
+test("allowlist → anyone omits allowlist (record keeps stale list by design)", () => {
+  // Flipping away from allowlist: the Rust update preserves the persisted
+  // list across mode toggles, so we don't send it — but we must send the mode.
+  const patch = computeRespondToWirePatch({
+    currentMode: "allowlist",
+    currentAllowlist: [PUBKEY_A],
+    submitMode: "anyone",
+    submitAllowlist: [PUBKEY_A],
+  });
+  assert.equal(patch.respondTo, "anyone");
+  assert.equal(patch.respondToAllowlist, undefined);
+});
+
+test("owner-only → owner-only (no-op) sends nothing", () => {
+  const patch = computeRespondToWirePatch({
+    currentMode: "owner-only",
+    currentAllowlist: [],
+    submitMode: "owner-only",
+    submitAllowlist: [],
+  });
+  assert.equal(patch.respondTo, undefined);
+  assert.equal(patch.respondToAllowlist, undefined);
+});
+
+test("anyone → allowlist with pre-existing list sends mode AND list", () => {
+  // Mode flip to allowlist where the list was already populated by an earlier
+  // session (the "Anyone → Allowlist, list already there" case): the payload
+  // must be re-sent so the harness applies the full {mode, list} atomically.
+  const patch = computeRespondToWirePatch({
+    currentMode: "anyone",
+    currentAllowlist: [PUBKEY_A],
+    submitMode: "allowlist",
+    submitAllowlist: [PUBKEY_A],
+  });
+  assert.equal(patch.respondTo, "allowlist");
+  assert.deepEqual(patch.respondToAllowlist, [PUBKEY_A]);
+});
+
+test("allowlist → owner-only omits allowlist", () => {
+  // Flipping away: same as the allowlist → anyone case — mode only.
+  const patch = computeRespondToWirePatch({
+    currentMode: "allowlist",
+    currentAllowlist: [PUBKEY_A],
+    submitMode: "owner-only",
+    submitAllowlist: [PUBKEY_A],
+  });
+  assert.equal(patch.respondTo, "owner-only");
+  assert.equal(patch.respondToAllowlist, undefined);
+});

desktop/src/features/agents/ui/personaRuntimeModel.ts

@@ -1,3 +1,5 @@
+import type { RespondToMode } from "@/shared/api/types";
+
 /** Runtime provider-capability tri-state used by the submit path. */
 export type ProviderRuntimeCapability = "capable" | "locked" | "unknown";
 
@@ -301,3 +303,72 @@ export function envVarsEqual(
     aKeys.every((key) => a[key] === b[key])
   );
 }
+
+/**
+ * Inputs for {@link computeRespondToWirePatch} — the persisted record shape
+ * alongside the dialog's submitted shape, both pre-derived primitives.
+ */
+export interface RespondToWirePatchInput {
+  /** The mode currently persisted on the agent record. */
+  currentMode: RespondToMode;
+  /** The allowlist currently persisted on the agent record. */
+  currentAllowlist: string[];
+  /** The mode the user is submitting from the dialog. */
+  submitMode: RespondToMode;
+  /** The allowlist the user is submitting from the dialog. */
+  submitAllowlist: string[];
+}
+
+/**
+ * The wire patch the edit dialog must send for the respond-to pair.
+ *
+ * - `respondTo` is `undefined` when the mode is unchanged (the Rust update
+ *   merges `undefined` as "leave unchanged").
+ * - `respondToAllowlist` is `undefined` when the payload need not be sent
+ *   (mode is not allowlist, OR the list is unchanged and the mode is already
+ *   allowlist).
+ */
+export interface RespondToWirePatch {
+  respondTo?: RespondToMode;
+  respondToAllowlist?: string[];
+}
+
+/**
+ * Compute the exact `{respondTo, respondToAllowlist}` wire patch the agent
+ * instance edit dialog must send for a given mode/allowlist transition.
+ *
+ * The dialog previously gated the allowlist payload on `submitMode ===
+ * "allowlist"` AND the list differing from the persisted record — which
+ * silently dropped the payload in two reachable shapes (#2501):
+ *
+ *   1. mode flip to allowlist with a list that happens to match the record
+ *      (e.g. anyone → allowlist where the allowlist was populated in an
+ *      earlier session), so the harness never receives the full
+ *      `{mode, allowlist}` tuple atomically and the agent stays unreachable;
+ *   2. mode already allowlist with an edited list — covered, but only because
+ *      the mode gate happened to coincide.
+ *
+ * The backend update merges `respondTo: undefined` as "mode unchanged" and
+ * `respondToAllowlist: undefined` as "payload unchanged", so the wire patch
+ * carries exactly the fields that must change on the record.
+ */
+export function computeRespondToWirePatch(
+  input: RespondToWirePatchInput,
+): RespondToWirePatch {
+  const modeChanged = input.submitMode !== input.currentMode;
+  const listChanged =
+    input.submitAllowlist.join(",") !== input.currentAllowlist.join(",");
+
+  const respondTo = modeChanged ? input.submitMode : undefined;
+
+  // Send the allowlist payload when the submitted mode needs it AND either the
+  // mode or the payload actually changed. A mode flip TO allowlist with an
+  // unchanged-but-nonempty list must still re-send the payload so the harness
+  // applies {mode, allowlist} atomically on the local record.
+  const respondToAllowlist =
+    input.submitMode === "allowlist" && (modeChanged || listChanged)
+      ? [...input.submitAllowlist]
+      : undefined;
+
+  return { respondTo, respondToAllowlist };
+}