From cee60ce18d6b995b5ffd0b2e73005beaee6d0d0d Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Tue, 1 Sep 2026 22:36:29 -0500 Subject: [PATCH] fix(desktop): resolve a branch parent's owner from every list that names one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit branchStoredSession looked its parent up in $sessions alone, and branchCurrentSession did the same. A conversation reachable through a profile-scoped project tree has no row there, and when it appears in both places the flat Recents copy is the ownerless one — so the lookup returned the row that cannot route, and the branch created its child on whichever backend happened to be active. cachedSessionRow spans Recents, cron, messaging and the project tree, and prefers the self-describing candidate. One ladder, used by both branch entry points and by resolveStoredSession. Co-authored-by: evan-bradford --- .../hooks/use-session-actions/index.ts | 11 ++-- .../resolve-stored-session.test.ts | 52 ++++++++++++++++++- .../hooks/use-session-actions/utils.ts | 37 +++++++++++-- 3 files changed, 91 insertions(+), 9 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts index 297e094807..46aa5f67ec 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/index.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/index.ts @@ -148,6 +148,7 @@ import { applyRuntimeInfo, applyStoredSessionPreviewRuntimeInfo, type BranchMessage, + cachedSessionRow, chatMessageArraysEquivalent, dedupeInflightUserAgainstTranscript, dropListedSession, @@ -2220,9 +2221,7 @@ export function useSessionActions({ // Same contract as branchStoredSession: the transcript read and the // branch RPC must both land on the backend that owns the parent, not on // whichever socket is active. - const ownerRoute = storedSessionId - ? sessionOwnerRouteFromRow($sessions.get().find(session => sessionMatchesStoredId(session, storedSessionId))) - : undefined + const ownerRoute = storedSessionId ? sessionOwnerRouteFromRow(cachedSessionRow(storedSessionId)) : undefined if (storedSessionId) { try { @@ -2291,9 +2290,11 @@ export function useSessionActions({ // Right-clicking a session outside the paginated sidebar window is a cache // miss: resolve it (cache → active backend → cross-profile) so the branch // is created on the parent's OWNING profile, not whichever is live (#67603). + // cachedSessionRow spans Recents, cron/messaging and the profile-scoped + // project tree, and prefers the self-describing row — an ownerless legacy + // Recents copy of the same id must not mask the row carrying the owner. const stored = - $sessions.get().find(session => sessionMatchesStoredId(session, storedSessionId)) ?? - (sessionProfile ? undefined : await resolveStoredSession(storedSessionId)) + cachedSessionRow(storedSessionId) ?? (sessionProfile ? undefined : await resolveStoredSession(storedSessionId)) const profile = sessionProfile ?? stored?.profile diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/resolve-stored-session.test.ts b/apps/desktop/src/app/session/hooks/use-session-actions/resolve-stored-session.test.ts index 468936f5b6..cccb323317 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/resolve-stored-session.test.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/resolve-stored-session.test.ts @@ -3,10 +3,11 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type * as HermesModule from '@/hermes' import { getSession } from '@/hermes' import { $activeGatewayProfile, $profiles } from '@/store/profile' +import { $projectTree } from '@/store/projects' import { $cronSessions, $messagingSessions, $sessions } from '@/store/session' import type { SessionInfo } from '@/types/hermes' -import { resolveSessionProfile, resolveStoredSession } from './utils' +import { cachedSessionRow, resolveSessionProfile, resolveStoredSession } from './utils' vi.mock('@/hermes', async importActual => ({ ...(await importActual()), @@ -24,6 +25,7 @@ describe('resolveStoredSession profile ownership', () => { $cronSessions.set([]) $messagingSessions.set([]) $sessions.set([]) + $projectTree.set([]) $profiles.set(profiles('default', 'meta')) $activeGatewayProfile.set('meta') mockGetSession.mockReset() @@ -33,6 +35,7 @@ describe('resolveStoredSession profile ownership', () => { $cronSessions.set([]) $messagingSessions.set([]) $sessions.set([]) + $projectTree.set([]) $profiles.set([]) $activeGatewayProfile.set('default') }) @@ -144,3 +147,50 @@ describe('resolveStoredSession profile ownership', () => { await expect(resolveSessionProfile('s1')).resolves.toBe('default') }) }) + +describe('cachedSessionRow owner preference', () => { + const projectNode = (sessions: SessionInfo[], preview: SessionInfo[] = []) => + ({ + previewSessions: preview, + repos: [{ groups: [{ sessions }] }] + }) as never + + beforeEach(() => { + $cronSessions.set([]) + $messagingSessions.set([]) + $sessions.set([]) + $projectTree.set([]) + mockGetSession.mockReset() + }) + + afterEach(() => { + $cronSessions.set([]) + $messagingSessions.set([]) + $sessions.set([]) + $projectTree.set([]) + }) + + it('prefers a self-describing project-tree row over an ownerless Recents duplicate', () => { + // The same conversation, listed twice: a legacy Recents row with no owner + // and the profile-scoped project-tree row the gateway stamped. Picking the + // Recents copy throws away the only routing information there is, and the + // branch then creates its child on whichever backend is active. + $sessions.set([session({ cwd: '/wrong', id: 's1' })]) + $projectTree.set([projectNode([session({ connection_id: 'pandora', cwd: '/right', id: 's1', profile: 'work' })])]) + + expect(cachedSessionRow('s1')).toMatchObject({ connection_id: 'pandora', cwd: '/right', profile: 'work' }) + }) + + it('finds a project-tree preview row when the session is in no other list', () => { + $projectTree.set([projectNode([], [session({ connection_id: 'rigremote', id: 's1', profile: 'default' })])]) + + expect(cachedSessionRow('s1')).toMatchObject({ connection_id: 'rigremote' }) + }) + + it('keeps the plain Recents row when nothing carries an owner', () => { + $sessions.set([session({ cwd: '/only', id: 's1' })]) + + expect(cachedSessionRow('s1')).toMatchObject({ cwd: '/only' }) + expect(cachedSessionRow('missing')).toBeUndefined() + }) +}) diff --git a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts index 83e599b589..113539132b 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts +++ b/apps/desktop/src/app/session/hooks/use-session-actions/utils.ts @@ -8,6 +8,7 @@ import { isMessagingSource, normalizeSessionSource } from '@/lib/session-source' import { reconcileApprovalModeForProfile } from '@/store/approval-mode' import { requestDesktopOnboardingForCredentialWarning } from '@/store/onboarding' import { $activeGatewayProfile, $profiles, normalizeProfileKey } from '@/store/profile' +import { $projectTree } from '@/store/projects' import { $cronSessions, $currentCwd, @@ -1387,13 +1388,43 @@ function upsertResolvedSession(session: SessionInfo, storedSessionId: string) { ]) } +// Every session row reachable through the profile-scoped project tree — +// preview rows on a collapsed project plus the drill-in lane rows. These are +// the only rows guaranteed to name their owning profile (the gateway stamps +// the request scope onto them), so owner resolution has to see them. +function projectTreeSessions(): SessionInfo[] { + return $projectTree + .get() + .flatMap(project => [ + ...(project.previewSessions ?? []), + ...project.repos.flatMap(repo => repo.groups.flatMap(group => group.sessions)) + ]) +} + +// The best cached row for a stored id, across every list that can hold one. +// "Best" means self-describing: the same conversation can appear both as an +// ownerless legacy Recents copy and as a profile-stamped project-tree row, and +// picking the ownerless one throws away the only routing information we have. +export function cachedSessionRow(storedSessionId: string): SessionInfo | undefined { + const candidates = [ + ...$sessions.get(), + ...$cronSessions.get(), + ...$messagingSessions.get(), + ...projectTreeSessions() + ].filter(session => sessionMatchesStoredId(session, storedSessionId)) + + return ( + candidates.find(session => session.connection_id?.trim()) ?? + candidates.find(session => session.profile?.trim()) ?? + candidates[0] + ) +} + export async function resolveStoredSession( storedSessionId: string, ownerRoute?: SessionProfileRoute ): Promise { - const cached = [...$sessions.get(), ...$cronSessions.get(), ...$messagingSessions.get()].find(session => - sessionMatchesStoredId(session, storedSessionId) - ) + const cached = cachedSessionRow(storedSessionId) if (ownerRoute) { const scope = {