refactor(desktop): re-host instance edit as AgentInstanceEditDialog behind AgentDialog (Phase 1B.3b) (#1639)

Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Brain <21994759fc7a6fa6b965551d35cfd7897d262f2495467f2d78694ddcfa6a5c7e@sprout-oss.stage.blox.sqprod.co>

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

@@ -4,6 +4,7 @@ import type {
   AcpRuntimeCatalogEntry,
   CreateManagedAgentResponse,
   CreatePersonaInput,
+  ManagedAgent,
   UpdatePersonaInput,
 } from "@/shared/api/types";
 import { Switch } from "@/shared/ui/switch";
@@ -12,14 +13,15 @@ import {
   intentForStartToggle,
   type AgentCreateIntent,
 } from "./agentCreateIntent";
+import { AgentInstanceEditDialog } from "./AgentInstanceEditDialog";
 import { CreateAgentDialog } from "./CreateAgentDialog";
 import { createPersonaDialogState } from "./personaDialogState";
 import { AgentDefinitionDialog } from "./AgentDefinitionDialog";
 
-export type AgentDialogMode = "definition" | "instance";
+export type AgentDialogCreateMode = "definition" | "instance";
 
-type AgentDialogProps = {
-  mode: AgentDialogMode;
+type AgentDialogCreateProps = {
+  mode: AgentDialogCreateMode;
   onOpenChange: (open: boolean) => void;
   definitionError: Error | null;
   isDefinitionPending: boolean;
@@ -32,14 +34,39 @@ type AgentDialogProps = {
   onInstanceCreated: (result: CreateManagedAgentResponse) => void;
 };
 
+type AgentDialogInstanceEditProps = {
+  mode: "instance-edit";
+  agent: ManagedAgent;
+  open: boolean;
+  onOpenChange: (open: boolean) => void;
+  onUpdated?: (agent: ManagedAgent) => void;
+};
+
+type AgentDialogProps = AgentDialogCreateProps | AgentDialogInstanceEditProps;
+
 /**
- * Unified create entry point (Phase 1B.2): routes a create intent to the
- * form that owns it. The definition family renders AgentDefinitionDialog in
- * create mode with a "start after create" toggle; the standalone-instance
- * intent renders CreateAgentDialog unchanged. Physical consolidation of the
- * two forms is Phase 1B.3.
+ * Unified entry point (Phase 1B.2/1B.3b): routes an intent to the form that
+ * owns it. The definition family renders AgentDefinitionDialog in create mode
+ * with a "start after create" toggle; the standalone-instance intent renders
+ * CreateAgentDialog unchanged; instance-edit renders AgentInstanceEditDialog
+ * (persistent mount + `open` toggle — its reset lifecycle is keyed on
+ * [open, agent.pubkey]). Physical consolidation of the forms is Phase 1B.3c+.
  */
-export function AgentDialog({
+export function AgentDialog(props: AgentDialogProps) {
+  if (props.mode === "instance-edit") {
+    return (
+      <AgentInstanceEditDialog
+        agent={props.agent}
+        onOpenChange={props.onOpenChange}
+        onUpdated={props.onUpdated}
+        open={props.open}
+      />
+    );
+  }
+  return <AgentCreateDialogRouter {...props} />;
+}
+
+function AgentCreateDialogRouter({
   mode,
   onOpenChange,
   definitionError,
@@ -48,7 +75,7 @@ export function AgentDialog({
   runtimesLoading,
   onSubmitDefinition,
   onInstanceCreated,
-}: AgentDialogProps) {
+}: AgentDialogCreateProps) {
   const [startAfterCreate, setStartAfterCreate] = React.useState(true);
   // Stable identity across toggle flips — AgentDefinitionDialog re-initializes its
   // fields whenever `initialValues` changes.

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

@@ -66,7 +66,7 @@ const ADVANCED_FIELDS_MOTION_TRANSITION = {
   ease: [0.23, 1, 0.32, 1],
 } as const;
 
-export function EditAgentDialog({
+export function AgentInstanceEditDialog({
   agent,
   open,
   onOpenChange,

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

@@ -5,7 +5,7 @@ import {
 } from "@/features/agents/openCreateAgentEvent";
 import { AddAgentToChannelDialog } from "./AddAgentToChannelDialog";
 import { AddTeamToChannelDialog } from "./AddTeamToChannelDialog";
-import { AgentDialog, type AgentDialogMode } from "./AgentDialog";
+import { AgentDialog, type AgentDialogCreateMode } from "./AgentDialog";
 import { BatchImportDialog } from "./BatchImportDialog";
 import { PersonaCatalogDialog } from "./PersonaCatalogDialog";
 import { AgentDefinitionDialog } from "./AgentDefinitionDialog";
@@ -33,9 +33,9 @@ export function AgentsView() {
   // so the unified create dialog and the edit/dup/import AgentDefinitionDialog
   // mount never coexist.
   const [createDialogMode, setCreateDialogMode] =
-    React.useState<AgentDialogMode | null>(null);
+    React.useState<AgentDialogCreateMode | null>(null);
 
-  function openUnifiedCreate(mode: AgentDialogMode) {
+  function openUnifiedCreate(mode: AgentDialogCreateMode) {
     if (mode === "definition") {
       personas.prepareCreate();
     }

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

@@ -226,7 +226,7 @@ export function CreateAgentDialog({
     [allRequiredEnvKeys, bakedSatisfiedEnvKeys, fileSatisfiedEnvKeys],
   );
 
-  // Clear model when provider scope changes, mirroring EditAgentDialog.
+  // Clear model when provider scope changes, mirroring AgentInstanceEditDialog.
   React.useEffect(() => {
     if (
       !open ||

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

@@ -0,0 +1,251 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import {
+  requiredCredentialEnvKeys,
+  runtimeSupportsLlmProviderSelection,
+} from "./personaDialogPickers.tsx";
+import {
+  computeEditAgentFormValidity,
+  hasMissingRequiredEnvKey,
+  resolveAgentCommandUpdate,
+  resolveInheritedRuntimeSubmission,
+  resolveRuntimeProviderCapability,
+} from "./personaRuntimeModel.ts";
+
+// ── Phase 1B.3b re-host pinning: inherit-toggle → gate → submit ─────────────
+//
+// AgentInstanceEditDialog (re-homed from EditAgentDialog) wires three seams
+// that must never disagree after the move:
+//   1. TOGGLE: flipping "Inherit runtime from persona" re-resolves
+//      prospectiveRuntimeId from the LINKED PERSONA's runtime (not the stale
+//      override) and feeds resolveInheritedRuntimeSubmission.
+//   2. GATE: useRequiredCredentialState validates the SAME inheritedSubmission
+//      provider/env the submit path will persist (gate ↔ record ↔ spawn).
+//   3. SUBMIT: UpdateManagedAgentInput persists that same snapshot, with
+//      resolveAgentCommandUpdate + harnessOverride derived from the toggle.
+// These tests chain the pure modules exactly as the component does, so a
+// re-host that re-derives any seam independently fails here.
+
+const runtimes = [
+  { id: "buzz-agent", command: "buzz-agent-cmd", defaultArgs: [] },
+  { id: "claude", command: "claude-cmd", defaultArgs: [] },
+];
+
+// Mirrors the component's prospectiveRuntimeId memo: when inheriting, resolve
+// from the linked persona's runtime; fall back to the agentCommand dual-match.
+function prospectiveRuntimeIdFor({
+  inheritHarness,
+  selectedRuntimeId,
+  linkedPersonaRuntime,
+  agentCommand,
+}) {
+  if (!inheritHarness) {
+    return selectedRuntimeId;
+  }
+  const personaRuntimeId = linkedPersonaRuntime?.trim();
+  if (personaRuntimeId) {
+    return (
+      runtimes.find((r) => r.id === personaRuntimeId)?.id ?? personaRuntimeId
+    );
+  }
+  return (
+    runtimes.find((r) => r.command?.trim() === agentCommand.trim())?.id ??
+    runtimes.find((r) => r.id === agentCommand.trim())?.id ??
+    ""
+  );
+}
+
+// A Claude-pinned agent linked to a buzz-agent/anthropic persona — the
+// inherit-transition scenario that exercises every seam at once.
+const pinnedAgent = {
+  name: "test-agent",
+  agentCommand: "claude-cmd",
+  agentCommandOverride: "claude-cmd",
+  acpCommand: "acp",
+  personaId: "p1",
+  provider: null,
+  model: null,
+  envVars: {},
+};
+const persona = {
+  runtime: "buzz-agent",
+  provider: "anthropic",
+  model: "claude-sonnet-4-5",
+  envVars: { ANTHROPIC_API_KEY: "sk-persona" },
+};
+
+function inheritTransitionState() {
+  const inheritHarness = true; // user just checked the toggle
+  const prospectiveRuntimeId = prospectiveRuntimeIdFor({
+    inheritHarness,
+    selectedRuntimeId: "claude",
+    linkedPersonaRuntime: persona.runtime,
+    agentCommand: pinnedAgent.agentCommand,
+  });
+  const inheritedSubmission = resolveInheritedRuntimeSubmission({
+    inheritHarness,
+    agentWasHarnessPinned: pinnedAgent.agentCommandOverride != null,
+    provider: "", // pinned Claude agent carries no provider
+    personaProvider: persona.provider,
+    model: "",
+    personaModel: persona.model,
+    envVars: {},
+    personaEnvVars: persona.envVars,
+  });
+  return { inheritHarness, prospectiveRuntimeId, inheritedSubmission };
+}
+
+test("rehost_toggle_resolvesProspectiveRuntimeFromPersona_notOverride", () => {
+  const { prospectiveRuntimeId } = inheritTransitionState();
+  assert.equal(
+    prospectiveRuntimeId,
+    "buzz-agent",
+    "inherit-toggle must resolve the prospective runtime from the linked persona, not the still-present Claude pin",
+  );
+});
+
+test("rehost_gate_validatesTheSubmissionSnapshot_sameValuesAsSubmit", () => {
+  const { prospectiveRuntimeId, inheritedSubmission } =
+    inheritTransitionState();
+
+  // Gate half: the credential requirement must be computed from the
+  // PROSPECTIVE runtime + the submission snapshot's provider/env.
+  const providerForRequiredKeys = runtimeSupportsLlmProviderSelection(
+    prospectiveRuntimeId,
+  )
+    ? (inheritedSubmission.provider ?? "")
+    : "";
+  const requiredKeys = requiredCredentialEnvKeys(
+    prospectiveRuntimeId,
+    providerForRequiredKeys,
+  );
+  const missing = hasMissingRequiredEnvKey(
+    requiredKeys,
+    inheritedSubmission.envVars,
+  );
+
+  // The persona snapshot carries the credential, so the gate must clear —
+  // and it must clear because of the SAME env map submit will persist.
+  assert.equal(inheritedSubmission.provider, "anthropic");
+  assert.deepEqual(inheritedSubmission.envVars, {
+    ANTHROPIC_API_KEY: "sk-persona",
+  });
+  assert.equal(
+    missing,
+    false,
+    "gate must validate the submission snapshot (persona-layered env), not the agent's own empty env",
+  );
+});
+
+test("rehost_gate_blocksSave_whenSubmissionSnapshotLacksCredential", () => {
+  const { prospectiveRuntimeId } = inheritTransitionState();
+  // Same transition but the persona carries no credential: the submission
+  // snapshot is credential-less and the SAME gate must now block Save.
+  const bareSubmission = resolveInheritedRuntimeSubmission({
+    inheritHarness: true,
+    agentWasHarnessPinned: true,
+    provider: "",
+    personaProvider: persona.provider,
+    model: "",
+    personaModel: persona.model,
+    envVars: {},
+    personaEnvVars: {}, // no key anywhere
+  });
+  const requiredKeys = requiredCredentialEnvKeys(
+    prospectiveRuntimeId,
+    bareSubmission.provider ?? "",
+  );
+  const requiredEnvKeyMissing = hasMissingRequiredEnvKey(
+    requiredKeys,
+    bareSubmission.envVars,
+  );
+  assert.equal(requiredEnvKeyMissing, true);
+
+  const canSubmit = computeEditAgentFormValidity({
+    name: pinnedAgent.name,
+    parallelism: "1",
+    turnTimeoutSeconds: "60",
+    agentAcpCommand: pinnedAgent.acpCommand,
+    acpCommand: pinnedAgent.acpCommand,
+    respondTo: "mentions",
+    respondToAllowlistLength: 0,
+    selectedRuntimeId: "claude",
+    inheritHarness: true,
+    agentCommand: pinnedAgent.agentCommand,
+    requiredEnvKeyMissing,
+  });
+  assert.equal(
+    canSubmit,
+    false,
+    "missing credential in the submission snapshot must block Save through the same validity path",
+  );
+});
+
+test("rehost_submit_persistsToggleAsCommandClear_andOmitsHarnessOverride", () => {
+  // Submit half of the toggle seam: inheriting with a persisted override must
+  // send the clear sentinel (""), and harnessOverride derives from the SAME
+  // agentCommandUpdate (omitted while inheriting — falsy per the component).
+  const agentCommandUpdate = resolveAgentCommandUpdate({
+    inheritHarness: true,
+    agentCommand: pinnedAgent.agentCommand,
+    originalAgentCommand: pinnedAgent.agentCommand,
+    agentCommandOverride: pinnedAgent.agentCommandOverride,
+  });
+  assert.equal(
+    agentCommandUpdate,
+    "",
+    "inherit with a persisted pin must persist the clear sentinel",
+  );
+  const harnessOverride =
+    agentCommandUpdate != null ? !true /* inheritHarness */ : undefined;
+  assert.equal(
+    harnessOverride,
+    false,
+    "harnessOverride must derive from the shared agentCommandUpdate, signalling the cleared pin",
+  );
+
+  // And the provider tri-state must classify the PROSPECTIVE runtime — the
+  // same id the gate used — so submit persists what the gate validated.
+  const { prospectiveRuntimeId, inheritedSubmission } =
+    inheritTransitionState();
+  const capability = resolveRuntimeProviderCapability(
+    prospectiveRuntimeId,
+    runtimeSupportsLlmProviderSelection(prospectiveRuntimeId),
+  );
+  assert.equal(capability, "capable");
+  const providerUpdate =
+    capability === "capable"
+      ? inheritedSubmission.provider !== (pinnedAgent.provider ?? null)
+        ? inheritedSubmission.provider
+        : undefined
+      : undefined;
+  assert.equal(
+    providerUpdate,
+    "anthropic",
+    "submit must persist the gate-validated submission provider for the prospective runtime",
+  );
+});
+
+test("rehost_steadyStateInherit_localEditsStayAuthoritative", () => {
+  // Wipe-on-poll companion: an agent ALREADY inheriting at open (no pin) with
+  // deliberate local edits — the submission snapshot must pass them through
+  // untouched, not resurrect persona values (the pre-move contract).
+  const submission = resolveInheritedRuntimeSubmission({
+    inheritHarness: true,
+    agentWasHarnessPinned: false, // steady state, not a transition
+    provider: "databricks", // deliberate local re-point
+    personaProvider: persona.provider,
+    model: "",
+    personaModel: persona.model,
+    envVars: { DATABRICKS_TOKEN: "tok" },
+    personaEnvVars: persona.envVars,
+  });
+  assert.equal(submission.provider, "databricks");
+  assert.equal(
+    submission.model,
+    null,
+    "empty local model in steady state stays empty (runtime default), never backfilled from the persona",
+  );
+  assert.deepEqual(submission.envVars, { DATABRICKS_TOKEN: "tok" });
+});

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

@@ -1186,7 +1186,7 @@ test("editAgent_findingE_capableBuzzAgentLoadedCatalog_preservedOnNoOpSave", ()
 // to id-based matching when command-path matching misses — same id-fallback used
 // by effectiveRuntimeIdForSubmit.
 
-// Helper mirrors the fixed seeding logic from EditAgentDialog.tsx open-effect /
+// Helper mirrors the fixed seeding logic from AgentInstanceEditDialog.tsx open-effect /
 // catalog-arrival effect.
 function deriveSelectedRuntimeId(agentCommand, runtimes) {
   const matched =
@@ -1390,7 +1390,7 @@ test("requiredCredentialEnvKeys: custom/unknown runtime → empty", () => {
 
 // ── Block-save gate: hasMissingRequiredEnvKey logic ────────────────────────
 //
-// The EditAgentDialog computes:
+// The AgentInstanceEditDialog computes:
 //   requiredEnvKeyMissing = hasMissingRequiredEnvKey(requiredEnvKeys, envVars)
 // and folds it into canSubmit (via computeEditAgentFormValidity). These tests
 // exercise the exported predicate directly.
@@ -1472,7 +1472,7 @@ test("blockSave_buzzAgentDatabricksHostProvided_allowed", () => {
 
 // ── Block-save gate: isMissingRequiredDropdownField ────────────────────────
 //
-// The EditAgentDialog also gates on modelRequired / providerRequired.
+// The AgentInstanceEditDialog also gates on modelRequired / providerRequired.
 // These tests guard the isMissingRequiredDropdownField predicate used for both.
 
 test("blockSave_missingRequiredModel_blocked", () => {

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

@@ -1,7 +1,7 @@
 /**
  * Shared provider and model field components for agent dialogs.
  *
- * Both CreateAgentDialog (local mode) and EditAgentDialog import these
+ * Both CreateAgentDialog (local mode) and AgentInstanceEditDialog import these
  * instead of duplicating the picker logic.
  */
 import type * as React from "react";

desktop/src/features/profile/ui/UserProfilePanel.tsx

@@ -34,7 +34,7 @@ import {
   stopManagedAgentWithRules,
 } from "@/features/agents/lib/managedAgentControlActions";
 import { describeLogFile } from "@/features/agents/ui/agentUi";
-import { EditAgentDialog } from "@/features/agents/ui/EditAgentDialog";
+import { AgentDialog } from "@/features/agents/ui/AgentDialog";
 import {
   consumePendingOpenEditAgent,
   subscribeOpenEditAgent,
@@ -918,8 +918,9 @@ export function UserProfilePanel({
   );
   const editAgentDialog =
     canEditAgent && managedAgent ? (
-      <EditAgentDialog
+      <AgentDialog
         agent={managedAgent}
+        mode="instance-edit"
         onOpenChange={setEditAgentOpen}
         open={editAgentOpen}
       />