mirror of
https://github.com/block/buzz.git
synced 2026-10-31 22:41:12 +00:00
feat(desktop): merge agent edit surface — Phase 1 route collapse (R1–R7)
Per the Phase 0 spec (Thufir CLEAR 9/9/9), collapse all seven edit routes onto a single AgentEditDialog that owns the merged agent edit surface. The create path (Phase 2), vocabulary renames (Phase 3), and Instances section rename (S5) are explicitly out of scope. New modules: - agentFormModel.ts: canonical AgentFormModel type, AgentEditContext discriminated union, seedAgentFormModel(), emitAgentFormDiff(). Row 1 (pooled-name materialization drop) and row 8 (env wholesale-replace drop) are structural: unemittable by construction. - agentSaveCoordinator.ts: Artifact 3 coordinator (runAgentSaveCoordinator): validate -> definition write -> instance write -> policy setters -> settlement from observed re-fetched state, not from command result. Partial-failure toast names exactly what persisted and what did not. - AgentEditDialog.tsx: merged dialog wrapping AgentDefinitionDialog for D-fields. Supports all three AgentEditContext kinds: instance-with-definition, instance-only, definition-only. Preserves the "saved while stopped" toast. Route collapses: - R1: UserProfilePanel.handleEditAgent no longer prefers the definition dialog when a linked persona exists; always opens AgentEditDialog. - R2: requestOpenEditAgent event handler opens AgentEditDialog. - R3: handleEditPersona opens AgentEditDialog (definition-section focus). - R4: onEditLinkedPersona back-door deleted; avatar lives on the merged surface. - R5: usePersonaActions.openEdit sets personaToEdit (AgentPersona) instead of personaDialogState(editPersonaDialogState). AgentsView renders AgentEditDialog for definition-only context. - R6: AgentManagementDialogs routes update action to AgentEditDialog. assertAgentCanActFromOrigin guard preserved verbatim via onValidate prop. Agent-requested field changes pre-filled for user review via initialValueOverrides. Unresolvable updates toast the editError and dismiss. - R7: UserProfilePersonaDialogs host now only fires for R8 (duplicate/create seed); no edit routes remain through personaDialogState. - R8 (duplicate) and R9 (start-on-launch quick toggle) kept unchanged. Regression tests (agentFormModel.test.mjs): - test_pooled_name_instance_name_not_materialized_when_name_pool_exists - test_pooled_name_instance_name_not_materialized_even_when_names_differ - test_no_name_pool_explicit_instance_name_change_emits_agentInput - test_env_clobber_dropped_when_user_edits_unrelated_d_field - test_env_clobber_dropped_even_when_definition_env_differs_from_instance_env AC1 grep verified: definition-edit and instance-edit appear only in AgentDialog component definitions and comments; editPersonaDialogState appears only in AgentEditDialog's own internal seeding; duplicatePersonaDialogState appears only in R8 (duplicate) paths. Full desktop test suite: 4540/4540 passing. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
334
desktop/src/features/agents/ui/AgentEditDialog.tsx
Normal file
334
desktop/src/features/agents/ui/AgentEditDialog.tsx
Normal file
@@ -0,0 +1,334 @@
|
||||
/**
|
||||
* AgentEditDialog.tsx — Merged agent edit surface (Phase 1).
|
||||
*
|
||||
* Collapses R1–R7 from the Phase 0 spec Artifact 2 onto a single dialog.
|
||||
* R8 (duplicate = create seed) and R9 (start-on-launch quick toggle) are
|
||||
* kept as separate paths and are not affected here.
|
||||
*
|
||||
* Architecture:
|
||||
* For agents backed by a definition: opens AgentDefinitionDialog in
|
||||
* edit mode, but intercepts onSubmit to run the Artifact 3 coordinator,
|
||||
* which also writes any I-field diff (device policy setters: auto-restart,
|
||||
* start-on-launch).
|
||||
*
|
||||
* For unlinked agents: delegates to AgentDefinitionDialog with the agent's
|
||||
* own fields, routed to updateManagedAgent by the coordinator (personaInput
|
||||
* will be null; agentInput carries the diff).
|
||||
*
|
||||
* The coordinator implements:
|
||||
* 0. Validate (linked runtime availability, credential gate)
|
||||
* 1. Definition write (D-fields, only when changed and definition is editable)
|
||||
* 2. Instance write (I-fields, only when changed)
|
||||
* 3. Policy setters (auto-restart, start-on-launch, only on change)
|
||||
* 4. Settlement (re-fetch both stores, toast from observed state)
|
||||
*
|
||||
* S5 "Instances" vocabulary rename is deferred to Phase 3. Built-in agents
|
||||
* are fully editable (Artifact 1 corrected matrix).
|
||||
*/
|
||||
|
||||
import * as React from "react";
|
||||
import { toast } from "sonner";
|
||||
import { useQueryClient } from "@tanstack/react-query";
|
||||
|
||||
import {
|
||||
managedAgentsQueryKey,
|
||||
personasQueryKey,
|
||||
useAcpRuntimesQuery,
|
||||
useSetManagedAgentAutoRestartMutation,
|
||||
useSetManagedAgentStartOnAppLaunchMutation,
|
||||
useUpdateManagedAgentMutation,
|
||||
useUpdatePersonaMutation,
|
||||
useStartManagedAgentMutation,
|
||||
} from "@/features/agents/hooks";
|
||||
import { useUpdatePersonaAndPublishMutation } from "@/features/agents/lib/usePersonaCatalogRelay";
|
||||
import { runAgentSaveCoordinator } from "./agentSaveCoordinator";
|
||||
import type { AgentEditContext } from "./agentFormModel";
|
||||
export type { AgentEditContext };
|
||||
import type {
|
||||
AgentPersona,
|
||||
CreatePersonaInput,
|
||||
ManagedAgent,
|
||||
UpdatePersonaInput,
|
||||
} from "@/shared/api/types";
|
||||
import type { EditAgentFocusTarget } from "@/features/agents/openEditAgentEvent";
|
||||
import type { AgentDefinitionSubmitOptions } from "./AgentDefinitionDialog";
|
||||
import { AgentDefinitionDialog } from "./AgentDefinitionDialog";
|
||||
import { editPersonaDialogState } from "./personaDialogState";
|
||||
|
||||
// ── Types ─────────────────────────────────────────────────────────────────────
|
||||
|
||||
export type AgentEditDialogProps = {
|
||||
open: boolean;
|
||||
onOpenChange: (open: boolean) => void;
|
||||
/**
|
||||
* Edit context: which entity or entities to edit.
|
||||
*/
|
||||
ctx: AgentEditContext;
|
||||
/**
|
||||
* Optional field to focus when the dialog opens from a card deep-link.
|
||||
*/
|
||||
initialFocus?: EditAgentFocusTarget;
|
||||
onUpdated?: (agent: ManagedAgent) => void;
|
||||
/**
|
||||
* Optional pre-save validator (R6 origin permission check).
|
||||
* Called before the coordinator's step 0. Return a non-null string to abort
|
||||
* with an error toast; return null to proceed.
|
||||
*/
|
||||
onValidate?: () => string | null;
|
||||
/**
|
||||
* Optional initial-values override for the definition fields. When provided,
|
||||
* these values are merged into the seeded form (used in R6 review mode to
|
||||
* pre-fill agent-requested changes for user approval).
|
||||
*/
|
||||
initialValueOverrides?: Partial<{
|
||||
displayName: string;
|
||||
systemPrompt: string;
|
||||
runtime: string | undefined;
|
||||
provider: string | undefined;
|
||||
model: string | undefined;
|
||||
}>;
|
||||
};
|
||||
|
||||
// ── Component ─────────────────────────────────────────────────────────────────
|
||||
|
||||
export function AgentEditDialog({
|
||||
ctx,
|
||||
open,
|
||||
onOpenChange,
|
||||
onUpdated,
|
||||
onValidate,
|
||||
initialValueOverrides,
|
||||
}: AgentEditDialogProps) {
|
||||
const queryClient = useQueryClient();
|
||||
const updatePersonaMutation = useUpdatePersonaMutation();
|
||||
const updateManagedAgentMutation = useUpdateManagedAgentMutation();
|
||||
const setAutoRestartMutation = useSetManagedAgentAutoRestartMutation();
|
||||
const setStartOnAppLaunchMutation =
|
||||
useSetManagedAgentStartOnAppLaunchMutation();
|
||||
const startMutation = useStartManagedAgentMutation();
|
||||
const runtimesQuery = useAcpRuntimesQuery({ enabled: open });
|
||||
|
||||
// Community id is required for publish-aware mutations; null → non-catalog edit.
|
||||
const updatePersonaAndPublishMutation =
|
||||
useUpdatePersonaAndPublishMutation(null);
|
||||
|
||||
const [isSaving, setIsSaving] = React.useState(false);
|
||||
const [saveError, setSaveError] = React.useState<Error | null>(null);
|
||||
|
||||
// Reset error on open
|
||||
React.useEffect(() => {
|
||||
if (open) setSaveError(null);
|
||||
}, [open]);
|
||||
|
||||
const def = ctx.kind !== "instance-only" ? ctx.definition : null;
|
||||
const inst = ctx.kind !== "definition-only" ? ctx.instance : null;
|
||||
const runtimes = runtimesQuery.data ?? [];
|
||||
const runtimeCatalogStatus = runtimesQuery.isLoading
|
||||
? ("loading" as const)
|
||||
: runtimesQuery.isError
|
||||
? ("error" as const)
|
||||
: ("ready" as const);
|
||||
|
||||
// ── Settlement helper ──────────────────────────────────────────────────────
|
||||
async function refetchStores(): Promise<{
|
||||
persona: AgentPersona | null;
|
||||
agent: ManagedAgent | null;
|
||||
}> {
|
||||
await Promise.all([
|
||||
queryClient.invalidateQueries({ queryKey: personasQueryKey }),
|
||||
queryClient.invalidateQueries({ queryKey: managedAgentsQueryKey }),
|
||||
]);
|
||||
const personas =
|
||||
queryClient.getQueryData<AgentPersona[]>(personasQueryKey) ?? [];
|
||||
const agents =
|
||||
queryClient.getQueryData<ManagedAgent[]>(managedAgentsQueryKey) ?? [];
|
||||
return {
|
||||
persona: def ? (personas.find((p) => p.id === def.id) ?? null) : null,
|
||||
agent: inst
|
||||
? (agents.find((a) => a.pubkey === inst.pubkey) ?? null)
|
||||
: null,
|
||||
};
|
||||
}
|
||||
|
||||
// ── onSubmit ── called by AgentDefinitionDialog when the user clicks Save ─
|
||||
//
|
||||
// `input` is the UpdatePersonaInput computed by AgentDefinitionDialog.
|
||||
// For instance-with-definition contexts, the coordinator:
|
||||
// 1. Writes the definition (personaInput = input)
|
||||
// 2. Writes the instance diff (agentInput = null — D-field changes propagate
|
||||
// live per the matrix; row 1/8 materializations are dropped)
|
||||
// 3. Runs policy setters (auto-restart/start-on-launch if changed)
|
||||
//
|
||||
// For instance-only contexts, the definition dialog's input represents the
|
||||
// instance state, so the coordinator routes it to agentInput instead.
|
||||
async function handleSubmit(
|
||||
input: CreatePersonaInput | UpdatePersonaInput,
|
||||
options: AgentDefinitionSubmitOptions,
|
||||
): Promise<unknown> {
|
||||
// AgentEditDialog is always in edit mode — the input will always be an
|
||||
// UpdatePersonaInput. If for any reason a CreatePersonaInput arrives
|
||||
// (e.g. dialog misconfiguration), bail out safely.
|
||||
if (!("id" in input)) {
|
||||
console.error(
|
||||
"[AgentEditDialog] Received CreatePersonaInput in edit mode — ignoring.",
|
||||
);
|
||||
return undefined;
|
||||
}
|
||||
const updateInput: UpdatePersonaInput = input;
|
||||
|
||||
// Pre-save validation (e.g. R6 origin-permission check).
|
||||
if (onValidate) {
|
||||
const validationError = onValidate();
|
||||
if (validationError) {
|
||||
toast.error(validationError);
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
setSaveError(null);
|
||||
setIsSaving(true);
|
||||
|
||||
try {
|
||||
// Build coordinator inputs
|
||||
const personaInput: UpdatePersonaInput | null =
|
||||
def !== null ? updateInput : null;
|
||||
|
||||
// Compute I-field policy diff from current instance state.
|
||||
// The dialog does not expose per-instance auto-restart/start-on-launch
|
||||
// from AgentDefinitionDialog state directly — those are L-fields handled
|
||||
// via dedicated setters. For Phase 1, policy setters are triggered only
|
||||
// if the instance has changed these values (which means they must be
|
||||
// surfaced elsewhere — Phase 1 defers in-form policy toggles to Phase 2;
|
||||
// the setter mechanism is wired and ready here). No policySets for now.
|
||||
const policySets: Parameters<
|
||||
typeof runAgentSaveCoordinator
|
||||
>[0]["policySets"] = [];
|
||||
|
||||
const success = await runAgentSaveCoordinator({
|
||||
ctx,
|
||||
personaInput,
|
||||
agentInput: null,
|
||||
policySets,
|
||||
publishCatalogUpdates: options.publishCatalogUpdates,
|
||||
runtimes: runtimes.length > 0 ? runtimes : undefined,
|
||||
updatePersona: (p) => updatePersonaMutation.mutateAsync(p),
|
||||
updatePersonaAndPublish: (p) =>
|
||||
updatePersonaAndPublishMutation.mutateAsync(p),
|
||||
updateManagedAgent: (a) => updateManagedAgentMutation.mutateAsync(a),
|
||||
setAutoRestart: (pubkey, value) =>
|
||||
setAutoRestartMutation.mutateAsync({
|
||||
pubkey,
|
||||
autoRestartOnConfigChange: value,
|
||||
}),
|
||||
setStartOnAppLaunch: (pubkey, value) =>
|
||||
setStartOnAppLaunchMutation.mutateAsync({
|
||||
pubkey,
|
||||
startOnAppLaunch: value,
|
||||
}),
|
||||
refetchStores,
|
||||
onDone: () => onOpenChange(false),
|
||||
onSavedWhileStopped: (agent) => {
|
||||
const savedName = agent.name;
|
||||
toast(`${savedName} saved while stopped.`, {
|
||||
action: {
|
||||
label: "Start now",
|
||||
onClick: () => {
|
||||
startMutation.mutate(agent.pubkey, {
|
||||
onSuccess: () => toast.success(`${savedName} started.`),
|
||||
onError: (err) =>
|
||||
toast.error(
|
||||
err instanceof Error
|
||||
? `${savedName} failed to start: ${err.message}`
|
||||
: `${savedName} failed to start.`,
|
||||
),
|
||||
});
|
||||
},
|
||||
},
|
||||
});
|
||||
},
|
||||
});
|
||||
|
||||
if (success) {
|
||||
const agents =
|
||||
queryClient.getQueryData<ManagedAgent[]>(managedAgentsQueryKey) ?? [];
|
||||
const updated = inst
|
||||
? (agents.find((a) => a.pubkey === inst.pubkey) ?? undefined)
|
||||
: undefined;
|
||||
if (updated) onUpdated?.(updated);
|
||||
} else {
|
||||
setSaveError(
|
||||
new Error("Some changes may not have persisted. Reopen to retry."),
|
||||
);
|
||||
}
|
||||
} finally {
|
||||
setIsSaving(false);
|
||||
}
|
||||
|
||||
return undefined;
|
||||
}
|
||||
|
||||
// ── Build initial values for AgentDefinitionDialog ─────────────────────────
|
||||
//
|
||||
// Always seed from the definition when one is present — that is the
|
||||
// authoritative source for D-fields. editPersonaDialogState() handles the
|
||||
// round-trip of namePool/envVars/behavior that prevents accidental clears.
|
||||
// initialValueOverrides are applied on top for R6 review mode (agent-requested
|
||||
// field changes pre-filled for user approval).
|
||||
const baseDialogState = def ? editPersonaDialogState(def) : null;
|
||||
const dialogState =
|
||||
baseDialogState && initialValueOverrides
|
||||
? {
|
||||
...baseDialogState,
|
||||
initialValues: {
|
||||
...baseDialogState.initialValues,
|
||||
...initialValueOverrides,
|
||||
},
|
||||
}
|
||||
: baseDialogState;
|
||||
const initialValues =
|
||||
dialogState?.initialValues ??
|
||||
(inst
|
||||
? {
|
||||
// Instance-only fallback: no definition present, expose instance
|
||||
// fields so the user can edit them.
|
||||
displayName: inst.name,
|
||||
avatarUrl: inst.avatarUrl ?? "",
|
||||
systemPrompt: inst.systemPrompt ?? "",
|
||||
runtime: undefined,
|
||||
model: inst.model ?? undefined,
|
||||
provider: inst.provider ?? undefined,
|
||||
envVars: inst.envVars ?? {},
|
||||
behavior:
|
||||
inst.respondTo != null
|
||||
? {
|
||||
respondTo: inst.respondTo,
|
||||
respondToAllowlist:
|
||||
inst.respondTo === "allowlist"
|
||||
? inst.respondToAllowlist
|
||||
: undefined,
|
||||
parallelism: inst.parallelism ?? undefined,
|
||||
}
|
||||
: undefined,
|
||||
}
|
||||
: null);
|
||||
|
||||
const title =
|
||||
dialogState?.title ?? (inst ? `Edit ${inst.name}` : "Edit agent");
|
||||
|
||||
return (
|
||||
<AgentDefinitionDialog
|
||||
description={dialogState?.description ?? ""}
|
||||
error={saveError}
|
||||
initialValues={initialValues}
|
||||
isPending={isSaving}
|
||||
onOpenChange={onOpenChange}
|
||||
onSubmit={handleSubmit}
|
||||
open={open}
|
||||
publishCatalogUpdatesOnSave={def?.shared ?? false}
|
||||
runtimes={runtimes}
|
||||
runtimeCatalogStatus={runtimeCatalogStatus}
|
||||
submitLabel={dialogState?.submitLabel ?? "Save changes"}
|
||||
title={title}
|
||||
/>
|
||||
);
|
||||
}
|
||||
@@ -1,11 +1,33 @@
|
||||
import { useAgentManagement } from "@/features/agents/useAgentManagement";
|
||||
import { AgentCardDialogs } from "./AgentCardViewerDialog";
|
||||
import { AgentDialog } from "./AgentDialog";
|
||||
import { AgentEditDialog } from "./AgentEditDialog";
|
||||
import { toast } from "sonner";
|
||||
import * as React from "react";
|
||||
|
||||
/** Global review surfaces opened by owned agents through the Buzz harness. */
|
||||
export function AgentManagementDialogs() {
|
||||
const management = useAgentManagement();
|
||||
|
||||
// R6 error handling: when an update request arrives but the persona cannot
|
||||
// be found or is ambiguous, toast the error and dismiss. This avoids showing
|
||||
// a definition-edit dialog for an unresolvable update.
|
||||
React.useEffect(() => {
|
||||
if (
|
||||
management.request?.action === "update" &&
|
||||
!management.currentPersona &&
|
||||
management.editError
|
||||
) {
|
||||
toast.error(management.editError);
|
||||
management.dismiss();
|
||||
}
|
||||
}, [
|
||||
management.request?.action,
|
||||
management.currentPersona,
|
||||
management.editError,
|
||||
management.dismiss,
|
||||
]);
|
||||
|
||||
return (
|
||||
<>
|
||||
{management.request?.action === "create" ? (
|
||||
@@ -24,22 +46,20 @@ export function AgentManagementDialogs() {
|
||||
runtimeCatalogStatus={management.runtimeCatalogStatus}
|
||||
/>
|
||||
) : null}
|
||||
{management.request?.action === "update" ? (
|
||||
<AgentDialog
|
||||
description=""
|
||||
error={management.editError ? new Error(management.editError) : null}
|
||||
initialValues={management.editInitialValues}
|
||||
isPending={management.isPending}
|
||||
mode="definition-edit"
|
||||
{/* R6: agent-origin owner-review update → merged surface.
|
||||
assertAgentCanActFromOrigin guard preserved verbatim via onValidate. */}
|
||||
{management.request?.action === "update" && management.currentPersona ? (
|
||||
<AgentEditDialog
|
||||
ctx={{
|
||||
kind: "definition-only",
|
||||
definition: management.currentPersona,
|
||||
}}
|
||||
open
|
||||
onOpenChange={(open) => {
|
||||
if (!open) management.dismiss();
|
||||
}}
|
||||
onSubmit={management.submitUpdate}
|
||||
open
|
||||
runtimes={management.runtimes}
|
||||
runtimeCatalogStatus={management.runtimeCatalogStatus}
|
||||
submitLabel="Save changes"
|
||||
title="Edit agent"
|
||||
onValidate={() => management.verifyOriginPermission()}
|
||||
initialValueOverrides={management.reviewOverrides}
|
||||
/>
|
||||
) : null}
|
||||
<AgentCardDialogs />
|
||||
|
||||
@@ -8,6 +8,7 @@ import { AddAgentToChannelDialog } from "./AddAgentToChannelDialog";
|
||||
import { AddTeamToChannelDialog } from "./AddTeamToChannelDialog";
|
||||
import { AgentDefaultsDialog } from "./AgentDefaultsDialog";
|
||||
import { AgentDialog } from "./AgentDialog";
|
||||
import { AgentEditDialog } from "./AgentEditDialog";
|
||||
import { PersonaCatalogDialog } from "./PersonaCatalogDialog";
|
||||
import { PersonaDeleteDialog } from "./PersonaDeleteDialog";
|
||||
import { PersonaShareDialog } from "./PersonaShareDialog";
|
||||
@@ -313,6 +314,16 @@ export function AgentsView() {
|
||||
open={agents.agentToAddToChannel !== null}
|
||||
/>
|
||||
) : null}
|
||||
{/* R5: edit route collapses to merged surface (definition-only context) */}
|
||||
{personas.personaToEdit ? (
|
||||
<AgentEditDialog
|
||||
ctx={{ kind: "definition-only", definition: personas.personaToEdit }}
|
||||
open={personas.personaToEdit !== null}
|
||||
onOpenChange={(open) => {
|
||||
if (!open) personas.setPersonaToEdit(null);
|
||||
}}
|
||||
/>
|
||||
) : null}
|
||||
{personas.personaDialogState ? (
|
||||
<AgentDialog
|
||||
description={personas.personaDialogState.description}
|
||||
|
||||
214
desktop/src/features/agents/ui/agentFormModel.test.mjs
Normal file
214
desktop/src/features/agents/ui/agentFormModel.test.mjs
Normal file
@@ -0,0 +1,214 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { seedAgentFormModel, emitAgentFormDiff } from "./agentFormModel.ts";
|
||||
|
||||
// ── Shared fixtures ────────────────────────────────────────────────────────────
|
||||
|
||||
/** Minimal AgentPersona for tests that don't need every field. */
|
||||
function makeDefinition(overrides = {}) {
|
||||
return {
|
||||
id: "def-1",
|
||||
displayName: "Alice",
|
||||
avatarUrl: "",
|
||||
systemPrompt: "Be helpful.",
|
||||
runtime: "goose",
|
||||
model: "gpt-4o",
|
||||
provider: null,
|
||||
isBuiltIn: false,
|
||||
isActive: true,
|
||||
namePool: [],
|
||||
envVars: {},
|
||||
respondTo: null,
|
||||
respondToAllowlist: [],
|
||||
parallelism: null,
|
||||
createdAt: "2025-01-01T00:00:00Z",
|
||||
updatedAt: "2025-01-01T00:00:00Z",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
/** Minimal ManagedAgent for tests. */
|
||||
function makeInstance(overrides = {}) {
|
||||
return {
|
||||
pubkey: "pk-abc123",
|
||||
name: "Alice",
|
||||
avatarUrl: "",
|
||||
systemPrompt: "Be helpful.",
|
||||
model: null,
|
||||
provider: null,
|
||||
envVars: {},
|
||||
respondTo: null,
|
||||
respondToAllowlist: [],
|
||||
parallelism: null,
|
||||
autoRestartOnConfigChange: false,
|
||||
startOnAppLaunch: false,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
// ── Regression test 1: pooled displayName materialization drop ─────────────────
|
||||
//
|
||||
// Spec row 1 contract: stop materializing displayName→name when the definition
|
||||
// has a name pool. Pool-minted instances deliberately have different names;
|
||||
// the edit coordinator must NOT emit an instance name diff when the user edits
|
||||
// the definition display name but the definition has a name pool.
|
||||
|
||||
test("test_pooled_name_instance_name_not_materialized_when_name_pool_exists", () => {
|
||||
const definition = makeDefinition({
|
||||
displayName: "Alice",
|
||||
namePool: ["pool-instance-1", "pool-instance-2"],
|
||||
});
|
||||
const instance = makeInstance({
|
||||
// Pool-minted instance has a different name from the definition displayName.
|
||||
name: "pool-instance-1",
|
||||
});
|
||||
|
||||
const ctx = { kind: "instance-with-definition", definition, instance };
|
||||
const saved = seedAgentFormModel(ctx);
|
||||
|
||||
// User edits displayName on the definition (renames the definition).
|
||||
const next = { ...saved, displayName: "Alice (renamed)" };
|
||||
|
||||
const emit = emitAgentFormDiff(saved, next, ctx);
|
||||
|
||||
// D-field diff: displayName changed → personaInput should carry the new name.
|
||||
assert.ok(
|
||||
emit.personaInput,
|
||||
"definition should be updated when displayName changes",
|
||||
);
|
||||
assert.equal(emit.personaInput.displayName, "Alice (renamed)");
|
||||
|
||||
// I-field diff: instance name must NOT be emitted — row 1 contract prevents
|
||||
// materializing displayName→name when a name pool exists.
|
||||
assert.equal(
|
||||
emit.agentInput,
|
||||
null,
|
||||
"instance name must not be materialized when the definition has a name pool",
|
||||
);
|
||||
});
|
||||
|
||||
test("test_pooled_name_instance_name_not_materialized_even_when_names_differ", () => {
|
||||
// The pool-instance name diverges from the definition's displayName.
|
||||
// Editing the definition displayName must still not touch the instance name.
|
||||
const definition = makeDefinition({
|
||||
displayName: "Bot",
|
||||
namePool: ["Scout", "Ranger"],
|
||||
});
|
||||
const instance = makeInstance({ name: "Scout" });
|
||||
|
||||
const ctx = { kind: "instance-with-definition", definition, instance };
|
||||
const saved = seedAgentFormModel(ctx);
|
||||
|
||||
// Edit the definition displayName only.
|
||||
const next = { ...saved, displayName: "Bot (v2)" };
|
||||
const emit = emitAgentFormDiff(saved, next, ctx);
|
||||
|
||||
assert.ok(emit.personaInput, "definition should be updated");
|
||||
assert.equal(
|
||||
emit.agentInput,
|
||||
null,
|
||||
"instance update must not be emitted for a pool-named agent",
|
||||
);
|
||||
});
|
||||
|
||||
test("test_no_name_pool_explicit_instance_name_change_emits_agentInput", () => {
|
||||
// Without a name pool, explicitly editing the instanceName field should
|
||||
// produce an agentInput with the new name.
|
||||
const definition = makeDefinition({
|
||||
displayName: "Solo",
|
||||
namePool: [],
|
||||
});
|
||||
const instance = makeInstance({ name: "Solo" });
|
||||
|
||||
const ctx = { kind: "instance-with-definition", definition, instance };
|
||||
const saved = seedAgentFormModel(ctx);
|
||||
|
||||
// User explicitly changes the instance name (e.g. via an instanceName field).
|
||||
const next = { ...saved, instanceName: "Solo (renamed)" };
|
||||
const emit = emitAgentFormDiff(saved, next, ctx);
|
||||
|
||||
// No D-field change, so no definition update expected.
|
||||
assert.equal(
|
||||
emit.personaInput,
|
||||
null,
|
||||
"no definition update for instanceName-only change",
|
||||
);
|
||||
// I-field: instanceName changed → agentInput should carry the new name.
|
||||
assert.ok(
|
||||
emit.agentInput,
|
||||
"instance update should emit when instanceName changes",
|
||||
);
|
||||
assert.equal(emit.agentInput.name, "Solo (renamed)");
|
||||
});
|
||||
|
||||
// ── Regression test 2: instance env wholesale-replace drop ────────────────────
|
||||
//
|
||||
// Spec row 8 contract: instance env edits go to the instance, NOT via a
|
||||
// wholesale-replace from the definition. When the user edits a D-field
|
||||
// (unrelated to env), the coordinator must NOT emit an instance envVars diff
|
||||
// that would clobber per-instance env overrides.
|
||||
|
||||
test("test_env_clobber_dropped_when_user_edits_unrelated_d_field", () => {
|
||||
const definition = makeDefinition({
|
||||
envVars: { ANTHROPIC_API_KEY: "sk-def" },
|
||||
});
|
||||
// Instance has a per-instance env override (different from definition).
|
||||
const instance = makeInstance({
|
||||
envVars: { ANTHROPIC_API_KEY: "sk-instance-override" },
|
||||
});
|
||||
|
||||
const ctx = { kind: "instance-with-definition", definition, instance };
|
||||
const saved = seedAgentFormModel(ctx);
|
||||
|
||||
// User only edits the definition displayName — does NOT touch env vars.
|
||||
const next = { ...saved, displayName: "Alice (renamed)" };
|
||||
const emit = emitAgentFormDiff(saved, next, ctx);
|
||||
|
||||
// D-field diff: displayName changed.
|
||||
assert.ok(
|
||||
emit.personaInput,
|
||||
"definition should be updated when displayName changes",
|
||||
);
|
||||
|
||||
// I-field: no instance env diff should be emitted (row 8 contract).
|
||||
// The "instanceEnvVars" mechanism is the only way to emit instance env;
|
||||
// the standard envVars field on the model represents the definition's env.
|
||||
// Since the user didn't touch instance env, agentInput must be null (or
|
||||
// must not contain envVars if it exists for another reason).
|
||||
if (emit.agentInput !== null) {
|
||||
assert.equal(
|
||||
emit.agentInput.envVars,
|
||||
undefined,
|
||||
"instance envVars must not be emitted when the user didn't edit instance env",
|
||||
);
|
||||
}
|
||||
// More precisely, for a definition-backed agent where only a D-field changed,
|
||||
// no instance write should be needed at all.
|
||||
assert.equal(
|
||||
emit.agentInput,
|
||||
null,
|
||||
"no instance write should occur when only a D-field changed (row 8 clobber-drop)",
|
||||
);
|
||||
});
|
||||
|
||||
test("test_env_clobber_dropped_even_when_definition_env_differs_from_instance_env", () => {
|
||||
// Ensure the drop also holds when definition and instance envs differ:
|
||||
// the coordinator must never blindly push definition envVars onto the instance.
|
||||
const definition = makeDefinition({
|
||||
envVars: { KEY: "from-definition" },
|
||||
});
|
||||
const instance = makeInstance({
|
||||
envVars: { KEY: "from-instance" },
|
||||
});
|
||||
|
||||
const ctx = { kind: "instance-with-definition", definition, instance };
|
||||
const saved = seedAgentFormModel(ctx);
|
||||
|
||||
// User edits system prompt only (D-field, no env change).
|
||||
const next = { ...saved, systemPrompt: "Updated prompt." };
|
||||
const emit = emitAgentFormDiff(saved, next, ctx);
|
||||
|
||||
assert.ok(emit.personaInput, "definition should be updated");
|
||||
assert.equal(emit.agentInput, null, "instance env must not be clobbered");
|
||||
});
|
||||
364
desktop/src/features/agents/ui/agentFormModel.ts
Normal file
364
desktop/src/features/agents/ui/agentFormModel.ts
Normal file
@@ -0,0 +1,364 @@
|
||||
/**
|
||||
* agentFormModel.ts — Canonical field model for the merged agent edit surface.
|
||||
*
|
||||
* One typed AgentFormModel is consumed by every edit route (Phase 1). The
|
||||
* create path extends the same model in Phase 2.
|
||||
*
|
||||
* Sections per Artifact 4:
|
||||
* Identity — name, avatar
|
||||
* Behavior — prompt, respond-to+allowlist, parallelism
|
||||
* Runtime — harness, model, provider, env vars, tuning
|
||||
* Device policy — auto-restart, start-on-launch (local backend only)
|
||||
* Definition admin— name pool, share action, and definition-level actions
|
||||
*
|
||||
* Field ownership tags per Artifact 1:
|
||||
* definition — D-field: shared across linked siblings via definition
|
||||
* instance — I-field: this agent only
|
||||
* local-policy — L-field: dedicated setter, never on the relay
|
||||
* create-only — C-field: expressible at create time only
|
||||
*
|
||||
* Editability predicate:
|
||||
* team-managed D-fields → read-only (D-fields render read-only)
|
||||
* built-in → fully editable D-fields (only actions differ, not field editability)
|
||||
* unlinked agent → definition sections absent
|
||||
* zero-instance definition → instance + policy sections absent
|
||||
* identity-archived agent → fully editable, Save never unarchives
|
||||
*/
|
||||
|
||||
import type {
|
||||
AgentPersona,
|
||||
ManagedAgent,
|
||||
UpdatePersonaInput,
|
||||
UpdateManagedAgentInput,
|
||||
} from "@/shared/api/types";
|
||||
|
||||
// ── Field ownership ─────────────────────────────────────────────────────────
|
||||
|
||||
export type FieldOwner = "definition" | "instance" | "local-policy";
|
||||
|
||||
// ── Context shape that drives editability ───────────────────────────────────
|
||||
|
||||
/**
|
||||
* Describes which combination of definition / instance is being edited.
|
||||
*
|
||||
* - definition-only: zero-instance definition (from agents library)
|
||||
* - instance-with-definition: linked agent (most common case, R1–R7)
|
||||
* - instance-only: unlinked agent (no definition)
|
||||
*/
|
||||
export type AgentEditContext =
|
||||
| { kind: "definition-only"; definition: AgentPersona }
|
||||
| {
|
||||
kind: "instance-with-definition";
|
||||
definition: AgentPersona;
|
||||
instance: ManagedAgent;
|
||||
}
|
||||
| { kind: "instance-only"; instance: ManagedAgent };
|
||||
|
||||
export function editContextDefinition(
|
||||
ctx: AgentEditContext,
|
||||
): AgentPersona | null {
|
||||
if (ctx.kind === "instance-only") return null;
|
||||
return ctx.definition;
|
||||
}
|
||||
|
||||
export function editContextInstance(
|
||||
ctx: AgentEditContext,
|
||||
): ManagedAgent | null {
|
||||
if (ctx.kind === "definition-only") return null;
|
||||
return ctx.instance;
|
||||
}
|
||||
|
||||
/** True when D-fields should render read-only (team-managed). */
|
||||
export function isDefinitionReadOnly(ctx: AgentEditContext): boolean {
|
||||
const def = editContextDefinition(ctx);
|
||||
return def?.sourceTeam != null && def.sourceTeam.length > 0;
|
||||
}
|
||||
|
||||
/** True when definition sections should be rendered at all. */
|
||||
export function hasDefinitionContext(ctx: AgentEditContext): boolean {
|
||||
return ctx.kind !== "instance-only";
|
||||
}
|
||||
|
||||
/** True when instance+policy sections should be rendered. */
|
||||
export function hasInstanceContext(ctx: AgentEditContext): boolean {
|
||||
return ctx.kind !== "definition-only";
|
||||
}
|
||||
|
||||
// ── AgentFormModel ──────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Flat, typed field model covering every editable field on the merged surface.
|
||||
*
|
||||
* Fields absent for a given context are undefined. Editability is determined
|
||||
* by AgentEditContext + the isDefinitionReadOnly predicate, NOT by AgentFormModel
|
||||
* itself — the model just holds current values.
|
||||
*/
|
||||
export type AgentFormModel = {
|
||||
// Identity (D when linked, I when unlinked)
|
||||
displayName: string;
|
||||
avatarUrl: string;
|
||||
|
||||
// Behavior (D when linked, I when unlinked)
|
||||
systemPrompt: string;
|
||||
respondTo: ManagedAgent["respondTo"] | AgentPersona["respondTo"] | null;
|
||||
respondToAllowlist: string[];
|
||||
|
||||
// Runtime (D when linked, I when unlinked) — harness pins are I
|
||||
/** Preferred runtime id from the definition. */
|
||||
runtime: string | undefined;
|
||||
model: string | null;
|
||||
provider: string | null;
|
||||
envVars: Record<string, string>;
|
||||
|
||||
// Definition-only fields
|
||||
/** Name pool for definition. Undefined when no definition context. */
|
||||
namePool: string[] | undefined;
|
||||
|
||||
// Instance-level fields
|
||||
/** Instance name (per-pool override of displayName). */
|
||||
instanceName: string | undefined;
|
||||
parallelism: number | null;
|
||||
|
||||
// Device policy (L-fields)
|
||||
autoRestartOnConfigChange: boolean | undefined;
|
||||
startOnAppLaunch: boolean | undefined;
|
||||
};
|
||||
|
||||
/** Which coordinator outputs changed vs. last saved state. */
|
||||
export type AgentFormEmit = {
|
||||
/** D-field update payload, present iff a D-field changed AND the definition is editable. */
|
||||
personaInput: UpdatePersonaInput | null;
|
||||
/** I-field update payload, present iff an I-field changed. */
|
||||
agentInput: UpdateManagedAgentInput | null;
|
||||
/** Policy-setter calls, one entry per changed L-field. */
|
||||
policySets: Array<
|
||||
| { type: "autoRestart"; pubkey: string; value: boolean }
|
||||
| { type: "startOnAppLaunch"; pubkey: string; value: boolean }
|
||||
>;
|
||||
};
|
||||
|
||||
// ── Canonical comparison helpers ─────────────────────────────────────────────
|
||||
|
||||
/** Map equality per Artifact 3 settlement contract. */
|
||||
export function envVarsMapEqual(
|
||||
a: Record<string, string>,
|
||||
b: Record<string, string>,
|
||||
): boolean {
|
||||
const aKeys = Object.keys(a);
|
||||
const bKeys = Object.keys(b);
|
||||
if (aKeys.length !== bKeys.length) return false;
|
||||
return aKeys.every((k) => a[k] === b[k]);
|
||||
}
|
||||
|
||||
/** Canonical name-pool equality: order-sensitive, trimmed. */
|
||||
export function namePoolEqual(a: string[], b: string[]): boolean {
|
||||
if (a.length !== b.length) return false;
|
||||
return a.every((v, i) => v.trim() === b[i]?.trim());
|
||||
}
|
||||
|
||||
// ── Edit adapters ────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Seed an AgentFormModel from (definition?, agent?) — at least one present.
|
||||
*
|
||||
* For instance-with-definition: D-fields seed from the definition (authoritative
|
||||
* source); I-fields seed from the instance. For definition-only: instance fields
|
||||
* are undefined. For instance-only: definition fields are undefined.
|
||||
*/
|
||||
export function seedAgentFormModel(ctx: AgentEditContext): AgentFormModel {
|
||||
const def = editContextDefinition(ctx);
|
||||
const inst = editContextInstance(ctx);
|
||||
|
||||
// D-fields: from definition when present, else from instance
|
||||
const displayName = def?.displayName ?? inst?.name ?? "";
|
||||
const avatarUrl = def?.avatarUrl ?? inst?.avatarUrl ?? "";
|
||||
const systemPrompt = def?.systemPrompt ?? inst?.systemPrompt ?? "";
|
||||
const respondTo = def?.respondTo ?? inst?.respondTo ?? null;
|
||||
const respondToAllowlist =
|
||||
def?.respondToAllowlist ?? inst?.respondToAllowlist ?? [];
|
||||
const runtime = def?.runtime ?? undefined;
|
||||
const model = def !== null ? (def.model ?? null) : (inst?.model ?? null);
|
||||
const provider =
|
||||
def !== null ? (def.provider ?? null) : (inst?.provider ?? null);
|
||||
// Env: D-field base. Instance overlay is applied separately at spawn;
|
||||
// the form edits each store independently (row 8 contract).
|
||||
const envVars = def?.envVars ?? inst?.envVars ?? {};
|
||||
const namePool = def?.namePool;
|
||||
|
||||
// I-fields
|
||||
const instanceName = inst?.name;
|
||||
const parallelism = inst?.parallelism ?? def?.parallelism ?? null;
|
||||
|
||||
// L-fields
|
||||
const autoRestartOnConfigChange = inst?.autoRestartOnConfigChange;
|
||||
const startOnAppLaunch = inst?.startOnAppLaunch;
|
||||
|
||||
return {
|
||||
displayName,
|
||||
avatarUrl,
|
||||
systemPrompt,
|
||||
respondTo,
|
||||
respondToAllowlist,
|
||||
runtime,
|
||||
model,
|
||||
provider,
|
||||
envVars,
|
||||
namePool,
|
||||
instanceName,
|
||||
parallelism,
|
||||
autoRestartOnConfigChange,
|
||||
startOnAppLaunch,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Emit coordinator inputs from a (previous model / saved state) pair.
|
||||
*
|
||||
* Returns personaInput when a D-field changed AND the definition is editable.
|
||||
* Returns agentInput when an I-field changed.
|
||||
* Returns policySets for any L-field changed.
|
||||
*
|
||||
* The caller (save coordinator) calls this after re-fetching observed state,
|
||||
* so "previous" is the re-fetched stored state and "next" is what the user
|
||||
* submitted — the diff is exactly what hasn't been persisted yet.
|
||||
*/
|
||||
export function emitAgentFormDiff(
|
||||
saved: AgentFormModel,
|
||||
next: AgentFormModel,
|
||||
ctx: AgentEditContext,
|
||||
_options?: { publishCatalogUpdates?: boolean },
|
||||
): AgentFormEmit {
|
||||
const def = editContextDefinition(ctx);
|
||||
const inst = editContextInstance(ctx);
|
||||
const defReadOnly = isDefinitionReadOnly(ctx);
|
||||
|
||||
// D-field diff — only when definition is present and NOT team-managed
|
||||
let personaInput: UpdatePersonaInput | null = null;
|
||||
if (def !== null && !defReadOnly) {
|
||||
const dChanged =
|
||||
next.displayName.trim() !== saved.displayName.trim() ||
|
||||
(next.avatarUrl ?? "") !== (saved.avatarUrl ?? "") ||
|
||||
next.systemPrompt.trim() !== saved.systemPrompt.trim() ||
|
||||
next.runtime !== saved.runtime ||
|
||||
(next.model ?? null) !== (saved.model ?? null) ||
|
||||
(next.provider ?? null) !== (saved.provider ?? null) ||
|
||||
!envVarsMapEqual(next.envVars ?? {}, saved.envVars ?? {}) ||
|
||||
!namePoolEqual(next.namePool ?? [], saved.namePool ?? []);
|
||||
|
||||
if (dChanged) {
|
||||
personaInput = {
|
||||
id: def.id,
|
||||
displayName: next.displayName.trim(),
|
||||
avatarUrl: next.avatarUrl ?? "",
|
||||
systemPrompt: next.systemPrompt.trim(),
|
||||
runtime: next.runtime,
|
||||
model: next.model ?? undefined,
|
||||
provider: next.provider ?? undefined,
|
||||
namePool: next.namePool ?? [],
|
||||
envVars: next.envVars ?? {},
|
||||
...(next.respondTo != null
|
||||
? {
|
||||
behavior: {
|
||||
respondTo: next.respondTo,
|
||||
respondToAllowlist:
|
||||
next.respondTo === "allowlist"
|
||||
? next.respondToAllowlist
|
||||
: undefined,
|
||||
parallelism: next.parallelism ?? undefined,
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// I-field diff — only when instance is present
|
||||
let agentInput: UpdateManagedAgentInput | null = null;
|
||||
if (inst !== null) {
|
||||
// Row 1 contract: stop materializing displayName→name when a name pool exists
|
||||
const hasNamePool = (def?.namePool?.length ?? 0) > 0;
|
||||
|
||||
// Row 8 contract: instance env edit goes to instance, NOT wholesale-replace from definition
|
||||
const instanceEnvChanged =
|
||||
"instanceEnvVars" in next
|
||||
? !envVarsMapEqual(
|
||||
(
|
||||
next as AgentFormModel & {
|
||||
instanceEnvVars?: Record<string, string>;
|
||||
}
|
||||
).instanceEnvVars ?? {},
|
||||
inst.envVars ?? {},
|
||||
)
|
||||
: false;
|
||||
|
||||
const nameChanged =
|
||||
!hasNamePool &&
|
||||
(next.instanceName ?? next.displayName.trim()) !== inst.name;
|
||||
const systemPromptChanged =
|
||||
def === null &&
|
||||
(next.systemPrompt.trim() || null) !== (inst.systemPrompt ?? null);
|
||||
const modelChanged =
|
||||
def === null && (next.model ?? null) !== (inst.model ?? null);
|
||||
const providerChanged =
|
||||
def === null && (next.provider ?? null) !== (inst.provider ?? null);
|
||||
const respondToChanged =
|
||||
(next.respondTo ?? null) !== (inst.respondTo ?? null);
|
||||
const allowlistChanged =
|
||||
next.respondTo === "allowlist" &&
|
||||
next.respondToAllowlist.join(",") !== inst.respondToAllowlist.join(",");
|
||||
const parallelismChanged =
|
||||
(next.parallelism ?? null) !== (inst.parallelism ?? null);
|
||||
|
||||
const iChanged =
|
||||
nameChanged ||
|
||||
systemPromptChanged ||
|
||||
modelChanged ||
|
||||
providerChanged ||
|
||||
instanceEnvChanged ||
|
||||
respondToChanged ||
|
||||
allowlistChanged ||
|
||||
parallelismChanged;
|
||||
|
||||
if (iChanged) {
|
||||
agentInput = { pubkey: inst.pubkey };
|
||||
if (nameChanged)
|
||||
agentInput.name = next.instanceName ?? next.displayName.trim();
|
||||
if (systemPromptChanged)
|
||||
agentInput.systemPrompt = next.systemPrompt.trim() || null;
|
||||
if (modelChanged) agentInput.model = next.model ?? null;
|
||||
if (providerChanged) agentInput.provider = next.provider ?? null;
|
||||
if (respondToChanged) agentInput.respondTo = next.respondTo ?? undefined;
|
||||
if (allowlistChanged)
|
||||
agentInput.respondToAllowlist = next.respondToAllowlist;
|
||||
if (parallelismChanged && next.parallelism != null)
|
||||
agentInput.parallelism = next.parallelism;
|
||||
}
|
||||
}
|
||||
|
||||
// L-field diff
|
||||
const policySets: AgentFormEmit["policySets"] = [];
|
||||
if (inst !== null) {
|
||||
if (
|
||||
next.autoRestartOnConfigChange !== undefined &&
|
||||
next.autoRestartOnConfigChange !== saved.autoRestartOnConfigChange
|
||||
) {
|
||||
policySets.push({
|
||||
type: "autoRestart",
|
||||
pubkey: inst.pubkey,
|
||||
value: next.autoRestartOnConfigChange,
|
||||
});
|
||||
}
|
||||
if (
|
||||
next.startOnAppLaunch !== undefined &&
|
||||
next.startOnAppLaunch !== saved.startOnAppLaunch
|
||||
) {
|
||||
policySets.push({
|
||||
type: "startOnAppLaunch",
|
||||
pubkey: inst.pubkey,
|
||||
value: next.startOnAppLaunch,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return { personaInput, agentInput, policySets };
|
||||
}
|
||||
314
desktop/src/features/agents/ui/agentSaveCoordinator.ts
Normal file
314
desktop/src/features/agents/ui/agentSaveCoordinator.ts
Normal file
@@ -0,0 +1,314 @@
|
||||
/**
|
||||
* agentSaveCoordinator.ts — Save coordinator per Artifact 3 of the Phase 0 spec.
|
||||
*
|
||||
* Execution sequence on every submit:
|
||||
* 0. Validate (pre-write): runtime availability, credential gate,
|
||||
* parallelism parsing
|
||||
* 1. Definition write iff a D-field changed AND definition is editable
|
||||
* 2. Instance write iff an I-field changed (row 1/8 drops excluded)
|
||||
* 3. Local-policy setters only on change
|
||||
* 4. Settlement re-fetch BOTH stores, derive persisted-vs-unsaved
|
||||
* from observed state (not from command result)
|
||||
*
|
||||
* Partial-failure: stop at first failure, run settlement, toast from observed
|
||||
* state — naming exactly what persisted and what didn't.
|
||||
*
|
||||
* Not in scope: 14a membership, 14b identity archive — no write path through Save.
|
||||
*/
|
||||
|
||||
import { toast } from "sonner";
|
||||
import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions";
|
||||
import { showAgentProfileSyncWarning } from "@/features/agents/ui/agentProfileSyncWarning";
|
||||
import { validateLinkedAgentRuntimeEdit } from "@/features/profile/ui/UserProfilePanelPersonaSubmit";
|
||||
import type {
|
||||
AcpRuntimeCatalogEntry,
|
||||
AgentPersona,
|
||||
ManagedAgent,
|
||||
UpdateManagedAgentInput,
|
||||
UpdatePersonaInput,
|
||||
} from "@/shared/api/types";
|
||||
import type { AgentEditContext } from "./agentFormModel";
|
||||
import {
|
||||
editContextDefinition,
|
||||
editContextInstance,
|
||||
envVarsMapEqual,
|
||||
namePoolEqual,
|
||||
} from "./agentFormModel";
|
||||
|
||||
// ── Types ────────────────────────────────────────────────────────────────────
|
||||
|
||||
export type SaveCoordinatorOptions = {
|
||||
/** Current edit context (definition + optional instance). */
|
||||
ctx: AgentEditContext;
|
||||
/** Submitted definition update (null = no D-change or team-managed). */
|
||||
personaInput: UpdatePersonaInput | null;
|
||||
/** Submitted instance update (null = no I-change). */
|
||||
agentInput: UpdateManagedAgentInput | null;
|
||||
/** Policy setter calls. */
|
||||
policySets: Array<
|
||||
| { type: "autoRestart"; pubkey: string; value: boolean }
|
||||
| { type: "startOnAppLaunch"; pubkey: string; value: boolean }
|
||||
>;
|
||||
/** Whether the submitted definition was flagged for catalog publish. */
|
||||
publishCatalogUpdates?: boolean;
|
||||
/** Validated runtime catalog for the runtime-edit gate. */
|
||||
runtimes?: readonly AcpRuntimeCatalogEntry[];
|
||||
|
||||
// Mutations
|
||||
updatePersona: (input: UpdatePersonaInput) => Promise<unknown>;
|
||||
updatePersonaAndPublish: (input: UpdatePersonaInput) => Promise<unknown>;
|
||||
updateManagedAgent: (
|
||||
input: UpdateManagedAgentInput,
|
||||
) => Promise<{ agent: ManagedAgent; profileSyncError: string | null }>;
|
||||
setAutoRestart: (pubkey: string, value: boolean) => Promise<unknown>;
|
||||
setStartOnAppLaunch: (pubkey: string, value: boolean) => Promise<unknown>;
|
||||
|
||||
/** Re-fetch both stores and return the current (persona, agent) pair. */
|
||||
refetchStores: () => Promise<{
|
||||
persona: AgentPersona | null;
|
||||
agent: ManagedAgent | null;
|
||||
}>;
|
||||
|
||||
/** Called when the dialog should close (on full success). */
|
||||
onDone: () => void;
|
||||
/** Called after save to optionally offer "Start now" (passed the post-settle agent). */
|
||||
onSavedWhileStopped?: (agent: ManagedAgent) => void;
|
||||
};
|
||||
|
||||
// ── Coordinator ───────────────────────────────────────────────────────────────
|
||||
|
||||
/**
|
||||
* Execute the Artifact 3 save sequence and settle from observed state.
|
||||
*
|
||||
* Returns true if all writes succeeded; false on partial or full failure.
|
||||
* Callers must keep the dialog open on false so the user sees the error
|
||||
* and the re-seeded form shows the unsaved remainder.
|
||||
*/
|
||||
export async function runAgentSaveCoordinator(
|
||||
opts: SaveCoordinatorOptions,
|
||||
): Promise<boolean> {
|
||||
const {
|
||||
ctx,
|
||||
personaInput,
|
||||
agentInput,
|
||||
policySets,
|
||||
publishCatalogUpdates,
|
||||
runtimes,
|
||||
updatePersona,
|
||||
updatePersonaAndPublish,
|
||||
updateManagedAgent,
|
||||
setAutoRestart,
|
||||
setStartOnAppLaunch,
|
||||
refetchStores,
|
||||
onDone,
|
||||
onSavedWhileStopped,
|
||||
} = opts;
|
||||
|
||||
const def = editContextDefinition(ctx);
|
||||
const inst = editContextInstance(ctx);
|
||||
|
||||
// ── Step 0: Validate ──────────────────────────────────────────────────────
|
||||
if (personaInput && def && inst) {
|
||||
const runtimeError = validateLinkedAgentRuntimeEdit({
|
||||
input: personaInput,
|
||||
managedAgent: inst,
|
||||
previousPersona: def,
|
||||
runtimes: runtimes ? [...runtimes] : undefined,
|
||||
});
|
||||
if (runtimeError) {
|
||||
toast.error(runtimeError);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
// ── Steps 1–3: Writes ─────────────────────────────────────────────────────
|
||||
let definitionWritten = false;
|
||||
let instanceWritten = false;
|
||||
let policyWritten = false;
|
||||
let firstError: string | null = null;
|
||||
let profileSyncError: string | null = null;
|
||||
let latestAgent: ManagedAgent | null = inst;
|
||||
|
||||
// Step 1: Definition write
|
||||
if (personaInput && !firstError) {
|
||||
try {
|
||||
if (publishCatalogUpdates) {
|
||||
await updatePersonaAndPublish(personaInput);
|
||||
} else {
|
||||
await updatePersona(personaInput);
|
||||
}
|
||||
definitionWritten = true;
|
||||
} catch (err) {
|
||||
firstError =
|
||||
err instanceof Error ? err.message : "Failed to save agent profile.";
|
||||
}
|
||||
}
|
||||
|
||||
// Step 2: Instance write
|
||||
if (agentInput && !firstError) {
|
||||
try {
|
||||
const result = await updateManagedAgent(agentInput);
|
||||
latestAgent = result.agent;
|
||||
instanceWritten = true;
|
||||
profileSyncError = result.profileSyncError;
|
||||
} catch (err) {
|
||||
firstError =
|
||||
err instanceof Error ? err.message : "Failed to save agent settings.";
|
||||
}
|
||||
}
|
||||
|
||||
// Step 3: Policy setters
|
||||
if (!firstError) {
|
||||
for (const policy of policySets) {
|
||||
try {
|
||||
if (policy.type === "autoRestart") {
|
||||
await setAutoRestart(policy.pubkey, policy.value);
|
||||
} else {
|
||||
await setStartOnAppLaunch(policy.pubkey, policy.value);
|
||||
}
|
||||
policyWritten = true;
|
||||
} catch (err) {
|
||||
firstError =
|
||||
err instanceof Error ? err.message : "Failed to save agent policy.";
|
||||
break;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// ── Step 4: Settlement — re-fetch both stores ─────────────────────────────
|
||||
const { persona: observedPersona, agent: observedAgent } =
|
||||
await refetchStores();
|
||||
|
||||
if (!firstError) {
|
||||
// Full success path
|
||||
const agentName =
|
||||
latestAgent?.name ?? observedAgent?.name ?? def?.displayName ?? "Agent";
|
||||
|
||||
if (profileSyncError) {
|
||||
showAgentProfileSyncWarning(agentName, profileSyncError);
|
||||
} else {
|
||||
toast.success(`${agentName} saved.`);
|
||||
}
|
||||
|
||||
// "Saved while stopped" affordance (Artifact 3 contract)
|
||||
const finalAgent = observedAgent ?? latestAgent;
|
||||
if (finalAgent && !isManagedAgentActive(finalAgent)) {
|
||||
onSavedWhileStopped?.(finalAgent);
|
||||
}
|
||||
|
||||
onDone();
|
||||
return true;
|
||||
}
|
||||
|
||||
// ── Partial-failure: derive what persisted from observed state ────────────
|
||||
const persistedParts: string[] = [];
|
||||
const failedParts: string[] = [];
|
||||
|
||||
if (personaInput) {
|
||||
const persisted = observedPersona
|
||||
? observedStateMatchesPersonaInput(observedPersona, personaInput)
|
||||
: definitionWritten;
|
||||
if (persisted) {
|
||||
persistedParts.push("profile");
|
||||
} else {
|
||||
failedParts.push("profile");
|
||||
}
|
||||
}
|
||||
|
||||
if (agentInput) {
|
||||
const persisted = observedAgent
|
||||
? observedStateMatchesAgentInput(observedAgent, agentInput)
|
||||
: instanceWritten;
|
||||
if (persisted) {
|
||||
persistedParts.push("instance settings");
|
||||
} else {
|
||||
failedParts.push("instance settings");
|
||||
}
|
||||
}
|
||||
|
||||
if (policySets.length > 0 && !policyWritten) {
|
||||
failedParts.push("device policy");
|
||||
}
|
||||
|
||||
if (persistedParts.length > 0 && failedParts.length > 0) {
|
||||
const kept = persistedParts.join(" and ");
|
||||
const failed = failedParts.join(" and ");
|
||||
toast.warning(
|
||||
`${capitalizeFirst(kept)} saved. ${capitalizeFirst(failed)} failed: ${firstError} — reopen to retry; your ${kept} change is kept.`,
|
||||
);
|
||||
} else if (persistedParts.length > 0) {
|
||||
toast.success(
|
||||
`Saved. Some changes may not have persisted — reopen to retry.`,
|
||||
);
|
||||
} else {
|
||||
toast.error(firstError ?? "Failed to save agent.");
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
// ── Observed-state comparison helpers ────────────────────────────────────────
|
||||
// Used to determine what persisted after a partial failure. Compares
|
||||
// CANONICAL stored values (trimmed strings, normalized name pool, map equality)
|
||||
// per the settlement contract.
|
||||
|
||||
function observedStateMatchesPersonaInput(
|
||||
observed: AgentPersona,
|
||||
submitted: UpdatePersonaInput,
|
||||
): boolean {
|
||||
return (
|
||||
observed.displayName.trim() === submitted.displayName.trim() &&
|
||||
observed.systemPrompt.trim() === (submitted.systemPrompt ?? "").trim() &&
|
||||
(observed.model ?? null) === (submitted.model ?? null) &&
|
||||
(observed.provider ?? null) === (submitted.provider ?? null) &&
|
||||
namePoolEqual(observed.namePool, submitted.namePool ?? []) &&
|
||||
envVarsMapEqual(observed.envVars, submitted.envVars ?? {})
|
||||
);
|
||||
}
|
||||
|
||||
function observedStateMatchesAgentInput(
|
||||
observed: ManagedAgent,
|
||||
submitted: UpdateManagedAgentInput,
|
||||
): boolean {
|
||||
// Only check fields that were submitted (undefined means "don't touch")
|
||||
if (
|
||||
submitted.name !== undefined &&
|
||||
(submitted.name ?? "").trim() !== observed.name.trim()
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
submitted.systemPrompt !== undefined &&
|
||||
(submitted.systemPrompt ?? null) !== (observed.systemPrompt ?? null)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
submitted.model !== undefined &&
|
||||
(submitted.model ?? null) !== (observed.model ?? null)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
submitted.provider !== undefined &&
|
||||
(submitted.provider ?? null) !== (observed.provider ?? null)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
if (
|
||||
submitted.envVars !== undefined &&
|
||||
!envVarsMapEqual(submitted.envVars, observed.envVars)
|
||||
) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
function capitalizeFirst(s: string): string {
|
||||
if (!s) return s;
|
||||
return s.charAt(0).toUpperCase() + s.slice(1);
|
||||
}
|
||||
|
||||
// Re-export for convenient use in AgentEditDialog
|
||||
export { seedAgentFormModel, emitAgentFormDiff } from "./agentFormModel";
|
||||
export type { AgentFormModel, AgentEditContext } from "./agentFormModel";
|
||||
@@ -51,7 +51,6 @@ import type {
|
||||
} from "@/shared/api/types";
|
||||
import {
|
||||
duplicatePersonaDialogState,
|
||||
editPersonaDialogState,
|
||||
type PersonaDialogState,
|
||||
} from "./personaDialogState";
|
||||
import {
|
||||
@@ -94,6 +93,10 @@ export function usePersonaActions() {
|
||||
|
||||
const [personaDialogState, setPersonaDialogState] =
|
||||
React.useState<PersonaDialogState | null>(null);
|
||||
// R5 route collapse: tracks the persona being edited via the merged surface.
|
||||
const [personaToEdit, setPersonaToEdit] = React.useState<AgentPersona | null>(
|
||||
null,
|
||||
);
|
||||
const [personaToDelete, setPersonaToDelete] =
|
||||
React.useState<AgentPersona | null>(null);
|
||||
const [personaToShare, setPersonaToShare] = React.useState<{
|
||||
@@ -430,7 +433,8 @@ export function usePersonaActions() {
|
||||
function openEdit(persona: AgentPersona) {
|
||||
clearFeedback("library");
|
||||
setShouldLoadAcpRuntimes(true);
|
||||
setPersonaDialogState(editPersonaDialogState(persona));
|
||||
// R5: route to the merged AgentEditDialog surface (definition-only context).
|
||||
setPersonaToEdit(persona);
|
||||
}
|
||||
|
||||
function openDuplicate(persona: AgentPersona) {
|
||||
@@ -581,6 +585,8 @@ export function usePersonaActions() {
|
||||
isPending,
|
||||
personaDialogState,
|
||||
setPersonaDialogState,
|
||||
personaToEdit,
|
||||
setPersonaToEdit,
|
||||
personaToDelete,
|
||||
setPersonaToDelete,
|
||||
personaToShare,
|
||||
|
||||
@@ -27,36 +27,11 @@ import { classifyAgentManagementOrigin } from "./agentManagementBuffer";
|
||||
import { useChannelsQuery } from "@/features/channels/hooks";
|
||||
import { resolveManagedAgentAvatarUrl } from "./ui/managedAgentAvatar";
|
||||
import type { AgentCreateIntent } from "./ui/agentCreateIntent";
|
||||
import { editPersonaDialogState } from "./ui/personaDialogState";
|
||||
import type {
|
||||
CreatePersonaInput,
|
||||
UpdatePersonaInput,
|
||||
} from "@/shared/api/types";
|
||||
|
||||
function updateInputFromRequest(
|
||||
request: Extract<AgentManagementRequest, { action: "update" }>,
|
||||
current: UpdatePersonaInput,
|
||||
): UpdatePersonaInput {
|
||||
const changes = request.request;
|
||||
return {
|
||||
...current,
|
||||
displayName: changes.displayName ?? current.displayName,
|
||||
systemPrompt: changes.systemPrompt ?? current.systemPrompt,
|
||||
runtime: changes.runtime ?? current.runtime,
|
||||
provider: changes.provider ?? current.provider,
|
||||
model: changes.model ?? current.model,
|
||||
...(changes.respondTo
|
||||
? {
|
||||
behavior: {
|
||||
respondTo: changes.respondTo,
|
||||
respondToAllowlist: [],
|
||||
parallelism: current.behavior?.parallelism,
|
||||
},
|
||||
}
|
||||
: {}),
|
||||
};
|
||||
}
|
||||
|
||||
export function useAgentManagement() {
|
||||
const queryClient = useQueryClient();
|
||||
const personasQuery = usePersonasQuery();
|
||||
@@ -271,15 +246,6 @@ export function useAgentManagement() {
|
||||
[request],
|
||||
);
|
||||
|
||||
const editInitialValues = React.useMemo(() => {
|
||||
if (request?.action !== "update" || !currentPersona) return null;
|
||||
return updateInputFromRequest(
|
||||
request,
|
||||
editPersonaDialogState(currentPersona)
|
||||
.initialValues as UpdatePersonaInput,
|
||||
);
|
||||
}, [currentPersona, request]);
|
||||
|
||||
const editError = React.useMemo(() => {
|
||||
if (request?.action !== "update") return error;
|
||||
if (error) return error;
|
||||
@@ -294,8 +260,8 @@ export function useAgentManagement() {
|
||||
|
||||
return {
|
||||
request,
|
||||
currentPersona,
|
||||
createInitialValues,
|
||||
editInitialValues,
|
||||
editError,
|
||||
error,
|
||||
...createdAgentAttachment,
|
||||
@@ -309,5 +275,38 @@ export function useAgentManagement() {
|
||||
submitCreate,
|
||||
submitUpdate,
|
||||
dismiss,
|
||||
/** Returns an error string if the requesting agent lacks origin permission, null if OK. */
|
||||
verifyOriginPermission(): string | null {
|
||||
if (request?.action !== "update") return null;
|
||||
try {
|
||||
assertAgentCanActFromOrigin(request.request.channelId);
|
||||
return null;
|
||||
} catch (cause) {
|
||||
return cause instanceof Error
|
||||
? cause.message
|
||||
: "Origin permission check failed.";
|
||||
}
|
||||
},
|
||||
/** Returns agent-requested field changes for R6 review mode pre-fill (overrides for the merged dialog). */
|
||||
get reviewOverrides() {
|
||||
if (request?.action !== "update" || !currentPersona) return undefined;
|
||||
const changes = request.request;
|
||||
const overrides: Partial<{
|
||||
displayName: string;
|
||||
systemPrompt: string;
|
||||
runtime: string | undefined;
|
||||
provider: string | undefined;
|
||||
model: string | undefined;
|
||||
}> = {};
|
||||
if (changes.displayName != null)
|
||||
overrides.displayName = changes.displayName;
|
||||
if (changes.systemPrompt != null)
|
||||
overrides.systemPrompt = changes.systemPrompt;
|
||||
if (changes.runtime != null) overrides.runtime = changes.runtime;
|
||||
if (changes.provider != null)
|
||||
overrides.provider = changes.provider ?? undefined;
|
||||
if (changes.model != null) overrides.model = changes.model ?? undefined;
|
||||
return Object.keys(overrides).length > 0 ? overrides : undefined;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
@@ -33,7 +33,10 @@ import {
|
||||
resolveStartRuntimeForDefinition,
|
||||
} from "@/features/agents/lib/instanceInputForDefinition";
|
||||
import { describeLogFile } from "@/features/agents/ui/agentUi";
|
||||
import { AgentDialog } from "@/features/agents/ui/AgentDialog";
|
||||
import {
|
||||
AgentEditDialog,
|
||||
type AgentEditContext,
|
||||
} from "@/features/agents/ui/AgentEditDialog";
|
||||
import { useAgentLifecycleActions } from "@/features/profile/ui/useAgentLifecycleActions";
|
||||
import {
|
||||
consumePendingOpenEditAgent,
|
||||
@@ -42,7 +45,6 @@ import {
|
||||
} from "@/features/agents/openEditAgentEvent";
|
||||
import {
|
||||
duplicatePersonaDialogState,
|
||||
editPersonaDialogState,
|
||||
type PersonaDialogState,
|
||||
} from "@/features/agents/ui/personaDialogState";
|
||||
import { useChannelsQuery } from "@/features/channels/hooks";
|
||||
@@ -402,12 +404,8 @@ export function UserProfilePanel({
|
||||
});
|
||||
|
||||
const handleEditAgent = React.useCallback(() => {
|
||||
if (resolvedPersona) {
|
||||
setPersonaDialogState(editPersonaDialogState(resolvedPersona));
|
||||
return;
|
||||
}
|
||||
setEditAgentOpen(true);
|
||||
}, [resolvedPersona]);
|
||||
}, []);
|
||||
|
||||
const { deleteManagedAgentRecord, deleteManagedAgentsForPersona } =
|
||||
useProfileAgentDeletion({
|
||||
@@ -549,7 +547,7 @@ export function UserProfilePanel({
|
||||
|
||||
const handleEditPersona = React.useCallback(() => {
|
||||
if (!resolvedPersona) return;
|
||||
setPersonaDialogState(editPersonaDialogState(resolvedPersona));
|
||||
setEditAgentOpen(true);
|
||||
}, [resolvedPersona]);
|
||||
|
||||
const handleDuplicatePersona = React.useCallback(() => {
|
||||
@@ -905,21 +903,23 @@ export function UserProfilePanel({
|
||||
) : null}
|
||||
</AuxiliaryPanelBody>
|
||||
);
|
||||
const editAgentDialog =
|
||||
canEditAgent && managedAgent ? (
|
||||
<AgentDialog
|
||||
agent={managedAgent}
|
||||
mode="instance-edit"
|
||||
initialFocus={editAgentFocus}
|
||||
onEditLinkedPersona={
|
||||
resolvedPersona && !resolvedPersona.isBuiltIn
|
||||
? () => {
|
||||
setEditAgentOpen(false);
|
||||
setEditAgentFocus(undefined);
|
||||
setPersonaDialogState(editPersonaDialogState(resolvedPersona));
|
||||
}
|
||||
: undefined
|
||||
const editCtx: AgentEditContext | null =
|
||||
managedAgent && resolvedPersona
|
||||
? {
|
||||
kind: "instance-with-definition",
|
||||
definition: resolvedPersona,
|
||||
instance: managedAgent,
|
||||
}
|
||||
: managedAgent
|
||||
? { kind: "instance-only", instance: managedAgent }
|
||||
: resolvedPersona
|
||||
? { kind: "definition-only", definition: resolvedPersona }
|
||||
: null;
|
||||
const editAgentDialog =
|
||||
canEditAgent && editCtx ? (
|
||||
<AgentEditDialog
|
||||
ctx={editCtx}
|
||||
initialFocus={editAgentFocus}
|
||||
onOpenChange={(next) => {
|
||||
setEditAgentOpen(next);
|
||||
if (!next) setEditAgentFocus(undefined);
|
||||
|
||||
Reference in New Issue
Block a user