fix(desktop): allow editing built-in agents (#1928)

Signed-off-by: Wes <wesbillman@users.noreply.github.com>
Co-authored-by: Pinky <44b8e82baa6e0e254e0208d68f335c283c94e7b78dd1fa10d5a49d3f13dd0435@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
Wes
2026-07-15 18:49:17 -07:00
committed by GitHub
co-authored by Pinky
parent 32957692eb
commit 202201f3aa
10 changed files with 77 additions and 107 deletions
+3 -1
View File
@@ -104,7 +104,9 @@ commit. **Pre-push hooks** run clippy (workspace + Tauri) and fast unit tests
in parallel (Rust, desktop JS, Tauri Rust, mobile Flutter) — no overlap with
pre-commit. Builds are CI-only. Run `just fix-all` to auto-fix all formatting
in one shot. Run `just ci` for the full local gate. Run `just hooks` to
re-install hooks after env changes.
re-install hooks after env changes. Before agents run Git or hooks, activate the
repo's Hermit environment (`. ./bin/activate-hermit`); do not rewrite hook
commands to compensate for an unconfigured shell `PATH`.
Additional rules:
- No `unsafe` code
@@ -175,10 +175,6 @@ pub async fn update_persona(
.find(|record| record.id == input.id)
.ok_or_else(|| format!("agent {} not found", input.id))?;
if persona.is_builtin {
return Err("Built-in agents cannot be edited.".to_string());
}
// Track what changed so we can propagate to linked agent records.
let avatar_changed = persona.avatar_url != avatar_url;
let name_changed = persona.display_name != display_name;
@@ -159,27 +159,8 @@ fn merge_personas(mut stored: Vec<AgentDefinition>, now: &str) -> (Vec<AgentDefi
for built_in in built_in_persona_records(now) {
if let Some(existing) = stored.iter_mut().find(|record| record.id == built_in.id) {
let created_at = existing.created_at.clone();
let updated_at = existing.updated_at.clone();
let is_active = existing.is_active;
// Built-in fields are canonical — user overrides on runtime/model are
// intentionally not preserved across restarts. Users who want a custom
// model or runtime should clone the built-in as a custom persona.
if existing.display_name != built_in.display_name
|| existing.avatar_url != built_in.avatar_url
|| existing.system_prompt != built_in.system_prompt
|| existing.name_pool != built_in.name_pool
|| existing.env_vars != built_in.env_vars
|| existing.runtime != built_in.runtime
|| existing.model != built_in.model
|| !existing.is_builtin
{
*existing = AgentDefinition {
created_at,
updated_at,
is_active,
..built_in
};
if !existing.is_builtin {
existing.is_builtin = true;
changed = true;
}
} else {
@@ -65,12 +65,33 @@ fn merge_personas_preserves_custom_records() {
}
#[test]
fn merge_personas_restores_builtin_defaults() {
fn merge_personas_preserves_builtin_edits() {
let mut edited_builtin = custom_persona("builtin:fizz", "My Fizz");
edited_builtin.is_builtin = true;
edited_builtin.is_active = true;
let original_created_at = edited_builtin.created_at.clone();
let original_updated_at = edited_builtin.updated_at.clone();
edited_builtin.system_prompt = "User-edited instructions".to_string();
edited_builtin.name_pool = vec!["User-edited name".to_string()];
edited_builtin.env_vars =
std::collections::BTreeMap::from([("USER_SETTING".to_string(), "value".to_string())]);
let (records, changed) = merge_personas(vec![edited_builtin.clone()], "2026-03-19T00:00:00Z");
assert!(changed); // The remaining seeded built-ins are added.
let fizz = records
.iter()
.find(|record| record.id == "builtin:fizz")
.expect("fizz built-in should exist");
assert_eq!(fizz.display_name, edited_builtin.display_name);
assert_eq!(fizz.system_prompt, edited_builtin.system_prompt);
assert_eq!(fizz.name_pool, edited_builtin.name_pool);
assert_eq!(fizz.env_vars, edited_builtin.env_vars);
assert_eq!(fizz.is_active, edited_builtin.is_active);
}
#[test]
fn merge_personas_restores_builtin_marker_without_resetting_edits() {
let mut edited_builtin = custom_persona("builtin:fizz", "My Fizz");
edited_builtin.is_builtin = false;
let (records, changed) = merge_personas(vec![edited_builtin], "2026-03-19T00:00:00Z");
@@ -79,71 +100,8 @@ fn merge_personas_restores_builtin_defaults() {
.iter()
.find(|record| record.id == "builtin:fizz")
.expect("fizz built-in should exist");
let canonical = BUILT_IN_PERSONAS
.iter()
.find(|persona| persona.id == "builtin:fizz")
.expect("fizz built-in definition should exist");
assert_eq!(fizz.display_name, canonical.display_name);
assert_eq!(fizz.avatar_url.as_deref(), canonical.avatar_url,);
assert_eq!(fizz.created_at, original_created_at);
assert_eq!(fizz.updated_at, original_updated_at);
assert!(fizz.is_active);
}
#[test]
fn merge_personas_restores_builtin_env_vars() {
// A hand-edited built-in record with stray env vars should be reset to
// the canonical (empty) env on merge. Built-ins are intended immutable —
// if a user wants per-persona credentials, they create or duplicate to a
// custom persona.
let mut tampered = custom_persona("builtin:fizz", "Fizz");
tampered.is_builtin = true;
tampered.avatar_url = None;
tampered.is_active = true;
tampered.env_vars =
std::collections::BTreeMap::from([("ANTHROPIC_API_KEY".to_string(), "leaked".to_string())]);
let (records, changed) = merge_personas(vec![tampered], "2026-03-19T00:00:00Z");
assert!(changed);
let fizz = records
.iter()
.find(|record| record.id == "builtin:fizz")
.expect("fizz built-in should exist");
// Built-in persona definitions have no `env_vars` field — they are
// always empty. The merge reset should clear the tampered key entirely.
assert!(
fizz.env_vars.is_empty(),
"expected empty, got {:?}",
fizz.env_vars
);
}
#[test]
fn merge_personas_restores_builtin_name_pool_and_preserves_is_active() {
let mut fizz = custom_persona("builtin:fizz", "Fizz");
fizz.is_builtin = true;
fizz.avatar_url = None;
fizz.is_active = true;
fizz.name_pool = vec!["Definitely Not Fizz".to_string()];
let (records, changed) = merge_personas(vec![fizz], "2026-03-19T00:00:00Z");
assert!(changed);
let fizz = records
.iter()
.find(|record| record.id == "builtin:fizz")
.expect("fizz built-in should exist");
let expected_name_pool = BUILT_IN_PERSONAS
.iter()
.find(|persona| persona.id == "builtin:fizz")
.expect("fizz built-in definition should exist")
.name_pool
.iter()
.map(|name| (*name).to_string())
.collect::<Vec<_>>();
assert_eq!(fizz.name_pool, expected_name_pool);
assert!(fizz.is_active);
assert!(fizz.is_builtin);
assert_eq!(fizz.display_name, "My Fizz");
}
#[test]
@@ -97,7 +97,7 @@ test("allows agents to update only personal, editable profiles", () => {
);
assert.equal(
requestTargetsEditablePersona({ isBuiltIn: true, sourceTeam: null }),
false,
true,
);
assert.equal(
requestTargetsEditablePersona({ isBuiltIn: false, sourceTeam: "team" }),
@@ -137,7 +137,7 @@ export function parseAgentManagementRequest(
export function requestTargetsEditablePersona(
persona: AgentPersona | undefined,
): persona is AgentPersona {
return Boolean(persona && !persona.isBuiltIn && !persona.sourceTeam);
return Boolean(persona && !persona.sourceTeam);
}
export function createInputFromRequest(
@@ -41,7 +41,7 @@ export function PersonaActionsMenu({
onDelete: (persona: AgentPersona) => void;
}) {
const disabled = isActionPending || isPending;
const canEdit = !persona.isBuiltIn && !persona.sourceTeam;
const canEdit = !persona.sourceTeam;
return (
<DropdownMenu modal={false}>
@@ -322,8 +322,7 @@ export function UserProfilePanel({
// covering both locally managed agents and declared-owned relay agents.
const canEditAgent =
isOwner === true &&
(managedAgent !== undefined ||
(resolvedPersona !== undefined && !resolvedPersona.isBuiltIn));
(managedAgent !== undefined || resolvedPersona !== undefined);
const memoryQuery = useAgentMemoryQuery(effectivePubkey, {
enabled: viewerIsOwner && Boolean(effectivePubkey),
});
@@ -395,7 +394,7 @@ export function UserProfilePanel({
});
const handleEditAgent = React.useCallback(() => {
if (resolvedPersona && !resolvedPersona.isBuiltIn) {
if (resolvedPersona) {
setPersonaDialogState(editPersonaDialogState(resolvedPersona));
return;
}
@@ -567,7 +566,7 @@ export function UserProfilePanel({
);
const handleEditPersona = React.useCallback(() => {
if (!resolvedPersona || resolvedPersona.isBuiltIn) return;
if (!resolvedPersona) return;
setPersonaDialogState(editPersonaDialogState(resolvedPersona));
}, [resolvedPersona]);
@@ -737,8 +736,7 @@ export function UserProfilePanel({
resolvedPersona,
);
const canManagePersona = isOwner === true && resolvedPersona !== undefined;
const canEditPersona =
canManagePersona && resolvedPersona?.isBuiltIn !== true;
const canEditPersona = canManagePersona;
const canDeletePersona = canManagePersona && !resolvedPersona?.sourceTeam;
const archiveActions = useIdentityArchive(effectivePubkey);
const agentSettingsMenu = (
-4
View File
@@ -6691,10 +6691,6 @@ async function handleUpdatePersona(args: {
if (!persona) {
throw new Error(`agent ${args.input.id} not found`);
}
if (persona.is_builtin) {
throw new Error("Built-in agents cannot be edited.");
}
persona.display_name = args.input.displayName.trim();
persona.avatar_url = args.input.avatarUrl?.trim() || null;
persona.system_prompt = args.input.systemPrompt.trim();
+39
View File
@@ -228,6 +228,45 @@ test("built-in personas are used from the catalog dialog", async ({ page }) => {
await expect.poll(() => getCatalogOrder(page)).toEqual(initialCatalogOrder);
});
test("built-in persona edits persist", async ({ page }) => {
await installMockBridge(page, {
activePersonaIds: ["builtin:fizz"],
globalAgentConfig: {
env_vars: { ANTHROPIC_API_KEY: "sk-ant-test" },
provider: "anthropic",
model: "claude-opus-4-5",
},
});
await gotoApp(page);
await page.getByTestId("open-agents-view").click();
await page.getByLabel("Open actions for Fizz").click();
await page.getByRole("menuitem", { name: "Edit" }).click();
const dialog = page.getByTestId("persona-dialog");
await dialog.getByLabel("Agent name").fill("My Fizz");
await dialog.getByLabel("Agent instruction").fill("User-edited instructions");
await dialog.getByRole("button", { name: "LLM provider" }).click();
await page
.getByRole("menuitemradio", { name: "Anthropic", exact: true })
.click();
await dialog.getByRole("button", { name: "Save changes" }).click();
await expect(dialog).toHaveCount(0);
await expect(page.getByTestId("agents-library-personas")).toContainText(
"My Fizz",
);
const personas = await invokeTauri<
Array<{ id: string; display_name: string; system_prompt: string }>
>(page, "list_personas");
expect(
personas.find((persona) => persona.id === "builtin:fizz"),
).toMatchObject({
display_name: "My Fizz",
system_prompt: "User-edited instructions",
});
});
test("agent avatar emoji picker scrolls inside its popover", async ({
page,
}) => {