diff --git a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts index a4b235fa04..c3f266b706 100644 --- a/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts +++ b/desktop/src/features/agents/lib/agentAutocompleteEligibility.ts @@ -1,3 +1,4 @@ +import { agentIdentityKey } from "@/features/agents/lib/agentIdentity"; import type { Channel, RelayAgent } from "@/shared/api/types"; import { normalizePubkey } from "@/shared/lib/pubkey"; @@ -285,16 +286,13 @@ type AgentAutocompleteCandidate = { personaId?: string | null; }; -function agentIdentityKey(candidate: T) { - if (candidate.isAgent !== true || !candidate.pubkey) { - return null; - } - - // Pubkeys—not persona metadata or a display name—are agent identities. - // A persona may be installed more than once, and an owner may intentionally - // create multiple same-named agents. Collapsing either case makes one agent - // impossible to choose from autocomplete. - return `pubkey:${normalizePubkey(candidate.pubkey)}`; +function agentAutocompleteIdentityKey( + candidate: T, +) { + // Only agents coalesce; two humans may legitimately share every other field. + // The identity itself comes from `agentIdentityKey` so this surface and the + // Agents library cannot drift into two different answers for "same agent?". + return candidate.isAgent === true ? agentIdentityKey(candidate) : null; } function agentCandidateRank( @@ -369,7 +367,7 @@ export function coalesceAgentAutocompleteCandidates< const indexesByKey = new Map(); for (const candidate of candidates) { - const key = agentIdentityKey(candidate); + const key = agentAutocompleteIdentityKey(candidate); if (!key) { output.push(candidate); continue; diff --git a/desktop/src/features/agents/lib/agentIdentity.test.mjs b/desktop/src/features/agents/lib/agentIdentity.test.mjs new file mode 100644 index 0000000000..59715f0d2c --- /dev/null +++ b/desktop/src/features/agents/lib/agentIdentity.test.mjs @@ -0,0 +1,138 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + agentDisplayGroupKey, + agentIdentityKey, + groupAgentsForDisplay, +} from "./agentIdentity.ts"; + +const PUBKEY_A = "a".repeat(64); +const PUBKEY_B = "b".repeat(64); + +test("agent identity is the pubkey, never persona metadata or a name", () => { + const left = { + pubkey: PUBKEY_A, + name: "Bumble", + personaId: "builtin:bumble", + }; + const right = { + pubkey: PUBKEY_B, + name: "Bumble", + personaId: "builtin:bumble", + }; + + assert.notEqual(agentIdentityKey(left), agentIdentityKey(right)); + assert.equal( + agentIdentityKey({ pubkey: ` ${PUBKEY_A.toUpperCase()} ` }), + agentIdentityKey({ pubkey: PUBKEY_A, personaId: "something-else" }), + ); + assert.equal(agentIdentityKey({ pubkey: null }), null); + assert.equal(agentIdentityKey({}), null); +}); + +test("the display group key separates renamed instances of one persona", () => { + const claude = { + pubkey: PUBKEY_A, + name: "Claude", + personaId: "builtin:fizz", + }; + const fizz = { pubkey: PUBKEY_B, name: "Fizz", personaId: "builtin:fizz" }; + + assert.notEqual(agentDisplayGroupKey(claude), agentDisplayGroupKey(fizz)); + assert.equal( + agentDisplayGroupKey(claude), + agentDisplayGroupKey({ ...claude, pubkey: PUBKEY_B, name: " claude " }), + ); + assert.notEqual( + agentDisplayGroupKey(claude), + agentDisplayGroupKey({ ...claude, personaId: "builtin:honey" }), + ); +}); + +test("a name is folded to NFC, so one fleet does not split on encoding", () => { + // macOS input methods and file systems commonly emit NFD, Windows emits NFC. + // The same name typed on two machines must land on one card. + const precomposed = "José"; // é as U+00E9 + const decomposed = "José"; // e + U+0301 combining acute + + assert.notEqual(precomposed, decomposed, "the inputs really do differ"); + assert.equal( + agentDisplayGroupKey({ personaId: "builtin:fizz", name: precomposed }), + agentDisplayGroupKey({ personaId: "builtin:fizz", name: decomposed }), + ); + + const groups = groupAgentsForDisplay([ + { pubkey: PUBKEY_A, name: precomposed, personaId: "builtin:fizz" }, + { pubkey: PUBKEY_B, name: decomposed, personaId: "builtin:fizz" }, + ]); + + assert.equal( + groups.length, + 1, + "two encodings of one name must not render two identical-looking cards", + ); + assert.equal(groups[0].agents.length, 2); +}); + +test("the group key cannot be forged by a name containing a separator", () => { + // Segments are length-prefixed. With a plain `|` join both of these render + // `persona:a|name:x|name:y`, silently merging two different agents onto one + // card and leaving one of them unopenable. + assert.notEqual( + agentDisplayGroupKey({ personaId: "a", name: "x|name:y" }), + agentDisplayGroupKey({ personaId: "a|name:x", name: "y" }), + ); + assert.notEqual( + agentDisplayGroupKey({ personaId: "builtin:fizz", name: "a:b" }), + agentDisplayGroupKey({ personaId: "builtin:fizz:a", name: "b" }), + ); + // A separator in a name is still just a name — same input, same key. + assert.equal( + agentDisplayGroupKey({ personaId: "a", name: "x|name:y" }), + agentDisplayGroupKey({ personaId: "a", name: " X|NAME:Y " }), + ); +}); + +test("unnamed instances of one persona share a card — documented, not accidental", () => { + // "", " ", null and undefined all fold to the same empty name, so several + // unnamed instances of one persona collapse onto the persona's card. They + // stay reachable through that card's profile panel, which lists every + // instance behind it. Asserted so a future change to the fold has to decide + // this deliberately rather than discover it. + const groups = groupAgentsForDisplay([ + { pubkey: PUBKEY_A, name: "", personaId: "builtin:fizz" }, + { pubkey: PUBKEY_B, name: " ", personaId: "builtin:fizz" }, + { pubkey: "c".repeat(64), name: null, personaId: "builtin:fizz" }, + { pubkey: "d".repeat(64), name: "Fizz", personaId: "builtin:fizz" }, + ]); + + assert.deepEqual( + groups.map((group) => group.name), + ["", "Fizz"], + ); + assert.equal(groups[0].agents.length, 3, "no unnamed instance is dropped"); +}); + +test("display grouping keeps every distinct identity and drops repeats", () => { + const agents = [ + { pubkey: PUBKEY_A, name: "Claude", personaId: "builtin:fizz" }, + { pubkey: PUBKEY_B, name: "Fizz", personaId: "builtin:fizz" }, + { pubkey: PUBKEY_A, name: "Claude", personaId: "builtin:fizz" }, + ]; + + const groups = groupAgentsForDisplay(agents); + + assert.deepEqual( + groups.map((group) => group.name), + ["Claude", "Fizz"], + ); + assert.deepEqual( + new Set(groups.flatMap((group) => group.agents).map(agentIdentityKey)), + new Set([ + agentIdentityKey({ pubkey: PUBKEY_A }), + agentIdentityKey(agents[1]), + ]), + ); + assert.equal(groups[0].agents.length, 1); +}); diff --git a/desktop/src/features/agents/lib/agentIdentity.ts b/desktop/src/features/agents/lib/agentIdentity.ts new file mode 100644 index 0000000000..aab8d20e32 --- /dev/null +++ b/desktop/src/features/agents/lib/agentIdentity.ts @@ -0,0 +1,110 @@ +import { normalizePubkey } from "@/shared/lib/pubkey"; + +/** + * THE definition of agent identity, shared by every surface that answers + * "which agents exist" (@-mention autocomplete, the Agents library, the + * profile panel). Two surfaces that hand-roll this drift apart, and the drift + * is invisible until an agent becomes unreachable on one of them. + * + * Pubkeys—not persona metadata or a display name—are agent identities. A + * persona may be installed more than once, and an owner may intentionally + * create multiple same-named agents. Collapsing either case makes one agent + * impossible to choose from autocomplete, and impossible to manage from the + * Agents library. + */ +export type AgentIdentityInput = { pubkey?: string | null }; + +export function agentIdentityKey(candidate: AgentIdentityInput): string | null { + const pubkey = candidate.pubkey?.trim(); + return pubkey ? `pubkey:${normalizePubkey(pubkey)}` : null; +} + +export type AgentDisplayInput = AgentIdentityInput & { + name?: string | null; + personaId?: string | null; +}; + +/** + * Presentation-only key: which agents may legitimately share ONE card in the + * Agents library. This is NOT an identity — it is a statement about what a + * single label can truthfully stand for. Instances of one persona that all + * carry the same name are interchangeable on a card (the card's profile panel + * lists every instance behind it); an instance the owner renamed is not, and + * must get a card of its own or it disappears from the library. + * + * Every caller must keep the full identity list of a display group reachable — + * grouping may never drop an `agentIdentityKey`. + */ +export function agentDisplayGroupKey(agent: AgentDisplayInput): string { + const personaId = agent.personaId?.trim() ?? ""; + // Length-prefixed segments, not a delimiter. A display name is free text and + // may contain any separator we could pick: with a plain `|` join, + // {personaId:"a", name:"x|name:y"} and {personaId:"a|name:x", name:"y"} both + // render `persona:a|name:x|name:y`, so two different agents share one card + // and one of them stops being openable. + const foldedName = foldAgentDisplayName(agent.name); + return `persona:${personaId.length}:${personaId}|name:${foldedName.length}:${foldedName}`; +} + +/** + * The one place a display name is folded for comparison. Callers that need a + * name-scoped key of their own (React keys, `data-testid`s) must fold through + * this rather than lowercasing inline, or their key and the group's disagree. + * + * Unicode is normalized to NFC before folding. macOS input methods and file + * systems commonly produce NFD while Windows produces NFC, so without this the + * same name typed on two machines in one fleet folds to two different keys and + * the library renders two cards with visually identical labels — a split the + * owner cannot see, explain, or fix from the UI. + */ +export function foldAgentDisplayName(name: string | null | undefined): string { + return name?.normalize("NFC").trim().toLowerCase() ?? ""; +} + +export type AgentDisplayGroup = { + key: string; + /** Trimmed display name shared by every member, empty when unnamed. */ + name: string; + /** `name` folded for comparison — the group's identity within a persona. */ + foldedName: string; + agents: T[]; +}; + +/** + * Split agents into display groups, in first-seen order, dropping repeated + * records of the same identity (the same pubkey read from two sources) but + * never dropping a distinct identity. + */ +export function groupAgentsForDisplay( + agents: readonly T[], +): AgentDisplayGroup[] { + const groups: AgentDisplayGroup[] = []; + const groupsByKey = new Map>(); + const seenIdentities = new Set(); + + for (const agent of agents) { + const identity = agentIdentityKey(agent); + if (identity) { + if (seenIdentities.has(identity)) continue; + seenIdentities.add(identity); + } + + const key = agentDisplayGroupKey(agent); + const existing = groupsByKey.get(key); + if (existing) { + existing.agents.push(agent); + continue; + } + + const group = { + key, + name: agent.name?.trim() ?? "", + foldedName: foldAgentDisplayName(agent.name), + agents: [agent], + }; + groupsByKey.set(key, group); + groups.push(group); + } + + return groups; +} diff --git a/desktop/src/features/agents/lib/pickProfileAgent.test.mjs b/desktop/src/features/agents/lib/pickProfileAgent.test.mjs index 710b5fc4be..6be35cc5c9 100644 --- a/desktop/src/features/agents/lib/pickProfileAgent.test.mjs +++ b/desktop/src/features/agents/lib/pickProfileAgent.test.mjs @@ -2,6 +2,7 @@ import assert from "node:assert/strict"; import test from "node:test"; import { + pickCanonicalProfileAgent, pickDirectProfileAgent, pickProfileAgent, } from "./pickProfileAgent.ts"; @@ -69,6 +70,156 @@ test("a fail-open predicate keeps every instance eligible while loading", () => assert.equal(pickProfileAgent([stopped, running], NONE_ARCHIVED), running); }); +test("opening a renamed instance opens that instance, not the persona's", () => { + const claude = { + name: "Claude", + personaId: "builtin:fizz", + pubkey: "a".repeat(64), + status: "running", + }; + const fizz = { + name: "Fizz", + personaId: "builtin:fizz", + pubkey: "b".repeat(64), + status: "stopped", + }; + const instances = [claude, fizz]; + + assert.equal(pickCanonicalProfileAgent(instances, fizz, NONE_ARCHIVED), fizz); + assert.equal( + pickCanonicalProfileAgent(instances, claude, NONE_ARCHIVED), + claude, + ); +}); + +test("same-named instances still canonicalise onto one profile target", () => { + const stopped = { + name: "Bumble", + personaId: "builtin:bumble", + pubkey: "a".repeat(64), + status: "stopped", + }; + const running = { + name: "Bumble", + personaId: "builtin:bumble", + pubkey: "b".repeat(64), + status: "running", + }; + const instances = [stopped, running]; + + assert.equal( + pickCanonicalProfileAgent(instances, stopped, NONE_ARCHIVED), + running, + ); + assert.equal( + pickCanonicalProfileAgent(instances, undefined, NONE_ARCHIVED), + running, + ); +}); + +test("a fully archived display group falls back to the persona's live instance", () => { + // Scoping runs before archive filtering, so a group whose every instance is + // archived must not resolve to nothing where the unscoped selector would + // still have found a live sibling. + const archivedFizz = { + name: "Fizz", + personaId: "builtin:fizz", + pubkey: "a".repeat(64), + status: "stopped", + }; + const liveClaude = { + name: "Claude", + personaId: "builtin:fizz", + pubkey: "b".repeat(64), + status: "running", + }; + const isArchived = (pubkey) => pubkey === archivedFizz.pubkey; + + assert.equal( + pickCanonicalProfileAgent( + [archivedFizz, liveClaude], + archivedFizz, + isArchived, + ), + liveClaude, + ); +}); + +test("a requested instance survives when every candidate is archived", () => { + const requested = { + name: "Fizz", + personaId: "builtin:fizz", + pubkey: "a".repeat(64), + status: "stopped", + }; + + assert.equal( + pickCanonicalProfileAgent([requested], requested, () => true), + requested, + ); +}); + +test("a renamed instance canonicalises within its own name, not the persona", () => { + // The message-avatar path: `builtin:fizz` holds two "Claude" and one "Fizz". + // Clicking an old "Claude" message must land on the running Claude, never on + // the persona-wide winner, or the profile panel and the library card that + // now exists for "Claude" disagree. + const claudeStopped = { + name: "Claude", + personaId: "builtin:fizz", + pubkey: "a".repeat(64), + status: "stopped", + }; + const claudeRunning = { + name: "Claude", + personaId: "builtin:fizz", + pubkey: "b".repeat(64), + status: "running", + }; + const fizzRunning = { + name: "Fizz", + personaId: "builtin:fizz", + pubkey: "c".repeat(64), + status: "running", + }; + const instances = [claudeStopped, claudeRunning, fizzRunning]; + + assert.equal( + pickCanonicalProfileAgent(instances, claudeStopped, NONE_ARCHIVED), + claudeRunning, + ); + assert.equal( + pickCanonicalProfileAgent(instances, fizzRunning, NONE_ARCHIVED), + fizzRunning, + ); +}); + +test("an instance missing from the persona list still resolves", () => { + // A historical agent read off an old message may no longer be in the + // persona's instance list; the request must not resolve to nothing. + const current = { + name: "Current", + personaId: "builtin:fizz", + pubkey: "a".repeat(64), + status: "running", + }; + const historical = { + name: "Retired", + personaId: "builtin:fizz", + pubkey: "b".repeat(64), + status: "stopped", + }; + + assert.equal( + pickCanonicalProfileAgent([current], historical, NONE_ARCHIVED), + current, + ); + assert.equal( + pickCanonicalProfileAgent([], historical, NONE_ARCHIVED), + historical, + ); +}); + test("a direct-opened active instance is never redirected to a sibling", () => { // "Alpha Sibling" sorts before "Tyler Agent"; without the direct guard an // access edit on Tyler would target the sibling. @@ -90,13 +241,18 @@ test("a direct-opened active instance is never redirected to a sibling", () => { }); test("a direct-opened inactive instance redirects to the active sibling", () => { + // The avatar on an old message points at a retired instance. Both carry the + // label the Agents library shows, so the retired one is not a card of its + // own and redirecting to the live instance matches what the library renders. const historical = { - name: "Earlier Parity Agent", + name: "Parity Agent", + personaId: "builtin:parity", pubkey: "a".repeat(64), status: "stopped", }; const current = { - name: "Current Parity Agent", + name: "Parity Agent", + personaId: "builtin:parity", pubkey: "b".repeat(64), status: "running", }; @@ -107,6 +263,29 @@ test("a direct-opened inactive instance redirects to the active sibling", () => ); }); +test("a direct-opened inactive instance never redirects across a rename", () => { + // The owner renamed one instance, so the library shows two cards. Redirecting + // the retired card to the differently named live instance would reopen the + // bug where a card refuses to open the agent it names. + const renamed = { + name: "Fizz", + personaId: "builtin:fizz", + pubkey: "a".repeat(64), + status: "stopped", + }; + const current = { + name: "Claude", + personaId: "builtin:fizz", + pubkey: "b".repeat(64), + status: "running", + }; + + assert.equal( + pickDirectProfileAgent(renamed, [renamed, current], NONE_ARCHIVED), + renamed, + ); +}); + test("a direct-opened inactive instance with no active sibling stays put", () => { const clicked = { name: "Only Instance", diff --git a/desktop/src/features/agents/lib/pickProfileAgent.ts b/desktop/src/features/agents/lib/pickProfileAgent.ts index dc2437c86e..6cd0a71622 100644 --- a/desktop/src/features/agents/lib/pickProfileAgent.ts +++ b/desktop/src/features/agents/lib/pickProfileAgent.ts @@ -1,12 +1,15 @@ +import { agentDisplayGroupKey } from "@/features/agents/lib/agentIdentity"; import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions"; import type { ManagedAgent } from "@/shared/api/types"; /** - * Pick the instance that represents a persona throughout the UI. + * Pick the instance that represents a *set of interchangeable instances* — + * a display group, or a whole persona when nothing has been renamed. * - * A persona can have several historical agent instances. Keeping this rule in - * one place prevents an avatar click on an older message from opening a - * different detail surface than the card in the Agents library. + * Active-first, then by name, so the answer is stable for a given set. Callers + * choose the set; do not call this with a whole persona when the caller is + * resolving a specific instance — use `pickCanonicalProfileAgent`, which scopes + * the set to the requested instance's display group first. * * Relay-archived instances are never eligible, so an archived record early in * file order can't hijack the persona target. Returns `undefined` when every @@ -29,6 +32,44 @@ export function pickProfileAgent( })[0]; } +/** + * Pick the instance a profile request should actually land on. This is the + * rule that keeps every profile entry point — Agents library card, message + * avatar, mention, deep link, Inbox — agreeing with each other. + * + * Collapsing onto the persona's representative instance is only correct while + * the instances are interchangeable presentations of that persona. Once the + * owner has renamed one, opening it must open *it*: the Agents library now + * shows the renamed instance its own card, and a message avatar must reach + * the same place that card does. So the requested instance's display group, + * not the whole persona, is the set we canonicalise over. Same-named siblings + * still collapse onto the active one, exactly as before. + * + * Scoping happens before archive filtering rather than after: the display group + * decides *which* siblings are candidates, and `pickProfileAgent` then drops the + * archived ones. A group whose every instance is archived falls back to the + * persona-wide list, so this never resolves to nothing where the unscoped + * selector would have found a live instance. + */ +export function pickCanonicalProfileAgent( + personaInstances: readonly ManagedAgent[], + requested: ManagedAgent | undefined, + isArchived: (pubkey: string) => boolean, +) { + if (!requested) return pickProfileAgent(personaInstances, isArchived); + + const key = agentDisplayGroupKey(requested); + const sameLabel = personaInstances.filter( + (instance) => agentDisplayGroupKey(instance) === key, + ); + + return ( + pickProfileAgent(sameLabel, isArchived) ?? + pickProfileAgent(personaInstances, isArchived) ?? + requested + ); +} + /** * Resolve which instance a profile panel opened for `directAgent` should * show, given every instance of the same persona. @@ -40,6 +81,12 @@ export function pickProfileAgent( * an old message from a retired instance), redirect to the active one so the * panel matches the Agents library. The `isArchived` predicate keeps that * redirect from ever landing on an archived sibling. + * + * The redirect resolves through `pickCanonicalProfileAgent`, so it also stays + * inside the clicked instance's display group: a retired instance the owner + * renamed falls back to an active sibling carrying *that* name, never to a + * differently named one. Redirecting across names would reopen the bug that + * made a renamed agent unreachable from its own card. */ export function pickDirectProfileAgent( directAgent: ManagedAgent, @@ -47,6 +94,10 @@ export function pickDirectProfileAgent( isArchived: (pubkey: string) => boolean, ) { if (isManagedAgentActive(directAgent)) return directAgent; - const canonical = pickProfileAgent(personaInstances, isArchived); + const canonical = pickCanonicalProfileAgent( + personaInstances, + directAgent, + isArchived, + ); return canonical && isManagedAgentActive(canonical) ? canonical : directAgent; } diff --git a/desktop/src/features/agents/ui/AgentIdentityCard.tsx b/desktop/src/features/agents/ui/AgentIdentityCard.tsx index b0668616ae..5dca2f2482 100644 --- a/desktop/src/features/agents/ui/AgentIdentityCard.tsx +++ b/desktop/src/features/agents/ui/AgentIdentityCard.tsx @@ -15,6 +15,8 @@ type AgentIdentityCardProps = { onClick: () => void; /** Optional badge rendered below the label (e.g. "Restart required"). */ statusBadge?: ReactNode; + /** Optional line under the label (e.g. the persona a split card belongs to). */ + subtitle?: string | null; }; export function AgentIdentityCard({ @@ -27,6 +29,7 @@ export function AgentIdentityCard({ modelLabel, onClick, statusBadge, + subtitle, }: AgentIdentityCardProps) { const trimmedAvatarUrl = avatarUrl?.trim() || null; @@ -72,6 +75,11 @@ export function AgentIdentityCard({ {label} + {subtitle ? ( + + {subtitle} + + ) : null} {modelLabel ? ( {modelLabel} diff --git a/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx b/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx index d0ff2e2738..1877a8f432 100644 --- a/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx +++ b/desktop/src/features/agents/ui/UnifiedAgentsSection.tsx @@ -8,7 +8,6 @@ import { import { resolveAgentCardModelLabel } from "@/features/agents/lib/agentCardModelLabel"; import { friendlyAgentLastError } from "@/features/agents/lib/friendlyAgentLastError"; import { isManagedAgentActive } from "@/features/agents/lib/managedAgentControlActions"; -import { pickProfileAgent } from "@/features/agents/lib/pickProfileAgent"; import { useIsArchivedPredicate } from "@/features/identity-archive/hooks"; import { useUserProfileQuery } from "@/features/profile/hooks"; import type { AgentPersona, ManagedAgent } from "@/shared/api/types"; @@ -130,42 +129,52 @@ export function UnifiedAgentsSection(props: UnifiedAgentsSectionProps) { disabled={isPersonasPending} onClick={onOpenCatalog} /> - {groups.map((group) => { - const profileAgent = pickProfileAgent(group.agents, isArchived); - return ( + {groups.flatMap((group) => + group.cards.map((card) => ( ( - - onSharePersona(persona, linkedAgent, effectiveAvatarUrl) - } - /> - )} - agent={profileAgent} + actions={ + card.ownsPersonaActions + ? (effectiveAvatarUrl, isEffectiveAvatarLoading) => ( + + onSharePersona( + persona, + linkedAgent, + effectiveAvatarUrl, + ) + } + /> + ) + : undefined + } + agent={card.agent} defaultModel={defaultModel} - key={group.persona.id} - persona={group.persona} + key={card.key} + label={card.label} + persona={card.persona} restartingAgentPubkey={restartingAgentPubkey} startingAgentPubkey={startingAgentPubkey} startingPersonaIds={startingPersonaIds} + subtitle={card.personaLabel} + testId={`persona-agent-row-${card.key}`} onOpenAgentProfile={onOpenAgentProfile} onOpenPersonaProfile={onOpenPersonaProfile} onRestartAgent={onRestartAgent} onStartAgent={onStartAgent} onStartPersona={onStartPersona} /> - ); - })} + )), + )} {unknown.length > 0 ? ( @@ -223,10 +232,13 @@ function AgentPersonaCard({ actions, agent, defaultModel, + label, persona, restartingAgentPubkey, startingAgentPubkey, startingPersonaIds, + subtitle, + testId, onOpenAgentProfile, onOpenPersonaProfile, onRestartAgent, @@ -239,10 +251,13 @@ function AgentPersonaCard({ ) => React.ReactNode; agent: ManagedAgent | undefined; defaultModel: string; + label: string; persona: AgentPersona; restartingAgentPubkey: string | null; startingAgentPubkey: string | null; startingPersonaIds: ReadonlySet; + subtitle: string | null; + testId: string; onOpenAgentProfile: ( pubkey: string, options?: ProfilePanelOpenOptions, @@ -252,7 +267,7 @@ function AgentPersonaCard({ onStartAgent: (pubkey: string) => void; onStartPersona: (persona: AgentPersona) => void; }) { - const title = persona.displayName; + const title = label; const modelLabel = resolveAgentCardModelLabel({ agent, personaModel: persona.model, @@ -310,7 +325,7 @@ function AgentPersonaCard({ ) } avatarUrl={avatarUrl} - dataTestId={`persona-agent-row-${persona.id}`} + dataTestId={testId} label={title} modelLabel={modelLabel} onClick={() => { @@ -333,6 +348,7 @@ function AgentPersonaCard({ ) : null } + subtitle={subtitle} /> ); } diff --git a/desktop/src/features/agents/ui/UnifiedAgentsSectionCardTarget.test.mjs b/desktop/src/features/agents/ui/UnifiedAgentsSectionCardTarget.test.mjs index 690a921040..9390fe9699 100644 --- a/desktop/src/features/agents/ui/UnifiedAgentsSectionCardTarget.test.mjs +++ b/desktop/src/features/agents/ui/UnifiedAgentsSectionCardTarget.test.mjs @@ -200,12 +200,14 @@ test("persona card main click records a persona target, never an explicit pubkey recordedPersona = persona; }; - // Archived sibling sorts first by name, so under fail-open pickProfileAgent - // selects it — the card displays the archived identity at click time. A - // durable pubkey target would strand the panel there after hydration. + // Both siblings carry the persona's name, so they stay on its single card. + // The archived one is first and ranks equal by name, so under fail-open + // pickProfileAgent selects it — the card displays the archived identity at + // click time. A durable pubkey target would strand the panel there after + // hydration. const agents = [ - agent({ pubkey: ARCHIVED_PK, name: "Archived Sibling" }), - agent({ pubkey: LIVE_PK, name: "Zed Sibling" }), + agent({ pubkey: ARCHIVED_PK, name: "Fizz Prime" }), + agent({ pubkey: LIVE_PK, name: "Fizz Prime" }), ]; await act(async () => { diff --git a/desktop/src/features/agents/ui/unifiedAgentGroups.test.mjs b/desktop/src/features/agents/ui/unifiedAgentGroups.test.mjs index b3ade7f229..fd134ce97e 100644 --- a/desktop/src/features/agents/ui/unifiedAgentGroups.test.mjs +++ b/desktop/src/features/agents/ui/unifiedAgentGroups.test.mjs @@ -1,6 +1,8 @@ import assert from "node:assert/strict"; import test from "node:test"; +import { coalesceAgentAutocompleteCandidates } from "../lib/agentAutocompleteEligibility.ts"; +import { agentIdentityKey } from "../lib/agentIdentity.ts"; import { buildUnifiedGroups } from "./unifiedAgentGroups.ts"; const NONE_ARCHIVED = () => false; @@ -19,6 +21,37 @@ function persona(overrides = {}) { return { id: "persona-1", displayName: "Persona", ...overrides }; } +/** Positional builders for the rename fixtures, where name is the subject. */ +function namedAgent(pubkey, name, personaId, status = "stopped") { + return { pubkey, name, personaId, status }; +} + +function namedPersona(id, displayName) { + return { id, displayName }; +} + +/** + * The owner's real shape: one builtin persona whose instances were renamed, so + * the group carries two distinct names across four distinct pubkeys. + */ +const FIZZ_PERSONA = namedPersona("builtin:fizz", "Fizz"); +const PUBKEYS = { + claudeOne: "1".repeat(64), + claudeTwo: "2".repeat(64), + fizzOne: "3".repeat(64), + fizzTwo: "4".repeat(64), +}; +const FIZZ_AGENTS = [ + namedAgent(PUBKEYS.claudeOne, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.claudeTwo, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.fizzOne, "Fizz", "builtin:fizz"), + namedAgent(PUBKEYS.fizzTwo, "Fizz", "builtin:fizz"), +]; + +function cardsOf(result) { + return result.groups.flatMap((group) => group.cards); +} + test("archived standalone custom agents are omitted while live peers remain", () => { const archived = agent({ pubkey: "a".repeat(64), personaId: null }); const live = agent({ pubkey: "b".repeat(64), personaId: null }); @@ -75,3 +108,299 @@ test("a fail-open predicate keeps every standalone agent discoverable", () => { assert.equal(ungrouped.length, 2); }); + +test("a renamed instance still gets a card in the agents library", () => { + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + NONE_ARCHIVED, + ); + + assert.equal(groups.length, 1); + assert.deepEqual( + groups[0].cards.map((card) => card.label), + ["Claude", "Fizz"], + ); + assert.deepEqual( + groups[0].cards.map((card) => card.agent.pubkey), + [PUBKEYS.claudeOne, PUBKEYS.fizzOne], + ); +}); + +test("a fully archived name gets no card of its own", () => { + // Archiving every "Claude" leaves only "Fizz" as a clickable card — an + // archived identity must never become a library card in its own right. + const isArchived = (pubkey) => + pubkey === PUBKEYS.claudeOne || pubkey === PUBKEYS.claudeTwo; + + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + isArchived, + ); + + assert.deepEqual( + groups[0].cards.map((card) => card.label), + ["Fizz"], + ); + assert.equal(groups[0].cards[0].ownsPersonaActions, true); +}); + +test("a split persona with every instance archived keeps one persona-only card", () => { + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + () => true, + ); + + assert.equal(groups[0].cards.length, 1); + assert.equal(groups[0].cards[0].key, FIZZ_PERSONA.id); + assert.equal(groups[0].cards[0].label, FIZZ_PERSONA.displayName); + assert.equal(groups[0].cards[0].agent, undefined); + assert.equal(groups[0].cards[0].ownsPersonaActions, true); +}); + +test("no managed agent is dropped by persona grouping", () => { + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + NONE_ARCHIVED, + ); + const reachable = cardsOf({ groups }) + .flatMap((card) => card.agents) + .map(agentIdentityKey); + + assert.deepEqual( + new Set(reachable), + new Set(FIZZ_AGENTS.map(agentIdentityKey)), + ); + assert.equal(reachable.length, FIZZ_AGENTS.length); +}); + +test("the agents library and @-mention autocomplete agree on which agents exist", () => { + const records = [ + ...FIZZ_AGENTS, + namedAgent("5".repeat(64), "Solo", "custom:solo", "running"), + ]; + const { groups, ungrouped, unknown } = buildUnifiedGroups( + [FIZZ_PERSONA, namedPersona("custom:solo", "Solo")], + records, + NONE_ARCHIVED, + ); + + const libraryIdentities = new Set( + [ + ...cardsOf({ groups }).flatMap((card) => card.agents), + ...ungrouped, + ...unknown, + ].map(agentIdentityKey), + ); + const autocompleteIdentities = new Set( + coalesceAgentAutocompleteCandidates( + records.map((record) => ({ ...record, isAgent: true })), + { getLabel: (candidate) => candidate.name }, + ).map(agentIdentityKey), + ); + + assert.deepEqual(libraryIdentities, autocompleteIdentities); +}); + +test("same-named instances of one persona stay on the persona's single card", () => { + const duplicates = [ + namedAgent( + PUBKEYS.claudeOne, + "Duplicate Auditor", + "custom:duplicate", + "running", + ), + namedAgent(PUBKEYS.claudeTwo, "Duplicate Auditor", "custom:duplicate"), + ]; + + const { groups } = buildUnifiedGroups( + [namedPersona("custom:duplicate", "Duplicate Auditor")], + duplicates, + NONE_ARCHIVED, + ); + + assert.equal(groups[0].cards.length, 1); + assert.equal(groups[0].cards[0].key, "custom:duplicate"); + assert.equal(groups[0].cards[0].label, "Duplicate Auditor"); + assert.equal(groups[0].cards[0].agents.length, 2); + assert.equal(groups[0].cards[0].ownsPersonaActions, true); +}); + +test("persona-level actions live on exactly one card per persona", () => { + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + NONE_ARCHIVED, + ); + + assert.equal( + groups[0].cards.filter((card) => card.ownsPersonaActions).length, + 1, + ); +}); + +test("persona actions stay put when an instance starts or stops", () => { + // Deriving the owner from an active-first pick relocated the only route to + // editing or deleting a persona the moment an agent started. + const ownerOf = (agents) => + buildUnifiedGroups( + [FIZZ_PERSONA], + agents, + NONE_ARCHIVED, + ).groups[0].cards.find((card) => card.ownsPersonaActions).label; + + assert.equal(ownerOf(FIZZ_AGENTS), "Fizz"); + assert.equal( + ownerOf([ + namedAgent(PUBKEYS.claudeOne, "Claude", "builtin:fizz", "running"), + namedAgent(PUBKEYS.claudeTwo, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.fizzOne, "Fizz", "builtin:fizz"), + namedAgent(PUBKEYS.fizzTwo, "Fizz", "builtin:fizz"), + ]), + "Fizz", + ); + assert.equal( + ownerOf([ + namedAgent(PUBKEYS.claudeOne, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.claudeTwo, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.fizzOne, "Fizz", "builtin:fizz", "running"), + namedAgent(PUBKEYS.fizzTwo, "Fizz", "builtin:fizz"), + ]), + "Fizz", + ); +}); + +test("persona actions fall back to the first card when every name was changed", () => { + const renamed = [ + namedAgent(PUBKEYS.claudeOne, "Cascade Instance A", "custom:cascade"), + namedAgent( + PUBKEYS.claudeTwo, + "Cascade Instance B", + "custom:cascade", + "running", + ), + ]; + const { groups } = buildUnifiedGroups( + [namedPersona("custom:cascade", "Cascade Test Agent")], + renamed, + NONE_ARCHIVED, + ); + + assert.deepEqual( + groups[0].cards.map((card) => card.ownsPersonaActions), + [true, false], + ); +}); + +test("a split persona keeps its own name on every card", () => { + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + NONE_ARCHIVED, + ); + + assert.deepEqual( + groups[0].cards.map((card) => card.personaLabel), + ["Fizz", "Fizz"], + ); +}); + +test("an unsplit persona has no second line to add", () => { + const { groups } = buildUnifiedGroups( + [namedPersona("custom:solo", "Solo")], + [namedAgent(PUBKEYS.claudeOne, "Solo", "custom:solo")], + NONE_ARCHIVED, + ); + + assert.equal(groups[0].cards[0].personaLabel, null); +}); + +test("card keys stay unique so cards cannot overwrite each other", () => { + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + NONE_ARCHIVED, + ); + const keys = groups[0].cards.map((card) => card.key); + + assert.equal(new Set(keys).size, keys.length); + // Pinned: e2e specs address split cards by `persona-agent-row-`. + assert.deepEqual(keys, ["builtin:fizz::claude", "builtin:fizz::fizz"]); +}); + +test("card keys do not move when an instance starts, stops, or is reordered", () => { + // A key derived from the current active-first winner remounts the card and + // refires its avatar query whenever a sibling's status changes. + const keysFor = (agents) => + buildUnifiedGroups( + [FIZZ_PERSONA], + agents, + NONE_ARCHIVED, + ).groups[0].cards.map((card) => card.key); + const baseline = keysFor(FIZZ_AGENTS); + + assert.deepEqual( + keysFor([ + namedAgent(PUBKEYS.claudeOne, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.claudeTwo, "Claude", "builtin:fizz", "running"), + namedAgent(PUBKEYS.fizzOne, "Fizz", "builtin:fizz"), + namedAgent(PUBKEYS.fizzTwo, "Fizz", "builtin:fizz", "running"), + ]), + baseline, + ); + assert.deepEqual( + keysFor([ + namedAgent(PUBKEYS.claudeTwo, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.claudeOne, "Claude", "builtin:fizz"), + namedAgent(PUBKEYS.fizzTwo, "Fizz", "builtin:fizz"), + namedAgent(PUBKEYS.fizzOne, "Fizz", "builtin:fizz"), + ]), + baseline, + ); +}); + +test("every distinct name is openable from a card the library actually renders", () => { + // `card.agent` is what the card's click handler opens; `card.agents` is the + // full set behind it. The render layer reads `agent`, so assert on it too — + // an invariant held only by a field no component reads proves nothing. + const { groups } = buildUnifiedGroups( + [FIZZ_PERSONA], + FIZZ_AGENTS, + NONE_ARCHIVED, + ); + + assert.deepEqual( + groups[0].cards.map((card) => card.agent.name), + ["Claude", "Fizz"], + ); +}); + +test("a persona with no instances keeps its single unchanged card", () => { + const { groups } = buildUnifiedGroups( + [namedPersona("custom:idle", "Idle")], + [], + NONE_ARCHIVED, + ); + + assert.equal(groups[0].cards.length, 1); + assert.equal(groups[0].cards[0].key, "custom:idle"); + assert.equal(groups[0].cards[0].label, "Idle"); + assert.equal(groups[0].cards[0].agent, undefined); +}); + +test("agents without a persona, and personas that vanished, are unchanged", () => { + const orphan = namedAgent("6".repeat(64), "Orphan", "custom:gone"); + const custom = namedAgent("7".repeat(64), "Custom", null); + + const { ungrouped, unknown } = buildUnifiedGroups( + [], + [orphan, custom], + NONE_ARCHIVED, + ); + + assert.deepEqual(ungrouped, [custom]); + assert.deepEqual(unknown, [orphan]); +}); diff --git a/desktop/src/features/agents/ui/unifiedAgentGroups.ts b/desktop/src/features/agents/ui/unifiedAgentGroups.ts index 2ddf34d840..1cffae6dab 100644 --- a/desktop/src/features/agents/ui/unifiedAgentGroups.ts +++ b/desktop/src/features/agents/ui/unifiedAgentGroups.ts @@ -1,6 +1,134 @@ +import { + foldAgentDisplayName, + groupAgentsForDisplay, +} from "@/features/agents/lib/agentIdentity"; +import { pickProfileAgent } from "@/features/agents/lib/pickProfileAgent"; import type { AgentPersona, ManagedAgent } from "@/shared/api/types"; -type PersonaGroup = { persona: AgentPersona; agents: ManagedAgent[] }; +/** + * One card in the Agents library. + * + * A card stands for every instance in `agents` and opens `agent` — never for + * an instance it does not list. Persona grouping decides which cards exist; + * it may never decide which agents exist (see `agentDisplayGroupKey`). + */ +export type UnifiedAgentCard = { + /** + * Stable card key, also used for the card's React key and `data-testid`. + * + * Derived only from the persona and the display group, never from runtime + * status or array position: a key that moves when an agent starts remounts + * the card and refires its avatar query. + */ + key: string; + label: string; + /** + * Persona name, rendered as the card's second line, and only when the + * persona has split into several cards — otherwise `label` already is the + * persona name. Without it a split persona's own name appears nowhere in + * the library, which is the same disappearing act this module exists to + * prevent. + */ + personaLabel: string | null; + persona: AgentPersona; + /** Instance the card opens; `undefined` for a persona with no instances. */ + agent: ManagedAgent | undefined; + /** Every instance the card stands for. */ + agents: ManagedAgent[]; + /** Persona-level actions (edit/share/delete) live on exactly one card. */ + ownsPersonaActions: boolean; +}; + +export type PersonaGroup = { + persona: AgentPersona; + agents: ManagedAgent[]; + cards: UnifiedAgentCard[]; +}; + +/** + * Cards for one persona. + * + * All of a persona's instances carrying the same name stay collapsed onto the + * persona's single card — that is the established gallery behaviour, and the + * card's profile panel lists every instance behind it. Once the owner renames + * an instance, the persona's name can no longer truthfully stand for all of + * them, so each surviving name gets its own card. Without this, a rename made + * an agent vanish from the library entirely. + * + * A split card only exists for a name that still has a live instance, so an + * archived identity never becomes a clickable card of its own. When archiving + * leaves no live instance under any name, the persona collapses back to a + * single persona-only card rather than disappearing from the library. + */ +function buildPersonaCards( + persona: AgentPersona, + agents: ManagedAgent[], + isArchived: (pubkey: string) => boolean, +): UnifiedAgentCard[] { + const displayGroups = groupAgentsForDisplay(agents); + const personaOnlyCard = (members: ManagedAgent[]): UnifiedAgentCard => ({ + key: persona.id, + label: persona.displayName, + personaLabel: null, + persona, + agent: pickProfileAgent(members, isArchived), + agents: members, + ownsPersonaActions: true, + }); + + if (displayGroups.length <= 1) { + return [personaOnlyCard(displayGroups[0]?.agents ?? [])]; + } + + const liveGroups = displayGroups.filter( + (group) => pickProfileAgent(group.agents, isArchived) !== undefined, + ); + if (liveGroups.length === 0) return [personaOnlyCard(agents)]; + + const ownerIndex = pickPersonaActionsIndex(persona, liveGroups); + return liveGroups.map((group, index) => ({ + // Plain `::` join rather than the length-prefixed form `agentDisplayGroupKey` + // uses, because this key is also the card's `data-testid` and e2e specs read + // it. That is safe only because the left segment cannot contain the + // separator. Every shape a persona id takes carries at most one colon: + // a `slugify()` output (which maps every non-alphanumeric to `-`, so it + // cannot contain one at all), a v4 UUID, a `builtin:` literal, or the + // store-namespaced `:` form — both halves of that + // last one are slugs, so it is a single colon too. The right segment is free + // text, but a forged separator there cannot shift the boundary when the left + // one is constrained. Revisit if persona ids ever become user-supplied. + key: `${persona.id}::${group.foldedName}`, + label: group.name || persona.displayName, + personaLabel: persona.displayName, + persona, + agent: pickProfileAgent(group.agents, isArchived), + agents: group.agents, + ownsPersonaActions: index === ownerIndex, + })); +} + +/** + * Which split card carries the persona menu — Edit / Duplicate / Share / + * Deactivate / Delete persona. + * + * Deliberately independent of runtime status. Deriving it from + * `pickProfileAgent` (active-first) relocated the only route to editing or + * deleting a persona whenever an instance started, leaving the card the owner + * was looking at with no menu and no hint where it went. The card that still + * carries the persona's own name is the natural home; if the owner renamed + * every instance — or archiving retired the card that held the persona's own + * name — the first surviving card gets it. + */ +function pickPersonaActionsIndex( + persona: AgentPersona, + displayGroups: readonly { foldedName: string }[], +): number { + const personaName = foldAgentDisplayName(persona.displayName); + const named = displayGroups.findIndex( + (group) => group.foldedName === personaName, + ); + return named === -1 ? 0 : named; +} /** * Group managed agents under their personas for the Agents library. @@ -34,7 +162,12 @@ export function buildUnifiedGroups( const matched = new Set(); const groups: PersonaGroup[] = personas.map((persona) => { matched.add(persona.id); - return { persona, agents: byPersonaId.get(persona.id) ?? [] }; + const personaAgents = byPersonaId.get(persona.id) ?? []; + return { + persona, + agents: personaAgents, + cards: buildPersonaCards(persona, personaAgents, isArchived), + }; }); const unknown: ManagedAgent[] = []; diff --git a/desktop/src/features/profile/lib/useCanonicalManagedAgentProfile.ts b/desktop/src/features/profile/lib/useCanonicalManagedAgentProfile.ts index 0393a795ce..664c7e4211 100644 --- a/desktop/src/features/profile/lib/useCanonicalManagedAgentProfile.ts +++ b/desktop/src/features/profile/lib/useCanonicalManagedAgentProfile.ts @@ -1,8 +1,8 @@ import * as React from "react"; import { + pickCanonicalProfileAgent, pickDirectProfileAgent, - pickProfileAgent, } from "@/features/agents/lib/pickProfileAgent"; import { useIsArchivedPredicate } from "@/features/identity-archive/hooks"; import { useUserProfileQuery } from "@/features/profile/hooks"; @@ -28,7 +28,9 @@ import { normalizePubkey } from "@/shared/lib/pubkey"; * opened active instance exact so an access edit targets it, only redirecting * an inactive click to a live sibling — see `pickDirectProfileAgent`. * - Otherwise persona-target and non-archived historical navigation resolve - * through the shared archive-aware selector: all instances archived yields + * through the shared archive-aware selector, scoped to the requested + * instance's display group so a renamed instance resolves to itself rather + * than to a differently named sibling: all candidates archived yields * `undefined` (persona-only mode), else the canonical live instance. */ export function resolveCanonicalManagedAgent(input: { @@ -60,7 +62,13 @@ export function resolveCanonicalManagedAgent(input: { isArchived, ); } - return pickProfileAgent(personaInstances, isArchived) ?? directManagedAgent; + return ( + pickCanonicalProfileAgent( + personaInstances, + directManagedAgent, + isArchived, + ) ?? directManagedAgent + ); } /** diff --git a/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts b/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts index de94bb230f..a20cdb5715 100644 --- a/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts +++ b/desktop/tests/e2e/agent-lifecycle-feedback.spec.ts @@ -106,16 +106,29 @@ test.describe("agent lifecycle feedback screenshots", () => { await openAgentsView(page); - // The custom persona card appears in the library. + // Both instances were renamed, so the persona splits into one card per + // name rather than hiding an instance behind a single persona card. await expect( - page.getByText("Cascade Test Agent", { exact: true }), + page.getByText("Cascade Instance A", { exact: true }), ).toBeVisible({ timeout: 10_000 }); + await expect( + page.getByText("Cascade Instance B", { exact: true }), + ).toBeVisible(); - // Open the actions menu for the custom persona. The trigger button carries - // an aria-label derived from the persona displayName. - await page - .getByRole("button", { name: "Open actions for Cascade Test Agent" }) - .click(); + // The persona's own name stays visible as each split card's second line, + // so a split persona is still findable in the library. + await expect( + page.getByText("Cascade Test Agent", { exact: true }), + ).toHaveCount(2); + + // Persona actions live on exactly one card — deterministically the first, + // since no instance kept the persona's name. The trigger button carries an + // aria-label derived from the persona displayName. + const personaActions = page.getByRole("button", { + name: "Open actions for Cascade Test Agent", + }); + await expect(personaActions).toHaveCount(1); + await personaActions.click(); // For a custom (non-builtin) persona, Delete opens PersonaDeleteDialog. await page.getByRole("menuitem", { name: "Delete" }).click(); diff --git a/desktop/tests/e2e/agents.spec.ts b/desktop/tests/e2e/agents.spec.ts index e191d29304..286beb52a5 100644 --- a/desktop/tests/e2e/agents.spec.ts +++ b/desktop/tests/e2e/agents.spec.ts @@ -2700,3 +2700,71 @@ test("duplicate instances move from the agents gallery into the agent profile", page.getByTestId(`user-profile-agent-delete-${additionalPubkey}`), ).toHaveCount(0); }); + +test("renamed instances of one persona each get their own gallery card", async ({ + page, +}) => { + // The regression this pins lived in JSX: the gallery rendered exactly one + // card per persona, so an instance the owner renamed had no card at all and + // was unreachable from Settings. Model-layer tests cannot catch a revert of + // the render layer; this can. + const personaId = "builtin:fizz"; + const claudePubkey = TEST_IDENTITIES.alice.pubkey; + const fizzPubkey = TEST_IDENTITIES.charlie.pubkey; + await installMockBridge(page, { + personas: [ + { + id: personaId, + displayName: "Fizz", + systemPrompt: "You are Fizz.", + }, + ], + managedAgents: [ + { pubkey: claudePubkey, name: "Claude", personaId, status: "running" }, + { pubkey: "1".repeat(64), name: "Claude", personaId, status: "stopped" }, + { pubkey: fizzPubkey, name: "Fizz", personaId, status: "stopped" }, + { pubkey: "2".repeat(64), name: "Fizz", personaId, status: "stopped" }, + ], + }); + await gotoApp(page); + await page.getByTestId("open-agents-view").click(); + + // Four instances, two names, two cards — not one card hiding "Fizz", and not + // four cards exploding same-named duplicates apart. + const claudeCard = page.getByTestId(`persona-agent-row-${personaId}::claude`); + const fizzCard = page.getByTestId(`persona-agent-row-${personaId}::fizz`); + await expect(claudeCard).toBeVisible(); + await expect(fizzCard).toBeVisible(); + await expect( + page.locator(`[data-testid^="persona-agent-row-${personaId}"]`), + ).toHaveCount(2); + await expect(claudeCard).toContainText("Claude"); + + // The persona's own name survives the split as each card's second line, so a + // split persona stays findable in the library. + await expect(claudeCard.getByText("Fizz", { exact: true })).toBeVisible(); + + // Persona actions live on exactly one card — the one still carrying the + // persona's name — rather than migrating to whichever instance is running. + await expect( + page.getByRole("button", { name: "Open actions for Fizz" }), + ).toHaveCount(1); + await expect( + fizzCard.getByRole("button", { name: "Open actions for Fizz" }), + ).toBeVisible(); + + // The renamed card opens its own instance, not the persona's active winner: + // the stopped "Fizz" offers Start, where running "Claude" would offer Stop. + await fizzCard.click(); + await expect(page.getByTestId("user-profile-panel")).toBeVisible(); + await expect( + page.getByTestId("user-profile-agent-primary-action"), + ).toHaveAttribute("aria-label", "Start agent"); + + // ...and the "Claude" card opens the running instance. + await page.getByTestId("auxiliary-panel-close").click(); + await claudeCard.click(); + await expect( + page.getByTestId("user-profile-agent-primary-action"), + ).toHaveAttribute("aria-label", "Stop"); +}); diff --git a/desktop/tests/e2e/profile.spec.ts b/desktop/tests/e2e/profile.spec.ts index 6c88b628ce..4493eaec64 100644 --- a/desktop/tests/e2e/profile.spec.ts +++ b/desktop/tests/e2e/profile.spec.ts @@ -1871,24 +1871,40 @@ test("an older agent message opens the same persona instance as the Agents libra await page.goto("/"); await page.getByTestId("open-agents-view").click(); - await page.getByTestId(`persona-agent-row-${personaId}`).click(); + + // The two instances carry different names, so the persona shows one card per + // name — neither is hidden — and each card opens its own instance. The + // persona's own name stays on both cards as a second line. + const earlierCard = page.getByTestId( + `persona-agent-row-${personaId}::earlier parity agent`, + ); + const currentCard = page.getByTestId( + `persona-agent-row-${personaId}::current parity agent`, + ); + await expect(earlierCard).toBeVisible(); + await expect(currentCard).toBeVisible(); + await expect(page.getByText("Parity Agent", { exact: true })).toHaveCount(2); + + // The running instance's card opens the running instance. + await currentCard.click(); + await expectHashSearchParam(page, "profile", currentPubkey); await expect( page.getByTestId("user-profile-agent-primary-action"), ).toHaveAttribute("aria-label", "Stop"); - const agentsLibraryContract = await readOwnedAgentProfileContract(page); + await page.getByTestId("auxiliary-panel-close").click(); - await page.getByTestId("user-profile-tab-runtime").click(); - await page.getByTestId("user-profile-instances").click(); - await page.getByTestId(`user-profile-instance-${historicalPubkey}`).click(); + // The renamed, stopped instance's card opens *that* instance rather than + // silently redirecting to its running sibling. + await earlierCard.click(); await expectHashSearchParam(page, "profile", historicalPubkey); - await expectHashSearchParam(page, "profileTab", "runtime"); await expect( page.getByTestId("user-profile-agent-primary-action"), ).toHaveAttribute("aria-label", "Start agent"); - await expect( - page.getByTestId(`user-profile-instance-${historicalPubkey}`), - ).toContainText("Current"); + const agentsLibraryContract = await readOwnedAgentProfileContract(page); + // Parity, restated for the new card model: an avatar click on an older + // message from the renamed instance must land on the same contract its card + // does — not on the persona's running instance. await page.getByTestId("auxiliary-panel-close").click(); await page.getByTestId("channel-agents").click(); const historicalMessage = page @@ -1898,7 +1914,7 @@ test("an older agent message opens the same persona instance as the Agents libra await historicalMessage.locator("button").first().click(); await expect( page.getByTestId("user-profile-agent-primary-action"), - ).toHaveAttribute("aria-label", "Stop"); + ).toHaveAttribute("aria-label", "Start agent"); const messageContract = await readOwnedAgentProfileContract(page); expect(messageContract).toEqual(agentsLibraryContract);