fix(desktop): resolve agent profiles through one archive-aware selector - #5706
fix(desktop): resolve agent profiles through one archive-aware selector#5706wpfleger96 wants to merge 1 commit into
Conversation
wesbillman
left a comment
There was a problem hiding this comment.
Reviewing on Wes Pfleger's behalf.
The archived-primary path described in this PR is still reachable from the main Agents library, so this does not fully fix the reported navigation bug.
UnifiedAgentsSection chooses profileAgent = pickProfileAgent(group.agents) without archive state (desktop/src/features/agents/ui/UnifiedAgentsSection.tsx:130-159). When a persona card has any instance, AgentPersonaCard then always calls onOpenAgentProfile(agent.pubkey) (:313-321). AgentsScreen serializes that as the explicit profile target (desktop/src/features/agents/ui/AgentsScreen.tsx:69-76). That means UserProfilePanel reaches the explicit-pubkey branch in resolveProfileManagedAgent, which intentionally returns the archived record verbatim, and never exercises the new archive-aware persona fallback.
Concretely, if pickProfileAgent selects the archived Duncan (it prioritizes active status, then name), clicking Duncan's card still displays that archived Duncan as the primary profile. The Instances list is filtered afterward, but the profile identity, status/actions, channels, activity, and queries remain bound to the archived pubkey.
Please make the persona-card entry path archive-aware, for example by selecting a non-archived profileAgent before opening an explicit pubkey, or by navigating the card via the persona target and reserving explicit-pubkey navigation for clicks on a specific instance. Add a regression test that follows the caller contract (archived selected candidate plus live sibling) rather than testing only the currently bypassed persona branch.
The new resolver tests intentionally codify behavior changes (including explicit archived selection and all-archived fallback); those semantics should remain visible in the PR review.
|
Correction to the attribution in my changes-requested review: I conflated two names. Wes Billman owns the authenticated GitHub account through which this automated review was submitted; Will Pfleger is the PR author. There is no “Wes Pfleger” involved. The review was produced by Carl, an automated reviewer, and the technical finding is unchanged. Sorry for the confusion. |
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Re-reviewed exact head 2e19767a88328ab66320b5580f3af626f0138f5d. My prior blocker is resolved.
The Agents-library caller now passes the archive predicate into pickProfileAgent, which removes archived candidates before active/name sorting. A mixed persona therefore opens the live sibling; an all-archived persona gets undefined and follows the existing persona-only navigation path. Archived standalone and unknown-persona cards are also removed, closing the adjacent explicit-pubkey entry paths. Direct explicit-pubkey profile navigation remains intentionally exact for manage/unarchive access.
The all-archived panel behavior is now consistent with the title: persona resolution returns no managed agent and the Instances section receives an empty list rather than restoring archived rows.
The added behavior-changing tests cover the actual caller selection contract, all-archived persona fallback, standalone/unknown filtering, fail-open loading, profile resolution, and Instances filtering. Focused resolver/group tests pass (25/25), desktop typecheck passes, and git diff --check is clean at this head.
I found no remaining material code issue. I am leaving this as a comment rather than approving because Wes has not explicitly authorized an approval review for this PR. Existing exact-head CI was still running in several Desktop jobs at my review snapshot.
wesbillman
left a comment
There was a problem hiding this comment.
Carl, an automated reviewer, commenting via Wes’s GitHub account.
Requesting changes after a fresh lifecycle review. Princess Donut identified a valid loading-state race that my previous re-review missed.
useIsArchivedPredicate() deliberately returns false before the archive snapshot exists. During that window, pickProfileAgent can select the archived active/name-preferred sibling. The persona card then navigates by that candidate's explicit pubkey (UnifiedAgentsSection.tsx → AgentsScreen), and resolveProfileManagedAgent deliberately preserves explicit-pubkey targets after hydration. When the archive snapshot arrives, the panel recomputes but remains pinned to the archived identity. This turns a transient fail-open window into a durable wrong-primary panel until it is closed and reopened.
Please keep persona-card navigation as a persona target so archive hydration can retarget the panel. The selected live agent may still drive card presentation/actions, and deliberate instance/error navigation may remain explicit-pubkey based. Add a regression composing the actual transition: archive state unavailable → persona card chooses the old preferred record → archive state arrives with that record archived → target resolves to the live sibling (or persona-only when all are archived).
The steady-state filtering added in this head is sound, but it does not close this lifecycle path.
wesbillman
left a comment
There was a problem hiding this comment.
Princess Donut, an automated reviewer, commenting via Wes’s GitHub account after alignment with Carl and Mongo.
Request changes — the persona-target fix introduces a selected-instance mismatch whenever a persona has multiple live siblings.
UnifiedAgentsSection chooses the instance represented by the card with pickProfileAgent(group.agents, isArchived) (desktop/src/features/agents/ui/UnifiedAgentsSection.tsx:135-156). That selector filters archived records, then deliberately ranks the remaining instances by active status and name (desktop/src/features/agents/ui/unifiedAgentGroups.ts:51-63). The card’s avatar, status, actions, error state, and optional Runtime-tab target all come from that selected instance.
The main click now records only the persona (UnifiedAgentsSection.tsx:321-332). In the panel, however, resolveProfileManagedAgent resolves that persona with a raw file-order find over the first non-archived sibling (desktop/src/features/profile/ui/UserProfilePanelUtils.ts:181-184). It does not apply the card’s active/name policy.
Concrete failure: with managed-agent order [stopped Abe, running Zed], the card displays and acts as running Zed, but clicking it opens Abe. The panel then binds profile queries, status/actions, channels, activity, logs, and archive controls to Abe (desktop/src/features/profile/ui/UserProfilePanel.tsx:192-234). The stopped-error variant is more misleading: an errored alphabetical instance can make the card request the Runtime tab, while the panel resolves a different healthy file-first sibling. The previous explicit-pubkey click preserved the exact represented instance; this head loses that contract while fixing hydration pinning.
The new mounted transition test misses the divergence because its expected live target is also the first remaining live record (UnifiedAgentsSectionCardTarget.test.mjs:226-273). The resolver tests likewise do not compose multiple live siblings in conflicting file-order versus active/name order.
Smallest safe remedy: retain persona URL targeting so archive hydration can self-correct, but make persona resolution consume the same shared archive-aware active→name selector as the card. Do not duplicate the policy. Add regressions with reversed file order for (1) active preference, (2) same-status name preference, and ideally (3) the stopped-error Runtime-tab path.
The archive filtering, all-archived persona-only behavior, standalone/orphan filtering, and fail-open hydration correction are otherwise sound. Exact head reviewed: f6c0045d10d94ff1b17559eeea5f92fac646b6a6; all reported exact-head checks are green, and git diff --check is clean. CI cannot detect this because the new tests encode matching order on both sides.
Pull request was closed
dfa2f41 to
17d2147
Compare
Profile resolution funnels every entry point through the shared pickProfileAgent selector, but it ranked instances purely by active/name — a relay-archived instance early in that order could hijack the persona card and the profile panel, and the persona card recorded a durable pubkey target that stranded the panel on an archived identity when the click landed during the archive-snapshot fail-open window. Make pickProfileAgent and the library grouping archive-aware via the fail-open useIsArchivedPredicate, and have the persona card record a persona target that re-resolves each render. A deliberately requested archived pubkey still stays exact so its archive controller can unarchive it, and Runtime instance pinning and the runtime-error affordance keep their explicit-pubkey path. Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
Agent profiles resolve through one shared selector (
pickProfileAgent) at every entry point — the persona card, the profile panel, and library grouping. That selector ranked instances only by active/name, with no archive awareness, so a relay-archived instance early in file order could hijack the persona card and the profile panel. The persona card also recorded a durable pubkey target, which could strand the panel on an archived identity when the click landed during the archive-snapshot fail-open window.This makes the shared resolution path archive-aware via the existing fail-open
useIsArchivedPredicate:pickProfileAgentfilters archived instances before ranking and returnsundefinedwhen every instance is archived (persona-only mode).buildUnifiedGroupsdrops archived agents from the standaloneCustom agentsandUnknown agentsbuckets; matched persona groups keep their full list and rely on the selector's persona-only fallback.useCanonicalManagedAgentProfileresolves through a pureresolveCanonicalManagedAgenthelper that applies the target-provenance rules: a deliberately requested archived pubkey stays exact (so its archive controller can unarchive it, even when a live sibling exists),preserveRequestedInstancestill pins a Runtime → Instances selection, and non-archived historical navigation keeps its canonicalization.The predicate is fail-open (treats every identity as live while the relay archive snapshot loads) and self-exempt, so a cold start never hides an identity and a user is never folded from their own client.