diff --git a/src/main/ipc/remote-workspace.test.ts b/src/main/ipc/remote-workspace.test.ts index 56eb4804a82..9a25ada9ec7 100644 --- a/src/main/ipc/remote-workspace.test.ts +++ b/src/main/ipc/remote-workspace.test.ts @@ -154,9 +154,10 @@ describe('remoteWorkspace:setForConnectedTargets', () => { const getRepoMock = vi.fn() const getWorkspaceSessionMock = vi.fn() // Ownership resolution reads the catalog, not one id-keyed row, so the fake has to project one. + const getReposMock = vi.fn(() => [getRepoMock('repo-target-1')].filter(Boolean)) const store = { getRepo: getRepoMock, - getRepos: () => [getRepoMock('repo-target-1')].filter(Boolean), + getRepos: getReposMock, getWorkspaceSession: getWorkspaceSessionMock } as unknown as Store @@ -176,6 +177,7 @@ describe('remoteWorkspace:setForConnectedTargets', () => { getTarget: (targetId: string) => targets.find((target) => target.id === targetId) }) getRepoMock.mockReset() + getReposMock.mockClear() getWorkspaceSessionMock.mockReset() getWorkspaceSessionMock.mockReturnValue(baseSession) getRepoMock.mockImplementation((repoId: string) => @@ -251,6 +253,31 @@ describe('remoteWorkspace:setForConnectedTargets', () => { return observed as RemoteWorkspaceObservedSnapshot } + it('reads the repo catalog once per publish, not once per worktree', async () => { + // `store.getRepos()` re-hydrates every repo row. The export asks "is this worktree mine?" once + // per worktree, so reading the catalog inside that callback multiplied hydration by the + // worktree count — 413 on the session that surfaced this. + const worktrees = Object.fromEntries( + Array.from({ length: 12 }, (_, index) => [`repo-target-1::/remote/repo-${index}`, []]) + ) + getWorkspaceSessionMock.mockReturnValue({ + ...baseSession, + tabsByWorktree: worktrees + } as WorkspaceSessionState) + const observed = await observeTarget('target-1') + getReposMock.mockClear() + + await callSetForConnectedTargets({ + hydratedTargetIds: ['target-1'], + expectedRevisionsByTargetId: { 'target-1': observed.snapshot?.revision ?? 7 }, + expectedHostObservationTokensByTargetId: { + 'target-1': observed.hostObservationToken + } + }) + + expect(getReposMock).toHaveBeenCalledTimes(1) + }) + it('does not write without an explicit non-empty hydrated target set', async () => { await expect(callSetForConnectedTargets({ session: baseSession })).resolves.toEqual([]) await expect( diff --git a/src/main/ipc/remote-workspace.ts b/src/main/ipc/remote-workspace.ts index f9479f15ce9..a530e55105c 100644 --- a/src/main/ipc/remote-workspace.ts +++ b/src/main/ipc/remote-workspace.ts @@ -1,5 +1,6 @@ import { ipcMain, type BrowserWindow } from 'electron' import type { Store } from '../persistence' +import type { Repo } from '../../shared/repo-types' import { getActiveMultiplexer, getSshConnectionStore } from './ssh' import { exportRemoteWorkspaceSession } from '../../shared/remote-workspace-session-projection' import { @@ -107,7 +108,7 @@ function getExpectedHostObservationTokens( } function targetForWorktree( - store: Store, + repoLookup: ReturnType>, worktreeId: string, executionHostId?: string ): string | null { @@ -115,21 +116,27 @@ function targetForWorktree( // `getRepo(id)?.connectionId`, which is host-blind — the same repo id can name rows on several // hosts, so a session could be published to a machine that never owned the worktree (#11163). // Unresolvable ownership exports to nobody rather than guessing. - const resolution = resolveWorktreeExecutionHost( - createRepoRowExecutionHostLookup(store.getRepos()), - { repoId: getRepoIdFromWorktreeId(worktreeId), hostId: executionHostId ?? null } - ) + const resolution = resolveWorktreeExecutionHost(repoLookup, { + repoId: getRepoIdFromWorktreeId(worktreeId), + hostId: executionHostId ?? null + }) return resolution.kind === 'resolved' ? resolution.connectionId : null } +/** + * Why the lookup is built by the caller: `isTargetWorktree` runs once per worktree in the session, + * and `store.getRepos()` re-hydrates every repo row on each call. Reading the repo list once per + * export — the rows cannot change inside one synchronous projection — keeps that hydration off the + * per-worktree path. + */ function exportSessionForTarget( - store: Store, + repoLookup: ReturnType>, targetId: string, session: WorkspaceSessionState ): RemoteWorkspaceSession { return exportRemoteWorkspaceSession(session, { isTargetWorktree: (worktreeId, executionHostId) => - targetForWorktree(store, worktreeId, executionHostId) === targetId + targetForWorktree(repoLookup, worktreeId, executionHostId) === targetId }) } @@ -246,11 +253,13 @@ export function registerRemoteWorkspaceHandlers( ) ?? [] const workspaceSession = args.session ?? store.getWorkspaceSession() + // One repo read for every target: the rows are the same for all of them. + const repoLookup = createRepoRowExecutionHostLookup(store.getRepos()) const results = await Promise.all( targets.map(async (target) => { // Why: each target has its own revision stream. Keep same-target // writes queued, but do not let one slow relay block others. - const session = exportSessionForTarget(store, target.id, workspaceSession) + const session = exportSessionForTarget(repoLookup, target.id, workspaceSession) const result = await queueRemoteWorkspacePatch(target.id, async () => { const current = getCachedRemoteWorkspaceSnapshot(target.id) ?? (await getRemoteSnapshot(target))