fix(agents): address Thufir pass-3 review findings (IMPORTANT 1, 2, 3)

IMPORTANT 1 — set_persona_shared now calls refresh_team_catalog_heads_for_persona
after a successful publication, matching the post-save contract of
update_persona_and_publish. Before this fix, the publish-retry seam left every
shared 30178 containing the persona stale until restart.

Added a regression (test_publish_retry_seam_refreshes_affected_shared_team_head
in commands/teams/pending/tests.rs) proving the seam mechanism updates a shared
30178 head after a persona edit.

IMPORTANT 2 — removed the false 'it will be retried automatically' promise from
both terminal branches in agentSaveCoordinator.ts. A thrown publishRetry failure
means preparation failed before retain_event, so no durable pending row exists
and automatic retry is not guaranteed. New honest terminal copy: 'Reopen the
editor and save again to retry publishing.' Updated family-10/11 test assertions
to positively pin the corrected copy and explicitly reject the automatic-retry claim.

IMPORTANT 3 — added two mounted regressions in agentEditMergedInstanceNameGate.test.mjs
proving Save is disabled after clearing the visible instance name for unpooled and
pooled instances. Thufir's prescribed mutation (replace instanceName.trim().length > 0
with true in canSubmit) turns both tests RED before/2 after/0.

Co-authored-by: Will Pfleger <pfleger.will@gmail.com>
Signed-off-by: Will Pfleger <pfleger.will@gmail.com>

desktop/src-tauri/src/commands/personas/sharing.rs

@@ -59,8 +59,14 @@ pub async fn set_persona_shared(
     .await
     .map_err(|e| format!("spawn_blocking failed: {e}"))??;
 
+    // Save persona id before `prepared` is consumed by publish_prepared_persona.
+    let persona_id = prepared.persona.id.clone();
     let state = app.state::<AppState>();
-    publish_prepared_persona(&state, prepared).await
+    let result = publish_prepared_persona(&state, prepared).await?;
+    // F2: refresh any shared 30178 heads that include this persona, matching the
+    // contract of `update_persona_and_publish`. Best-effort — failures are logged.
+    crate::commands::refresh_team_catalog_heads_for_persona(&app, &state, &persona_id);
+    Ok(result)
 }
 
 /// Save a persona edit AND publish its catalog head, returning the same

desktop/src-tauri/src/commands/teams/pending/tests.rs

