Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 37 additions & 10 deletions desktop/src/features/agents/ui/AgentDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import type {
AcpRuntimeCatalogEntry,
CreateManagedAgentResponse,
CreatePersonaInput,
ManagedAgent,
UpdatePersonaInput,
} from "@/shared/api/types";
import { Switch } from "@/shared/ui/switch";
Expand All @@ -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;
Expand All @@ -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,
Expand All @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions desktop/src/features/agents/ui/AgentsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
}
Expand Down
2 changes: 1 addition & 1 deletion desktop/src/features/agents/ui/CreateAgentDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 ||
Expand Down
251 changes: 251 additions & 0 deletions desktop/src/features/agents/ui/agentInstanceEditPinning.test.mjs
Original file line number Diff line number Diff line change
@@ -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" });
});
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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", () => {
Expand Down
Loading
Loading