refactor(desktop): converge the three definition→instance mappings (Phase 1B.3.5) (#1645)

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

desktop/src/features/agents/lib/instanceInputForDefinition.test.mjs

@@ -0,0 +1,196 @@
+import assert from "node:assert/strict";
+import test from "node:test";
+
+import {
+  availableRuntimesForStart,
+  buildInstanceInputForDefinition,
+  resolveStartRuntimeForDefinition,
+} from "./instanceInputForDefinition.ts";
+
+// ── Phase 1B.3.5: the single definition→instance mapping ────────────────────
+//
+// Every surface that starts an agent from a definition maps through
+// buildInstanceInputForDefinition + resolveStartRuntimeForDefinition +
+// availableRuntimesForStart. These tests pin the decided rows:
+//   row 1: refuse (actionable error) when the configured runtime is missing
+//   row 2: harnessOverride = !persona.runtime || persona.runtime === runtime.id
+//   row 3: avatar through resolveManagedAgentAvatarUrl (injectable upload)
+//   row 4: create input NEVER contains definition env vars
+//   row 6: runtime list acquisition is refetch-aware
+
+const gooseRuntime = {
+  id: "goose",
+  label: "Goose",
+  avatarUrl: "https://runtime/goose.png",
+  availability: "available",
+  command: "goose-cmd",
+  binaryPath: "/bin/goose",
+  defaultArgs: ["--acp"],
+  mcpCommand: "goose-mcp",
+  installHint: "",
+  installInstructionsUrl: "",
+  canAutoInstall: false,
+  underlyingCliPath: null,
+};
+
+const claudeRuntime = {
+  ...gooseRuntime,
+  id: "claude",
+  label: "Claude",
+  command: "claude-cmd",
+  mcpCommand: null,
+};
+
+function persona(overrides = {}) {
+  return {
+    id: "p-1",
+    displayName: "Test Agent",
+    systemPrompt: "prompt",
+    model: null,
+    runtime: "goose",
+    avatarUrl: "https://example.com/a.png",
+    envVars: { ANTHROPIC_API_KEY: "persona-secret" },
+    isBuiltIn: false,
+    ...overrides,
+  };
+}
+
+test("row 4: create input never contains definition env vars", async () => {
+  const input = await buildInstanceInputForDefinition(persona(), gooseRuntime);
+  assert.equal(
+    "envVars" in input,
+    false,
+    "definition env must never be seeded into the create input — " +
+      "record.env_vars is overrides-only and spawn merges the live definition env",
+  );
+});
+
+test("row 2: harnessOverride follows the backend-aligned formula", async () => {
+  const match = await buildInstanceInputForDefinition(
+    persona({ runtime: "goose" }),
+    gooseRuntime,
+  );
+  assert.equal(match.harnessOverride, true, "picked == configured → true");
+
+  const noPreference = await buildInstanceInputForDefinition(
+    persona({ runtime: undefined }),
+    gooseRuntime,
+  );
+  assert.equal(noPreference.harnessOverride, true, "no preference → true");
+
+  const differs = await buildInstanceInputForDefinition(
+    persona({ runtime: "claude" }),
+    gooseRuntime,
+  );
+  assert.equal(
+    differs.harnessOverride,
+    false,
+    "picked != configured → false (definition stays authoritative)",
+  );
+});
+
+test("row 3: plain avatar URLs pass through; base64 data URIs upload via the injectable", async () => {
+  const plain = await buildInstanceInputForDefinition(persona(), gooseRuntime);
+  assert.equal(plain.avatarUrl, "https://example.com/a.png");
+
+  const uploads = [];
+  const uploaded = await buildInstanceInputForDefinition(
+    persona({ avatarUrl: "data:image/png;base64,aGk=" }),
+    gooseRuntime,
+    async (bytes) => {
+      uploads.push(bytes);
+      return {
+        url: "https://cdn/blob.png",
+        sha256: "x",
+        size: 2,
+        type: "image/png",
+        uploaded: 0,
+      };
+    },
+  );
+  assert.equal(uploaded.avatarUrl, "https://cdn/blob.png");
+  assert.equal(uploads.length, 1, "upload must go through the injected fn");
+});
+
+test("mapping carries the runtime and definition fields", async () => {
+  const input = await buildInstanceInputForDefinition(persona(), gooseRuntime);
+  assert.equal(input.name, "Test Agent");
+  assert.equal(input.acpCommand, "buzz-acp");
+  assert.equal(input.agentCommand, "goose-cmd");
+  assert.deepEqual(input.agentArgs, ["--acp"]);
+  assert.equal(input.mcpCommand, "goose-mcp");
+  assert.equal(input.personaId, "p-1");
+  assert.equal(input.systemPrompt, "prompt");
+  assert.equal(input.model, undefined);
+  assert.equal(input.spawnAfterCreate, true);
+  assert.equal(input.startOnAppLaunch, true);
+  assert.deepEqual(input.backend, { type: "local" });
+});
+
+test("row 1: refuses when the configured runtime is not available", () => {
+  assert.throws(
+    () =>
+      resolveStartRuntimeForDefinition(persona({ runtime: "missing" }), [
+        gooseRuntime,
+        claudeRuntime,
+      ]),
+    /not available|No available runtime/i,
+    "configured-but-missing runtime must refuse, never silently fall back",
+  );
+});
+
+test("row 1: resolves the configured runtime when available", () => {
+  const { runtime, warnings } = resolveStartRuntimeForDefinition(
+    persona({ runtime: "claude" }),
+    [gooseRuntime, claudeRuntime],
+  );
+  assert.equal(runtime.id, "claude");
+  assert.deepEqual(warnings, []);
+});
+
+test("row 1: no preference resolves the default with no warnings", () => {
+  const { runtime, warnings } = resolveStartRuntimeForDefinition(
+    persona({ runtime: undefined }),
+    [gooseRuntime, claudeRuntime],
+  );
+  assert.equal(runtime.id, "goose");
+  assert.deepEqual(warnings, []);
+});
+
+test("row 1: refuses when no runtimes exist at all", () => {
+  assert.throws(
+    () => resolveStartRuntimeForDefinition(persona({ runtime: undefined }), []),
+    /No available runtime/,
+  );
+});
+
+test("row 6: fetched query uses cached data without refetching", async () => {
+  let refetched = false;
+  const runtimes = await availableRuntimesForStart({
+    isFetched: true,
+    data: [gooseRuntime, { ...claudeRuntime, availability: "missing" }],
+    refetch: async () => {
+      refetched = true;
+      return { data: [] };
+    },
+  });
+  assert.equal(refetched, false);
+  assert.deepEqual(
+    runtimes.map((r) => r.id),
+    ["goose"],
+    "unavailable runtimes are filtered out",
+  );
+});
+
+test("row 6: unfetched query refetches instead of resolving empty", async () => {
+  const runtimes = await availableRuntimesForStart({
+    isFetched: false,
+    data: undefined,
+    refetch: async () => ({ data: [claudeRuntime] }),
+  });
+  assert.deepEqual(
+    runtimes.map((r) => r.id),
+    ["claude"],
+    "an unfetched query must fetch, not spuriously report no runtimes",
+  );
+});

desktop/src/features/agents/lib/instanceInputForDefinition.ts

@@ -0,0 +1,107 @@
+import type {
+  AcpRuntime,
+  AcpRuntimeCatalogEntry,
+  AgentPersona,
+  CreateManagedAgentInput,
+} from "@/shared/api/types";
+import {
+  resolvePersonaRuntime,
+  type ResolvePersonaRuntimeResult,
+} from "./resolvePersonaRuntime";
+import {
+  resolveManagedAgentAvatarUrl,
+  type UploadMediaBytes,
+} from "../ui/managedAgentAvatar";
+
+type RuntimesQueryLike = {
+  isFetched: boolean;
+  data: readonly AcpRuntimeCatalogEntry[] | undefined;
+  refetch: () => Promise<{
+    data?: readonly AcpRuntimeCatalogEntry[] | undefined;
+  }>;
+};
+
+/**
+ * Acquire the available-runtime list for a start action (Phase 1B.3.5
+ * row 6). Refetch-aware: an unfetched query is fetched instead of being
+ * treated as an empty list (which would spuriously refuse every start).
+ */
+export async function availableRuntimesForStart(
+  query: RuntimesQueryLike,
+): Promise<AcpRuntime[]> {
+  const entries = query.isFetched ? query.data : (await query.refetch()).data;
+  return (entries ?? []).filter(
+    (runtime): runtime is AcpRuntime => runtime.availability === "available",
+  );
+}
+
+/**
+ * Resolve the runtime a definition should start on, refusing when the
+ * definition's configured runtime is not available (Phase 1B.3.5 row 1,
+ * Wes's call: one consistent refuse-with-actionable-error everywhere —
+ * never silently start on a different runtime than configured).
+ */
+export function resolveStartRuntimeForDefinition(
+  persona: AgentPersona,
+  runtimes: readonly AcpRuntime[],
+): { runtime: AcpRuntime; warnings: string[] } {
+  const defaultRuntime = runtimes[0] ?? null;
+  const { runtime, warnings, isOverridden }: ResolvePersonaRuntimeResult =
+    resolvePersonaRuntime(persona.runtime, runtimes, defaultRuntime);
+
+  if (!runtime) {
+    throw new Error("No available runtime found for this agent.");
+  }
+  if (isOverridden) {
+    throw new Error(
+      warnings[0] ??
+        "This agent's configured runtime is not available. Install the runtime or edit the agent before starting it.",
+    );
+  }
+  return { runtime, warnings };
+}
+
+/**
+ * The single definition→instance mapping (Phase 1B.3.5 rows 2–4). Every
+ * surface that creates a running instance from a definition builds its
+ * CreateManagedAgentInput here so the mapping cannot drift per-site.
+ *
+ * - harnessOverride uses the backend-aligned formula: true only when the
+ *   definition has no runtime preference or the picked runtime matches it
+ *   (`create_time_agent_command_override` stores None when picked ==
+ *   inherited; on fallback `harness_override: false` keeps the definition
+ *   authoritative).
+ * - avatarUrl goes through resolveManagedAgentAvatarUrl (base64 data URIs
+ *   upload via the injectable `upload`; other URLs pass through unchanged).
+ * - envVars are never seeded from the definition: record.env_vars is
+ *   agent overrides only and spawn merges the live definition env
+ *   underneath. Seeding would manufacture pseudo-overrides that mask
+ *   later definition edits made before the first spawn.
+ */
+export async function buildInstanceInputForDefinition(
+  persona: AgentPersona,
+  runtime: AcpRuntime,
+  upload?: UploadMediaBytes,
+): Promise<CreateManagedAgentInput> {
+  const avatarUrl = await resolveManagedAgentAvatarUrl(
+    persona.avatarUrl,
+    upload,
+    runtime.avatarUrl,
+  );
+
+  return {
+    name: persona.displayName,
+    acpCommand: "buzz-acp",
+    agentCommand: runtime.command,
+    agentArgs: runtime.defaultArgs,
+    mcpCommand: runtime.mcpCommand ?? "",
+    personaId: persona.id,
+    harnessOverride: !persona.runtime || persona.runtime === runtime.id,
+    systemPrompt: persona.systemPrompt,
+    avatarUrl,
+    model: persona.model ?? undefined,
+    spawnAfterCreate: true,
+    startOnAppLaunch: true,
+    backend: { type: "local" },
+  };
+}

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

@@ -6,7 +6,7 @@ type BlobDescriptor = {
   uploaded: number;
 };
 
-type UploadMediaBytes = (
+export type UploadMediaBytes = (
   data: number[],
   filename?: string,
 ) => Promise<BlobDescriptor>;

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

@@ -15,11 +15,8 @@ import {
 import { useChannelsQuery } from "@/features/channels/hooks";
 import { usePresenceQuery } from "@/features/presence/hooks";
 import type {
-  AcpRuntime,
-  AcpRuntimeCatalogEntry,
   AgentPersona,
   Channel,
-  CreateManagedAgentInput,
   CreateManagedAgentResponse,
   ManagedAgent,
 } from "@/shared/api/types";
@@ -31,7 +28,11 @@ import {
   startManagedAgentWithRules,
   stopManagedAgentWithRules,
 } from "../lib/managedAgentControlActions";
-import { resolvePersonaRuntime } from "../lib/resolvePersonaRuntime";
+import {
+  availableRuntimesForStart,
+  buildInstanceInputForDefinition,
+  resolveStartRuntimeForDefinition,
+} from "../lib/instanceInputForDefinition";
 
 export function useManagedAgentActions() {
   const relayAgentsQuery = useRelayAgentsQuery();
@@ -170,15 +171,6 @@ export function useManagedAgentActions() {
     }
   }
 
-  async function getAvailableRuntimesForStart() {
-    if (availableRuntimesQuery.isFetched) {
-      return availableRuntimesQuery.data ?? [];
-    }
-
-    const result = await availableRuntimesQuery.refetch();
-    return filterAvailableRuntimes(result.data);
-  }
-
   function setPersonaStartPending(personaId: string, pending: boolean) {
     const next = new Set(startingPersonaIdsRef.current);
     if (pending) {
@@ -197,40 +189,12 @@ export function useManagedAgentActions() {
     setPersonaStartPending(persona.id, true);
     clearFeedback();
     try {
-      const runtimes = await getAvailableRuntimesForStart();
-      const defaultRuntime = runtimes[0] ?? null;
-      const { runtime, warnings, isOverridden } = resolvePersonaRuntime(
-        persona.runtime,
+      const runtimes = await availableRuntimesForStart(availableRuntimesQuery);
+      const { runtime, warnings } = resolveStartRuntimeForDefinition(
+        persona,
         runtimes,
-        defaultRuntime,
       );
-
-      if (!runtime) {
-        throw new Error("No available runtime found for this agent.");
-      }
-      if (isOverridden) {
-        throw new Error(
-          warnings[0] ??
-            "This agent's configured runtime is not available. Install the runtime or edit the agent before starting it.",
-        );
-      }
-
-      const input: CreateManagedAgentInput = {
-        name: persona.displayName,
-        acpCommand: "buzz-acp",
-        agentCommand: runtime.command,
-        agentArgs: runtime.defaultArgs,
-        mcpCommand: runtime.mcpCommand ?? "",
-        personaId: persona.id,
-        systemPrompt: persona.systemPrompt,
-        avatarUrl: persona.avatarUrl ?? undefined,
-        model: persona.model ?? undefined,
-        envVars: persona.envVars,
-        spawnAfterCreate: true,
-        startOnAppLaunch: true,
-        backend: { type: "local" },
-        harnessOverride: !persona.runtime || persona.runtime === runtime.id,
-      };
+      const input = await buildInstanceInputForDefinition(persona, runtime);
 
       const created = await createAgentMutation.mutateAsync(input);
       setCreatedAgent(created);
@@ -489,11 +453,3 @@ export function useManagedAgentActions() {
     refetchRelayAgents: () => void relayAgentsQuery.refetch(),
   };
 }
-
-function filterAvailableRuntimes(
-  runtimes: readonly AcpRuntimeCatalogEntry[] | undefined,
-): AcpRuntime[] {
-  return (runtimes ?? []).filter(
-    (runtime): runtime is AcpRuntime => runtime.availability === "available",
-  );
-}

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

@@ -21,7 +21,6 @@ import { isSingleItemFile } from "@/shared/lib/fileMagic";
 import type {
   AcpRuntime,
   AgentPersona,
-  CreateManagedAgentInput,
   CreateManagedAgentResponse,
   CreatePersonaInput,
   UpdatePersonaInput,
@@ -38,6 +37,7 @@ import {
   type AgentCreateIntent,
 } from "./agentCreateIntent";
 import { resolveManagedAgentAvatarUrl } from "./managedAgentAvatar";
+import { buildInstanceInputForDefinition } from "../lib/instanceInputForDefinition";
 import { usePersonaImportActions } from "./usePersonaImportActions";
 
 type PersonaFeedbackSurface = "catalog" | "library";
@@ -198,21 +198,10 @@ export function usePersonaActions() {
           setPersonaDialogState(null);
           return true;
         }
-        const agentInput: CreateManagedAgentInput = {
-          name: persona.displayName,
-          acpCommand: "buzz-acp",
-          agentCommand: runtime.command,
-          agentArgs: runtime.defaultArgs,
-          mcpCommand: runtime.mcpCommand ?? "",
-          personaId: persona.id,
-          harnessOverride: true,
-          systemPrompt: persona.systemPrompt,
-          avatarUrl: persona.avatarUrl ?? avatarUrl,
-          model: persona.model ?? undefined,
-          spawnAfterCreate: true,
-          startOnAppLaunch: true,
-          backend: { type: "local" },
-        };
+        const agentInput = await buildInstanceInputForDefinition(
+          persona,
+          runtime,
+        );
 
         try {
           const created = await createAgentMutation.mutateAsync(agentInput);

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

@@ -27,7 +27,11 @@ import {
   useUpdatePersonaMutation,
 } from "@/features/agents/hooks";
 import { AddAgentToChannelDialog } from "@/features/agents/ui/AddAgentToChannelDialog";
-import { resolvePersonaRuntime } from "@/features/agents/lib/resolvePersonaRuntime";
+import {
+  availableRuntimesForStart,
+  buildInstanceInputForDefinition,
+  resolveStartRuntimeForDefinition,
+} from "@/features/agents/lib/instanceInputForDefinition";
 import {
   isManagedAgentActive,
   startManagedAgentWithRules,
@@ -92,7 +96,6 @@ import { cn } from "@/shared/lib/cn";
 import type {
   AgentPersona,
   Channel,
-  CreateManagedAgentInput,
   CreatePersonaInput,
   UpdatePersonaInput,
 } from "@/shared/api/types";
@@ -403,37 +406,20 @@ export function UserProfilePanel({
 
   const createManagedAgentForPersona = React.useCallback(
     async (personaToStart: AgentPersona) => {
-      const runtimes = availableRuntimesQuery.data ?? [];
-      const defaultRuntime = runtimes[0] ?? null;
-      const { runtime, warnings } = resolvePersonaRuntime(
-        personaToStart.runtime,
+      const runtimes = await availableRuntimesForStart(availableRuntimesQuery);
+      const { runtime, warnings } = resolveStartRuntimeForDefinition(
+        personaToStart,
         runtimes,
-        defaultRuntime,
       );
 
       for (const warning of warnings) {
         toast.warning(warning);
       }
 
-      if (!runtime) {
-        throw new Error("No available runtime found for this agent.");
-      }
-
-      const input: CreateManagedAgentInput = {
-        name: personaToStart.displayName,
-        acpCommand: "buzz-acp",
-        agentCommand: runtime.command,
-        agentArgs: runtime.defaultArgs,
-        mcpCommand: runtime.mcpCommand ?? "",
-        personaId: personaToStart.id,
-        systemPrompt: personaToStart.systemPrompt,
-        avatarUrl: personaToStart.avatarUrl ?? undefined,
-        model: personaToStart.model ?? undefined,
-        envVars: personaToStart.envVars,
-        spawnAfterCreate: true,
-        startOnAppLaunch: true,
-        backend: { type: "local" },
-      };
+      const input = await buildInstanceInputForDefinition(
+        personaToStart,
+        runtime,
+      );
 
       const created = await createAgentMutation.mutateAsync(input);
       void managedAgentsQuery.refetch();
@@ -441,7 +427,7 @@ export function UserProfilePanel({
       return created;
     },
     [
-      availableRuntimesQuery.data,
+      availableRuntimesQuery,
       createAgentMutation.mutateAsync,
       managedAgentsQuery.refetch,
       relayAgentsQuery.refetch,