From 1c7ec93aea90f77124a821f08392936d975b44b6 Mon Sep 17 00:00:00 2001 From: Benjamin Brumbaugh Date: Fri, 4 Sep 2026 12:50:21 -0700 Subject: [PATCH] fix(desktop): preserve branch compatibility with older gateways --- .../hooks/use-session-actions.test.tsx | 77 +++++++++++++------ .../hooks/use-session-actions/index.ts | 62 +++++++++++---- tui_gateway/methods_session.py | 39 +++++++++- 3 files changed, 138 insertions(+), 40 deletions(-) diff --git a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx index b403069409..437303416f 100644 --- a/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx +++ b/apps/desktop/src/app/session/hooks/use-session-actions.test.tsx @@ -219,7 +219,7 @@ describe('desktop branch creation idempotency', () => { const createReady = deferred<{ session_id: string; stored_session_id: string }>() const requestGateway = vi.fn(async (method: string, params?: Record) => { - if (method === 'session.create') { + if (method === 'session.branch_stored') { return createReady.promise as never } @@ -249,7 +249,7 @@ describe('desktop branch creation idempotency', () => { }) await waitFor(() => - expect(requestGateway.mock.calls.filter(([method]) => method === 'session.create')).toHaveLength(1) + expect(requestGateway.mock.calls.filter(([method]) => method === 'session.branch_stored')).toHaveLength(1) ) await act(async () => { @@ -257,9 +257,9 @@ describe('desktop branch creation idempotency', () => { await expect(Promise.all([first, second])).resolves.toEqual([true, true]) }) - expect(requestGateway.mock.calls.filter(([method]) => method === 'session.create')).toHaveLength(1) + expect(requestGateway.mock.calls.filter(([method]) => method === 'session.branch_stored')).toHaveLength(1) expect(requestGateway).toHaveBeenCalledWith( - 'session.create', + 'session.branch_stored', expect.objectContaining({ copy_parent_history: true, omit_messages: true, @@ -283,7 +283,7 @@ describe('desktop branch creation idempotency', () => { const otherCreate = deferred<{ session_id: string; stored_session_id: string }>() routedCreate.mockImplementation((async (connectionId: string, _profile: string, method: string) => { - if (method !== 'session.create') { + if (method !== 'session.branch_stored') { return {} as never } @@ -317,7 +317,7 @@ describe('desktop branch creation idempotency', () => { await act(async () => { second = actions!.branchStoredSession('parent') await waitFor(() => - expect(routedCreate.mock.calls.filter(([, , method]) => method === 'session.create')).toHaveLength(2) + expect(routedCreate.mock.calls.filter(([, , method]) => method === 'session.branch_stored')).toHaveLength(2) ) }) @@ -327,7 +327,7 @@ describe('desktop branch creation idempotency', () => { await expect(Promise.all([first, second])).resolves.toEqual([true, true]) }) - const creates = routedCreate.mock.calls.filter(([, , method]) => method === 'session.create') + const creates = routedCreate.mock.calls.filter(([, , method]) => method === 'session.branch_stored') expect(creates.map(([connectionId]) => connectionId)).toEqual(['pandora', 'other-box']) // Two distinct children, not one child claimed twice. @@ -672,7 +672,7 @@ async function createWith( let createParams: Record | undefined const requestGateway = vi.fn(async (method: string, params?: Record) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { createParams = params return { session_id: RUNTIME_SESSION_ID, stored_session_id: null } as never @@ -1952,7 +1952,7 @@ describe('branchStoredSession desktop source tagging', () => { it('opens the branch as the primary session in the main workspace (#93444)', async () => { const requestGateway = vi.fn(async (method: string) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never } @@ -2005,7 +2005,7 @@ describe('branchStoredSession desktop source tagging', () => { it('keeps the current view when branching a different session from the sidebar (does not reintroduce #69750)', async () => { const requestGateway = vi.fn(async (method: string) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never } @@ -2068,7 +2068,7 @@ describe('branchStoredSession desktop source tagging', () => { _profile: string, method: string ) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never } @@ -2090,10 +2090,10 @@ describe('branchStoredSession desktop source tagging', () => { await expect(branchStoredSession!('stored-parent')).resolves.toBe(true) // The create must ride the parent's own (connection, profile) socket... - expect(requestGatewayForAgent).toHaveBeenCalledWith( + expect(requestGatewayForAgent).toHaveBeenLastCalledWith( 'pandora', 'default', - 'session.create', + 'session.branch_stored', expect.objectContaining({ parent_session_id: 'stored-parent', source: 'desktop' }) ) // ...and never the ambient socket, which may serve a different machine. @@ -2108,7 +2108,7 @@ describe('branchStoredSession desktop source tagging', () => { _profile: string, method: string ) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never } @@ -2124,10 +2124,10 @@ describe('branchStoredSession desktop source tagging', () => { await expect(branchStoredSession!('stored-parent')).resolves.toBe(true) - expect(requestGatewayForAgent).toHaveBeenCalledWith( + expect(requestGatewayForAgent).toHaveBeenLastCalledWith( 'pandora', 'default', - 'session.create', + 'session.branch_stored', expect.objectContaining({ copy_parent_history: true, omit_messages: true, @@ -2149,7 +2149,7 @@ describe('branchStoredSession desktop source tagging', () => { _profile: string, method: string ) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never } @@ -2182,7 +2182,7 @@ describe('branchStoredSession desktop source tagging', () => { let createParams: Record | undefined const ambientRequest = vi.fn(async (method: string, params?: Record) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { createParams = params return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never @@ -2263,7 +2263,7 @@ describe('branchStoredSession desktop source tagging', () => { let branchParams: Record | undefined const requestGateway = vi.fn(async (method: string, params?: Record) => { - if (method === 'session.branch') { + if (method === 'session.branch_whole') { branchParams = params return { @@ -2354,7 +2354,7 @@ describe('branchStoredSession desktop source tagging', () => { let createParams: Record | undefined const requestGateway = vi.fn(async (method: string, params?: Record) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { createParams = params return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never @@ -2391,7 +2391,7 @@ describe('branchStoredSession desktop source tagging', () => { let createParams: Record | undefined const requestGateway = vi.fn(async (method: string, params?: Record) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { createParams = params return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never @@ -2420,7 +2420,7 @@ describe('branchStoredSession desktop source tagging', () => { let createParams: Record | undefined const requestGateway = vi.fn(async (method: string, params?: Record) => { - if (method === 'session.create') { + if (method === 'session.create' || method === 'session.branch_stored') { createParams = params return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never @@ -2438,6 +2438,39 @@ describe('branchStoredSession desktop source tagging', () => { expect(createParams).toBeDefined() expect(createParams).not.toHaveProperty('profile') }) + + it('falls back without creating an empty branch against an older backend', async () => { + const requestGateway = vi.fn(async (method: string, params?: Record) => { + if (method === 'session.branch_stored') { + throw new Error('unknown method: session.branch_stored') + } + + if (method === 'session.create') { + expect(params?.messages).toEqual([{ content: 'persisted context', role: 'user' }]) + + return { session_id: 'branch-runtime', stored_session_id: 'branch-stored' } as never + } + + return {} as never + }) + + vi.mocked(getAllSessionMessages).mockResolvedValue({ + messages: [{ content: 'persisted context', role: 'user', timestamp: 1 }], + session_id: 'stored-parent' + } as never) + + let branchStoredSession: ((storedSessionId: string) => Promise) | null = null + render( (branchStoredSession = branch)} requestGateway={requestGateway} />) + await waitFor(() => expect(branchStoredSession).not.toBeNull()) + + await expect(branchStoredSession!('stored-parent')).resolves.toBe(true) + expect(requestGateway).toHaveBeenCalledWith( + 'session.create', + expect.objectContaining({ + parent_session_id: 'stored-parent' + }) + ) + }) }) // ── Main/tile dedup (the "same session open in main AND its own tab" bug) ───── 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 e820c44df2..abe2d346bc 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 @@ -2360,23 +2360,57 @@ export function useSessionActions({ // No title: the backend auto-names the branch from its parent's lineage. if (!createFlight) { + const branchParams = { + session_id: sourceSessionId, + omit_messages: true, + ...(branchCount !== undefined ? { count: branchCount } : {}) + } + + const createParams = { + cols: 96, + copy_parent_history: true, + omit_messages: true, + source: 'desktop', + ...(cwd && { cwd }), + ...(profile ? { profile } : {}), + ...(parentStoredId && { parent_session_id: parentStoredId }) + } + createFlight = ( sourceSessionId - ? requestBranchGateway('session.branch', { - session_id: sourceSessionId, - omit_messages: true, - ...(branchCount !== undefined ? { count: branchCount } : {}) - }) - : requestBranchGateway('session.create', { - cols: 96, - source: 'desktop', - ...(cwd && { cwd }), - ...(profile ? { profile } : {}), - ...(branchMessages.length - ? { messages: branchMessages.map(({ content, role }) => ({ content, role })) } - : { copy_parent_history: true, omit_messages: true }), - ...(parentStoredId && { parent_session_id: parentStoredId }) + ? requestBranchGateway( + branchCount === undefined ? 'session.branch_whole' : 'session.branch', + branchParams + ).catch(err => { + if (!isMissingRpcMethod(err)) { + throw err + } + + return requestBranchGateway('session.branch', branchParams) }) + : branchMessages.length + ? requestBranchGateway('session.create', { + ...createParams, + messages: branchMessages.map(({ content, role }) => ({ content, role })) + }) + : requestBranchGateway('session.branch_stored', createParams).catch( + async err => { + if (!isMissingRpcMethod(err)) { + throw err + } + + const { messages } = await getAllSessionMessages(parentStoredId ?? '', ownerRoute ?? profile) + + if (!messages.length) { + throw new Error('nothing to branch — send a message first') + } + + return requestBranchGateway('session.create', { + ...createParams, + messages: messages.map(({ content, role }) => ({ content, role })) + }) + } + ) ).catch(err => { // Drop the flight so a genuine retry re-issues the create; a // resolved flight is cleared once the child is fully published. diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index ef397fb42e..6af42fd87b 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -331,8 +331,7 @@ def _create_overrides(params: dict) -> tuple: return model_override, reasoning_override, service_tier_override -@method("session.create") -def _(rid, params: dict) -> dict: +def _create_session(rid, params: dict) -> dict: # ``profile`` (app-global remote mode): stored so the build and every turn re-bind HERMES_HOME. profile_home = _profile_home(profile := (params.get("profile") or "").strip() or None) # Reject an incoherent model×provider pair BEFORE any state exists: minting it only defers the @@ -429,6 +428,25 @@ def _(rid, params: dict) -> dict: "profile_name": _response_profile_name(profile)}}) +@method("session.create") +def _(rid, params: dict) -> dict: + return _create_session(rid, params) + + +@method("session.branch_stored") +def _(rid, params: dict) -> dict: + """Create a whole-session branch from a stored parent without routing its transcript through the client. + + This is a distinct method, rather than an overloaded ``session.create`` flag, so older Desktop clients can + safely detect the capability and older gateways cannot silently create an empty branch when they ignore an + unknown parameter. + """ + branch_params = dict(params) + branch_params["copy_parent_history"] = True + branch_params["omit_messages"] = True + return _create_session(rid, branch_params) + + def _unarchive_recoverable(db, session_id: str) -> bool: """``unarchive_recoverable_session`` that works on a read-only listing handle (foreign profile): the rare write escalates to a short-lived registry writer instead of writing on the reader.""" @@ -2063,8 +2081,7 @@ def _branch_source_history(db, session: dict, old_key: str) -> list: return history or _visible_branch_history(in_memory_history) -@_session_method("session.branch", live=True) -def _(rid, params: dict, session: dict) -> dict: +def _branch_live(rid, params: dict, session: dict) -> dict: # Write into the parent's profile-scoped state.db; the launch handle would orphan rows. with _session_db(session) as db: if db is None: @@ -2099,6 +2116,20 @@ def _(rid, params: dict, session: dict) -> dict: return _ok(rid, response) +@_session_method("session.branch", live=True) +def _(rid, params: dict, session: dict) -> dict: + return _branch_live(rid, params, session) + + +@_session_method("session.branch_whole", live=True) +def _(rid, params: dict, session: dict) -> dict: + """Whole-session branch response seam for Desktop clients that do not need the copied transcript echoed back.""" + branch_params = dict(params) + branch_params.pop("count", None) + branch_params["omit_messages"] = True + return _branch_live(rid, branch_params, session) + + # ── interrupt / steer / redirect ───────────────────────────────────── @method("session.interrupt") def _(rid, params: dict) -> dict: