refactor(desktop): unify EditAgentDialog styling with PersonaDialog (#1540)

Signed-off-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
Co-authored-by: npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5 <8a675edd33677aa0389f6650d467b2041fb0df4ca820eacb009babb95e3715d4@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
klopez4212
2026-07-07 16:03:39 +01:00
committed by GitHub
co-authored by npub13fn4ahfnvaa2qwylvegdgeajqs0mph6v4qsw4jcqnw4mjh3hzh2quuucm5
parent 564ead3856
commit c258ffc4a8
13 changed files with 2120 additions and 425 deletions
+8 -2
View File
@@ -134,7 +134,11 @@ const overrides = new Map([
// config-parity: max_tokens_env_var + context_limit_env_var fields added to
// KnownAcpRuntime (2 fields × 4 runtimes + discovery tests = ~13 lines).
// Load-bearing — required for buzz-agent normalized config parity.
["src-tauri/src/managed_agents/discovery.rs", 1124],
// same-runtime-pin: update_time_agent_command_override + its override /
// same-runtime / alias / sentinel / non-override / persona-less test matrix
// (~135 lines, mostly tests) so a deliberate Custom pin survives the update
// path instead of being dropped back to inherit. Load-bearing, not debt.
["src-tauri/src/managed_agents/discovery.rs", 1259],
// migration_tests.rs carries the harness-sync migration coverage plus the
// patch_json_records owner-only writeback regression test (SECURITY.md:90
// crash-safe 0o600 fallback). Load-bearing security + feature coverage, not
@@ -207,7 +211,9 @@ const overrides = new Map([
// a GUI-launched DMG (the discovery_env_with_baked_floor fold).
// +3: provider tri-state applied in update_managed_agent handler
// (if let Some(provider_update) = input.provider { record.provider = provider_update; }).
["src-tauri/src/commands/agent_models.rs", 1071],
// +8: harness_override thread-through in update_managed_agent so a deliberate
// Custom pin routes to update_time_agent_command_override (comment + call).
["src-tauri/src/commands/agent_models.rs", 1079],
// draft-persistence predicate: submit-time `loadDraft` check + inline comment
// + deps-array entry in submitMessage closes the never-persisted-boundary
// defect (Thufir Pass-3 finding). Load-bearing correctness fix; queued to
+13 -5
View File
@@ -860,13 +860,21 @@ pub async fn update_managed_agent(
// that diverges from the persona. An empty/whitespace value (the
// "Inherit from persona" sentinel) clears the pin back to `None`. A
// name-only edit (`agent_command == None`) leaves the pin intact.
//
// `harness_override` threads the user's explicit intent: when they pick
// a runtime/Custom command in the dialog it is a real pin even if it
// maps to the persona's own runtime, so a same-runtime pick is kept
// rather than dropped back to inherit (see
// `update_time_agent_command_override`).
if let Some(agent_command) = input.agent_command {
let personas = load_personas(&app).unwrap_or_default();
record.agent_command_override = crate::managed_agents::divergent_agent_command_override(
record.persona_id.as_deref(),
&personas,
Some(&agent_command),
);
record.agent_command_override =
crate::managed_agents::update_time_agent_command_override(
record.persona_id.as_deref(),
&personas,
Some(&agent_command),
input.harness_override,
);
}
if let Some(agent_args) = input.agent_args {
record.agent_args = agent_args;
@@ -348,6 +348,47 @@ pub fn divergent_agent_command_override(
}
}
/// Decide the `agent_command_override` to persist at AGENT UPDATE time.
///
/// The edit dialog sends `agent_command` as a tri-state string: the empty
/// "inherit from persona" sentinel (clear the pin), or a concrete command
/// (pin). Resolution:
///
/// - EMPTY / whitespace → the inherit sentinel: always `None` regardless of
/// `harness_override`, so toggling "Inherit runtime from persona" clears the
/// pin.
/// - DELIBERATE OVERRIDE (`harness_override` true, persona linked): the user
/// explicitly picked a runtime/Custom command in the dialog. This is a real
/// pin and is preserved VERBATIM — even when the picked command maps to, or
/// is byte-identical to, the persona's own runtime command. Selecting "Custom
/// command" and saving e.g. `goose` for a goose persona is a deliberate act
/// to freeze the harness against future persona runtime edits; dropping it
/// back to inherit (as [`divergent_agent_command_override`] would) defeats
/// that intent. Unlike the create-time path, there is no byte-identical
/// exception here: at create the command is machine-derived from the persona,
/// so equality means "no user divergence"; at update an equal command reached
/// the force branch only because the user picked Custom, which IS the
/// divergence.
/// - NO OVERRIDE INTENT (`harness_override` false) or NO PERSONA: defer to
/// [`divergent_agent_command_override`], which keeps the persona authoritative
/// and treats a same-runtime restatement as inherit.
pub fn update_time_agent_command_override(
persona_id: Option<&str>,
personas: &[crate::managed_agents::types::PersonaRecord],
picked_command: Option<&str>,
harness_override: bool,
) -> Option<String> {
let picked = picked_command
.map(str::trim)
.filter(|value| !value.is_empty())?;
if persona_id.is_some() && harness_override {
return Some(picked.to_string());
}
divergent_agent_command_override(persona_id, personas, Some(picked))
}
/// Decide the `agent_command_override` to persist at AGENT CREATE time.
///
/// A persona-backed create receives its harness command from
@@ -729,8 +770,8 @@ mod tests {
use super::{
classify_runtime, create_time_agent_command_override, default_agent_command,
divergent_agent_command_override, effective_agent_command, find_via_login_shell,
managed_agent_avatar_url, normalize_agent_args, BUZZ_AGENT_AVATAR_URL,
CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL,
managed_agent_avatar_url, normalize_agent_args, update_time_agent_command_override,
BUZZ_AGENT_AVATAR_URL, CLAUDE_CODE_AVATAR_URL, CODEX_AVATAR_URL, GOOSE_AVATAR_URL,
};
use crate::managed_agents::AcpAvailabilityStatus;
@@ -1120,4 +1161,98 @@ mod tests {
Some("codex-acp".to_string())
);
}
#[test]
fn update_time_override_preserves_same_runtime_pin_when_overriding() {
// The bug this fixes: the user picks "Custom command" in the edit
// dialog and saves `goose` verbatim for a goose persona. That is a
// deliberate pin (harness_override true) — it must be kept so future
// persona runtime edits stop propagating, even though it maps to the
// persona's own runtime. `divergent_agent_command_override` alone would
// wrongly drop it to `None`.
let personas = vec![persona_with_runtime("p1", Some("goose"))];
assert_eq!(
update_time_agent_command_override(Some("p1"), &personas, Some("goose"), true),
Some("goose".to_string())
);
}
#[test]
fn update_time_override_preserves_exact_persona_command_when_overriding() {
// Even when the pick is byte-identical to the persona's own command, an
// explicit Custom selection (harness_override true) is a deliberate pin
// and is preserved. This is the core divergence from the create-time
// contract: at update, equality reached the force branch only because
// the user picked Custom.
let personas = vec![persona_with_runtime("p1", Some("claude"))];
assert_eq!(
update_time_agent_command_override(
Some("p1"),
&personas,
Some("claude-agent-acp"),
true
),
Some("claude-agent-acp".to_string())
);
}
#[test]
fn update_time_override_preserves_alias_pin_when_overriding() {
// A `claude` persona with an installed `claude-code-acp` alias: picking
// it as a Custom pin is a deliberate divergence from the primary
// command and must be preserved when overriding.
let personas = vec![persona_with_runtime("p1", Some("claude"))];
assert_eq!(
update_time_agent_command_override(
Some("p1"),
&personas,
Some("claude-code-acp"),
true
),
Some("claude-code-acp".to_string())
);
}
#[test]
fn update_time_override_defers_to_divergent_when_not_overriding() {
// Without the explicit intent bit (e.g. a name-only edit that still
// echoes the command), the persona stays authoritative: a same-runtime
// command inherits, a different runtime pins.
let personas = vec![persona_with_runtime("p1", Some("goose"))];
assert_eq!(
update_time_agent_command_override(Some("p1"), &personas, Some("goose"), false),
None
);
assert_eq!(
update_time_agent_command_override(Some("p1"), &personas, Some("codex-acp"), false),
Some("codex-acp".to_string())
);
}
#[test]
fn update_time_override_clears_pin_for_inherit_sentinel() {
// The empty "Inherit from persona" sentinel always clears the pin,
// regardless of the override flag.
let personas = vec![persona_with_runtime("p1", Some("goose"))];
assert_eq!(
update_time_agent_command_override(Some("p1"), &personas, Some(" "), true),
None
);
assert_eq!(
update_time_agent_command_override(Some("p1"), &personas, None, true),
None
);
}
#[test]
fn update_time_override_preserves_pin_for_persona_less_agent() {
// A persona-less agent has no runtime to inherit, so any picked command
// is a real pin — preserved even without the override flag (mirrors the
// create-time persona-less contract).
let personas = vec![persona_with_runtime("p1", Some("goose"))];
assert_eq!(
update_time_agent_command_override(None, &personas, Some("codex-acp"), false),
Some("codex-acp".to_string())
);
}
}
@@ -503,6 +503,14 @@ pub struct UpdateManagedAgentRequest {
pub acp_command: Option<String>,
#[serde(default)]
pub agent_command: Option<String>,
/// True when the accompanying `agent_command` is a runtime/Custom command
/// the user deliberately picked for a linked persona (i.e. the dialog is
/// not inheriting). Distinguishes a real pin — including one that maps to
/// the persona's own runtime — from a persona-authoritative restatement,
/// so a same-runtime pick is preserved instead of being dropped back to
/// inherit. Ignored when `agent_command` is absent or the inherit sentinel.
#[serde(default)]
pub harness_override: bool,
#[serde(default)]
pub agent_args: Option<Vec<String>>,
#[serde(default)]
@@ -0,0 +1,378 @@
import { cn } from "@/shared/lib/cn";
import { Input } from "@/shared/ui/input";
import { Textarea } from "@/shared/ui/textarea";
import { EnvVarsEditor, type EnvVarsValue } from "./EnvVarsEditor";
import {
PERSONA_FIELD_CONTROL_CLASS,
PERSONA_FIELD_SHELL_CLASS,
PERSONA_LABEL_OPTIONAL_CLASS,
} from "./personaDialogPickers";
import type { AgentPersona } from "@/shared/api/types";
export function EditAgentAdvancedFields({
acpCommand,
agentArgs,
agentCommand,
disabled,
envVars,
fileSatisfiedEnvKeys,
inheritedEnvVars,
inheritHarness,
linkedPersona,
mcpCommand,
mcpToolsets,
parallelism,
relayUrl,
requiredEnvKeys,
selectedRuntimeId,
systemPrompt,
turnTimeoutSeconds,
onAcpCommandChange,
onAgentArgsChange,
onAgentCommandChange,
onEnvVarsChange,
onInheritHarnessChange,
onMcpCommandChange,
onMcpToolsetsChange,
onParallelismChange,
onRelayUrlChange,
onSystemPromptChange,
onTurnTimeoutChange,
}: {
acpCommand: string;
agentArgs: string;
agentCommand: string;
disabled: boolean;
envVars: EnvVarsValue;
fileSatisfiedEnvKeys: readonly string[];
inheritedEnvVars: Record<string, string>;
inheritHarness: boolean;
linkedPersona: AgentPersona | null;
mcpCommand: string;
mcpToolsets: string;
parallelism: string;
relayUrl: string;
requiredEnvKeys: readonly string[];
selectedRuntimeId: string;
systemPrompt: string;
turnTimeoutSeconds: string;
onAcpCommandChange: (value: string) => void;
onAgentArgsChange: (value: string) => void;
onAgentCommandChange: (value: string) => void;
onEnvVarsChange: (value: EnvVarsValue) => void;
onInheritHarnessChange: (value: boolean) => void;
onMcpCommandChange: (value: string) => void;
onMcpToolsetsChange: (value: string) => void;
onParallelismChange: (value: string) => void;
onRelayUrlChange: (value: string) => void;
onSystemPromptChange: (value: string) => void;
onTurnTimeoutChange: (value: string) => void;
}) {
return (
<div className="space-y-5 pt-2">
{/* Inherit runtime from persona */}
{linkedPersona ? (
<div className="space-y-1.5">
<label
className="flex items-center gap-2 text-sm font-medium"
htmlFor="edit-agent-inherit-harness"
>
<input
checked={inheritHarness}
id="edit-agent-inherit-harness"
onChange={(event) => onInheritHarnessChange(event.target.checked)}
type="checkbox"
/>
Inherit runtime from persona
</label>
<p className="text-xs text-muted-foreground">
{inheritHarness
? `Uses the ${linkedPersona.displayName} persona's runtime${
linkedPersona.runtime ? ` (${linkedPersona.runtime})` : ""
}. Editing the persona and respawning propagates the new runtime.`
: "Pins this agent to a specific runtime command, overriding the persona's runtime."}
</p>
</div>
) : null}
{/* Custom agent command (when custom runtime) */}
{selectedRuntimeId === "custom" && !inheritHarness ? (
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-command"
>
Agent command
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-command"
onChange={(event) => onAgentCommandChange(event.target.value)}
placeholder="Full path or shell command"
value={agentCommand}
/>
</div>
</div>
) : null}
{/* Agent runtime args */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-args"
>
Agent runtime args
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-args"
onChange={(event) => onAgentArgsChange(event.target.value)}
placeholder="Comma-separated"
value={agentArgs}
/>
</div>
</div>
{/* MCP command */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-mcp-command"
>
MCP command
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-mcp-command"
onChange={(event) => onMcpCommandChange(event.target.value)}
placeholder="Optional MCP server command"
value={mcpCommand}
/>
</div>
</div>
{/* MCP toolsets */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-mcp-toolsets"
>
MCP toolsets
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-mcp-toolsets"
onChange={(event) => onMcpToolsetsChange(event.target.value)}
placeholder="default,canvas,forums,dms,media"
value={mcpToolsets}
/>
</div>
<p className="text-xs text-muted-foreground">
Comma-separated list of toolsets to expose via BUZZ_TOOLSETS.
</p>
</div>
{/* Turn timeout + Parallelism side by side */}
<div className="grid gap-4 md:grid-cols-2">
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-timeout"
>
Turn timeout
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>seconds</span>
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-timeout"
onChange={(event) => onTurnTimeoutChange(event.target.value)}
placeholder="300"
value={turnTimeoutSeconds}
/>
</div>
</div>
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-parallelism"
>
Parallelism
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-parallelism"
inputMode="numeric"
onChange={(event) => onParallelismChange(event.target.value)}
placeholder="1"
value={parallelism}
/>
</div>
</div>
</div>
{/* Relay URL */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-relay-url"
>
Relay URL
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-relay-url"
onChange={(event) => onRelayUrlChange(event.target.value)}
placeholder="Leave blank to use the workspace relay"
value={relayUrl}
/>
</div>
</div>
{/* ACP command */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-acp-command"
>
ACP command
</label>
<div
className={cn(
"flex min-h-11 items-center px-3",
PERSONA_FIELD_SHELL_CLASS,
)}
>
<Input
autoCorrect="off"
className={cn(
"h-8 px-0 py-0 leading-6",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-acp-command"
onChange={(event) => onAcpCommandChange(event.target.value)}
value={acpCommand}
/>
</div>
</div>
{/* System prompt override */}
<div className="space-y-1.5">
<label
className="text-sm font-medium text-foreground"
htmlFor="edit-agent-system-prompt"
>
System prompt override
<span className={PERSONA_LABEL_OPTIONAL_CLASS}>Optional</span>
</label>
<div className={PERSONA_FIELD_SHELL_CLASS}>
<Textarea
className={cn(
"min-h-24 resize-y px-3 py-3 leading-5",
PERSONA_FIELD_CONTROL_CLASS,
)}
disabled={disabled}
id="edit-agent-system-prompt"
onChange={(event) => onSystemPromptChange(event.target.value)}
placeholder="Leave blank to send no ACP system prompt"
value={systemPrompt}
/>
</div>
</div>
{/* Env vars */}
<EnvVarsEditor
disabled={disabled}
fileSatisfiedKeys={fileSatisfiedEnvKeys}
helperText="Per-agent env vars. Override the persona's vars on collision."
inheritedFrom={inheritedEnvVars}
inheritedLabel="persona"
onChange={onEnvVarsChange}
requiredKeys={requiredEnvKeys}
value={envVars}
/>
</div>
);
}
File diff suppressed because it is too large Load Diff
+172 -85
View File
@@ -12,6 +12,8 @@ import { cn } from "@/shared/lib/cn";
import { Input } from "@/shared/ui/input";
import { Textarea } from "@/shared/ui/textarea";
import { UserAvatar } from "@/shared/ui/UserAvatar";
import { PersonaDropdownField } from "./PersonaDropdownField";
import type { PersonaDropdownOption } from "./personaDialogPickers";
/**
* Inbound author gate UI for create/edit agent dialogs.
@@ -47,6 +49,12 @@ function formatSearchUserSecondary(user: UserSearchResult) {
return formatPubkey(user.pubkey);
}
const RESPOND_TO_OPTIONS: PersonaDropdownOption[] = [
{ label: "Only me (default)", value: "owner-only" },
{ label: "Anyone", value: "anyone" },
{ label: "Allowlist", value: "allowlist" },
];
export function CreateAgentRespondToField({
mode,
allowlist,
@@ -54,6 +62,7 @@ export function CreateAgentRespondToField({
onAllowlistChange,
ownerPubkey,
disabled,
variant,
}: {
mode: RespondToMode;
allowlist: string[];
@@ -66,6 +75,8 @@ export function CreateAgentRespondToField({
*/
ownerPubkey?: string | null;
disabled?: boolean;
/** When "persona", uses PersonaDropdownField styling to match the persona dialog. */
variant?: "default" | "persona";
}) {
const [query, setQuery] = React.useState("");
const [isDirectEntryOpen, setIsDirectEntryOpen] = React.useState(false);
@@ -101,6 +112,11 @@ export function CreateAgentRespondToField({
setQuery("");
}
function handleAddRawPubkey(pubkey: string) {
onAllowlistChange(mergeAllowlist(allowlist, [pubkey]));
setQuery("");
}
function handleRemove(pubkey: string) {
onAllowlistChange(
allowlist.filter((p) => p.toLowerCase() !== pubkey.toLowerCase()),
@@ -113,28 +129,50 @@ export function CreateAgentRespondToField({
setPasteText("");
}
const isPersonaVariant = variant === "persona";
return (
<div className="space-y-2" data-testid="agent-respond-to">
<label className="text-sm font-medium" htmlFor="agent-respond-to">
<label
className={
isPersonaVariant
? "text-sm font-medium text-foreground"
: "text-sm font-medium"
}
htmlFor="agent-respond-to"
>
Who can talk to this agent
</label>
<select
className="flex h-9 w-full rounded-md border border-input bg-background px-3 py-2 text-sm shadow-xs"
data-testid="agent-respond-to-select"
disabled={disabled}
id="agent-respond-to"
onChange={(e) => onModeChange(e.target.value as RespondToMode)}
value={mode}
>
<option value="owner-only">Owner only (default)</option>
<option value="anyone">Anyone</option>
<option value="allowlist">Allowlist</option>
</select>
<p className="text-xs text-muted-foreground">
Controls which Nostr authors the agent listens to (@mentions, DMs,
thread replies). The agent&apos;s owner can always shut it down with
<span className="font-mono"> !shutdown</span>.
</p>
{isPersonaVariant ? (
<PersonaDropdownField
disabled={disabled}
id="agent-respond-to"
onValueChange={(value) => onModeChange(value as RespondToMode)}
options={RESPOND_TO_OPTIONS}
placeholder="Only me (default)"
value={mode}
/>
) : (
<select
className="flex h-9 w-full rounded-md border border-input bg-background px-3 py-2 text-sm shadow-xs"
data-testid="agent-respond-to-select"
disabled={disabled}
id="agent-respond-to"
onChange={(e) => onModeChange(e.target.value as RespondToMode)}
value={mode}
>
<option value="owner-only">Owner only (default)</option>
<option value="anyone">Anyone</option>
<option value="allowlist">Allowlist</option>
</select>
)}
{!isPersonaVariant ? (
<p className="text-xs text-muted-foreground">
Controls which Nostr authors the agent listens to (@mentions, DMs,
thread replies). The agent&apos;s owner can always shut it down with
<span className="font-mono"> !shutdown</span>.
</p>
) : null}
{mode === "allowlist" ? (
<AllowlistPicker
allowlist={allowlist}
@@ -142,6 +180,7 @@ export function CreateAgentRespondToField({
disabled={disabled}
isDirectEntryOpen={isDirectEntryOpen}
onAddFromPaste={handleAddFromPaste}
onAddRawPubkey={handleAddRawPubkey}
onAddSearchResult={handleAddSearchResult}
onPasteTextChange={setPasteText}
onQueryChange={setQuery}
@@ -159,18 +198,22 @@ export function CreateAgentRespondToField({
}
searchIsLoading={userSearchQuery.isLoading}
searchResults={searchResults}
variant={isPersonaVariant ? "persona" : "default"}
/>
) : null}
</div>
);
}
const HEX_64_RE = /^[0-9a-f]{64}$/i;
function AllowlistPicker({
allowlist,
deferredQuery,
disabled,
isDirectEntryOpen,
onAddFromPaste,
onAddRawPubkey,
onAddSearchResult,
onPasteTextChange,
onQueryChange,
@@ -184,12 +227,14 @@ function AllowlistPicker({
searchError,
searchIsLoading,
searchResults,
variant = "default",
}: {
allowlist: string[];
deferredQuery: string;
disabled?: boolean;
isDirectEntryOpen: boolean;
onAddFromPaste: () => void;
onAddRawPubkey: (pubkey: string) => void;
onAddSearchResult: (user: UserSearchResult) => void;
onPasteTextChange: (value: string) => void;
onQueryChange: (value: string) => void;
@@ -203,28 +248,42 @@ function AllowlistPicker({
searchError: string | null;
searchIsLoading: boolean;
searchResults: UserSearchResult[];
variant?: "default" | "persona";
}) {
const isPersona = variant === "persona";
// Detect if the query is a valid hex pubkey that's not already in the list.
const queryIsHexPubkey =
HEX_64_RE.test(deferredQuery) &&
!allowlist.some((p) => p.toLowerCase() === deferredQuery.toLowerCase());
return (
<div
className="space-y-2.5 rounded-xl border border-border/80 bg-muted/15 p-3"
className={
isPersona
? "space-y-2.5"
: "space-y-2.5 rounded-xl border border-border/80 bg-muted/15 p-3"
}
data-testid="agent-respond-to-allowlist"
>
<div className="flex items-center justify-between gap-2">
<span className="text-sm font-medium">Allowed pubkeys</span>
<span className="rounded-full bg-background px-2 py-1 text-2xs font-medium leading-none text-muted-foreground">
{allowlist.length} selected
</span>
</div>
{ownerPubkey ? (
{!isPersona ? (
<div className="flex items-center justify-between gap-2">
<span className="text-sm font-medium">Allowed pubkeys</span>
<span className="rounded-full bg-background px-2 py-1 text-2xs font-medium leading-none text-muted-foreground">
{allowlist.length} selected
</span>
</div>
) : null}
{!isPersona && ownerPubkey ? (
<p className="text-xs text-muted-foreground">
Owner (<span className="font-mono">{formatPubkey(ownerPubkey)}</span>)
is always implicitly allowed by the harness — no need to add it here.
</p>
) : (
) : !isPersona ? (
<p className="text-xs text-muted-foreground">
The agent&apos;s owner is always implicitly allowed.
</p>
)}
) : null}
<div className="rounded-lg border border-border/80 bg-background">
<div className="flex items-center gap-2 px-2.5 py-2">
<Search className="h-4 w-4 text-muted-foreground" />
@@ -233,7 +292,9 @@ function AllowlistPicker({
data-testid="agent-respond-to-search"
disabled={disabled}
onChange={(event) => onQueryChange(event.target.value)}
placeholder="Search by name or NIP-05."
placeholder={
isPersona ? "Search people" : "Search by name or NIP-05."
}
value={query}
/>
</div>
@@ -299,6 +360,30 @@ function AllowlistPicker({
</button>
))}
</div>
) : queryIsHexPubkey ? (
<button
className="flex w-full items-center justify-between rounded-md px-2.5 py-1.5 text-left transition-colors hover:bg-accent hover:text-accent-foreground"
data-testid="agent-respond-to-add-raw-pubkey"
onClick={() => onAddRawPubkey(deferredQuery.toLowerCase())}
type="button"
>
<div className="flex items-center gap-2 min-w-0">
<UserAvatar
avatarUrl={null}
displayName={formatPubkey(deferredQuery)}
size="xs"
/>
<div className="min-w-0">
<p className="truncate text-sm font-medium leading-5">
{formatPubkey(deferredQuery)}
</p>
<p className="truncate text-xs text-muted-foreground">
Add pubkey directly
</p>
</div>
</div>
<span className="text-xs text-muted-foreground">Add</span>
</button>
) : (
<p className="px-2 py-1 text-sm text-muted-foreground">
No matching users.
@@ -310,66 +395,68 @@ function AllowlistPicker({
{searchError ? (
<p className="text-sm text-destructive">{searchError}</p>
) : null}
<div className="space-y-2">
<button
aria-controls="agent-respond-to-direct-panel"
aria-expanded={isDirectEntryOpen}
className="inline-flex items-center gap-1.5 text-xs font-medium text-muted-foreground transition-colors hover:text-foreground"
data-testid="agent-respond-to-toggle-direct"
onClick={onToggleDirectEntry}
type="button"
>
<ChevronDown
className={cn(
"h-4 w-4 transition-transform",
isDirectEntryOpen && "rotate-180",
)}
/>
<span>Paste pubkeys</span>
</button>
{isDirectEntryOpen ? (
<div
className="space-y-2 rounded-lg border border-dashed border-border/80 bg-background/70 p-2.5"
id="agent-respond-to-direct-panel"
{!isPersona ? (
<div className="space-y-2">
<button
aria-controls="agent-respond-to-direct-panel"
aria-expanded={isDirectEntryOpen}
className="inline-flex items-center gap-1.5 text-xs font-medium text-muted-foreground transition-colors hover:text-foreground"
data-testid="agent-respond-to-toggle-direct"
onClick={onToggleDirectEntry}
type="button"
>
<p className="text-xs text-muted-foreground">
One per line, or comma/space-separated. 64-char lowercase hex only
— npub decoding is not yet supported here.
</p>
<Textarea
className="min-h-20 font-mono text-xs"
data-testid="agent-respond-to-paste"
disabled={disabled}
onChange={(event) => onPasteTextChange(event.target.value)}
placeholder="abcdef0123…"
value={pasteText}
<ChevronDown
className={cn(
"h-4 w-4 transition-transform",
isDirectEntryOpen && "rotate-180",
)}
/>
{pasteInvalid.length > 0 ? (
<p className="text-xs text-destructive">
{pasteInvalid.length} entr
{pasteInvalid.length === 1 ? "y is" : "ies are"} not 64-char hex
and will be ignored.
<span>Paste pubkeys</span>
</button>
{isDirectEntryOpen ? (
<div
className="space-y-2 rounded-lg border border-dashed border-border/80 bg-background/70 p-2.5"
id="agent-respond-to-direct-panel"
>
<p className="text-xs text-muted-foreground">
One per line, or comma/space-separated. 64-char lowercase hex
only — npub decoding is not yet supported here.
</p>
) : null}
<div className="flex items-center justify-between gap-2">
<span className="text-xs text-muted-foreground">
{pasteValidCount > 0
? `${pasteValidCount} valid pubkey${pasteValidCount === 1 ? "" : "s"} ready.`
: "No valid pubkeys yet."}
</span>
<button
className="rounded-md border border-border/80 bg-background px-2.5 py-1 text-xs font-medium transition-colors hover:bg-accent hover:text-accent-foreground disabled:cursor-not-allowed disabled:opacity-50"
data-testid="agent-respond-to-paste-add"
disabled={disabled || pasteValidCount === 0}
onClick={onAddFromPaste}
type="button"
>
Add to allowlist
</button>
<Textarea
className="min-h-20 font-mono text-xs"
data-testid="agent-respond-to-paste"
disabled={disabled}
onChange={(event) => onPasteTextChange(event.target.value)}
placeholder="abcdef0123…"
value={pasteText}
/>
{pasteInvalid.length > 0 ? (
<p className="text-xs text-destructive">
{pasteInvalid.length} entr
{pasteInvalid.length === 1 ? "y is" : "ies are"} not 64-char
hex and will be ignored.
</p>
) : null}
<div className="flex items-center justify-between gap-2">
<span className="text-xs text-muted-foreground">
{pasteValidCount > 0
? `${pasteValidCount} valid pubkey${pasteValidCount === 1 ? "" : "s"} ready.`
: "No valid pubkeys yet."}
</span>
<button
className="rounded-md border border-border/80 bg-background px-2.5 py-1 text-xs font-medium transition-colors hover:bg-accent hover:text-accent-foreground disabled:cursor-not-allowed disabled:opacity-50"
data-testid="agent-respond-to-paste-add"
disabled={disabled || pasteValidCount === 0}
onClick={onAddFromPaste}
type="button"
>
Add to allowlist
</button>
</div>
</div>
</div>
) : null}
</div>
) : null}
</div>
) : null}
</div>
);
}
@@ -7,7 +7,12 @@ import {
requiredCredentialEnvKeys,
isMissingRequiredDropdownField,
} from "./personaDialogPickers.tsx";
import { shouldClearModelForRuntimeChange } from "./personaRuntimeModel.ts";
import {
computeEditAgentFormValidity,
hasMissingRequiredEnvKey,
resolveAgentCommandUpdate,
shouldClearModelForRuntimeChange,
} from "./personaRuntimeModel.ts";
// ── LLM provider field visibility ──────────────────────────────────────────
//
@@ -359,6 +364,311 @@ test("editAgent_runtimeDropdown_pinsHarnessWhenConcreteCatalogRuntimeSelected",
);
});
// ── Custom command as a runtime pin ──────────────────────────────────────────
//
// "Custom command" has no catalog entry (nextRuntime === undefined), so it must
// clear inheritance directly in the handler rather than relying on the
// concrete-runtime branch. Without this, an inheriting persona-linked agent that
// picks "Custom command" keeps inheritHarness=true, the command input stays
// gated behind !inheritHarness, and Save silently follows the inherit path —
// discarding the custom-command intent.
test("editAgent_runtimeDropdown_pinsHarnessWhenCustomCommandSelected", () => {
// Simulate handleRuntimeDropdownChange("custom") for an inherited agent.
let inheritHarness = true; // starts inherited
const NO_RUNTIME_DROPDOWN_VALUE = "__none__";
const nextValue = "custom";
const nextRuntimeId =
nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue;
const nextRuntime = undefined; // "custom" has no catalog entry
// The fixed handler clears inheritance for ANY explicit selection, before the
// concrete-runtime branch (which never runs for a custom command).
inheritHarness = false;
if (nextRuntime?.command) {
// concrete-runtime branch — not taken for custom command
}
assert.equal(nextRuntimeId, "custom");
assert.equal(
inheritHarness,
false,
"selecting 'Custom command' must set inheritHarness=false so the command input is editable and Save takes the pin path",
);
});
test("editAgent_runtimeDropdown_keepsInheritWhenCatalogEntryHasNoCommand", () => {
// A catalog entry whose adapter is missing/not installed has command:null.
// Selecting it must NOT clear inheritance: the concrete-runtime branch can't
// set a command, so pinning would leave agentCommand unchanged on Save while
// the provider/model logic treats the new runtime as effective — an inherited
// Claude agent could persist a Databricks provider while still running Claude.
let inheritHarness = true; // inherited Claude agent
const NO_RUNTIME_DROPDOWN_VALUE = "__none__";
const nextValue = "buzz-agent"; // catalog entry, but adapter missing
const nextRuntimeId =
nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue;
const resolvedRuntimeId = nextRuntimeId || "custom";
const nextRuntime = { id: "buzz-agent", command: null, defaultArgs: [] };
// Mirror the guarded handler: only pin when a command can be supplied.
const isCustomCommand = resolvedRuntimeId === "custom";
if (isCustomCommand || nextRuntime?.command) {
inheritHarness = false;
}
assert.equal(
inheritHarness,
true,
"selecting a command:null catalog entry must keep inheritHarness=true to avoid a mismatched command/provider pair",
);
});
test("editAgent_resolveAgentCommandUpdate_pinsCustomCommandNotInherit", () => {
// After the custom-command selection clears inheritance, the submit path must
// pin the (edited) custom command rather than following the inherit sentinel.
assert.equal(
resolveAgentCommandUpdate({
inheritHarness: false,
agentCommand: "/opt/bin/my-custom-agent",
originalAgentCommand: "", // was inheriting, no command
agentCommandOverride: null,
}),
"/opt/bin/my-custom-agent",
"edited custom command must be persisted as a pin",
);
});
test("editAgent_resolveAgentCommandUpdate_pinsUnchangedPrefillOnInheritTransition", () => {
// Codex scenario: a persona-linked agent that was inheriting selects Custom
// command and Saves the visible prefilled command without editing it. The
// command equals the resolved original, but because the agent had no override
// (agentCommandOverride == null) this is an inherit→pin transition and the
// command MUST be sent as the pin — otherwise the update is omitted and the
// agent keeps inheriting (silent no-op).
assert.equal(
resolveAgentCommandUpdate({
inheritHarness: false,
agentCommand: "goose run",
originalAgentCommand: "goose run", // prefilled, unchanged
agentCommandOverride: null, // was inheriting
}),
"goose run",
"unchanged prefilled command must still be pinned on inherit→pin transition",
);
});
test("editAgent_resolveAgentCommandUpdate_noOpWhenPinnedAndUnchanged", () => {
// An already-pinned agent (had an override) whose command is unchanged must
// stay a no-op so an unrelated edit does not rewrite the command.
assert.equal(
resolveAgentCommandUpdate({
inheritHarness: false,
agentCommand: "claude",
originalAgentCommand: "claude",
agentCommandOverride: "claude", // already pinned
}),
undefined,
"unchanged command on an already-pinned agent must be omitted",
);
});
test("editAgent_resolveAgentCommandUpdate_inheritSentinelOnlyWhenPinToClear", () => {
// Reverting to inherit sends the empty sentinel only when there's a pin to
// clear; a name-only edit on an already-inheriting agent leaves it alone.
assert.equal(
resolveAgentCommandUpdate({
inheritHarness: true,
agentCommand: "claude",
originalAgentCommand: "claude",
agentCommandOverride: "claude", // had a pin → clear it
}),
"",
"reverting to inherit with a prior pin must send the clear sentinel",
);
assert.equal(
resolveAgentCommandUpdate({
inheritHarness: true,
agentCommand: "claude",
originalAgentCommand: "claude",
agentCommandOverride: null, // was already inheriting → no-op
}),
undefined,
"name-only edit on an inheriting agent must leave the command alone",
);
});
test("editAgent_customCommandSelected_autoExpandsAdvancedSection", () => {
// Selecting "Custom command" must reveal the Advanced command input, which is
// otherwise collapsed. Without this the user can Save without ever seeing the
// field, leaving agentCommand equal to the original effective command (so the
// update is omitted) and the custom selection silently no-ops.
let showAdvancedFields = false; // starts collapsed on open
const NO_RUNTIME_DROPDOWN_VALUE = "__none__";
const nextValue = "custom";
const nextRuntimeId =
nextValue === NO_RUNTIME_DROPDOWN_VALUE ? "" : nextValue;
const resolvedRuntimeId = nextRuntimeId || "custom";
const isCustomCommand = resolvedRuntimeId === "custom";
// Mirror the handler's auto-expand branch.
if (isCustomCommand) {
showAdvancedFields = true;
}
assert.equal(
showAdvancedFields,
true,
"selecting 'Custom command' must auto-expand Advanced so the command input is visible",
);
});
test("editAgent_missingRequiredEnvKey_autoExpandsAdvancedOnTransition", () => {
// Codex P2: when a provider change makes a credential newly required, the
// EnvVarsEditor lives inside the collapsed Advanced section, so the amber
// required row would stay unmounted (invisible) while Save is disabled. The
// effect auto-expands Advanced on the missing→present-requirement transition.
let showAdvancedFields = false; // collapsed by default on open
let previousMissing = false;
// Mirror the effect's transition guard.
function applyMissingEffect(requiredEnvKeyMissing) {
if (requiredEnvKeyMissing && !previousMissing) {
showAdvancedFields = true;
}
previousMissing = requiredEnvKeyMissing;
}
// Initial render: buzz-agent with no provider — nothing required yet.
applyMissingEffect(
hasMissingRequiredEnvKey(requiredCredentialEnvKeys("buzz-agent", ""), {}),
);
assert.equal(
showAdvancedFields,
false,
"Advanced stays collapsed while no credential is required",
);
// User picks anthropic → ANTHROPIC_API_KEY becomes required and is unset.
applyMissingEffect(
hasMissingRequiredEnvKey(
requiredCredentialEnvKeys("buzz-agent", "anthropic"),
{},
),
);
assert.equal(
showAdvancedFields,
true,
"Advanced auto-expands when a required credential is newly missing",
);
// User fills the key, then collapses Advanced manually — no re-expand.
showAdvancedFields = false;
applyMissingEffect(
hasMissingRequiredEnvKey(
requiredCredentialEnvKeys("buzz-agent", "anthropic"),
{ ANTHROPIC_API_KEY: "sk-ant-test" },
),
);
assert.equal(
showAdvancedFields,
false,
"Advanced does not re-expand once the required credential is filled",
);
});
test("editAgent_missingRequiredEnvKey_blocksSaveViaValidity", () => {
// The block-save gate is folded into computeEditAgentFormValidity so the
// Save button disables when a runtime/provider-required credential is unset.
const base = {
name: "My Agent",
parallelism: "",
turnTimeoutSeconds: "",
agentAcpCommand: "",
acpCommand: "",
respondTo: "all",
respondToAllowlistLength: 0,
selectedRuntimeId: "buzz-agent",
inheritHarness: false,
agentCommand: "buzz-agent",
requiredEnvKeyMissing: false,
};
assert.equal(
computeEditAgentFormValidity({ ...base, requiredEnvKeyMissing: true }),
false,
"Save must be blocked when a required credential key is missing",
);
assert.equal(
computeEditAgentFormValidity({ ...base, requiredEnvKeyMissing: false }),
true,
"Save must be allowed once the required credential key is present",
);
});
test("editAgent_customCommandPinned_blocksSaveWhenCommandEmpty", () => {
// A pinned custom command with an empty command field must block Save — the
// backend would spawn a runtime with no command otherwise. Exercises the real
// computeEditAgentFormValidity helper.
const base = {
name: "My Agent",
parallelism: "",
turnTimeoutSeconds: "",
agentAcpCommand: "",
acpCommand: "",
respondTo: "all",
respondToAllowlistLength: 0,
selectedRuntimeId: "custom",
inheritHarness: false,
agentCommand: "",
requiredEnvKeyMissing: false,
};
assert.equal(
computeEditAgentFormValidity(base),
false,
"empty pinned custom command must block Save",
);
assert.equal(
computeEditAgentFormValidity({ ...base, agentCommand: "/opt/bin/agent" }),
true,
"non-empty custom command must allow Save",
);
// An inherited (not pinned) selection is never gated by this rule, even with
// an empty command — the inherit path resolves the command server-side.
assert.equal(
computeEditAgentFormValidity({ ...base, inheritHarness: true }),
true,
"inheriting agents must not be gated by the custom-command rule",
);
// The other validity gates still apply through the helper.
assert.equal(
computeEditAgentFormValidity({
...base,
agentCommand: "/opt/bin/agent",
name: " ",
}),
false,
"blank name must block Save",
);
assert.equal(
computeEditAgentFormValidity({
...base,
agentCommand: "/opt/bin/agent",
respondTo: "allowlist",
respondToAllowlistLength: 0,
}),
false,
"empty allowlist must block Save",
);
});
test("editAgent_inheritedAgentRuntimeSwitch_producesConsistentCommandProviderPair", () => {
// Bad path before fix: inheritHarness stays true, so agentCommandUpdate is
// undefined (agent still inherits Claude), but provider="databricks_v2" persists.
@@ -1078,15 +1388,14 @@ test("requiredCredentialEnvKeys: custom/unknown runtime → empty", () => {
assert.deepEqual(keys, []);
});
// ── Block-save gate: hasRequiredEnvKeyMissing logic ────────────────────────
// ── Block-save gate: hasMissingRequiredEnvKey logic ────────────────────────
//
// The EditAgentDialog computes:
// hasRequiredEnvKeyMissing = requiredEnvKeys.some(k => (envVars[k] ?? "").length === 0)
// and folds it into canSubmit. These tests exercise that predicate directly.
// requiredEnvKeyMissing = hasMissingRequiredEnvKey(requiredEnvKeys, envVars)
// and folds it into canSubmit (via computeEditAgentFormValidity). These tests
// exercise the exported predicate directly.
function hasRequiredEnvKeyMissing(requiredKeys, envVars) {
return requiredKeys.some((key) => (envVars[key] ?? "").length === 0);
}
const hasRequiredEnvKeyMissing = hasMissingRequiredEnvKey;
test("blockSave_buzzAgentAnthropicMissingKey_blocked", () => {
// Will's exact case: buzz-agent / anthropic / opus / no ANTHROPIC_API_KEY
@@ -1,7 +1,11 @@
import assert from "node:assert/strict";
import test from "node:test";
import { shouldClearModelForRuntimeChange } from "./personaRuntimeModel.ts";
import {
resolveInheritedRuntimeSubmission,
resolveRuntimeProviderCapability,
shouldClearModelForRuntimeChange,
} from "./personaRuntimeModel.ts";
test("shouldClearModelForRuntimeChange preserves model for first runtime selection", () => {
assert.equal(shouldClearModelForRuntimeChange("", "goose"), false);
@@ -18,3 +22,188 @@ test("shouldClearModelForRuntimeChange clears model when runtime is removed", ()
test("shouldClearModelForRuntimeChange keeps model for unchanged runtime", () => {
assert.equal(shouldClearModelForRuntimeChange("goose", "goose"), false);
});
test("resolveInheritedRuntimeSubmission passes through local edit state when not inheriting", () => {
const result = resolveInheritedRuntimeSubmission({
inheritHarness: false,
agentWasHarnessPinned: false,
provider: "databricks",
personaProvider: "anthropic",
model: "",
personaModel: "claude-sonnet",
envVars: { FOO: "bar" },
personaEnvVars: { ANTHROPIC_API_KEY: "sk-persona" },
});
assert.equal(result.provider, "databricks");
assert.deepEqual(result.envVars, { FOO: "bar" });
// Not inheriting → persona model is never substituted; empty local → null.
assert.equal(result.model, null);
});
test("resolveInheritedRuntimeSubmission normalizes an empty local provider to null when not inheriting", () => {
const result = resolveInheritedRuntimeSubmission({
inheritHarness: false,
agentWasHarnessPinned: true,
provider: " ",
personaProvider: "anthropic",
model: "",
personaModel: "claude-sonnet",
envVars: {},
personaEnvVars: {},
});
assert.equal(result.provider, null);
});
test("resolveInheritedRuntimeSubmission persists the persona provider + layered env on the inherit-transition from a harness pin", () => {
// The core fix: a previously harness-pinned agent has a cleared provider and
// no credential locally, but on the inherit-transition the persona snapshot
// must be persisted so the record (which spawn reads) carries the provider +
// credential. Requires agentWasHarnessPinned to distinguish this from a
// steady-state inherit.
const result = resolveInheritedRuntimeSubmission({
inheritHarness: true,
agentWasHarnessPinned: true,
provider: "",
personaProvider: "anthropic",
model: "",
personaModel: "claude-sonnet",
envVars: {},
personaEnvVars: { ANTHROPIC_API_KEY: "sk-persona" },
});
assert.equal(result.provider, "anthropic");
assert.deepEqual(result.envVars, { ANTHROPIC_API_KEY: "sk-persona" });
// Empty local model on the transition inherits the persona model so a
// provider-backed runtime isn't saved model-less (readiness requires one).
assert.equal(result.model, "claude-sonnet");
});
test("resolveInheritedRuntimeSubmission layers the agent's own env over the persona's on the inherit-transition", () => {
const result = resolveInheritedRuntimeSubmission({
inheritHarness: true,
agentWasHarnessPinned: true,
provider: "",
personaProvider: "anthropic",
model: "",
personaModel: "claude-sonnet",
envVars: { ANTHROPIC_API_KEY: "sk-agent", EXTRA: "1" },
personaEnvVars: { ANTHROPIC_API_KEY: "sk-persona" },
});
// Agent layer wins on key collision, mirroring spawn-time layering.
assert.deepEqual(result.envVars, {
ANTHROPIC_API_KEY: "sk-agent",
EXTRA: "1",
});
});
test("resolveInheritedRuntimeSubmission preserves a user-edited provider + env while inheriting", () => {
// Regression: an already-inheriting agent (e.g. an Anthropic persona) that
// the user re-points to Databricks with its own DATABRICKS_HOST must persist
// that deliberate edit verbatim — NOT get overwritten with the persona's
// provider/env. The provider field is user-editable even while inheriting.
const result = resolveInheritedRuntimeSubmission({
inheritHarness: true,
agentWasHarnessPinned: false,
provider: "databricks",
personaProvider: "anthropic",
model: "",
personaModel: "claude-sonnet",
envVars: { DATABRICKS_HOST: "https://dbc-x.cloud.databricks.com" },
personaEnvVars: { ANTHROPIC_API_KEY: "sk-persona" },
});
assert.equal(result.provider, "databricks");
assert.deepEqual(result.envVars, {
DATABRICKS_HOST: "https://dbc-x.cloud.databricks.com",
});
// Not the transition branch → persona model is NOT substituted.
assert.equal(result.model, null);
});
test("resolveInheritedRuntimeSubmission clears an already-inheriting agent's provider override when the user picks Default", () => {
// Regression: an already-inheriting agent had a saved provider override
// (databricks). The user picks the "Default" option → empty local provider.
// Because the agent was NOT harness-pinned at open, this is a deliberate
// clear, not the inherit-transition — persist null (runtime default), do NOT
// resurrect the persona provider.
const result = resolveInheritedRuntimeSubmission({
inheritHarness: true,
agentWasHarnessPinned: false,
provider: "",
personaProvider: "anthropic",
model: "",
personaModel: "claude-sonnet",
envVars: {},
personaEnvVars: { ANTHROPIC_API_KEY: "sk-persona" },
});
assert.equal(result.provider, null);
assert.deepEqual(result.envVars, {});
});
test("resolveInheritedRuntimeSubmission normalizes a whitespace-only local provider on the inherit-transition (unset persona)", () => {
// The inherit-transition branch (was harness-pinned, now inheriting, empty
// local provider); an unset persona provider normalizes to null.
const result = resolveInheritedRuntimeSubmission({
inheritHarness: true,
agentWasHarnessPinned: true,
provider: " ",
personaProvider: "",
model: "",
personaModel: null,
envVars: {},
personaEnvVars: {},
});
assert.equal(result.provider, null);
assert.deepEqual(result.envVars, {});
});
test("resolveInheritedRuntimeSubmission keeps a deliberate local model on the inherit-transition", () => {
// A non-empty local model is an explicit pick and wins over the persona
// model even on the transition branch.
const result = resolveInheritedRuntimeSubmission({
inheritHarness: true,
agentWasHarnessPinned: true,
provider: "",
personaProvider: "anthropic",
model: "claude-opus",
personaModel: "claude-sonnet",
envVars: {},
personaEnvVars: { ANTHROPIC_API_KEY: "sk-persona" },
});
assert.equal(result.model, "claude-opus");
});
test("resolveInheritedRuntimeSubmission yields a null model on the inherit-transition when the persona has none", () => {
const result = resolveInheritedRuntimeSubmission({
inheritHarness: true,
agentWasHarnessPinned: true,
provider: "",
personaProvider: "anthropic",
model: "",
personaModel: null,
envVars: {},
personaEnvVars: { ANTHROPIC_API_KEY: "sk-persona" },
});
assert.equal(result.model, null);
});
test("resolveRuntimeProviderCapability classifies provider-capable runtimes as capable", () => {
assert.equal(resolveRuntimeProviderCapability("buzz-agent", true), "capable");
assert.equal(resolveRuntimeProviderCapability("goose", true), "capable");
});
test("resolveRuntimeProviderCapability classifies known CLI-login runtimes as locked before the catalog loads", () => {
// The core fix: a not-yet-loaded catalog must not force these to "unknown".
assert.equal(resolveRuntimeProviderCapability("claude", false), "locked");
assert.equal(resolveRuntimeProviderCapability("codex", false), "locked");
assert.equal(resolveRuntimeProviderCapability(" claude ", false), "locked");
});
test("resolveRuntimeProviderCapability leaves genuinely unknown/custom runtimes as unknown", () => {
// Preserves the tri-state's "omit rather than destructively write" behavior
// for ids we can't statically classify (custom command, empty, unknown).
assert.equal(resolveRuntimeProviderCapability("custom", false), "unknown");
assert.equal(resolveRuntimeProviderCapability("", false), "unknown");
assert.equal(
resolveRuntimeProviderCapability("some-vendor-cli", false),
"unknown",
);
});
@@ -1,3 +1,44 @@
/** Runtime provider-capability tri-state used by the submit path. */
export type ProviderRuntimeCapability = "capable" | "locked" | "unknown";
/**
* Classify a runtime id's provider-selection capability as a tri-state,
* independent of whether the runtime catalog has loaded yet.
*
* The submit path keys its provider write on this: "capable" persists the
* provider, "locked" clears it, and "unknown" OMITS the field so a transient
* loading/error state (or a genuinely unknown/custom command) never becomes a
* destructive write.
*
* Before the catalog loads, `prospectiveRuntimeId` can already be a known
* persona runtime string (e.g. `buzz-agent`). A catalog lookup then returns
* `undefined`, which would misclassify a provider-backed runtime as "unknown"
* and omit the provider while still clearing the command override / writing env
* — leaving the record inheriting a provider-backed runtime with a null
* provider. To avoid that, we resolve capability STATICALLY for known ids:
*
* - buzz-agent / goose → "capable" (`isProviderCapable`, id-based).
* - claude / codex → "locked" (CLI-login runtimes; no LLM provider selection).
* - anything else (custom, empty, genuinely unknown) → "unknown".
*
* `isProviderCapable` is the caller-supplied {@link
* runtimeSupportsLlmProviderSelection} result, kept as the single source of
* truth for the capable set rather than re-hardcoding it here.
*/
export function resolveRuntimeProviderCapability(
runtimeId: string,
isProviderCapable: boolean,
): ProviderRuntimeCapability {
if (isProviderCapable) {
return "capable";
}
const id = runtimeId.trim();
if (id === "claude" || id === "codex") {
return "locked";
}
return "unknown";
}
export function shouldClearModelForRuntimeChange(
previousRuntime: string,
nextRuntime: string,
@@ -7,3 +48,228 @@ export function shouldClearModelForRuntimeChange(
return previous.length > 0 && previous !== next;
}
/**
* Resolve the `agentCommand` field to send on Save for the harness pin.
*
* The backend treats an empty string as the "inherit from persona" sentinel
* (clears the override) and any concrete command as an explicit pin.
* `undefined` means "leave the record's command alone".
*
* - Inheriting: send the sentinel only if there's a pin to clear, so a
* name-only edit leaves the record untouched.
* - Pinning: normally send the command only when it diverges from the resolved
* value the dialog opened with, so an unchanged save stays a no-op. The
* exception is an inherit→pin transition (no override at open): the command
* field is prefilled with the resolved effective command, so accepting it
* as-is leaves it equal to `agentCommand` — without forcing the pin the
* update would be omitted and the agent would keep inheriting. An empty
* command never reaches the force branch (the caller blocks Save for an empty
* pinned custom command; catalog runtimes always set a concrete command).
*/
export function resolveAgentCommandUpdate(input: {
inheritHarness: boolean;
/** The command currently in the (possibly prefilled) input. */
agentCommand: string;
/** The resolved effective command the dialog opened with. */
originalAgentCommand: string;
/** The persisted override, or null when the agent was inheriting. */
agentCommandOverride: string | null;
}): string | undefined {
if (input.inheritHarness) {
return input.agentCommandOverride != null ? "" : undefined;
}
const pinnedCommand = input.agentCommand.trim();
const pinningFromInherit = input.agentCommandOverride == null;
if (
pinnedCommand !== input.originalAgentCommand ||
(pinningFromInherit && pinnedCommand.length > 0)
) {
return pinnedCommand;
}
return undefined;
}
/**
* Whether any of the runtime/provider-required credential keys is unset.
*
* A key counts as missing when its env value is absent or an empty string
* (matching {@link EnvVarsEditor}'s own `isMissing` rendering). The
* `requiredEnvKeys` list is already filtered to keys the dialog can fix —
* CLI-login runtimes (claude/codex) and keys satisfied by the runtime file
* config contribute no entries, so this never blocks on out-of-band auth.
*/
export function hasMissingRequiredEnvKey(
requiredEnvKeys: string[],
envVars: Record<string, string>,
): boolean {
return requiredEnvKeys.some((key) => (envVars[key] ?? "").length === 0);
}
/**
* Resolve the provider and env-vars to PERSIST on Save.
*
* The spawn path reads ONLY the record snapshot (`record.provider`/
* `record.env_vars`), never the live persona, and the record is authoritative:
* `env_vars` is the complete pinned map (persona env snapshotted at create,
* already merged UNDER the agent's own overrides), and the provider field is
* user-editable in the dialog even while inheriting. So the local edit state IS
* the record's own value and is honored verbatim in the normal case.
*
* The ONE exception is the inherit-TRANSITION-from-a-harness-pin: a previously
* harness-pinned agent (e.g. Claude — `agent.agentCommandOverride != null` at
* dialog open) has its `provider` cleared and carries no persona credential,
* then the user checks "Inherit runtime from persona" for a provider-backed
* persona (e.g. buzz-agent/Anthropic). Persisting the local (empty) provider +
* credential-less env would save an agent that fails readiness on next start.
* Only in that case — inheriting AND the local provider is empty AND the agent
* was harness-pinned at open — do we substitute the persona snapshot: the
* persona's provider and the persona-layered env (`{ ...personaEnv,
* ...agentEnv }`, agent layer wins to mirror spawn-time layering), matching
* create-time record pinning.
*
* A NON-empty local provider while inheriting (e.g. an Anthropic-persona agent
* the user re-points to Databricks) is a deliberate edit and passes through
* unchanged — we never overwrite it with the persona provider.
*
* MODEL follows the same transition rule. buzz-agent/goose readiness requires a
* model, but a Claude-pinned agent's record often carries no model (Claude
* resolves its own). On the inherit-transition to a provider-backed persona with
* a set `persona.model`, persisting the empty local model would save an agent
* that inherits the provider + credentials but no model and fails readiness on
* next start — so we substitute `personaModel` in that same case. A non-empty
* local model (deliberate pick) always passes through; an empty local model in
* steady state stays empty (runtime default), same authoritative logic.
*
* An EMPTY local provider while inheriting on an agent that was ALREADY
* inheriting at open (`agentWasHarnessPinned` false) is ALSO authoritative: the
* user either never set one or deliberately picked the "Default" option to
* clear a saved override, so we persist `null` (runtime default) rather than
* resurrect the persona provider — otherwise the Default option could never
* actually clear an inherited agent's provider override.
*
* The result is the SAME effective value the required-credential gate
* validates, so the gate, the submitted record, and the spawn snapshot agree.
* Provider is normalized: trimmed, empty → `null`.
*/
export function resolveInheritedRuntimeSubmission(input: {
inheritHarness: boolean;
/**
* Whether the agent was harness-pinned (`agentCommandOverride != null`) at
* dialog open. Distinguishes the inherit-transition (was pinned, now
* inheriting) from steady-state inherit (was already inheriting), so an empty
* provider in steady state clears the override instead of resurrecting the
* persona provider.
*/
agentWasHarnessPinned: boolean;
/** Local provider edit state (from the agent record, user-editable). */
provider: string;
/** The linked persona's provider, or empty when none/unset. */
personaProvider: string;
/** Local model edit state (from the agent record, user-editable). */
model: string;
/** The linked persona's model, or empty/null when none/unset. */
personaModel: string | null;
/** Local env-vars edit state (the agent's own layer). */
envVars: Record<string, string>;
/** The persona's env vars, layered under the agent's own on transition. */
personaEnvVars: Record<string, string>;
}): {
provider: string | null;
model: string | null;
envVars: Record<string, string>;
} {
const localProvider = input.provider.trim();
const localModel = input.model.trim();
// Substitute the persona snapshot ONLY on the true inherit-transition: the
// agent was harness-pinned at open, is now inheriting, and has an empty local
// provider. Otherwise the local edit state is authoritative — a non-empty
// provider is a deliberate pick, and an empty provider on an already-
// inheriting agent is a deliberate clear (Default) — and passes through.
if (
input.inheritHarness &&
input.agentWasHarnessPinned &&
localProvider.length === 0
) {
return {
provider: input.personaProvider.trim() || null,
// Fill an empty local model from the persona so a provider-backed runtime
// isn't saved model-less; a deliberate local model still wins.
model: localModel || input.personaModel?.trim() || null,
envVars: { ...input.personaEnvVars, ...input.envVars },
};
}
return {
provider: localProvider || null,
model: localModel || null,
envVars: input.envVars,
};
}
/** Inputs for {@link computeEditAgentFormValidity} — all pre-derived primitives. */
export interface EditAgentFormValidityInput {
name: string;
parallelism: string;
turnTimeoutSeconds: string;
/** The command already persisted on the agent (empty when inheriting). */
agentAcpCommand: string;
acpCommand: string;
respondTo: string;
respondToAllowlistLength: number;
selectedRuntimeId: string;
inheritHarness: boolean;
agentCommand: string;
/**
* Whether a runtime/provider-required credential key is still unset. When
* true the Save button is blocked — the agent would otherwise persist with a
* missing credential and crash-loop on next start. See
* {@link hasMissingRequiredEnvKey}.
*/
requiredEnvKeyMissing: boolean;
}
/**
* Pure field-validity check for the Edit Agent dialog's Save button.
*
* Mirrors the harness/backend validation so the user sees a disabled button
* instead of a round-tripped error:
* - name is required;
* - parallelism / timeout must be blank or parseable integers;
* - a previously-set ACP command cannot be cleared to empty (spawn failure);
* - allowlist respond-to mode needs at least one entry;
* - a pinned "Custom command" runtime (custom selection with inheritance
* cleared) must carry a concrete command — an empty command would spawn a
* runtime with no command.
* - a runtime/provider-required credential key must be present — persisting
* with a missing key would crash-loop the agent on next start.
*/
export function computeEditAgentFormValidity(
input: EditAgentFormValidityInput,
): boolean {
const parallelismValid =
input.parallelism.trim() === "" ||
!Number.isNaN(Number.parseInt(input.parallelism, 10));
const timeoutValid =
input.turnTimeoutSeconds.trim() === "" ||
!Number.isNaN(Number.parseInt(input.turnTimeoutSeconds, 10));
const acpCommandValid = !(
input.agentAcpCommand && input.acpCommand.trim() === ""
);
const respondToValid =
input.respondTo !== "allowlist" || input.respondToAllowlistLength > 0;
const customCommandValid = !(
input.selectedRuntimeId === "custom" &&
!input.inheritHarness &&
input.agentCommand.trim() === ""
);
return (
input.name.trim().length > 0 &&
parallelismValid &&
timeoutValid &&
acpCommandValid &&
respondToValid &&
customCommandValid &&
!input.requiredEnvKeyMissing
);
}
@@ -0,0 +1,109 @@
import * as React from "react";
import { useRuntimeFileConfigQuery } from "@/features/agents/hooks";
import {
requiredCredentialEnvKeys,
runtimeSupportsLlmProviderSelection,
} from "./personaDialogPickers";
import { hasMissingRequiredEnvKey } from "./personaRuntimeModel";
/** Derived required-credential state for the Edit Agent dialog's Advanced section. */
export interface RequiredCredentialState {
/** Required env keys still unset and not satisfied by the runtime file config. */
requiredEnvKeys: string[];
/** Required keys already satisfied by the runtime file config (shown as info rows). */
fileSatisfiedEnvKeys: string[];
/** Whether any required env key is still missing (blocks Save). */
requiredEnvKeyMissing: boolean;
}
/**
* Compute the runtime/provider-required credential state and keep the Advanced
* section's required-credential row visible when a key is newly missing.
*
* All keys are derived from the PROSPECTIVE post-submit runtime (not the
* current dropdown). On an inherit transition (claude→buzz-agent or the
* reverse) the current dropdown would suppress the provider to "" and falsely
* unblock Save; using the prospective id keeps the gate honest about what will
* actually be saved.
*
* The `EnvVarsEditor` (and its amber required-key row) lives inside the
* collapsed-by-default Advanced section, so a provider change that newly
* requires a key would otherwise leave the row unmounted while Save is disabled
* with no on-screen reason. This hook auto-expands Advanced on the
* missing→present-requirement transition, so the user can still collapse it
* again once the key is filled.
*/
export function useRequiredCredentialState(params: {
open: boolean;
prospectiveRuntimeId: string;
provider: string;
envVars: Record<string, string>;
setShowAdvancedFields: React.Dispatch<React.SetStateAction<boolean>>;
}): RequiredCredentialState {
const {
open,
prospectiveRuntimeId,
provider,
envVars,
setShowAdvancedFields,
} = params;
const providerForRequiredKeys = runtimeSupportsLlmProviderSelection(
prospectiveRuntimeId,
)
? provider
: "";
const { data: runtimeFileConfig } = useRuntimeFileConfigQuery(
prospectiveRuntimeId,
{ enabled: open },
);
const fileSatisfiedEnvKeys = React.useMemo(() => {
if (!runtimeFileConfig) return [] as string[];
return requiredCredentialEnvKeys(
prospectiveRuntimeId,
providerForRequiredKeys,
).filter(
(key) =>
(envVars[key] ?? "").length === 0 &&
runtimeFileConfig.satisfiedEnvKeys.includes(key),
);
}, [
runtimeFileConfig,
prospectiveRuntimeId,
providerForRequiredKeys,
envVars,
]);
const requiredEnvKeys = React.useMemo(
() =>
requiredCredentialEnvKeys(
prospectiveRuntimeId,
providerForRequiredKeys,
).filter((key) => !fileSatisfiedEnvKeys.includes(key)),
[prospectiveRuntimeId, providerForRequiredKeys, fileSatisfiedEnvKeys],
);
const requiredEnvKeyMissing = React.useMemo(
() => hasMissingRequiredEnvKey(requiredEnvKeys, envVars),
[requiredEnvKeys, envVars],
);
// Auto-expand Advanced on the missing→present-requirement transition only.
const previousMissing = React.useRef(false);
React.useEffect(() => {
if (!open) {
previousMissing.current = false;
return;
}
if (requiredEnvKeyMissing && !previousMissing.current) {
setShowAdvancedFields(true);
}
previousMissing.current = requiredEnvKeyMissing;
}, [open, requiredEnvKeyMissing, setShowAdvancedFields]);
return { requiredEnvKeys, fileSatisfiedEnvKeys, requiredEnvKeyMissing };
}
+7
View File
@@ -673,6 +673,13 @@ export type UpdateManagedAgentInput = {
relayUrl?: string;
acpCommand?: string;
agentCommand?: string;
/**
* True when `agentCommand` is a runtime/Custom command the user deliberately
* picked (the dialog is not inheriting). Preserves a pin that maps to the
* linked persona's own runtime instead of letting the backend drop it back to
* inherit. Ignored when `agentCommand` is absent or the inherit sentinel.
*/
harnessOverride?: boolean;
agentArgs?: string[];
mcpCommand?: string;
/** Absent = don't touch. Present = set the mode. */
@@ -70,8 +70,10 @@ async function openEditDialog(
await page.getByTestId("user-profile-edit-agent").click();
// Wait for the Edit dialog's provider field (goose runtime supports it).
await expect(page.locator("#agent-provider")).toBeVisible({
// Wait for the Edit dialog's LLM provider field (goose runtime supports it).
// The Edit dialog renders provider selection via PersonaDropdownField, whose
// trigger button carries this id (the Create dialog uses #agent-provider).
await expect(page.locator("#edit-agent-llm-provider")).toBeVisible({
timeout: 10_000,
});
}