diff --git a/src/main/runtime/orca-runtime-close-mobile-session-tab.ts b/src/main/runtime/orca-runtime-close-mobile-session-tab.ts index fa624d6f3ce..d7c32c297d4 100644 --- a/src/main/runtime/orca-runtime-close-mobile-session-tab.ts +++ b/src/main/runtime/orca-runtime-close-mobile-session-tab.ts @@ -18,7 +18,6 @@ import { } from './mobile-session-tab-close-outcome' import { getRuntimeBrowserPageRegistry } from './runtime-browser-page-registry' import type { RuntimeCommandSurfaceHost } from './orca-runtime-core' -import { structuredAgentSessionTabId } from '../../shared/structured-agent-session-projection' import { SESSION_TAB_NOT_FOUND_ERROR } from '../../shared/session-tab-close' import { rendererPublicationThrottle } from '../window/renderer-publication-throttle' @@ -296,10 +295,8 @@ export class OrcaRuntimeWithCloseMobileSessionTab extends OrcaRuntimeWithRefuseU } else if (tab.type === 'agent-session') { if (this.notifier?.closeSessionTab) { try { - await this.notifier.closeSessionTab( - structuredAgentSessionTabId(tab.sessionId), - worktreeId - ) + // Why: a reopened chat's window tab id is not derivable from its session; the window maps ours. + await this.notifier.closeSessionTab(tab.id, worktreeId) } catch (error) { // The renderer already having removed the tab is an idempotent close, not a veto. if (!(error instanceof Error && error.message === SESSION_TAB_NOT_FOUND_ERROR)) { diff --git a/src/main/runtime/orca-runtime-structured-session-restore.test.ts b/src/main/runtime/orca-runtime-structured-session-restore.test.ts index d6ec1e22782..1d8105d07f8 100644 --- a/src/main/runtime/orca-runtime-structured-session-restore.test.ts +++ b/src/main/runtime/orca-runtime-structured-session-restore.test.ts @@ -307,10 +307,7 @@ describe('structured session cold restoration', () => { reason: 'user' }) - expect(closeSessionTab).toHaveBeenCalledWith( - 'structured-agent-session-restored-session', - 'workspace-1' - ) + expect(closeSessionTab).toHaveBeenCalledWith('agent-session:restored-session', 'workspace-1') expect(closeStructuredSession).toHaveBeenCalledWith('restored-session') expect(setSessionTabVisibility).toHaveBeenCalledWith('restored-session', false) expect(setSessionTabVisibility.mock.invocationCallOrder[0]).toBeLessThan( diff --git a/src/renderer/src/hooks/ipc-events/host-session-tab-target.ts b/src/renderer/src/hooks/ipc-events/host-session-tab-target.ts new file mode 100644 index 00000000000..86397dd464d --- /dev/null +++ b/src/renderer/src/hooks/ipc-events/host-session-tab-target.ts @@ -0,0 +1,13 @@ +import { LOCAL_STRUCTURED_SESSION_OWNER } from '@/runtime/local-structured-session-owner' +import { resolveLocalTabIdForHostSessionTab } from '@/runtime/web-session-tabs-sync/tracking-mappings' + +/** Main addresses its own session-tab ids; a mirrored chat tab lives here under a different id. */ +export function resolveWindowTabIdForHostTab(worktreeId: string, tabId: string): string { + return ( + resolveLocalTabIdForHostSessionTab({ + environmentId: LOCAL_STRUCTURED_SESSION_OWNER, + worktreeId, + hostTabId: tabId + }) ?? tabId + ) +} diff --git a/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts index 47464072034..7cd2b65efb9 100644 --- a/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge.ts @@ -13,6 +13,7 @@ import { } from '../../store/pinned-tab-close-guard' import { useAppStore } from '../../store' import { resolveBrowserSessionTabTarget } from './browser-session-tab-target' +import { resolveWindowTabIdForHostTab } from './host-session-tab-target' export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { unsubs.push( @@ -20,8 +21,9 @@ export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { if (isLocalSessionTabCloseOwned(worktreeId, tabId)) { return } + const localTabId = resolveWindowTabIdForHostTab(worktreeId, tabId) const store = useAppStore.getState() - const browserTarget = resolveBrowserSessionTabTarget(store, worktreeId, tabId) + const browserTarget = resolveBrowserSessionTabTarget(store, worktreeId, localTabId) if (browserTarget) { guardPinnedTabClose({ isPinned: isUnifiedTabPinned(store, worktreeId, browserTarget.workspaceId), @@ -31,11 +33,11 @@ export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { return } guardPinnedTabClose({ - isPinned: isUnifiedTabPinned(store, worktreeId, tabId), - tabLabel: resolvePinnedTabLabel(store, worktreeId, tabId), + isPinned: isUnifiedTabPinned(store, worktreeId, localTabId), + tabLabel: resolvePinnedTabLabel(store, worktreeId, localTabId), onClose: () => { const currentStore = useAppStore.getState() - closeMobileSessionTabInStore(currentStore, worktreeId, tabId) + closeMobileSessionTabInStore(currentStore, worktreeId, localTabId) } }) }) @@ -43,8 +45,9 @@ export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { unsubs.push( window.api.ui.onSessionTabCloseRequest(({ requestId, tabId, worktreeId, expiresAt }) => { + const localTabId = resolveWindowTabIdForHostTab(worktreeId, tabId) const store = useAppStore.getState() - const browserTarget = resolveBrowserSessionTabTarget(store, worktreeId, tabId) + const browserTarget = resolveBrowserSessionTabTarget(store, worktreeId, localTabId) let cancelConfirmation: (() => void) | undefined let timeout: ReturnType | undefined let settled = false @@ -78,7 +81,11 @@ export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { respond() return } - const closed = closeMobileSessionTabInStore(useAppStore.getState(), worktreeId, tabId) + const closed = closeMobileSessionTabInStore( + useAppStore.getState(), + worktreeId, + localTabId + ) respond(closed ? undefined : SESSION_TAB_NOT_FOUND_ERROR) } catch (error) { respond(error instanceof Error ? error.message : SESSION_TAB_CLOSE_FAILED_ERROR) @@ -92,7 +99,7 @@ export function registerSessionTabIpcBridge(unsubs: (() => void)[]): void { ) return } - const visibleId = browserTarget?.workspaceId ?? tabId + const visibleId = browserTarget?.workspaceId ?? localTabId cancelConfirmation = guardPinnedTabClose({ isPinned: isUnifiedTabPinned(store, worktreeId, visibleId), tabLabel: resolvePinnedTabLabel(store, worktreeId, visibleId), diff --git a/src/renderer/src/hooks/ipc-events/terminal-ui-routing-ipc-bridge.ts b/src/renderer/src/hooks/ipc-events/terminal-ui-routing-ipc-bridge.ts index 85460c88841..29ddc098726 100644 --- a/src/renderer/src/hooks/ipc-events/terminal-ui-routing-ipc-bridge.ts +++ b/src/renderer/src/hooks/ipc-events/terminal-ui-routing-ipc-bridge.ts @@ -9,6 +9,7 @@ import { activateTabAndFocusPane } from '@/lib/activate-tab-and-focus-pane' import { useAppStore } from '../../store' import type { AppState } from '../../store/types' import { resolveBrowserSessionTabTarget } from './browser-session-tab-target' +import { resolveWindowTabIdForHostTab } from './host-session-tab-target' import { activateTerminalInitiatedWorktree, focusTerminalInitiatedTab @@ -146,9 +147,12 @@ export function registerTerminalUiRoutingIpcBridge(unsubs: (() => void)[]): void unsubs.push( window.api.ui.onFocusEditorTab(({ tabId, worktreeId, userInitiated }) => { + const localTabId = resolveWindowTabIdForHostTab(worktreeId, tabId) const store = useAppStore.getState() - const tab = (store.unifiedTabsByWorktree[worktreeId] ?? []).find((item) => item.id === tabId) - const browserTarget = resolveBrowserSessionTabTarget(store, worktreeId, tabId) + const tab = (store.unifiedTabsByWorktree[worktreeId] ?? []).find( + (item) => item.id === localTabId + ) + const browserTarget = resolveBrowserSessionTabTarget(store, worktreeId, localTabId) // Why: chat-completion focus is a courtesy reveal, not navigation — never yank the user // back into a workspace they deliberately left. A notification click is the opposite: the // user asked for this workspace, and the activateWorktree that precedes it is async, so diff --git a/src/renderer/src/runtime/structured-agent-session-tab-retirement.test.ts b/src/renderer/src/runtime/structured-agent-session-tab-retirement.test.ts index 7963b1c150e..5c3017998c0 100644 --- a/src/renderer/src/runtime/structured-agent-session-tab-retirement.test.ts +++ b/src/renderer/src/runtime/structured-agent-session-tab-retirement.test.ts @@ -84,7 +84,6 @@ describe('structured agent session tab retirement', () => { beginStructuredAgentSessionTabClose({ target, worktreeId: 'wt-1', - tabId: 'agent-tab-1', sessionId: 'session-1', provisional: true }) @@ -119,13 +118,11 @@ describe('structured agent session tab retirement', () => { retireStructuredAgentSessionTab({ target, worktreeId: 'wt-1', - tabId: 'agent-tab-1', sessionId: 'session-1' }) retireStructuredAgentSessionTab({ target, worktreeId: 'wt-1', - tabId: 'agent-tab-1', sessionId: 'session-1' }) expect(mocks.closeSession).toHaveBeenCalledTimes(1) diff --git a/src/renderer/src/runtime/structured-agent-session-tab-retirement.ts b/src/renderer/src/runtime/structured-agent-session-tab-retirement.ts index 8489aa0e6a6..567e7459603 100644 --- a/src/renderer/src/runtime/structured-agent-session-tab-retirement.ts +++ b/src/renderer/src/runtime/structured-agent-session-tab-retirement.ts @@ -19,7 +19,6 @@ function retirementKey(target: RuntimeClientTarget, worktreeId: string, sessionI export function retireStructuredAgentSessionTab(args: { target: RuntimeClientTarget worktreeId: string - tabId: string sessionId: string onError?: (error: unknown) => void }): void { @@ -28,11 +27,13 @@ export function retireStructuredAgentSessionTab(args: { if (existing) { return } + const hostTabId = `agent-session:${args.sessionId}` + // Why: main echoes the host tab id it was asked to close, never this window's tab id. const closeHostTab = () => - withLocalSessionTabCloseOwner(args.worktreeId, args.tabId, () => + withLocalSessionTabCloseOwner(args.worktreeId, hostTabId, () => callRuntimeRpc(args.target, 'session.tabs.close', { worktree: toRuntimeWorktreeSelector(args.worktreeId), - tabId: `agent-session:${args.sessionId}`, + tabId: hostTabId, reason: 'user' }) ) @@ -58,7 +59,6 @@ export function retireStructuredAgentSessionTab(args: { export function beginStructuredAgentSessionTabClose(args: { target: RuntimeClientTarget worktreeId: string - tabId: string sessionId: string provisional: boolean onError?: (error: unknown) => void @@ -92,7 +92,6 @@ export function suppressCancelledStructuredSessionTabs( retireStructuredAgentSessionTab({ target, worktreeId: snapshot.worktree, - tabId: `agent-session:${sessionId}`, sessionId, onError }) diff --git a/src/renderer/src/runtime/web-session-tabs-sync/tracking-mappings.ts b/src/renderer/src/runtime/web-session-tabs-sync/tracking-mappings.ts index 83f8633f253..ed6ba998a1e 100644 --- a/src/renderer/src/runtime/web-session-tabs-sync/tracking-mappings.ts +++ b/src/renderer/src/runtime/web-session-tabs-sync/tracking-mappings.ts @@ -73,3 +73,20 @@ export function hostSessionTabIdsByLocalTabForWorktree( } return entries } + +/** The local tab mirroring a host tab, or null when no mirrored tab claims it. */ +export function resolveLocalTabIdForHostSessionTab(args: { + environmentId: string + worktreeId: string + hostTabId: string +}): string | null { + for (const [tabId, hostTabId] of hostSessionTabIdsByLocalTabForWorktree( + args.environmentId, + args.worktreeId + )) { + if (hostTabId === args.hostTabId) { + return tabId + } + } + return null +} diff --git a/src/renderer/src/store/slices/tabs/tabs-close-actions.ts b/src/renderer/src/store/slices/tabs/tabs-close-actions.ts index 35a7be70ced..b9187c8b0d8 100644 --- a/src/renderer/src/store/slices/tabs/tabs-close-actions.ts +++ b/src/renderer/src/store/slices/tabs/tabs-close-actions.ts @@ -64,7 +64,6 @@ export function createTabsCloseActions( activeRuntimeEnvironmentId: getRuntimeEnvironmentIdForWorktree(state, worktreeId) }), worktreeId, - tabId: tab.id, sessionId: tab.entityId, provisional }) diff --git a/tests/e2e/structured-session-reopened-tab-close.unit.test.ts b/tests/e2e/structured-session-reopened-tab-close.unit.test.ts new file mode 100644 index 00000000000..35547145c32 --- /dev/null +++ b/tests/e2e/structured-session-reopened-tab-close.unit.test.ts @@ -0,0 +1,231 @@ +// @vitest-environment happy-dom + +/** + * After /clear the current chat keeps the tab id derived from its first session, so reopening that + * first session from history lands in a suffixed tab. Closing or focusing the reopened tab from + * main must reach that tab, not the current chat. Runs the real main close/focus path against the + * real window store, mirror and IPC bridges; only the process boundary is stubbed. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeMobileSessionTabsResult } from '../../src/shared/runtime-types' +import type * as RuntimeRpcClient from '../../src/renderer/src/runtime/runtime-rpc-client' +import { OrcaRuntimeService } from '../../src/main/runtime/orca-runtime' +import { setStructuredAgentSessionHost } from '../../src/main/native-chat/agent-session-wire/structured-agent-session-registry' +import { useAppStore } from '../../src/renderer/src/store' +import { applyStructuredSessionTabSnapshots } from '../../src/renderer/src/runtime/local-structured-session-tabs-sync/snapshot-apply' +import { resetLocalStructuredSessionVersionForTests } from '../../src/renderer/src/runtime/local-structured-session-tabs-sync' +import { resetWebSessionTabsSnapshotFreshnessForTests } from '../../src/renderer/src/runtime/web-session-tabs-sync' +import { registerSessionTabIpcBridge } from '../../src/renderer/src/hooks/ipc-events/session-tab-ipc-bridge' +import { registerTerminalUiRoutingIpcBridge } from '../../src/renderer/src/hooks/ipc-events/terminal-ui-routing-ipc-bridge' + +const WORKTREE = 'repo-1::/tmp/wt-reopen' +const CURRENT_SESSION = 'clear-x' +const REOPENED_SESSION = 'session-s' +const CURRENT_TAB = `structured-agent-session-${REOPENED_SESSION}` +const REOPENED_TAB = `${CURRENT_TAB}:history-1` + +type CloseRequest = { requestId: string; tabId: string; worktreeId: string; expiresAt?: number } +type Listener = (payload: T) => void + +type ProcessBoundary = { + runtime: Pick | null + rendererSessionCloses: string[] +} + +const bridge = vi.hoisted((): ProcessBoundary => ({ + runtime: null, + rendererSessionCloses: [] +})) + +vi.mock('../../src/renderer/src/runtime/runtime-rpc-client', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + callRuntimeRpc: async ( + _target: unknown, + method: string, + params: { worktree: string; tabId: string; reason?: 'user' } + ) => { + if (method !== 'session.tabs.close') { + throw new Error(`unexpected rpc ${method}`) + } + return bridge.runtime!.closeMobileSessionTab(params.worktree, params.tabId, { + reason: params.reason + }) + } + } +}) + +vi.mock('../../src/renderer/src/runtime/structured-agent-session-close', () => ({ + closeStructuredAgentSession: async (_target: unknown, sessionId: string) => { + bridge.rendererSessionCloses.push(sessionId) + return 'closed' + } +})) + +async function setup() { + const listeners: { + closeSessionTab?: Listener<{ tabId: string; worktreeId: string }> + closeRequest?: Listener + focusEditorTab?: Listener<{ tabId: string; worktreeId: string; userInitiated?: boolean }> + } = {} + const pendingResponses = new Map void>() + const register = + (assign: (listener: Listener) => void) => + (listener: Listener) => { + assign(listener) + return () => {} + } + const noop = () => () => {} + vi.stubGlobal('api', { + ui: { + onCloseSessionTab: register((l) => (listeners.closeSessionTab = l)), + onSessionTabCloseRequest: register((l) => (listeners.closeRequest = l)), + onFocusEditorTab: register((l) => (listeners.focusEditorTab = l)), + onMoveSessionTab: noop, + onSplitTerminal: noop, + onRenameTerminal: noop, + onFocusTerminal: noop, + respondSessionTabClose: ({ requestId, error }: { requestId: string; error?: string }) => + pendingResponses.get(requestId)?.(error) + } + }) + registerSessionTabIpcBridge([]) + registerTerminalUiRoutingIpcBridge([]) + + const hostSessionCloses: string[] = [] + const closeResponses: (string | undefined)[] = [] + const runtime = new OrcaRuntimeService() + bridge.runtime = runtime + let nextRequest = 0 + runtime.setNotifier( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the chat close and focus paths call only these two notifier members. + { + // Same contract as the window relay: a request the renderer answers with an optional error. + closeSessionTab: (tabId: string, worktreeId: string) => + new Promise((resolve, reject) => { + const requestId = `close-${++nextRequest}` + pendingResponses.set(requestId, (error) => { + closeResponses.push(error) + return error ? reject(new Error(error)) : resolve() + }) + listeners.closeRequest!({ requestId, tabId, worktreeId }) + }), + focusEditorTab: (tabId: string, worktreeId: string) => + listeners.focusEditorTab!({ tabId, worktreeId }) + } as never + ) + // No durable store, PTYs or workspace session on disk: the host tab list below is the whole state. + Object.assign(runtime, { + hasPersistedStructuredAgentSessionStore: () => true, + getKnownWorkspaceSessionWorktreeIds: () => new Set(), + hydrateHeadlessMobileSessionTabsFromWorkspaceSession: () => new Set(), + refreshMobileSessionPtyRecords: async () => new Set(), + ensureStructuredAgentSessionHost: async () => undefined + }) + setStructuredAgentSessionHost( + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: restore and tab close call only these host members. + { + reconcileRestartLeases: async () => undefined, + restoreReadableSessions: async () => undefined, + close: async (sessionId: string) => { + hostSessionCloses.push(sessionId) + }, + setSessionTabVisibility: async () => undefined, + listSessionTabs: () => [ + { sessionId: CURRENT_SESSION, workspaceId: WORKTREE, agent: 'claude' }, + { sessionId: REOPENED_SESSION, workspaceId: WORKTREE, agent: 'claude' } + ] + } as never + ) + await runtime.restoreStructuredAgentSessionTabs() + const hostSnapshot: RuntimeMobileSessionTabsResult = await runtime.listMobileSessionTabs( + `id:${WORKTREE}` + ) + + // The current chat after /clear: first session's tab id, new session's entity. + useAppStore.setState({ activeWorktreeId: WORKTREE }) + useAppStore.getState().createUnifiedTab(WORKTREE, 'agent-session', { + id: CURRENT_TAB, + entityId: CURRENT_SESSION, + label: 'Claude Chat' + }) + applyStructuredSessionTabSnapshots([hostSnapshot]) + + const agentTabs = () => + (useAppStore.getState().unifiedTabsByWorktree[WORKTREE] ?? []) + .filter((tab) => tab.contentType === 'agent-session') + .map((tab) => ({ id: tab.id, entityId: tab.entityId })) + return { runtime, agentTabs, hostSessionCloses, closeResponses } +} + +const initialStoreState = useAppStore.getState() + +beforeEach(() => { + bridge.rendererSessionCloses = [] +}) + +afterEach(() => { + setStructuredAgentSessionHost(null) + bridge.runtime = null + useAppStore.setState(initialStoreState, true) + resetLocalStructuredSessionVersionForTests() + resetWebSessionTabsSnapshotFreshnessForTests() + vi.unstubAllGlobals() +}) + +describe('closing a chat reopened from history after /clear', () => { + it('mirrors the reopened session into a suffixed tab beside the current chat', async () => { + const { agentTabs } = await setup() + + expect(agentTabs()).toEqual( + expect.arrayContaining([ + { id: CURRENT_TAB, entityId: CURRENT_SESSION }, + { id: REOPENED_TAB, entityId: REOPENED_SESSION } + ]) + ) + }) + + it('closes only the reopened tab when the user closes it on the desktop', async () => { + const { agentTabs, hostSessionCloses, closeResponses } = await setup() + + useAppStore.getState().closeUnifiedTab(REOPENED_TAB) + + await vi.waitFor(() => expect(hostSessionCloses).toEqual([REOPENED_SESSION])) + // Let any stray echo reach the window before asserting the current chat survived. + await new Promise((resolve) => setTimeout(resolve, 0)) + expect(agentTabs()).toEqual([{ id: CURRENT_TAB, entityId: CURRENT_SESSION }]) + expect(bridge.rendererSessionCloses).toEqual([REOPENED_SESSION]) + expect(hostSessionCloses).toEqual([REOPENED_SESSION]) + // The window already removed the tab, so main's relay is acknowledged, not reported missing. + expect(closeResponses).toEqual([undefined]) + }) + + it('closes only the reopened tab when a phone closes it through the runtime', async () => { + const { runtime, agentTabs, hostSessionCloses } = await setup() + + await runtime.closeMobileSessionTab(`id:${WORKTREE}`, `agent-session:${REOPENED_SESSION}`, { + reason: 'user' + }) + + await vi.waitFor(() => + expect(agentTabs()).toEqual([{ id: CURRENT_TAB, entityId: CURRENT_SESSION }]) + ) + expect(bridge.rendererSessionCloses).not.toContain(CURRENT_SESSION) + expect(hostSessionCloses).not.toContain(CURRENT_SESSION) + }) + + it('focuses the reopened tab when a phone activates it through the runtime', async () => { + const { runtime } = await setup() + useAppStore.getState().activateTab(CURRENT_TAB, { worktreeId: WORKTREE }) + + await runtime.activateMobileSessionTab(`id:${WORKTREE}`, `agent-session:${REOPENED_SESSION}`) + + const state = useAppStore.getState() + const group = state.groupsByWorktree[WORKTREE]?.find((candidate) => + candidate.tabOrder.includes(REOPENED_TAB) + ) + expect(group?.activeTabId).toBe(REOPENED_TAB) + }) +})