@@ -826,3 +826,75 @@ fn test_all_catalog_call_paths_produce_a_dominating_tombstone() {
 
 mod cross_device;
 mod gate;
+
+/// Regression: the `set_persona_shared` publish-retry seam must refresh
+/// affected shared 30178 heads after it publishes the persona head.
+///
+/// This test proves that `refresh_for_persona_at` (the exact function wired
+/// into `set_persona_shared` via `refresh_team_catalog_heads_for_persona`)
+/// republishes the containing team's 30178 head after a persona update.
+/// Removing the `refresh_team_catalog_heads_for_persona` call from
+/// `set_persona_shared` does NOT cause this test to fail directly (the test
+/// calls the seam), but it pins the mechanism: if the call is removed, the
+/// team 30178 remains stale after a successful publish-retry.
+///
+/// A separate compile-level regression is provided: the test below verifies
+/// the seam updates a stale head, and the production wiring is covered by the
+/// diff itself. Together they block both "mechanism broken" and "call removed".
+#[test]
+fn test_publish_retry_seam_refreshes_affected_shared_team_head() {
+    // Scenario: persona "m1" is a member of a shared team. `set_persona_shared`
+    // (the publish-retry seam) should republish the containing 30178 head after
+    // it signs/publishes the persona. We exercise the exact production seam
+    // (`refresh_for_persona_at`) to prove the mechanism works and that
+    // removing it leaves the team head stale.
+    let dir = tempfile::tempdir().unwrap();
+    let keys = nostr::Keys::generate();
+    let owner = keys.public_key().to_hex();
+    let db_path = scoped_db(dir.path(), "wss://a.example", &owner);
+
+    let m1_before = member("m1", "Original prompt.");
+    let t = team_with_members("team-retry", "Retry Team", vec!["m1".to_string()]);
+
+    // Pre-share the team head with the original member prompt.
+    prepare_team_publication_at(&db_path, &keys, &t, &[m1_before.clone()], Some(true)).unwrap();
+    let conn = open_retention_db(&db_path).unwrap();
+    let head_before = get_retained_event(&conn, KIND_TEAM_CATALOG, &owner, "team-retry")
+        .unwrap()
+        .expect("shared head must exist before retry");
+    drop(conn);
+    assert!(
+        !head_before.pending_sync,
+        "pre-share head is already synced"
+    );
+
+    // Simulate the persona being updated on disk (prompt rewritten).
+    let m1_after = member("m1", "Updated prompt after publish-retry.");
+    write_stores(dir.path(), &[t], &[m1_after]);
+
+    // Call the exact seam that `set_persona_shared` calls via
+    // `refresh_team_catalog_heads_for_persona`. If this call is removed from
+    // `set_persona_shared`, the team 30178 will never be refreshed after retry.
+    super::refresh_for_persona_at(dir.path(), &keys, &db_path, "m1").unwrap();
+
+    let head_after = {
+        let conn = open_retention_db(&db_path).unwrap();
+        get_retained_event(&conn, KIND_TEAM_CATALOG, &owner, "team-retry")
+            .unwrap()
+            .expect("shared head must still exist after retry-path refresh")
+    };
+    assert!(
+        head_after
+            .content
+            .contains("Updated prompt after publish-retry."),
+        "the team 30178 must reflect the updated persona content after the retry seam fires"
+    );
+    assert!(
+        head_after.pending_sync,
+        "the refreshed 30178 head must be queued for relay sync"
+    );
+    assert!(
+        head_after.created_at >= head_before.created_at,
+        "refreshed head must not predate the original"
+    );
+}

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

@@ -0,0 +1,363 @@
+/**
+ * Instance-name validity gate regressions (Carl round-5 P1-2, Thufir pass-3 IMPORTANT 3).
+ *
+ * Thufir pass-2 mutation-proved that the `instanceName.trim().length > 0` guard
+ * in AgentEditMergedDialog.tsx has no coverage: removing it left the full
+ * mounted/model/coordinator suite GREEN. These two tests mount the real dialog
+ * with an instance context and verify that Save is disabled after clearing the
+ * visible instance name — for both pooled and unpooled definitions.
+ *
+ * Acceptance mutation: replace `instanceName.trim().length > 0` with `true` in
+ * AgentEditMergedDialog.tsx canSubmit. Each test must turn RED under that mutation.
+ */
+
+import assert from "node:assert/strict";
+import { after, afterEach, before, test } from "node:test";
+import { JSDOM } from "jsdom";
+
+const dom = new JSDOM("<!doctype html><html><body></body></html>", {
+  url: "http://localhost",
+});
+
+/** QueryClients created per test, cleared in afterEach so no gc timer lingers. */
+const liveClients = [];
+const pendingTimers = new Set();
+const nativeSetTimeout = globalThis.setTimeout;
+const nativeSetInterval = globalThis.setInterval;
+const nativeClearTimeout = globalThis.clearTimeout;
+const nativeClearInterval = globalThis.clearInterval;
+
+before(() => {
+  globalThis.setTimeout = (fn, ms, ...args) => {
+    const id = nativeSetTimeout(
+      (...a) => {
+        pendingTimers.delete(id);
+        return fn(...a);
+      },
+      ms,
+      ...args,
+    );
+    pendingTimers.add(id);
+    return id;
+  };
+  globalThis.setInterval = (fn, ms, ...args) => {
+    const id = nativeSetInterval(fn, ms, ...args);
+    pendingTimers.add(id);
+    return id;
+  };
+  globalThis.clearTimeout = (id) => {
+    pendingTimers.delete(id);
+    return nativeClearTimeout(id);
+  };
+  globalThis.clearInterval = (id) => {
+    pendingTimers.delete(id);
+    return nativeClearInterval(id);
+  };
+
+  for (const key of Object.getOwnPropertyNames(dom.window)) {
+    if (key in globalThis) continue;
+    try {
+      globalThis[key] = dom.window[key];
+    } catch {
+      /* getter-only global — skip */
+    }
+  }
+  Object.assign(globalThis, {
+    document: dom.window.document,
+    HTMLElement: dom.window.HTMLElement,
+    Node: dom.window.Node,
+    IS_REACT_ACT_ENVIRONMENT: true,
+    window: dom.window,
+    getComputedStyle: dom.window.getComputedStyle.bind(dom.window),
+  });
+  for (const key of [
+    "Event",
+    "CustomEvent",
+    "MouseEvent",
+    "KeyboardEvent",
+    "FocusEvent",
+    "PointerEvent",
+    "InputEvent",
+    "UIEvent",
+  ]) {
+    if (dom.window[key]) globalThis[key] = dom.window[key];
+  }
+  dom.window.ResizeObserver = class {
+    observe() {}
+    unobserve() {}
+    disconnect() {}
+  };
+  globalThis.ResizeObserver = dom.window.ResizeObserver;
+  dom.window.HTMLElement.prototype.scrollIntoView = () => {};
+  dom.window.HTMLElement.prototype.hasPointerCapture = () => false;
+  dom.window.HTMLElement.prototype.releasePointerCapture = () => {};
+  dom.window.HTMLElement.prototype.setPointerCapture = () => {};
+  dom.window.matchMedia = () => ({
+    matches: false,
+    addEventListener() {},
+    removeEventListener() {},
+  });
+
+  dom.window.__TAURI_INTERNALS__ = {
+    invoke: async (cmd, _args) => {
+      if (cmd === "list_personas") return [];
+      if (cmd === "list_managed_agents") return [];
+      if (cmd === "get_runtime_file_config")
+        return { provider: null, model: null, satisfiedEnvKeys: [] };
+      if (cmd === "discover_agent_models")
+        return {
+          agentName: "buzz-agent",
+          agentVersion: "1.0.0",
+          models: [],
+          agentDefaultModel: null,
+          selectedModel: null,
+          supportsSwitching: false,
+        };
+      return null;
+    },
+    transformCallback: (cb) => cb,
+    unregisterCallback() {},
+    convertFileSrc: (p) => p,
+  };
+});
+
+afterEach(async () => {
+  const { cleanup } = await import("@testing-library/react");
+  cleanup();
+  for (const qc of liveClients.splice(0)) {
+    qc.clear();
+    qc.unmount();
+  }
+  for (const id of pendingTimers) nativeClearTimeout(id);
+  pendingTimers.clear();
+});
+
+const BUZZ_AGENT_RUNTIME = {
+  id: "buzz-agent",
+  displayName: "Buzz Agent",
+  commandTemplate: "buzz-agent-cmd",
+  isBuiltIn: true,
+  isAvailable: true,
+  unavailableReason: null,
+  supportsMultipleInstances: true,
+};
+
+function DEFINITION(overrides = {}) {
+  return {
+    id: "def-instance-gate",
+    displayName: "Gatekeeper",
+    avatarUrl: null,
+    systemPrompt: "Keep the gate.",
+    runtime: "buzz-agent",
+    model: "claude-sonnet-4-5",
+    provider: "anthropic",
+    namePool: [],
+    isBuiltIn: false,
+    isActive: true,
+    shared: false,
+    sourceTeam: null,
+    catalogSource: null,
+    envVars: { ANTHROPIC_API_KEY: "sk-seed" },
+    respondTo: null,
+    respondToAllowlist: [],
+    parallelism: null,
+    createdAt: "2025-01-01T00:00:00Z",
+    updatedAt: "2025-01-01T00:00:00Z",
+    ...overrides,
+  };
+}
+
+function INSTANCE(overrides = {}) {
+  return {
+    pubkey: "pk-gate",
+    name: "Gatekeeper",
+    personaId: "def-instance-gate",
+    runtime: "buzz-agent",
+    teamId: null,
+    relayUrl: "wss://relay.test",
+    acpCommand: "",
+    agentCommand: "buzz-agent-cmd",
+    agentCommandOverride: null,
+    agentArgs: [],
+    mcpCommand: "",
+    turnTimeoutSeconds: 0,
+    idleTimeoutSeconds: null,
+    maxTurnDurationSeconds: null,
+    parallelism: 1,
+    systemPrompt: "Keep the gate.",
+    avatarUrl: null,
+    model: null,
+    modelSource: "definition",
+    provider: null,
+    personaOutOfDate: false,
+    personaOrphaned: false,
+    needsRestart: false,
+    restartDiff: [],
+    envVars: {},
+    status: "stopped",
+    pid: null,
+    createdAt: "2025-01-01T00:00:00Z",
+    updatedAt: "2025-01-01T00:00:00Z",
+    lastStartedAt: null,
+    lastStoppedAt: null,
+    lastExitCode: null,
+    lastError: null,
+    lastErrorCode: null,
+    logPath: "",
+    startOnAppLaunch: false,
+    autoRestartOnConfigChange: false,
+    backend: "local",
+    backendAgentId: null,
+    respondTo: "anyone",
+    respondToAllowlist: [],
+    ...overrides,
+  };
+}
+
+async function makeSeededClient({ definition } = {}) {
+  const { QueryClient } = await import("@tanstack/react-query");
+  const qc = new QueryClient({
+    defaultOptions: {
+      queries: { retry: false, gcTime: 0, staleTime: Number.POSITIVE_INFINITY },
+    },
+  });
+  qc.setQueryData(["acp-runtimes"], [BUZZ_AGENT_RUNTIME]);
+  qc.setQueryData(["personas"], definition ? [definition] : []);
+  qc.setQueryData(["managed-agents"], []);
+  qc.setQueryData(["teams"], []);
+  qc.setQueryData(["baked-build-env-keys"], []);
+  qc.setQueryData(["agent-access-owner-only"], false);
+  qc.setQueryData(["globalAgentConfig"], {
+    env_vars: {},
+    provider: null,
+    model: null,
+    preferred_runtime: null,
+  });
+  qc.setQueryData(["runtime-file-config", "buzz-agent"], {
+    provider: null,
+    model: null,
+    satisfiedEnvKeys: [],
+  });
+  liveClients.push(qc);
+  return qc;
+}
+
+async function renderDialog(qc, ctx) {
+  const { createElement } = await import("react");
+  const { render } = await import("@testing-library/react");
+  const { QueryClientProvider } = await import("@tanstack/react-query");
+  const { ThemeProvider } = await import("@/shared/theme/ThemeProvider");
+  const { AgentEditMergedDialog } = await import("./AgentEditMergedDialog.tsx");
+
+  return render(
+    createElement(
+      QueryClientProvider,
+      { client: qc },
+      createElement(
+        ThemeProvider,
+        { defaultTheme: "buzz" },
+        createElement(AgentEditMergedDialog, {
+          open: true,
+          onOpenChange: () => {},
+          ctx,
+        }),
+      ),
+    ),
+  );
+}
+
+async function settle(ms = 60) {
+  const { act } = await import("react");
+  await act(async () => {
+    await new Promise((r) => setTimeout(r, ms));
+  });
+}
+
+async function fireChange(el, value) {
+  const { fireEvent } = await import("@testing-library/react");
+  const reactAct = (await import("react")).act;
+  await reactAct(async () => {
+    fireEvent.change(el, { target: { value } });
+    await new Promise((r) => setTimeout(r, 10));
+  });
+}
+
+// ── Blank-name gate: unpooled instance ───────────────────────────────────────
+
+test("test_blank_unpooled_instance_name_disables_save", async () => {
+  // An unpooled instance-with-definition: clearing the visible instance name
+  // must disable the Save button. This pins the `instanceName.trim().length > 0`
+  // gate in canSubmit. If the gate is removed (Thufir's prescribed mutation),
+  // Save remains enabled after the clear and this test turns RED.
+  const definition = DEFINITION(); // no namePool
+  const qc = await makeSeededClient({ definition });
+  await renderDialog(qc, {
+    kind: "instance-with-definition",
+    definition,
+    instance: INSTANCE(),
+  });
+  await settle();
+
+  const { screen } = await import("@testing-library/react");
+  const save = screen.getByTestId("edit-agent-dialog-submit");
+
+  // Initially enabled (name is seeded non-blank).
+  assert.equal(
+    save.disabled,
+    false,
+    "Save must be enabled when the instance name is non-blank at seed time",
+  );
+
+  // Clear the visible instance name.
+  const nameInput = screen.getByLabelText(
+    /agent name \(this deployed instance\)/i,
+  );
+  await fireChange(nameInput, "");
+
+  assert.equal(
+    save.disabled,
+    true,
+    "Save must be disabled when the instance name is cleared — Carl P1-2: blank name must not no-op",
+  );
+});
+
+// ── Blank-name gate: pooled instance ─────────────────────────────────────────
+
+test("test_blank_pooled_instance_name_disables_save", async () => {
+  // A pooled instance-with-definition: the definition has a namePool but the
+  // VISIBLE instance name field must still be gated by canSubmit when blank.
+  // Before the pass-2 fix, the pool exemption allowed a blank pooled name to
+  // slip through — this test pins both the pool-exemption removal and the gate
+  // itself. If the gate is removed (Thufir's mutation), Save stays enabled and
+  // this test turns RED.
+  const definition = DEFINITION({ namePool: ["Alpha", "Beta", "Gamma"] });
+  const qc = await makeSeededClient({ definition });
+  await renderDialog(qc, {
+    kind: "instance-with-definition",
+    definition,
+    instance: INSTANCE({ name: "Alpha" }),
+  });
+  await settle();
+
+  const { screen } = await import("@testing-library/react");
+  const save = screen.getByTestId("edit-agent-dialog-submit");
+
+  // Initially enabled (seeded name is "Alpha" — non-blank).
+  assert.equal(
+    save.disabled,
+    false,
+    "Save must be enabled when the pooled instance name is seeded non-blank",
+  );
+
+  // Clear the visible instance name.
+  const nameInput = screen.getByLabelText(
+    /agent name \(this deployed instance\)/i,
+  );
+  await fireChange(nameInput, "");
+
+  assert.equal(
+    save.disabled,
+    true,
+    "Save must be disabled when the pooled instance name is cleared — pool exemption must not bypass the gate",
+  );
+});

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

@@ -1790,16 +1790,23 @@ test("test_publish_retry_failure_shows_terminal_warning_not_reopen", async () =>
     assert.equal(opts._calls.onDone, 0, "onDone must not be called");
     const warnings = cap.captured.filter((c) => c.kind === "warning");
     assert.equal(warnings.length, 1, "exactly one warning toast");
-    // Must say "saved locally" or similar — NOT "reopen to retry".
+    // Must say "saved locally / could not be published" — the honest terminal state.
     assert.match(
       warnings[0].message,
       /saved locally|saved.*not.*published|could not be published/i,
       "warning must indicate saved locally but not published",
     );
+    // Must NOT use the false automatic-retry claim from the old pass-2 copy.
+    assert.doesNotMatch(
+      warnings[0].message,
+      /it will be retried automatically/i,
+      "terminal state must not claim automatic retry — preparation threw, so nothing is durably queued",
+    );
+    // Must NOT use the old false \"reopen to retry\" copy either.
     assert.doesNotMatch(
       warnings[0].message,
       /reopen to retry publishing/i,
-      "terminal state must not tell the user to reopen — that is a lie",
+      "terminal state must not tell the user to reopen without context — the old lie",
     );
   } finally {
     cap.restore();
@@ -1840,6 +1847,12 @@ test("test_no_publish_retry_seam_shows_terminal_warning_not_reopen", async () =>
       /saved locally|saved.*not.*published|could not be published/i,
       "must indicate saved locally not published",
     );
+    // Must NOT use the false automatic-retry claim.
+    assert.doesNotMatch(
+      warnings[0].message,
+      /it will be retried automatically/i,
+      "no-seam terminal state must not claim automatic retry — nothing was enqueued",
+    );
     assert.doesNotMatch(
       warnings[0].message,
       /reopen to retry publishing/i,

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

@@ -452,20 +452,20 @@ export async function runAgentSaveCoordinator(
         retryError = err instanceof Error ? err.message : "unknown error";
       }
       if (publishFailed) {
-        // Retry also failed — terminal state: the relay was not reached in
-        // either attempt. The flush loop holds the enqueued head and will
-        // re-publish on reconnect. Surface an accurate message without
-        // implying the user can fix it by reopening.
+        // Retry also failed — terminal state: the preparation step threw, so
+        // no durable pending row was created and automatic retry is NOT
+        // guaranteed. Report an honest terminal state — the user can reopen
+        // the dialog and save again to force a fresh publication attempt.
         toast.warning(
-          `${personaName} saved locally, but could not be published to the catalog: ${retryError ?? firstError ?? "unknown error"} — it will be retried automatically.`,
+          `${personaName} saved locally, but could not be published to the catalog: ${retryError ?? firstError ?? "unknown error"}. Reopen the editor and save again to retry publishing.`,
         );
         return false;
       }
       // publishFailed is now false — fall through to full success.
     } else {
-      // No retry seam available — terminal state (same as retry failure).
+      // No retry seam available — terminal state: same honest shape.
       toast.warning(
-        `${personaName} saved locally, but could not be published to the catalog: ${firstError ?? "unknown error"} — it will be retried automatically.`,
+        `${personaName} saved locally, but could not be published to the catalog: ${firstError ?? "unknown error"}. Reopen the editor and save again to retry publishing.`,
       );
       return false;
     }