fix(desktop): resolve a branch parent's owner from every list that names one
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 <evan-bradford@users.noreply.github.com>
This commit is contained in:
committed by
brooklyn!
parent
208549432e
commit
cee60ce18d
@@ -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
|
||||
|
||||
|
||||
+51
-1
@@ -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<typeof HermesModule>()),
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<SessionInfo | undefined> {
|
||||
const cached = [...$sessions.get(), ...$cronSessions.get(), ...$messagingSessions.get()].find(session =>
|
||||
sessionMatchesStoredId(session, storedSessionId)
|
||||
)
|
||||
const cached = cachedSessionRow(storedSessionId)
|
||||
|
||||
if (ownerRoute) {
|
||||
const scope = {
|
||||
|
||||
Reference in New Issue
Block a user