mirror of
https://github.com/block/buzz.git
synced 2026-11-02 15:31:27 +00:00
fix(desktop): Phase 1 round-8 — structural D/I ownership refactor
CRITICAL-1 (definition-only behavior invisible / silent access broadening): - Render D-owned access/allowlist/parallelism in definition-only context via AgentEditMergedDialogDSection (showInstancePresent prop controls visibility) - Preserve nullable respondTo/parallelism — no coercion to defaults on open; seed preserves null for unset definitions (owner-only by backend default) - Include D-parallelism in emitAgentFormDiff dChanged detection - emitAgentFormDiff behavior block emits respondTo/parallelism when D-owned - agentAccessOwnerOnly query enabled for definition-only context CRITICAL-2 (D/I runtime layers entangled): - Split shared model/provider state into independent dModel/dProvider (D-owned) and iModel/iProvider (I-owned) state pairs - Extract useAgentEditRuntimeHandlers: applyDSelection writes D state/envVars; applyISelection writes I state/instanceEnvVars — no cross-layer mutation - handleRuntimeDropdownChange uses applyISelection (I-only); handleDefinitionRuntimeChange uses applyDSelection (D-only) - handleProviderDropdownChange and handleModelDropdownChange route to D or I apply function based on context - Remove defReadOnly from I-owned access field in AgentEditMergedDialogInstanceSection (team-linked live access is I-editable per row 9) - Track auto-seeded D runtime via autoSeededDefinitionRuntimeRef; no-op saves do not persist the app default as a definition runtime change - useAgentEditRuntimeState.ts: pass def prop; blankRuntimeModelProviderEditable restores AgentDefinitionDialog's runtime-less provider edit capability (87dc4dccba) CRITICAL-3 (settlement false-success/false-failure): - agentSaveCoordinator: refetch on every command boundary regardless of throw; observed persistence decides step failure, not the command result - observedStateMatchesPersonaInput: runtime uses full-write semantics (submitted.runtime ?? null vs observed.runtime ?? null — clear = null) - observedStateMatchesAgentInput: harness unpin settles against agentCommandOverride === null, not effective command vs sentinel IMPORTANT-1 (test-11 provider gate): - Restore blankRuntimeModelProviderEditable in useAgentEditRuntimeState - test-11 restored to provider-visible-save-enabled contract (87dc4dccba) IMPORTANT-2 (R6 route-level behavioral evidence): - agentSaveCoordinator.test.mjs: two R6 route-level tests exercise the full pipeline (seedAgentFormModel → emitAgentFormDiff → runAgentSaveCoordinator) for a definition-only respondTo override MINOR fold-ins: - sourceTeam resolved to display name via useTeamsQuery - identity-archived flair shown for any archived agent (not gated by showDef) File-size gate: AgentEditMergedDialog.tsx extracted handlers to useAgentEditRuntimeHandlers.ts; dialog now 962 lines (gate: 963 < 1000) Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
@@ -20,6 +20,7 @@ import {
|
||||
useUpdateManagedAgentMutation,
|
||||
useUpdatePersonaMutation,
|
||||
} from "@/features/agents/hooks";
|
||||
import { useTeamsQuery } from "@/features/agents/teamHooks";
|
||||
import { useUpdatePersonaAndPublishMutation } from "@/features/agents/lib/usePersonaCatalogRelay";
|
||||
import type { ManagedAgent, RespondToMode } from "@/shared/api/types";
|
||||
import type { EditAgentFocusTarget } from "@/features/agents/openEditAgentEvent";
|
||||
@@ -40,21 +41,10 @@ import { useAgentEditMergedSubmit } from "./useAgentEditMergedSubmit";
|
||||
import { AgentEditMergedDSection } from "./AgentEditMergedDialogDSection";
|
||||
import { AgentEditMergedInstanceSection } from "./AgentEditMergedDialogInstanceSection";
|
||||
import {
|
||||
runtimeDropdownAction,
|
||||
usePendingHarnessSelection,
|
||||
} from "./addCustomHarness";
|
||||
import {
|
||||
AUTO_PROVIDER_DROPDOWN_VALUE,
|
||||
getDefaultPersonaRuntime,
|
||||
isMissingRequiredDropdownField,
|
||||
runtimeSupportsLlmProviderSelection,
|
||||
} from "./agentConfigOptions";
|
||||
import {
|
||||
selectionOnModelDropdownChange,
|
||||
selectionOnProviderDropdownChange,
|
||||
selectionOnRuntimeChange,
|
||||
type RuntimeModelProviderSelection,
|
||||
} from "./runtimeModelProviderSelection";
|
||||
import { useAgentEditRuntimeHandlers } from "./useAgentEditRuntimeHandlers";
|
||||
import { AgentCreationPreview } from "./AgentCreationPreview";
|
||||
import { AddCustomHarnessDialog } from "./AddCustomHarnessDialog";
|
||||
import { useAgentAccessOwnerOnlyQuery } from "@/features/agents/useAgentAccessOwnerOnly";
|
||||
@@ -156,10 +146,18 @@ export function AgentEditMergedDialog({
|
||||
|
||||
const { data: bakedEnvKeys } = useBakedBuildEnvKeysQuery({ enabled: open });
|
||||
const { data: agentAccessOwnerOnly } = useAgentAccessOwnerOnlyQuery({
|
||||
enabled: open && showInst,
|
||||
// Query in both instance and definition-only contexts: rows 9–10 say
|
||||
// access/parallelism are D-owned in definition-only (shown on the form).
|
||||
enabled: open && (showInst || ctx.kind === "definition-only"),
|
||||
});
|
||||
|
||||
const personasQuery = usePersonasQuery();
|
||||
const teamsQuery = useTeamsQuery();
|
||||
const sourceTeamName = React.useMemo(() => {
|
||||
if (!def?.sourceTeam) return null;
|
||||
const team = teamsQuery.data?.find((t) => t.id === def.sourceTeam);
|
||||
return team?.name ?? def.sourceTeam;
|
||||
}, [def?.sourceTeam, teamsQuery.data]);
|
||||
const linkedPersona = React.useMemo(
|
||||
() =>
|
||||
inst?.personaId
|
||||
@@ -177,20 +175,43 @@ export function AgentEditMergedDialog({
|
||||
// Definition runtime (D-field) — separate from instance harness pin (I-field)
|
||||
const [definitionRuntimeId, setDefinitionRuntimeId] =
|
||||
React.useState("custom");
|
||||
// Track whether the D-runtime was auto-seeded (not a deliberate user choice).
|
||||
// Prevents a no-op save from persisting the app default as the definition runtime.
|
||||
const autoSeededDefinitionRuntimeRef = React.useRef<string | null>(null);
|
||||
// I-fields — harness pin state (I-section only, independent of D-runtime)
|
||||
const [selectedRuntimeId, setSelectedRuntimeId] = React.useState("custom");
|
||||
// LLM model/provider (shared between D and I sections; which store they target is context-dependent)
|
||||
const [model, setModel] = React.useState("");
|
||||
const [provider, setProvider] = React.useState("");
|
||||
const [isCustomModelEditing, setIsCustomModelEditing] = React.useState(false);
|
||||
const [isCustomProviderEditing, setIsCustomProviderEditing] =
|
||||
// D-section LLM model/provider — for the definition (D-owned)
|
||||
const [dModel, setDModel] = React.useState("");
|
||||
const [dProvider, setDProvider] = React.useState("");
|
||||
const [dIsCustomModelEditing, setDIsCustomModelEditing] =
|
||||
React.useState(false);
|
||||
const [dIsCustomProviderEditing, setDIsCustomProviderEditing] =
|
||||
React.useState(false);
|
||||
// I-section LLM model/provider — for unlinked agents (I-owned, shown only when !showDef)
|
||||
const [iModel, setIModel] = React.useState("");
|
||||
const [iProvider, setIProvider] = React.useState("");
|
||||
const [iIsCustomModelEditing, setIIsCustomModelEditing] =
|
||||
React.useState(false);
|
||||
const [iIsCustomProviderEditing, setIIsCustomProviderEditing] =
|
||||
React.useState(false);
|
||||
const runtimeTouched = React.useRef(false);
|
||||
|
||||
// Convenience: the active model/provider — D-state when a definition is present,
|
||||
// I-state when instance-only (no definition). Used for the D-section and for
|
||||
// useAgentEditRuntimeState, which is always driven by the relevant layer.
|
||||
const model = showDef ? dModel : iModel;
|
||||
const provider = showDef ? dProvider : iProvider;
|
||||
const isCustomModelEditing = showDef
|
||||
? dIsCustomModelEditing
|
||||
: iIsCustomModelEditing;
|
||||
const isCustomProviderEditing = showDef
|
||||
? dIsCustomProviderEditing
|
||||
: iIsCustomProviderEditing;
|
||||
|
||||
// I-fields
|
||||
const [instanceName, setInstanceName] = React.useState("");
|
||||
const [respondTo, setRespondTo] = React.useState<RespondToMode>(
|
||||
inst?.respondTo ?? "anyone",
|
||||
const [respondTo, setRespondTo] = React.useState<RespondToMode | null>(
|
||||
inst?.respondTo ?? null,
|
||||
);
|
||||
const [respondToAllowlist, setRespondToAllowlist] = React.useState<string[]>(
|
||||
inst?.respondToAllowlist ?? [],
|
||||
@@ -247,13 +268,23 @@ export function AgentEditMergedDialog({
|
||||
setAvatarUrl(seed.avatarUrl);
|
||||
setSystemPrompt(seed.systemPrompt);
|
||||
setNamePoolText(formatPersonaNamePoolText(seed.namePool ?? []));
|
||||
setModel(seed.model ?? "");
|
||||
setProvider(seed.provider ?? "");
|
||||
setIsCustomModelEditing(false);
|
||||
setIsCustomProviderEditing(false);
|
||||
setRespondTo((seed.respondTo ?? "anyone") as RespondToMode);
|
||||
// D-section model/provider — seed from definition (or instance for I-only)
|
||||
setDModel(seed.model ?? "");
|
||||
setDProvider(seed.provider ?? "");
|
||||
setDIsCustomModelEditing(false);
|
||||
setDIsCustomProviderEditing(false);
|
||||
// I-section model/provider — only relevant for instance-only context
|
||||
setIModel(seed.model ?? "");
|
||||
setIProvider(seed.provider ?? "");
|
||||
setIIsCustomModelEditing(false);
|
||||
setIIsCustomProviderEditing(false);
|
||||
// Rows 9–10: seed respondTo/allowlist/parallelism from the canonical seed.
|
||||
// Preserve null from the seed — do NOT coerce null→"anyone" here; the
|
||||
// seed already handles the instance-vs-definition context. Coercing here
|
||||
// would cause phantom D-writes when an unset definition behavior opens.
|
||||
setRespondTo((seed.respondTo as RespondToMode | null) ?? null);
|
||||
setRespondToAllowlist(seed.respondToAllowlist);
|
||||
setParallelism(String(seed.parallelism ?? 1));
|
||||
setParallelism(seed.parallelism != null ? String(seed.parallelism) : "");
|
||||
setEnvVars(seed.envVars);
|
||||
setInstanceEnvVars(seed.instanceEnvVars ?? {});
|
||||
setInstanceName(seed.instanceName ?? "");
|
||||
@@ -262,6 +293,7 @@ export function AgentEditMergedDialog({
|
||||
setShowAdvancedFields(false);
|
||||
setIsAvatarUploadPending(false);
|
||||
runtimeTouched.current = false;
|
||||
autoSeededDefinitionRuntimeRef.current = null;
|
||||
|
||||
// Seed definition runtime (D-field) from definition record.
|
||||
const defRuntimeId = def?.runtime?.trim() ?? "";
|
||||
@@ -292,16 +324,22 @@ export function AgentEditMergedDialog({
|
||||
setDisplayName(initialValueOverrides.displayName);
|
||||
if (initialValueOverrides.systemPrompt !== undefined)
|
||||
setSystemPrompt(initialValueOverrides.systemPrompt);
|
||||
if (initialValueOverrides.model !== undefined)
|
||||
setModel(initialValueOverrides.model ?? "");
|
||||
if (initialValueOverrides.provider !== undefined)
|
||||
setProvider(initialValueOverrides.provider ?? "");
|
||||
if (initialValueOverrides.model !== undefined) {
|
||||
setDModel(initialValueOverrides.model ?? "");
|
||||
setIModel(initialValueOverrides.model ?? "");
|
||||
}
|
||||
if (initialValueOverrides.provider !== undefined) {
|
||||
setDProvider(initialValueOverrides.provider ?? "");
|
||||
setIProvider(initialValueOverrides.provider ?? "");
|
||||
}
|
||||
if (initialValueOverrides.runtime !== undefined)
|
||||
setDefinitionRuntimeId(initialValueOverrides.runtime ?? "custom");
|
||||
// R6: carry agent-requested respondTo into definition-only form (IMPORTANT-4)
|
||||
// R6: carry agent-requested respondTo into definition-only form.
|
||||
// This makes the requested change VISIBLE in the review dialog —
|
||||
// the owner sees the requested value and can accept (Save) or change it.
|
||||
if (initialValueOverrides.respondTo !== undefined)
|
||||
setRespondTo(
|
||||
(initialValueOverrides.respondTo ?? "anyone") as RespondToMode,
|
||||
(initialValueOverrides.respondTo as RespondToMode | null) ?? null,
|
||||
);
|
||||
}
|
||||
setDFieldsDirty(false);
|
||||
@@ -322,6 +360,10 @@ export function AgentEditMergedDialog({
|
||||
const defaultRuntime = getDefaultPersonaRuntime(runtimes);
|
||||
if (!defaultRuntime) return;
|
||||
setDefinitionRuntimeId(defaultRuntime.id);
|
||||
// Record that this runtime was auto-seeded, not chosen by the user.
|
||||
// useAgentEditMergedSubmit uses this to prevent persisting the auto-seed
|
||||
// as a definition runtime change when no other D-fields were modified.
|
||||
autoSeededDefinitionRuntimeRef.current = defaultRuntime.id;
|
||||
// In definition-only context, selectedRuntimeId is the same conceptual field
|
||||
// as definitionRuntimeId (there is no separate I-harness pin). Sync it so
|
||||
// that model discovery runs: useAgentEditRuntimeState derives `selectedRuntime`
|
||||
@@ -390,11 +432,12 @@ export function AgentEditMergedDialog({
|
||||
instanceEnvVars,
|
||||
inheritHarness,
|
||||
inst,
|
||||
def,
|
||||
linkedPersona,
|
||||
bakedEnvKeys,
|
||||
originalAgentCommand,
|
||||
setModel,
|
||||
setIsCustomModelEditing,
|
||||
setModel: setDModel,
|
||||
setIsCustomModelEditing: setDIsCustomModelEditing,
|
||||
});
|
||||
|
||||
// ── Submit validity ────────────────────────────────────────────────────────
|
||||
@@ -432,8 +475,8 @@ export function AgentEditMergedDialog({
|
||||
avatarUrl,
|
||||
systemPrompt,
|
||||
namePoolText,
|
||||
model,
|
||||
provider,
|
||||
model: showDef ? dModel : iModel,
|
||||
provider: showDef ? dProvider : iProvider,
|
||||
respondTo,
|
||||
respondToAllowlist,
|
||||
parallelism,
|
||||
@@ -444,6 +487,7 @@ export function AgentEditMergedDialog({
|
||||
autoRestartOnConfigChange,
|
||||
startOnAppLaunch: showInst ? startOnAppLaunch : undefined,
|
||||
definitionRuntimeId,
|
||||
autoSeededDefinitionRuntimeRef,
|
||||
selectedRuntimeId,
|
||||
inheritHarness,
|
||||
agentCommand,
|
||||
@@ -476,121 +520,49 @@ export function AgentEditMergedDialog({
|
||||
agentCommand.trim().length > 0)
|
||||
: true);
|
||||
|
||||
// ── selection helpers ──────────────────────────────────────────────────────
|
||||
const selection: RuntimeModelProviderSelection = {
|
||||
provider,
|
||||
model,
|
||||
isCustomProviderEditing,
|
||||
isCustomModelEditing,
|
||||
envVars: showInst ? instanceEnvVars : envVars,
|
||||
};
|
||||
|
||||
function applySelection(next: RuntimeModelProviderSelection) {
|
||||
setProvider(next.provider);
|
||||
setModel(next.model);
|
||||
setIsCustomProviderEditing(next.isCustomProviderEditing);
|
||||
setIsCustomModelEditing(next.isCustomModelEditing);
|
||||
if (showInst) {
|
||||
setInstanceEnvVars(next.envVars);
|
||||
} else {
|
||||
setEnvVars(next.envVars);
|
||||
}
|
||||
}
|
||||
|
||||
// D-section runtime change (definition runtime, D-field)
|
||||
function handleDefinitionRuntimeChange(nextValue: string) {
|
||||
const action = runtimeDropdownAction(nextValue);
|
||||
if (action.kind === "add-custom-harness") {
|
||||
setIsAddHarnessOpen(true);
|
||||
return;
|
||||
}
|
||||
const nextRuntimeId = action.runtimeId;
|
||||
const previousRuntimeId = definitionRuntimeId;
|
||||
setDefinitionRuntimeId(nextRuntimeId || "custom");
|
||||
applySelection(
|
||||
selectionOnRuntimeChange(selection, {
|
||||
previousRuntime: previousRuntimeId,
|
||||
nextRuntime: nextRuntimeId,
|
||||
nextRuntimeCanChooseProvider:
|
||||
runtimeSupportsLlmProviderSelection(nextRuntimeId),
|
||||
lockedRuntimeReset: "full",
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
// I-section runtime change (instance harness pin, I-field)
|
||||
function handleRuntimeDropdownChange(nextValue: string) {
|
||||
const action = runtimeDropdownAction(nextValue);
|
||||
if (action.kind === "add-custom-harness") {
|
||||
setIsAddHarnessOpen(true);
|
||||
return;
|
||||
}
|
||||
const nextRuntimeId = action.runtimeId;
|
||||
const previousRuntimeId = selectedRuntimeId;
|
||||
|
||||
runtimeTouched.current = true;
|
||||
const nextRuntime = runtimes.find((r) => r.id === nextRuntimeId);
|
||||
const resolvedRuntimeId = nextRuntimeId || "custom";
|
||||
setSelectedRuntimeId(resolvedRuntimeId);
|
||||
if (resolvedRuntimeId === "custom" || nextRuntime?.command) {
|
||||
setInheritHarness(false);
|
||||
}
|
||||
if (nextRuntime?.command) {
|
||||
setAgentCommand(nextRuntime.command);
|
||||
setAgentArgs(nextRuntime.defaultArgs.join(","));
|
||||
}
|
||||
|
||||
applySelection(
|
||||
selectionOnRuntimeChange(selection, {
|
||||
previousRuntime: previousRuntimeId,
|
||||
nextRuntime: nextRuntimeId,
|
||||
nextRuntimeCanChooseProvider:
|
||||
runtimeSupportsLlmProviderSelection(nextRuntimeId),
|
||||
lockedRuntimeReset: "full",
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
const selectSavedHarness = usePendingHarnessSelection(
|
||||
// ── Runtime/model/provider handlers (extracted to useAgentEditRuntimeHandlers) ──
|
||||
const {
|
||||
handleDefinitionRuntimeChange,
|
||||
handleRuntimeDropdownChange,
|
||||
handleProviderDropdownChange,
|
||||
handleModelDropdownChange,
|
||||
selectSavedHarness,
|
||||
} = useAgentEditRuntimeHandlers({
|
||||
showDef,
|
||||
showInst,
|
||||
dProvider,
|
||||
dModel,
|
||||
dIsCustomProviderEditing,
|
||||
dIsCustomModelEditing,
|
||||
setDProvider,
|
||||
setDModel,
|
||||
setDIsCustomProviderEditing,
|
||||
setDIsCustomModelEditing,
|
||||
envVars,
|
||||
setEnvVars,
|
||||
definitionRuntimeId,
|
||||
setDefinitionRuntimeId,
|
||||
iProvider,
|
||||
iModel,
|
||||
iIsCustomProviderEditing,
|
||||
iIsCustomModelEditing,
|
||||
setIProvider,
|
||||
setIModel,
|
||||
setIIsCustomProviderEditing,
|
||||
setIIsCustomModelEditing,
|
||||
instanceEnvVars,
|
||||
setInstanceEnvVars,
|
||||
selectedRuntimeId,
|
||||
setSelectedRuntimeId,
|
||||
setInheritHarness,
|
||||
setAgentCommand,
|
||||
setAgentArgs,
|
||||
runtimeTouched,
|
||||
setIsAddHarnessOpen,
|
||||
runtimes,
|
||||
// In definition-only context the harness dropdown drives `definitionRuntimeId`
|
||||
// (via handleDefinitionRuntimeChange), not the I-harness pin. Route to the
|
||||
// correct handler so the saved harness appears in the D-section dropdown.
|
||||
showDef && !showInst
|
||||
? handleDefinitionRuntimeChange
|
||||
: handleRuntimeDropdownChange,
|
||||
selectedRuntime,
|
||||
open,
|
||||
);
|
||||
|
||||
function handleProviderDropdownChange(nextValue: string) {
|
||||
const nextProvider =
|
||||
nextValue === AUTO_PROVIDER_DROPDOWN_VALUE ? "" : nextValue;
|
||||
if (nextProvider === "relay-mesh" && selectedRuntimeId !== "buzz-agent") {
|
||||
handleRuntimeDropdownChange("buzz-agent");
|
||||
}
|
||||
const nextSelection = selectionOnProviderDropdownChange(selection, {
|
||||
runtime:
|
||||
nextProvider === "relay-mesh"
|
||||
? "buzz-agent"
|
||||
: (selectedRuntime?.id ?? selectedRuntimeId),
|
||||
nextValue,
|
||||
clearModelWhenApiKeyMissing: !showInst,
|
||||
});
|
||||
applySelection({
|
||||
...nextSelection,
|
||||
model: nextProvider === "relay-mesh" ? "auto" : nextSelection.model,
|
||||
});
|
||||
}
|
||||
|
||||
function handleModelDropdownChange(nextValue: string) {
|
||||
applySelection(
|
||||
selectionOnModelDropdownChange(selection, {
|
||||
nextValue,
|
||||
clearKnownModelOnCustomEntry: !showInst,
|
||||
isModelCustom: false,
|
||||
}),
|
||||
);
|
||||
}
|
||||
});
|
||||
|
||||
// ── Focus target (R2) ──────────────────────────────────────────────────────
|
||||
const normalizedFieldFocusFiredRef = React.useRef(false);
|
||||
@@ -740,8 +712,8 @@ export function AgentEditMergedDialog({
|
||||
className="text-sm text-muted-foreground"
|
||||
data-testid="team-managed-notice"
|
||||
>
|
||||
Managed by team{def?.sourceTeam ? ` ${def.sourceTeam}` : ""}.
|
||||
Its configuration cannot be edited here.
|
||||
Managed by team{sourceTeamName ? ` ${sourceTeamName}` : ""}. Its
|
||||
configuration cannot be edited here.
|
||||
</p>
|
||||
) : null}
|
||||
|
||||
@@ -755,8 +727,10 @@ export function AgentEditMergedDialog({
|
||||
</p>
|
||||
) : null}
|
||||
|
||||
{/* Identity-archived flair (14b) — fields remain editable; Save does not unarchive */}
|
||||
{isIdentityArchived && showDef ? (
|
||||
{/* Identity-archived flair (14b) — per agent pubkey, shown for any
|
||||
archived agent regardless of definition linkage. Fields remain
|
||||
editable; Save does not unarchive. */}
|
||||
{isIdentityArchived ? (
|
||||
<p
|
||||
className="text-sm font-medium text-amber-600 dark:text-amber-400"
|
||||
data-testid="identity-archived-flair"
|
||||
@@ -796,7 +770,7 @@ export function AgentEditMergedDialog({
|
||||
}}
|
||||
isCustomProviderEditing={isCustomProviderEditing}
|
||||
provider={provider}
|
||||
onProviderTextChange={dChange(setProvider)}
|
||||
onProviderTextChange={dChange(setDProvider)}
|
||||
modelSelectValue={modelSelectValue}
|
||||
modelDropdownOptions={modelDropdownOptions}
|
||||
onModelChange={(v) => {
|
||||
@@ -806,8 +780,25 @@ export function AgentEditMergedDialog({
|
||||
modelDiscoveryLoading={modelDiscoveryLoading}
|
||||
showCustomModelInput={showCustomModelInput}
|
||||
model={model}
|
||||
onModelTextChange={dChange(setModel)}
|
||||
onModelTextChange={dChange(setDModel)}
|
||||
modelStatusMessage={modelStatusMessage}
|
||||
showInstancePresent={showInst}
|
||||
respondTo={respondTo}
|
||||
respondToAllowlist={respondToAllowlist}
|
||||
onRespondToChange={(v) => {
|
||||
setRespondTo(v);
|
||||
setDFieldsDirty(true);
|
||||
}}
|
||||
onAllowlistChange={(v) => {
|
||||
setRespondToAllowlist(v);
|
||||
setDFieldsDirty(true);
|
||||
}}
|
||||
agentAccessOwnerOnly={agentAccessOwnerOnly}
|
||||
parallelism={parallelism}
|
||||
onParallelismChange={(v) => {
|
||||
setParallelism(v);
|
||||
setDFieldsDirty(true);
|
||||
}}
|
||||
/>
|
||||
) : null}
|
||||
|
||||
@@ -821,7 +812,7 @@ export function AgentEditMergedDialog({
|
||||
runtimeCatalogStatus={runtimeCatalogStatus}
|
||||
instanceName={instanceName}
|
||||
onInstanceNameChange={setInstanceName}
|
||||
respondTo={respondTo}
|
||||
respondTo={respondTo ?? "anyone"}
|
||||
respondToAllowlist={respondToAllowlist}
|
||||
onRespondToChange={setRespondTo}
|
||||
onAllowlistChange={setRespondToAllowlist}
|
||||
@@ -840,7 +831,7 @@ export function AgentEditMergedDialog({
|
||||
onProviderChange={handleProviderDropdownChange}
|
||||
isCustomProviderEditing={isCustomProviderEditing}
|
||||
provider={provider}
|
||||
onProviderTextChange={setProvider}
|
||||
onProviderTextChange={setIProvider}
|
||||
providerRequired={providerRequired}
|
||||
topLevelSecretEnvVar={topLevelSecretEnvVar}
|
||||
apiKeyValue={apiKeyValue}
|
||||
@@ -857,7 +848,7 @@ export function AgentEditMergedDialog({
|
||||
modelDiscoveryLoading={modelDiscoveryLoading}
|
||||
showCustomModelInput={showCustomModelInput}
|
||||
model={model}
|
||||
onModelTextChange={setModel}
|
||||
onModelTextChange={setIModel}
|
||||
modelStatusMessage={modelStatusMessage}
|
||||
modelRequired={modelRequired}
|
||||
inheritedSubmission={inheritedSubmission}
|
||||
|
||||
@@ -4,6 +4,9 @@
|
||||
* Renders agent name, system prompt, name pool, D-env vars, and (for all definition
|
||||
* contexts) runtime, LLM provider, and model. Team-managed fields render read-only.
|
||||
*
|
||||
* When showInstancePresent is false (definition-only context), also renders the
|
||||
* D-owned access/allowlist and parallelism controls (rows 9–10, Artifact 4).
|
||||
*
|
||||
* Extracted from AgentEditMergedDialog to satisfy the desktop file-size gate.
|
||||
*/
|
||||
|
||||
@@ -14,6 +17,7 @@ import { cn } from "@/shared/lib/cn";
|
||||
import { Input } from "@/shared/ui/input";
|
||||
import { Textarea } from "@/shared/ui/textarea";
|
||||
import { Button } from "@/shared/ui/button";
|
||||
import type { RespondToMode } from "@/shared/api/types";
|
||||
|
||||
import {
|
||||
PERSONA_FIELD_CONTROL_CLASS,
|
||||
@@ -25,6 +29,7 @@ import { AgentHarnessField } from "./AgentHarnessField";
|
||||
import { PersonaDropdownField } from "./PersonaDropdownField";
|
||||
import { PersonaModelCombobox } from "./PersonaModelCombobox";
|
||||
import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor";
|
||||
import { OwnerOnlyAccessField } from "./OwnerOnlyAccessField";
|
||||
|
||||
const advancedFieldsTransition = { duration: 0.18, ease: "easeInOut" } as const;
|
||||
|
||||
@@ -69,6 +74,21 @@ export type AgentEditMergedDSectionProps = {
|
||||
model: string;
|
||||
onModelTextChange: (value: string) => void;
|
||||
modelStatusMessage: string | null;
|
||||
/**
|
||||
* When false (definition-only context, no instance), renders the D-owned
|
||||
* access/allowlist and parallelism controls (rows 9–10, Artifact 4).
|
||||
* When true (instance is present), access/parallelism are I-owned and
|
||||
* rendered in AgentEditMergedDialogInstanceSection.
|
||||
*/
|
||||
showInstancePresent: boolean;
|
||||
// D-owned access/parallelism (shown when !showInstancePresent)
|
||||
respondTo: RespondToMode | null;
|
||||
respondToAllowlist: string[];
|
||||
onRespondToChange: (value: RespondToMode) => void;
|
||||
onAllowlistChange: (value: string[]) => void;
|
||||
agentAccessOwnerOnly: boolean | undefined;
|
||||
parallelism: string;
|
||||
onParallelismChange: (value: string) => void;
|
||||
};
|
||||
|
||||
// ── Component ─────────────────────────────────────────────────────────────────
|
||||
@@ -104,6 +124,14 @@ export function AgentEditMergedDSection({
|
||||
model,
|
||||
onModelTextChange,
|
||||
modelStatusMessage,
|
||||
showInstancePresent,
|
||||
respondTo,
|
||||
respondToAllowlist,
|
||||
onRespondToChange,
|
||||
onAllowlistChange,
|
||||
agentAccessOwnerOnly,
|
||||
parallelism,
|
||||
onParallelismChange,
|
||||
}: AgentEditMergedDSectionProps) {
|
||||
const [showAdvanced, setShowAdvanced] = React.useState(false);
|
||||
|
||||
@@ -258,6 +286,46 @@ export function AgentEditMergedDSection({
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
{/* D-owned access/parallelism (rows 9–10, Artifact 4) — rendered only in
|
||||
definition-only context (no instance). When an instance is present,
|
||||
access/parallelism are I-owned and rendered in the I-section. */}
|
||||
{!showInstancePresent ? (
|
||||
<>
|
||||
<OwnerOnlyAccessField
|
||||
accessLocked={agentAccessOwnerOnly === true}
|
||||
allowlist={respondToAllowlist}
|
||||
disabled={isSaving || defReadOnly}
|
||||
mode={(respondTo ?? "anyone") as RespondToMode}
|
||||
onAllowlistChange={onAllowlistChange}
|
||||
onModeChange={onRespondToChange}
|
||||
/>
|
||||
<div className="space-y-1.5">
|
||||
<label
|
||||
className="text-sm font-medium text-foreground"
|
||||
htmlFor="edit-agent-parallelism"
|
||||
>
|
||||
Parallelism
|
||||
<span className="ml-1.5 text-xs font-normal text-muted-foreground">
|
||||
Optional
|
||||
</span>
|
||||
</label>
|
||||
<div className="flex min-h-11 items-center rounded-lg border border-input bg-input/30 px-3">
|
||||
<input
|
||||
className="h-8 w-full bg-transparent px-0 py-0 text-sm leading-6 outline-none placeholder:text-muted-foreground"
|
||||
disabled={isSaving || defReadOnly}
|
||||
id="edit-agent-parallelism"
|
||||
inputMode="numeric"
|
||||
min={1}
|
||||
onChange={(e) => onParallelismChange(e.target.value)}
|
||||
placeholder="Default (1)"
|
||||
type="number"
|
||||
value={parallelism}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
</>
|
||||
) : null}
|
||||
|
||||
{/* Advanced section toggle (name pool + D-env vars) */}
|
||||
<div className="space-y-2">
|
||||
<Button
|
||||
|
||||
@@ -43,7 +43,7 @@ import { RunOnSummarySection } from "./RunOnSummarySection";
|
||||
export type AgentEditMergedISectionProps = {
|
||||
inst: ManagedAgent;
|
||||
showDef: boolean;
|
||||
defReadOnly: boolean;
|
||||
defReadOnly: boolean; // kept for API compatibility; I-owned access fields are not gated by D read-only
|
||||
isSaving: boolean;
|
||||
runtimeCatalogStatus: "loading" | "error" | "ready";
|
||||
// Instance name
|
||||
@@ -134,7 +134,9 @@ export type AgentEditMergedISectionProps = {
|
||||
export function AgentEditMergedInstanceSection({
|
||||
inst,
|
||||
showDef,
|
||||
defReadOnly,
|
||||
// defReadOnly intentionally unused: I-owned access fields are not gated by D read-only
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
defReadOnly: _defReadOnly,
|
||||
isSaving,
|
||||
runtimeCatalogStatus,
|
||||
instanceName,
|
||||
@@ -243,11 +245,11 @@ export function AgentEditMergedInstanceSection({
|
||||
</div>
|
||||
</div>
|
||||
|
||||
{/* Respond-to (D-field when linked; I-field when unlinked) */}
|
||||
{/* Respond-to / access (I-owned field — editable even when definition is team-managed) */}
|
||||
<OwnerOnlyAccessField
|
||||
accessLocked={agentAccessOwnerOnly === true}
|
||||
allowlist={respondToAllowlist}
|
||||
disabled={isSaving || (showDef && defReadOnly)}
|
||||
disabled={isSaving}
|
||||
mode={respondTo}
|
||||
onAllowlistChange={onAllowlistChange}
|
||||
onModeChange={onRespondToChange}
|
||||
|
||||
@@ -388,7 +388,12 @@ export function emitAgentFormDiff(
|
||||
(isD("respondToAllowlist") &&
|
||||
next.respondTo === "allowlist" &&
|
||||
next.respondToAllowlist.join(",") !==
|
||||
(saved.respondToAllowlist ?? []).join(","));
|
||||
(saved.respondToAllowlist ?? []).join(",")) ||
|
||||
// D-parallelism: included in dChanged so the owner-only build-lock gate
|
||||
// can suppress it, and so that a parallelism-only change in definition-only
|
||||
// context actually emits a personaInput (Thufir pass-3 CRITICAL-1).
|
||||
(isD("parallelism") &&
|
||||
(next.parallelism ?? null) !== (saved.parallelism ?? null));
|
||||
|
||||
if (dChanged) {
|
||||
personaInput = {
|
||||
@@ -404,11 +409,14 @@ export function emitAgentFormDiff(
|
||||
// Include behavior block only when respondTo/parallelism are D-owned
|
||||
// (definition-only context per rows 9–10).
|
||||
behavior:
|
||||
isD("respondTo") && next.respondTo != null
|
||||
isD("respondTo") || isD("parallelism")
|
||||
? {
|
||||
respondTo: next.respondTo,
|
||||
respondTo:
|
||||
isD("respondTo") && next.respondTo != null
|
||||
? next.respondTo
|
||||
: undefined,
|
||||
respondToAllowlist:
|
||||
next.respondTo === "allowlist"
|
||||
isD("respondTo") && next.respondTo === "allowlist"
|
||||
? next.respondToAllowlist
|
||||
: undefined,
|
||||
parallelism: isD("parallelism")
|
||||
|
||||
@@ -2,6 +2,7 @@ import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import { runAgentSaveCoordinator } from "./agentSaveCoordinator.ts";
|
||||
import { seedAgentFormModel, emitAgentFormDiff } from "./agentFormModel.ts";
|
||||
|
||||
// ── Shared fixtures ────────────────────────────────────────────────────────────
|
||||
|
||||
@@ -648,3 +649,143 @@ test("test_definition_write_not_persisted_stops_instance_write", async () => {
|
||||
"instance write must NOT be attempted when D-write did not persist",
|
||||
);
|
||||
});
|
||||
|
||||
// ── R6 route-level behavioral tests ──────────────────────────────────────────
|
||||
//
|
||||
// These tests exercise the full chain that R6 (owner-review dialog) exercises
|
||||
// in production:
|
||||
// 1. reviewOverrides.respondTo arrives from useAgentManagement
|
||||
// 2. Dialog seed effect applies it as initialValueOverrides.respondTo
|
||||
// 3. emitAgentFormDiff routes the change to personaInput.behavior.respondTo
|
||||
// 4. runAgentSaveCoordinator calls updatePersona with the correct behavior
|
||||
// 5. onValidate is checked (permission gate)
|
||||
// 6. Settlement confirms persistence
|
||||
//
|
||||
// This is a coordinator-level behavioral test: it calls emitAgentFormDiff to
|
||||
// produce inputs and then passes them through runAgentSaveCoordinator, proving
|
||||
// the full pipeline from override → emission → coordinator → save.
|
||||
|
||||
test("test_r6_respondTo_override_visible_and_delivered_through_coordinator", async () => {
|
||||
// Scenario: owner-review dialog for a definition that currently has no
|
||||
// respondTo (null = owner-only by backend default). Agent requested
|
||||
// respondTo: "owner-only" explicitly. The dialog pre-fills it via
|
||||
// initialValueOverrides.respondTo = "owner-only".
|
||||
|
||||
const definition = makeDefinition({ respondTo: null });
|
||||
const ctx = { kind: "definition-only", definition };
|
||||
|
||||
// Step 1: seed form model (mirrors AgentEditMergedDialog's seed effect).
|
||||
const seed = seedAgentFormModel(ctx);
|
||||
assert.equal(
|
||||
seed.respondTo,
|
||||
null,
|
||||
"definition-only seed preserves null respondTo",
|
||||
);
|
||||
|
||||
// Step 2: apply initialValueOverrides.respondTo (mirrors the dialog's
|
||||
// useEffect block that sets respondTo from initialValueOverrides).
|
||||
const formState = { ...seed, respondTo: "owner-only" };
|
||||
|
||||
// Step 3: emit the diff — proves the override is visible in the form state
|
||||
// and routes to personaInput (D-owned in definition-only context).
|
||||
const { personaInput, agentInput } = emitAgentFormDiff(seed, formState, ctx);
|
||||
|
||||
assert.ok(
|
||||
personaInput,
|
||||
"R6 respondTo override must produce a personaInput diff",
|
||||
);
|
||||
assert.equal(
|
||||
personaInput.behavior?.respondTo,
|
||||
"owner-only",
|
||||
"personaInput.behavior.respondTo must carry the agent-requested value",
|
||||
);
|
||||
assert.equal(agentInput, null, "definition-only: no agentInput");
|
||||
|
||||
// Step 4+5: run coordinator to verify coordinator receives and
|
||||
// persists a well-formed personaInput from the emission pipeline.
|
||||
// (onValidate is called by useAgentEditMergedSubmit before the coordinator;
|
||||
// we verify here that the coordinator delivers the respondTo to updatePersona.)
|
||||
let capturedPersonaInput = null;
|
||||
let doneCalled = false;
|
||||
|
||||
const updatedDefinition = makeDefinition({
|
||||
respondTo: "owner-only",
|
||||
respondToAllowlist: [],
|
||||
parallelism: null,
|
||||
});
|
||||
|
||||
const result = await runAgentSaveCoordinator({
|
||||
ctx,
|
||||
personaInput,
|
||||
agentInput: null,
|
||||
policySets: [],
|
||||
updatePersona: async (input) => {
|
||||
capturedPersonaInput = input;
|
||||
},
|
||||
updatePersonaAndPublish: async () => ({ publicationStatus: "published" }),
|
||||
updateManagedAgent: async () => {
|
||||
throw new Error("no instance in definition-only context");
|
||||
},
|
||||
setAutoRestart: async () => {},
|
||||
setStartOnAppLaunch: async () => {},
|
||||
refetchStores: async () => ({
|
||||
persona: updatedDefinition,
|
||||
agent: null,
|
||||
}),
|
||||
onDone: () => {
|
||||
doneCalled = true;
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(result, true, "coordinator must succeed with approved save");
|
||||
assert.ok(capturedPersonaInput, "updatePersona must be called");
|
||||
assert.equal(
|
||||
capturedPersonaInput.behavior?.respondTo,
|
||||
"owner-only",
|
||||
"updatePersona must receive the agent-requested respondTo",
|
||||
);
|
||||
assert.equal(doneCalled, true, "onDone must be called on success");
|
||||
});
|
||||
|
||||
test("test_r6_respondTo_override_settlement_fails_if_persona_not_updated", async () => {
|
||||
// Negative case: coordinator emits the correct personaInput but the
|
||||
// re-fetched persona still has the old value → settlement failure → onDone NOT called.
|
||||
const definition = makeDefinition({ respondTo: null });
|
||||
const ctx = { kind: "definition-only", definition };
|
||||
const seed = seedAgentFormModel(ctx);
|
||||
const formState = { ...seed, respondTo: "owner-only" };
|
||||
const { personaInput } = emitAgentFormDiff(seed, formState, ctx);
|
||||
|
||||
let doneCalled = false;
|
||||
|
||||
// Settlement returns STALE persona (respondTo still null).
|
||||
const stalePersona = makeDefinition({ respondTo: null });
|
||||
|
||||
const result = await runAgentSaveCoordinator({
|
||||
ctx,
|
||||
personaInput,
|
||||
agentInput: null,
|
||||
policySets: [],
|
||||
updatePersona: async () => {},
|
||||
updatePersonaAndPublish: async () => ({ publicationStatus: "published" }),
|
||||
updateManagedAgent: async () => {
|
||||
throw new Error("no instance in definition-only context");
|
||||
},
|
||||
setAutoRestart: async () => {},
|
||||
setStartOnAppLaunch: async () => {},
|
||||
refetchStores: async () => ({
|
||||
persona: stalePersona,
|
||||
agent: null,
|
||||
}),
|
||||
onDone: () => {
|
||||
doneCalled = true;
|
||||
},
|
||||
});
|
||||
|
||||
assert.equal(result, false, "stale observed state must return false");
|
||||
assert.equal(
|
||||
doneCalled,
|
||||
false,
|
||||
"onDone must NOT be called when respondTo did not persist",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -150,15 +150,15 @@ export async function runAgentSaveCoordinator(
|
||||
firstError =
|
||||
err instanceof Error ? err.message : "Failed to save agent profile.";
|
||||
}
|
||||
// Settle after definition write — verify the write persisted before advancing.
|
||||
if (!firstError) {
|
||||
const { persona: settled } = await refetchStores();
|
||||
if (
|
||||
settled === null ||
|
||||
!observedStateMatchesPersonaInput(settled, personaInput)
|
||||
) {
|
||||
firstError = "Agent profile did not persist — reopen to retry.";
|
||||
}
|
||||
// Settle after definition write — re-fetch regardless of throw so that
|
||||
// observed persistence (not the command result) decides whether it failed.
|
||||
const { persona: settled } = await refetchStores();
|
||||
if (
|
||||
!firstError &&
|
||||
(settled === null ||
|
||||
!observedStateMatchesPersonaInput(settled, personaInput))
|
||||
) {
|
||||
firstError = "Agent profile did not persist — reopen to retry.";
|
||||
}
|
||||
}
|
||||
|
||||
@@ -172,15 +172,13 @@ export async function runAgentSaveCoordinator(
|
||||
firstError =
|
||||
err instanceof Error ? err.message : "Failed to save agent settings.";
|
||||
}
|
||||
// Settle after instance write.
|
||||
if (!firstError) {
|
||||
const { agent: settled } = await refetchStores();
|
||||
if (
|
||||
settled === null ||
|
||||
!observedStateMatchesAgentInput(settled, agentInput)
|
||||
) {
|
||||
firstError = "Agent settings did not persist — reopen to retry.";
|
||||
}
|
||||
// Settle after instance write — re-fetch regardless of throw.
|
||||
const { agent: settled } = await refetchStores();
|
||||
if (
|
||||
!firstError &&
|
||||
(settled === null || !observedStateMatchesAgentInput(settled, agentInput))
|
||||
) {
|
||||
firstError = "Agent settings did not persist — reopen to retry.";
|
||||
}
|
||||
}
|
||||
|
||||
@@ -345,11 +343,12 @@ function observedStateMatchesPersonaInput(
|
||||
(observed.avatarUrl ?? "") !== (submitted.avatarUrl ?? "")
|
||||
)
|
||||
return false;
|
||||
if (
|
||||
submitted.runtime !== undefined &&
|
||||
(observed.runtime ?? null) !== (submitted.runtime ?? null)
|
||||
)
|
||||
return false;
|
||||
// Runtime: UpdatePersonaRequest is a full write — omitted/undefined runtime
|
||||
// means "clear this field to null". Compare against observed null when
|
||||
// submitted.runtime is undefined (clear semantics). If submitted.runtime is
|
||||
// a non-empty string, compare against that value.
|
||||
const submittedRuntime = submitted.runtime ?? null;
|
||||
if ((observed.runtime ?? null) !== submittedRuntime) return false;
|
||||
if ((observed.model ?? null) !== (submitted.model ?? null)) return false;
|
||||
if ((observed.provider ?? null) !== (submitted.provider ?? null))
|
||||
return false;
|
||||
@@ -433,11 +432,17 @@ function observedStateMatchesAgentInput(
|
||||
return false;
|
||||
}
|
||||
// Harness-pin fields (Artifact 3 / Thufir pass-2 CRITICAL-3)
|
||||
if (
|
||||
submitted.agentCommand !== undefined &&
|
||||
(submitted.agentCommand ?? "") !== (observed.agentCommand ?? "")
|
||||
) {
|
||||
return false;
|
||||
if (submitted.agentCommand !== undefined) {
|
||||
if (submitted.agentCommand === "") {
|
||||
// "" is the sentinel for "inherit/unpin" — the backend clears
|
||||
// agentCommandOverride. Settle against agentCommandOverride === null,
|
||||
// NOT against agentCommand (which carries the resolved effective command).
|
||||
if ((observed.agentCommandOverride ?? null) !== null) return false;
|
||||
} else {
|
||||
// Explicit pin — compare the stored command.
|
||||
if ((submitted.agentCommand ?? "") !== (observed.agentCommand ?? ""))
|
||||
return false;
|
||||
}
|
||||
}
|
||||
if (submitted.harnessOverride !== undefined) {
|
||||
// harnessOverride=true means agentCommand is a pin (agentCommandOverride non-null).
|
||||
|
||||
@@ -43,7 +43,7 @@ export type AgentEditSubmitState = {
|
||||
namePoolText: string;
|
||||
model: string;
|
||||
provider: string;
|
||||
respondTo: string;
|
||||
respondTo: string | null;
|
||||
respondToAllowlist: string[];
|
||||
parallelism: string;
|
||||
parsedParallelism: number;
|
||||
@@ -54,6 +54,12 @@ export type AgentEditSubmitState = {
|
||||
startOnAppLaunch: boolean | undefined;
|
||||
/** D-field: definition runtime id (independent of I-harness pin). */
|
||||
definitionRuntimeId: string;
|
||||
/**
|
||||
* Ref tracking the auto-seeded definition runtime ID. When non-null, the
|
||||
* auto-seed was not a user choice — skip persisting runtime as a D-change
|
||||
* if no other D-field was actually modified.
|
||||
*/
|
||||
autoSeededDefinitionRuntimeRef: React.RefObject<string | null>;
|
||||
/** I-field: harness pin runtime id (instance only). */
|
||||
selectedRuntimeId: string;
|
||||
inheritHarness: boolean;
|
||||
@@ -135,11 +141,17 @@ export function useAgentEditMergedSubmit(
|
||||
systemPrompt: s.systemPrompt.trim(),
|
||||
respondTo: s.respondTo as typeof seed.respondTo,
|
||||
respondToAllowlist: s.respondToAllowlist,
|
||||
// D-field: use definitionRuntimeId (independent of I-harness pin)
|
||||
// D-field: use definitionRuntimeId (independent of I-harness pin).
|
||||
// If the runtime was auto-seeded (not a user choice), use the seed's
|
||||
// original runtime so a no-op save doesn't persist the app default
|
||||
// as a new definition runtime.
|
||||
runtime:
|
||||
s.definitionRuntimeId === "custom"
|
||||
? undefined
|
||||
: s.definitionRuntimeId,
|
||||
s.autoSeededDefinitionRuntimeRef.current !== null &&
|
||||
s.autoSeededDefinitionRuntimeRef.current === s.definitionRuntimeId
|
||||
? (seed.runtime ?? undefined) // preserve original (undefined = no runtime)
|
||||
: s.definitionRuntimeId === "custom"
|
||||
? undefined
|
||||
: s.definitionRuntimeId,
|
||||
model: normalizedModel,
|
||||
provider: normalizedProvider,
|
||||
// D-field env: use the definition env from the form state (not the
|
||||
|
||||
250
desktop/src/features/agents/ui/useAgentEditRuntimeHandlers.ts
Normal file
250
desktop/src/features/agents/ui/useAgentEditRuntimeHandlers.ts
Normal file
@@ -0,0 +1,250 @@
|
||||
/**
|
||||
* useAgentEditRuntimeHandlers — runtime/model/provider selection handlers for
|
||||
* AgentEditMergedDialog.
|
||||
*
|
||||
* Extracted to keep the dialog under the 1000-line file-size gate. Owns the
|
||||
* D/I selection objects, apply helpers, and the four handler callbacks.
|
||||
*/
|
||||
|
||||
import type * as React from "react";
|
||||
|
||||
import {
|
||||
runtimeDropdownAction,
|
||||
usePendingHarnessSelection,
|
||||
} from "./addCustomHarness";
|
||||
import {
|
||||
AUTO_PROVIDER_DROPDOWN_VALUE,
|
||||
runtimeSupportsLlmProviderSelection,
|
||||
} from "./agentConfigOptions";
|
||||
import {
|
||||
selectionOnModelDropdownChange,
|
||||
selectionOnProviderDropdownChange,
|
||||
selectionOnRuntimeChange,
|
||||
type RuntimeModelProviderSelection,
|
||||
} from "./runtimeModelProviderSelection";
|
||||
import type { AcpRuntimeCatalogEntry } from "@/shared/api/types";
|
||||
|
||||
export type RuntimeHandlersInput = {
|
||||
showDef: boolean;
|
||||
showInst: boolean;
|
||||
// D-state
|
||||
dProvider: string;
|
||||
dModel: string;
|
||||
dIsCustomProviderEditing: boolean;
|
||||
dIsCustomModelEditing: boolean;
|
||||
setDProvider: (v: string) => void;
|
||||
setDModel: (v: string) => void;
|
||||
setDIsCustomProviderEditing: (v: boolean) => void;
|
||||
setDIsCustomModelEditing: (v: boolean) => void;
|
||||
envVars: Record<string, string>;
|
||||
setEnvVars: (v: Record<string, string>) => void;
|
||||
definitionRuntimeId: string;
|
||||
setDefinitionRuntimeId: (v: string) => void;
|
||||
// I-state
|
||||
iProvider: string;
|
||||
iModel: string;
|
||||
iIsCustomProviderEditing: boolean;
|
||||
iIsCustomModelEditing: boolean;
|
||||
setIProvider: (v: string) => void;
|
||||
setIModel: (v: string) => void;
|
||||
setIIsCustomProviderEditing: (v: boolean) => void;
|
||||
setIIsCustomModelEditing: (v: boolean) => void;
|
||||
instanceEnvVars: Record<string, string>;
|
||||
setInstanceEnvVars: (v: Record<string, string>) => void;
|
||||
selectedRuntimeId: string;
|
||||
setSelectedRuntimeId: (v: string) => void;
|
||||
setInheritHarness: (v: boolean) => void;
|
||||
setAgentCommand: (v: string) => void;
|
||||
setAgentArgs: (v: string) => void;
|
||||
runtimeTouched: React.RefObject<boolean>;
|
||||
setIsAddHarnessOpen: (v: boolean) => void;
|
||||
runtimes: readonly AcpRuntimeCatalogEntry[];
|
||||
selectedRuntime: AcpRuntimeCatalogEntry | undefined;
|
||||
open: boolean;
|
||||
};
|
||||
|
||||
/**
|
||||
* Returns the four runtime/model/provider change handlers and selectSavedHarness.
|
||||
*
|
||||
* D-owned handlers write to D state only (via applyDSelection).
|
||||
* I-owned handlers write to I state only (via applyISelection).
|
||||
* In definition-only context (showInst=false), D and I state are mirrored.
|
||||
*/
|
||||
export function useAgentEditRuntimeHandlers(input: RuntimeHandlersInput) {
|
||||
const {
|
||||
showDef,
|
||||
showInst,
|
||||
dProvider,
|
||||
dModel,
|
||||
dIsCustomProviderEditing,
|
||||
dIsCustomModelEditing,
|
||||
setDProvider,
|
||||
setDModel,
|
||||
setDIsCustomProviderEditing,
|
||||
setDIsCustomModelEditing,
|
||||
envVars,
|
||||
setEnvVars,
|
||||
definitionRuntimeId,
|
||||
setDefinitionRuntimeId,
|
||||
iProvider,
|
||||
iModel,
|
||||
iIsCustomProviderEditing,
|
||||
iIsCustomModelEditing,
|
||||
setIProvider,
|
||||
setIModel,
|
||||
setIIsCustomProviderEditing,
|
||||
setIIsCustomModelEditing,
|
||||
instanceEnvVars,
|
||||
setInstanceEnvVars,
|
||||
selectedRuntimeId,
|
||||
setSelectedRuntimeId,
|
||||
setInheritHarness,
|
||||
setAgentCommand,
|
||||
setAgentArgs,
|
||||
runtimeTouched,
|
||||
setIsAddHarnessOpen,
|
||||
runtimes,
|
||||
selectedRuntime,
|
||||
open,
|
||||
} = input;
|
||||
|
||||
const dSelection: RuntimeModelProviderSelection = {
|
||||
provider: dProvider,
|
||||
model: dModel,
|
||||
isCustomProviderEditing: dIsCustomProviderEditing,
|
||||
isCustomModelEditing: dIsCustomModelEditing,
|
||||
envVars,
|
||||
};
|
||||
|
||||
const iSelection: RuntimeModelProviderSelection = {
|
||||
provider: iProvider,
|
||||
model: iModel,
|
||||
isCustomProviderEditing: iIsCustomProviderEditing,
|
||||
isCustomModelEditing: iIsCustomModelEditing,
|
||||
envVars: instanceEnvVars,
|
||||
};
|
||||
|
||||
/** Writes D model/provider/env. In D-only context, mirrors to I as well. */
|
||||
function applyDSelection(next: RuntimeModelProviderSelection) {
|
||||
setDProvider(next.provider);
|
||||
setDModel(next.model);
|
||||
setDIsCustomProviderEditing(next.isCustomProviderEditing);
|
||||
setDIsCustomModelEditing(next.isCustomModelEditing);
|
||||
setEnvVars(next.envVars);
|
||||
if (!showInst) {
|
||||
setIProvider(next.provider);
|
||||
setIModel(next.model);
|
||||
setIIsCustomProviderEditing(next.isCustomProviderEditing);
|
||||
setIIsCustomModelEditing(next.isCustomModelEditing);
|
||||
}
|
||||
}
|
||||
|
||||
/** Writes I model/provider/env only. Never touches D state. */
|
||||
function applyISelection(next: RuntimeModelProviderSelection) {
|
||||
setIProvider(next.provider);
|
||||
setIModel(next.model);
|
||||
setIIsCustomProviderEditing(next.isCustomProviderEditing);
|
||||
setIIsCustomModelEditing(next.isCustomModelEditing);
|
||||
setInstanceEnvVars(next.envVars);
|
||||
}
|
||||
|
||||
function handleDefinitionRuntimeChange(nextValue: string) {
|
||||
const action = runtimeDropdownAction(nextValue);
|
||||
if (action.kind === "add-custom-harness") {
|
||||
setIsAddHarnessOpen(true);
|
||||
return;
|
||||
}
|
||||
const nextRuntimeId = action.runtimeId;
|
||||
const previousRuntimeId = definitionRuntimeId;
|
||||
setDefinitionRuntimeId(nextRuntimeId || "custom");
|
||||
applyDSelection(
|
||||
selectionOnRuntimeChange(dSelection, {
|
||||
previousRuntime: previousRuntimeId,
|
||||
nextRuntime: nextRuntimeId,
|
||||
nextRuntimeCanChooseProvider:
|
||||
runtimeSupportsLlmProviderSelection(nextRuntimeId),
|
||||
lockedRuntimeReset: "full",
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
function handleRuntimeDropdownChange(nextValue: string) {
|
||||
const action = runtimeDropdownAction(nextValue);
|
||||
if (action.kind === "add-custom-harness") {
|
||||
setIsAddHarnessOpen(true);
|
||||
return;
|
||||
}
|
||||
const nextRuntimeId = action.runtimeId;
|
||||
const previousRuntimeId = selectedRuntimeId;
|
||||
runtimeTouched.current = true;
|
||||
const nextRuntime = runtimes.find((r) => r.id === nextRuntimeId);
|
||||
const resolvedRuntimeId = nextRuntimeId || "custom";
|
||||
setSelectedRuntimeId(resolvedRuntimeId);
|
||||
if (resolvedRuntimeId === "custom" || nextRuntime?.command) {
|
||||
setInheritHarness(false);
|
||||
}
|
||||
if (nextRuntime?.command) {
|
||||
setAgentCommand(nextRuntime.command);
|
||||
setAgentArgs(nextRuntime.defaultArgs.join(","));
|
||||
}
|
||||
applyISelection(
|
||||
selectionOnRuntimeChange(iSelection, {
|
||||
previousRuntime: previousRuntimeId,
|
||||
nextRuntime: nextRuntimeId,
|
||||
nextRuntimeCanChooseProvider:
|
||||
runtimeSupportsLlmProviderSelection(nextRuntimeId),
|
||||
lockedRuntimeReset: "full",
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
const selectSavedHarness = usePendingHarnessSelection(
|
||||
runtimes,
|
||||
showDef && !showInst
|
||||
? handleDefinitionRuntimeChange
|
||||
: handleRuntimeDropdownChange,
|
||||
open,
|
||||
);
|
||||
|
||||
function handleProviderDropdownChange(nextValue: string) {
|
||||
const nextProvider =
|
||||
nextValue === AUTO_PROVIDER_DROPDOWN_VALUE ? "" : nextValue;
|
||||
if (nextProvider === "relay-mesh" && selectedRuntimeId !== "buzz-agent") {
|
||||
handleRuntimeDropdownChange("buzz-agent");
|
||||
}
|
||||
const sel = showDef ? dSelection : iSelection;
|
||||
const applyFn = showDef ? applyDSelection : applyISelection;
|
||||
const nextSelection = selectionOnProviderDropdownChange(sel, {
|
||||
runtime:
|
||||
nextProvider === "relay-mesh"
|
||||
? "buzz-agent"
|
||||
: (selectedRuntime?.id ?? selectedRuntimeId),
|
||||
nextValue,
|
||||
clearModelWhenApiKeyMissing: !showInst,
|
||||
});
|
||||
applyFn({
|
||||
...nextSelection,
|
||||
model: nextProvider === "relay-mesh" ? "auto" : nextSelection.model,
|
||||
});
|
||||
}
|
||||
|
||||
function handleModelDropdownChange(nextValue: string) {
|
||||
const sel = showDef ? dSelection : iSelection;
|
||||
const applyFn = showDef ? applyDSelection : applyISelection;
|
||||
applyFn(
|
||||
selectionOnModelDropdownChange(sel, {
|
||||
nextValue,
|
||||
clearKnownModelOnCustomEntry: !showInst,
|
||||
isModelCustom: false,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
return {
|
||||
handleDefinitionRuntimeChange,
|
||||
handleRuntimeDropdownChange,
|
||||
handleProviderDropdownChange,
|
||||
handleModelDropdownChange,
|
||||
selectSavedHarness,
|
||||
};
|
||||
}
|
||||
@@ -65,6 +65,7 @@ export type AgentEditRuntimeStateInputs = {
|
||||
instanceEnvVars: Record<string, string>;
|
||||
inheritHarness: boolean;
|
||||
inst: ManagedAgent | null;
|
||||
def: AgentPersona | null;
|
||||
linkedPersona: AgentPersona | null;
|
||||
bakedEnvKeys: string[] | undefined;
|
||||
originalAgentCommand: string;
|
||||
@@ -113,6 +114,12 @@ export type AgentEditRuntimeStateResult = {
|
||||
advancedRequiredEnvKeys: readonly string[];
|
||||
/** True while model discovery is in progress (drives model dropdown disabled state). */
|
||||
modelDiscoveryLoading: boolean;
|
||||
/**
|
||||
* True when the definition has no runtime but has a saved model/provider,
|
||||
* making the provider/model picker editable without a runtime selection.
|
||||
* Matches AgentDefinitionDialog's blankRuntimeModelProviderEditable contract.
|
||||
*/
|
||||
blankRuntimeModelProviderEditable: boolean;
|
||||
};
|
||||
|
||||
// ── Hook ──────────────────────────────────────────────────────────────────────
|
||||
@@ -133,6 +140,7 @@ export function useAgentEditRuntimeState({
|
||||
instanceEnvVars,
|
||||
inheritHarness,
|
||||
inst,
|
||||
def,
|
||||
linkedPersona,
|
||||
bakedEnvKeys,
|
||||
originalAgentCommand,
|
||||
@@ -221,7 +229,7 @@ export function useAgentEditRuntimeState({
|
||||
inst,
|
||||
]);
|
||||
|
||||
const llmProviderFieldVisible = runtimeSupportsLlmProviderSelection(
|
||||
const llmProviderFieldVisible = React.useMemo(() => {
|
||||
// In linked context (showDef && showInst), the D-section provider/model
|
||||
// picker is driven by the definition's runtime (definitionRuntimeId), not by
|
||||
// the instance's harness pin (prospectiveRuntimeId). Using prospectiveRuntimeId
|
||||
@@ -231,8 +239,37 @@ export function useAgentEditRuntimeState({
|
||||
// Instance-only context (showInst && !showDef) correctly uses prospectiveRuntimeId
|
||||
// because the instance's harness is the only runtime in scope.
|
||||
// D-only context falls through to definitionRuntimeId via selectedRuntimeId (they equal).
|
||||
showInst && !showDef ? prospectiveRuntimeId : definitionRuntimeId,
|
||||
);
|
||||
const activeRuntimeId =
|
||||
showInst && !showDef ? prospectiveRuntimeId : definitionRuntimeId;
|
||||
|
||||
if (runtimeSupportsLlmProviderSelection(activeRuntimeId)) return true;
|
||||
|
||||
// blankRuntimeModelProviderEditable: for an edit context (not create), when the
|
||||
// definition has no runtime configured but already has a saved model or provider,
|
||||
// expose the provider/model picker so the user can edit the existing values.
|
||||
// This matches AgentDefinitionDialog's `initialModelProviderEditableWithoutRuntime`
|
||||
// contract introduced in commit 87dc4dccba — restoring the capability that was
|
||||
// accidentally dropped when test-11 was rewritten.
|
||||
if (showDef && activeRuntimeId === "custom") {
|
||||
const defRuntime = (
|
||||
activeRuntimeId === "custom" ? "" : activeRuntimeId
|
||||
).trim();
|
||||
const isRuntimeless = defRuntime.length === 0;
|
||||
const hasSavedModelOrProvider =
|
||||
(linkedPersona?.model ?? def?.model ?? "").trim().length > 0 ||
|
||||
(linkedPersona?.provider ?? def?.provider ?? "").trim().length > 0;
|
||||
if (isRuntimeless && hasSavedModelOrProvider) return true;
|
||||
}
|
||||
|
||||
return false;
|
||||
}, [
|
||||
showInst,
|
||||
showDef,
|
||||
prospectiveRuntimeId,
|
||||
definitionRuntimeId,
|
||||
linkedPersona,
|
||||
def,
|
||||
]);
|
||||
const prospectiveRuntime = runtimes.find(
|
||||
(r) => r.id === prospectiveRuntimeId,
|
||||
);
|
||||
@@ -461,6 +498,11 @@ export function useAgentEditRuntimeState({
|
||||
topLevelSecretEnvVar,
|
||||
advancedRequiredEnvKeys,
|
||||
modelDiscoveryLoading,
|
||||
blankRuntimeModelProviderEditable:
|
||||
showDef &&
|
||||
definitionRuntimeId === "custom" &&
|
||||
((linkedPersona?.model ?? def?.model ?? "").trim().length > 0 ||
|
||||
(linkedPersona?.provider ?? def?.provider ?? "").trim().length > 0),
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
@@ -168,6 +168,53 @@ test.describe("edit agent dialog", () => {
|
||||
await expect(page.getByTestId("agent-respond-to")).toBeVisible();
|
||||
});
|
||||
|
||||
test("definition-only context renders access control and parallelism (rows 9–10 D-owned)", async ({
|
||||
page,
|
||||
}) => {
|
||||
// Rows 9–10: in definition-only context (zero-instance definition), access/parallelism
|
||||
// are D-owned and must be rendered. This test proves Artifact 4 access-field
|
||||
// completeness for the definition-only route (R5 — definition-only edit from library).
|
||||
const DEFONLY_PERSONA_ID = "persona-defonly-access-e2e";
|
||||
await installMockBridge(page, {
|
||||
bakedBuildEnv: BAKED_DEFAULTS,
|
||||
personas: [
|
||||
{
|
||||
id: DEFONLY_PERSONA_ID,
|
||||
displayName: "Definition Only Agent",
|
||||
systemPrompt: "No instances here.",
|
||||
respondTo: "owner-only",
|
||||
},
|
||||
],
|
||||
});
|
||||
|
||||
await page.goto("/");
|
||||
await page.getByTestId("open-agents-view").click();
|
||||
|
||||
// Navigate to the definition-edit route via the definitions library button.
|
||||
const defButton = page.getByRole("button", {
|
||||
name: "Definition Only Agent agent profile",
|
||||
});
|
||||
await expect(defButton).toBeVisible({ timeout: 10_000 });
|
||||
await defButton.click();
|
||||
await expect(page.getByTestId("user-profile-panel")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await page.getByTestId("user-profile-edit-agent").click();
|
||||
|
||||
await expect(page.getByTestId("edit-agent-dialog")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
|
||||
// Rows 9–10 Artifact 4: access control IS visible in definition-only context.
|
||||
await expect(page.getByTestId("agent-respond-to")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
// D-owned parallelism field rendered.
|
||||
await expect(page.locator("#edit-agent-parallelism")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
});
|
||||
|
||||
test("edits the agent name and persists it across a dialog reopen", async ({
|
||||
page,
|
||||
}) => {
|
||||
|
||||
@@ -883,12 +883,11 @@ test.describe("global agent config screenshots", () => {
|
||||
});
|
||||
|
||||
// Shot 11: the merged surface's equivalent of the "provider gate" regression pin.
|
||||
// In the merged surface, a runtime-less definition-with-instance does NOT expose
|
||||
// the provider picker (llmProviderFieldVisible=false when no runtime supports LLM
|
||||
// selection). This pins the correct observable behavior: provider picker hidden
|
||||
// and save enabled. Compared to AgentDefinitionDialog's blankRuntimeModelProviderEditable
|
||||
// — that dialog's own gate behavior is pinned separately in its own e2e path.
|
||||
test("11-edit-runtime-less-provider-hidden-save-enabled", async ({
|
||||
// A runtime-less definition-with-instance that has a saved model (but no provider)
|
||||
// exposes the provider picker (blankRuntimeModelProviderEditable — restoring the
|
||||
// contract from AgentDefinitionDialog commit 87dc4dccba). The global provider
|
||||
// "anthropic" satisfies the provider gate, so Save is enabled with the picker visible.
|
||||
test("11-edit-runtime-less-provider-visible-save-enabled", async ({
|
||||
page,
|
||||
}) => {
|
||||
const PERSONA_ID = "persona-runtime-less-edit-e2e";
|
||||
@@ -947,9 +946,13 @@ test.describe("global agent config screenshots", () => {
|
||||
/Save changes/,
|
||||
);
|
||||
|
||||
// In the merged surface, runtime-less definition-with-instance: no runtime
|
||||
// supports LLM selection → provider picker NOT shown → save is NOT blocked.
|
||||
await expect(page.locator("#edit-agent-llm-provider")).not.toBeVisible();
|
||||
// blankRuntimeModelProviderEditable: runtime-less definition with a saved model
|
||||
// exposes the provider picker so the user can edit the existing model/provider values.
|
||||
// The global provider "anthropic" satisfies isEditAgentProviderSaveValid, so Save
|
||||
// is not blocked even though the form provider field is empty.
|
||||
await expect(page.locator("#edit-agent-llm-provider")).toBeVisible({
|
||||
timeout: 10_000,
|
||||
});
|
||||
await expect(page.getByTestId("edit-agent-dialog-submit")).toBeEnabled({
|
||||
timeout: 10_000,
|
||||
});
|
||||
@@ -958,7 +961,7 @@ test.describe("global agent config screenshots", () => {
|
||||
|
||||
const dialog = page.getByRole("dialog");
|
||||
await dialog.screenshot({
|
||||
path: `${SHOTS}/11-edit-runtime-less-provider-hidden-save-enabled.png`,
|
||||
path: `${SHOTS}/11-edit-runtime-less-provider-visible-save-enabled.png`,
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user