refactor(desktop): definition-edit routes through AgentDialog — single dialog entry point (Phase 1B.3c) (#1643)

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

@@ -42,15 +42,42 @@ type AgentDialogInstanceEditProps = {
   onUpdated?: (agent: ManagedAgent) => void;
 };
 
-type AgentDialogProps = AgentDialogCreateProps | AgentDialogInstanceEditProps;
+type AgentDialogDefinitionEditProps = {
+  mode: "definition-edit";
+  open: boolean;
+  title: string;
+  description: string;
+  submitLabel: string;
+  initialValues: CreatePersonaInput | UpdatePersonaInput | null;
+  error: Error | null;
+  isPending: boolean;
+  isImportPending?: boolean;
+  runtimes: AcpRuntimeCatalogEntry[];
+  runtimesLoading?: boolean;
+  onOpenChange: (open: boolean) => void;
+  onSubmit: (
+    input: CreatePersonaInput | UpdatePersonaInput,
+  ) => Promise<unknown>;
+  onImportUpdateFile?: (
+    personaId: string,
+    fileBytes: number[],
+    fileName: string,
+  ) => Promise<void>;
+};
+
+type AgentDialogProps =
+  | AgentDialogCreateProps
+  | AgentDialogInstanceEditProps
+  | AgentDialogDefinitionEditProps;
 
 /**
- * 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+.
+ * Unified entry point (Phase 1B.2/1B.3b/1B.3c): routes an intent to the form
+ * that owns it. The definition family renders AgentDefinitionDialog — create
+ * mode adds a "start after create" toggle, definition-edit passes the caller's
+ * PersonaDialogState-derived props through unchanged (edit/duplicate/import).
+ * The standalone-instance intent renders CreateAgentDialog unchanged;
+ * instance-edit renders AgentInstanceEditDialog (persistent mount + `open`
+ * toggle — its reset lifecycle is keyed on [open, agent.pubkey]).
  */
 export function AgentDialog(props: AgentDialogProps) {
   if (props.mode === "instance-edit") {
@@ -63,6 +90,10 @@ export function AgentDialog(props: AgentDialogProps) {
       />
     );
   }
+  if (props.mode === "definition-edit") {
+    const { mode: _mode, ...definitionProps } = props;
+    return <AgentDefinitionDialog {...definitionProps} />;
+  }
   return <AgentCreateDialogRouter {...props} />;
 }
 

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

@@ -8,7 +8,6 @@ import { AddTeamToChannelDialog } from "./AddTeamToChannelDialog";
 import { AgentDialog, type AgentDialogCreateMode } from "./AgentDialog";
 import { BatchImportDialog } from "./BatchImportDialog";
 import { PersonaCatalogDialog } from "./PersonaCatalogDialog";
-import { AgentDefinitionDialog } from "./AgentDefinitionDialog";
 import { PersonaDeleteDialog } from "./PersonaDeleteDialog";
 import { PersonaImportUpdateDialog } from "./PersonaImportUpdateDialog";
 import { PersonaShareDialog } from "./PersonaShareDialog";
@@ -30,8 +29,7 @@ export function AgentsView() {
   const agents = useManagedAgentActions();
   const personas = usePersonaActions();
   // Exclusivity: create never sets `personaDialogState` (edit/dup/import do),
-  // so the unified create dialog and the edit/dup/import AgentDefinitionDialog
-  // mount never coexist.
+  // so the create-mode and definition-edit AgentDialog mounts never coexist.
   const [createDialogMode, setCreateDialogMode] =
     React.useState<AgentDialogCreateMode | null>(null);
 
@@ -241,7 +239,7 @@ export function AgentsView() {
         />
       ) : null}
       {personas.personaDialogState ? (
-        <AgentDefinitionDialog
+        <AgentDialog
           description={personas.personaDialogState.description}
           error={
             personas.updatePersonaMutation.error instanceof Error
@@ -255,6 +253,7 @@ export function AgentsView() {
             personas.personaImportActions.isApplyingPersonaImportUpdate
           }
           isPending={personas.isPending}
+          mode="definition-edit"
           runtimes={personas.acpRuntimesQuery.data ?? []}
           runtimesLoading={personas.acpRuntimesQuery.isLoading}
           onImportUpdateFile={

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

@@ -0,0 +1,90 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import { AgentDialog } from "./AgentDialog.tsx";
+import { AgentDefinitionDialog } from "./AgentDefinitionDialog.tsx";
+import { AgentInstanceEditDialog } from "./AgentInstanceEditDialog.tsx";
+
+// ── Phase 1B.3c routing pinning ─────────────────────────────────────────────
+//
+// AgentDialog is the single dialog entry point for every intent. It is
+// hook-free by design, so each union arm can be exercised as a plain function
+// call and the returned element inspected: the arm must route to the form
+// component that owns the intent, and pass-through arms must forward the
+// caller's props byte-for-byte (minus the `mode` discriminant).
+
+const noop = () => {};
+
+test("definition-edit routes to AgentDefinitionDialog with exact pass-through", () => {
+  const props = {
+    description: "Edit the agent definition.",
+    error: null,
+    initialValues: { displayName: "Brain" },
+    isImportPending: true,
+    isPending: false,
+    onImportUpdateFile: async () => {},
+    onOpenChange: noop,
+    onSubmit: async () => {},
+    open: true,
+    runtimes: [],
+    runtimesLoading: false,
+    submitLabel: "Save",
+    title: "Edit agent",
+  };
+
+  const element = AgentDialog({ mode: "definition-edit", ...props });
+
+  assert.equal(element.type, AgentDefinitionDialog);
+  assert.deepEqual(element.props, props, "props must pass through unchanged");
+  assert.equal(
+    "mode" in element.props,
+    false,
+    "the mode discriminant must not leak into AgentDefinitionDialog",
+  );
+});
+
+test("instance-edit routes to AgentInstanceEditDialog with its contract props", () => {
+  const agent = { pubkey: "abc", name: "test-agent" };
+  const onOpenChange = noop;
+  const onUpdated = noop;
+
+  const element = AgentDialog({
+    mode: "instance-edit",
+    agent,
+    onOpenChange,
+    onUpdated,
+    open: true,
+  });
+
+  assert.equal(element.type, AgentInstanceEditDialog);
+  assert.deepEqual(element.props, {
+    agent,
+    onOpenChange,
+    onUpdated,
+    open: true,
+  });
+});
+
+test("create modes route to the internal create router, not a form directly", () => {
+  for (const mode of ["definition", "instance"]) {
+    const element = AgentDialog({
+      mode,
+      definitionError: null,
+      isDefinitionPending: false,
+      onInstanceCreated: noop,
+      onOpenChange: noop,
+      onSubmitDefinition: async () => true,
+      runtimes: [],
+      runtimesLoading: false,
+    });
+
+    assert.notEqual(element.type, AgentDefinitionDialog);
+    assert.notEqual(element.type, AgentInstanceEditDialog);
+    assert.equal(
+      typeof element.type,
+      "function",
+      `${mode} must route through the internal create router`,
+    );
+    assert.equal(element.type.name, "AgentCreateDialogRouter");
+  }
+});

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

@@ -5,7 +5,7 @@ import type {
   UpdatePersonaInput,
 } from "@/shared/api/types";
 import { PersonaDeleteDialog } from "@/features/agents/ui/PersonaDeleteDialog";
-import { AgentDefinitionDialog } from "@/features/agents/ui/AgentDefinitionDialog";
+import { AgentDialog } from "@/features/agents/ui/AgentDialog";
 import type { PersonaDialogState } from "@/features/agents/ui/personaDialogState";
 
 export function UserProfilePersonaDialogs({
@@ -35,11 +35,12 @@ export function UserProfilePersonaDialogs({
 }) {
   return (
     <>
-      <AgentDefinitionDialog
+      <AgentDialog
         description={personaDialogState?.description ?? ""}
         error={updateError ?? createError}
         initialValues={personaDialogState?.initialValues ?? null}
         isPending={isPending}
+        mode="definition-edit"
         runtimes={runtimes}
         runtimesLoading={runtimesLoading}
         onOpenChange={(open) => {

desktop/test-loader-hooks.mjs

@@ -14,7 +14,10 @@ const repoRoot = path.resolve(
 );
 
 function resolveSourcePath(basePath) {
-  if (path.extname(basePath)) {
+  // Existence decides, not path.extname — a dotted basename like
+  // `ProfileAvatarEditor.utils` (→ .utils.ts on disk) looks like an
+  // extension but still needs resolving.
+  if (fs.existsSync(basePath) && fs.statSync(basePath).isFile()) {
     return basePath;
   }
 
@@ -32,7 +35,7 @@ function resolveSourcePath(basePath) {
     }
   }
 
-  return `${basePath}.ts`;
+  return null;
 }
 
 // emoji-mart ships a bundled CJS main that node's cjs-module-lexer cannot
@@ -70,22 +73,26 @@ export function resolve(specifier, context, nextResolve) {
     // Otherwise paths like `@/.../foo.mjs` would be coerced into `foo.mjs.ts`
     // and fail to resolve.
     const resolved = resolveSourcePath(`${srcRoot}/${stripped}`);
-    return nextResolve(resolved, context);
+    return nextResolve(resolved ?? `${srcRoot}/${stripped}`, context);
   }
   // Resolve extensionless relative TS imports (e.g. `./parseImeta`) — the app's
   // bundler adds the extension, but node's ESM resolver does not. Without this,
   // any .ts that relative-imports a sibling .ts can't be imported from a test,
   // which previously forced stale inlined copies of the source under test.
+  // Dotted basenames (`./ProfileAvatarEditor.utils`) look like extensions to
+  // path.extname, so resolveSourcePath existence-checks instead.
   if (
     (specifier.startsWith("./") || specifier.startsWith("../")) &&
-    !path.extname(specifier) &&
-    context.parentURL
+    context.parentURL?.startsWith("file:")
   ) {
     const parentPath = fileURLToPath(context.parentURL);
     const resolved = resolveSourcePath(
       path.resolve(path.dirname(parentPath), specifier),
     );
-    return nextResolve(resolved, context);
+    if (resolved) {
+      return nextResolve(resolved, context);
+    }
+    return nextResolve(specifier, context);
   }
   return nextResolve(specifier, context);
 }