diff --git a/src/renderer/src/components/sidebar/worktree-list-groups-mobile-parity.test.ts b/src/renderer/src/components/sidebar/worktree-list-groups-mobile-parity.test.ts index 3192d1cb411..667129aef92 100644 --- a/src/renderer/src/components/sidebar/worktree-list-groups-mobile-parity.test.ts +++ b/src/renderer/src/components/sidebar/worktree-list-groups-mobile-parity.test.ts @@ -86,7 +86,7 @@ async function desktopCatalog(): Promise<{ repos: readonly Repo[]; rows: Project let repos: readonly Repo[] = [] let rows: ProjectCatalogRows = { projects: [], projectHostSetups: [] } for (const host of PROJECT_SECTION_PARITY_HOSTS) { - const catalog = await fetchRepoCatalogForTarget(host.target) + const catalog = await fetchRepoCatalogForTarget(host.target, repos) const result = mergeFetchedRepoCatalog(catalog, repos) const reconciled = reconcileSupersededSshRepos(result.repos, { pendingSshRepoReadoptions: [] }) const finalized = applyManualRepoOrder(reconciled.repos, []) diff --git a/src/renderer/src/store/repos/all-host-repo-catalog-actions.ts b/src/renderer/src/store/repos/all-host-repo-catalog-actions.ts index 90ee84f35ab..21d23fec027 100644 --- a/src/renderer/src/store/repos/all-host-repo-catalog-actions.ts +++ b/src/renderer/src/store/repos/all-host-repo-catalog-actions.ts @@ -27,7 +27,7 @@ import { filterSetupsForPrunedRepoRows, filterTrustedOrcaHooksToValidRepos, mergeFetchedRepoCatalog, - reconcileReadoptedSshWorktreeState, + reconcileFetchedCatalogWorktreeState, reconcileSupersededSshRepos } from './repo-catalog-merge' import { getRuntimeTargetHostId } from '../../runtime/runtime-client-target' @@ -85,7 +85,7 @@ export function createAllHostRepoCatalogActions( return { repos: finalizedRepos, pendingSshRepoReadoptions: reconciliation.pendingReadoptions, - ...reconcileReadoptedSshWorktreeState(s, s.pendingSshRepoReadoptions), + ...reconcileFetchedCatalogWorktreeState(s, catalog, finalizedRepos), ...mergedProjectCompatibility, ...(arrayElementsUnchanged(finalizedRepos, s.repos) ? {} @@ -121,7 +121,9 @@ export function createAllHostRepoCatalogActions( let localCatalogOutcome: LocalRepoCatalogFetchOutcome = { status: 'fulfilled' } const localFence = claimHostCatalogFence(get, 'repos', { kind: 'local' }) try { - if (!applyCatalog(await fetchRepoCatalogForTarget({ kind: 'local' }), localFence)) { + if ( + !applyCatalog(await fetchRepoCatalogForTarget({ kind: 'local' }, get().repos), localFence) + ) { failed = true } } catch (err) { @@ -146,7 +148,7 @@ export function createAllHostRepoCatalogActions( 'repos', async (target, fence) => { const [catalogResult, visibilitySnapshot] = await Promise.all([ - fetchRepoCatalogForTarget(target).then( + fetchRepoCatalogForTarget(target, get().repos).then( (catalog) => ({ ok: true as const, catalog }), (error: unknown) => ({ ok: false as const, error }) ), diff --git a/src/renderer/src/store/repos/dropped-repo-worktree-rows.test.ts b/src/renderer/src/store/repos/dropped-repo-worktree-rows.test.ts new file mode 100644 index 00000000000..080823a7ab3 --- /dev/null +++ b/src/renderer/src/store/repos/dropped-repo-worktree-rows.test.ts @@ -0,0 +1,141 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest' +import type { Repo } from '../../../../shared/repo-types' +import type { DetectedWorktreeListResult } from '../../../../shared/worktree/types' +import { toRuntimeExecutionHostId } from '../../../../shared/execution-host' +import { + createCompatibleRuntimeStatusResponseIfNeeded, + type RuntimeEnvironmentCallRequest +} from '../../runtime/runtime-compatibility-test-fixture' +import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' +import { createTestStore, makeWorktree, seedStore } from '../slices/store-test-helpers' + +const SERVER = toRuntimeExecutionHostId('env-1') + +function serverRepo(id: string, path = '/srv/app'): Repo { + return { + id, + path, + displayName: 'app', + badgeColor: '#737373', + addedAt: 1, + executionHostId: SERVER + } +} + +function serverRow(repoId: string, path = '/srv/app') { + return makeWorktree({ id: `${repoId}::${path}`, repoId, path, hostId: SERVER }) +} + +let serverRepos: Repo[] = [] +let repoListGate: Promise | null = null +const call = vi.fn(async (args: RuntimeEnvironmentCallRequest) => { + const compatible = createCompatibleRuntimeStatusResponseIfNeeded(args) + if (compatible) { + return compatible + } + if (args.method === 'repo.list') { + const answer = structuredClone(serverRepos) + await repoListGate + return { id: 'rpc-repo.list', ok: true, result: { repos: answer }, _meta: {} } + } + return { id: `rpc-${args.method}`, ok: true, result: {}, _meta: { runtimeId: 'remote-runtime' } } +}) + +describe('a fetched catalog that drops a repo registration', () => { + beforeEach(() => { + clearRuntimeCompatibilityCacheForTests() + call.mockClear() + repoListGate = null + vi.stubGlobal('window', { + api: { runtimeEnvironments: { call, list: vi.fn().mockResolvedValue([]) } }, + dispatchEvent: vi.fn() + }) + }) + + it('drops that host’s rows when the server relists its checkout under a new repo id', async () => { + const store = createTestStore() + const localTwin: Repo = { ...serverRepo('old'), path: '/local/app', executionHostId: 'local' } + const localRow = makeWorktree({ + id: 'old::/local/app', + repoId: 'old', + path: '/local/app', + hostId: 'local' + }) + const legacyRow = makeWorktree({ id: 'old::/legacy', repoId: 'old', path: '/legacy' }) + const detected: DetectedWorktreeListResult = { + repoId: 'old', + authoritative: true, + source: 'git', + worktrees: [serverRow('old'), localRow].map((row) => ({ + ...row, + ownership: 'orca-managed' as const, + selectedCheckout: true, + visible: true + })) + } + seedStore(store, { + repos: [serverRepo('old'), localTwin], + worktreesByRepo: { old: [serverRow('old'), localRow, legacyRow] }, + detectedWorktreesByRepo: { old: detected } + }) + serverRepos = [serverRepo('copied')] + + await store.getState().fetchRuntimeEnvironmentRepos('env-1') + + const state = store.getState() + expect(state.repos.map((repo) => `${repo.executionHostId}:${repo.id}`).sort()).toEqual([ + 'local:old', + `${SERVER}:copied` + ]) + // The local twin keeps its row and the pre-host row it may own; only the server's row goes. + expect(state.worktreesByRepo.old?.map((worktree) => worktree.id)).toEqual([ + 'old::/local/app', + 'old::/legacy' + ]) + expect(state.detectedWorktreesByRepo.old?.worktrees.map((worktree) => worktree.id)).toEqual([ + 'old::/local/app' + ]) + }) + + it('drops the pre-host rows too once no host registers the repo id', async () => { + const store = createTestStore() + seedStore(store, { + repos: [serverRepo('old')], + worktreesByRepo: { + old: [ + serverRow('old'), + makeWorktree({ id: 'old::/legacy', repoId: 'old', path: '/legacy' }) + ] + } + }) + serverRepos = [serverRepo('copied')] + + await store.getState().fetchRuntimeEnvironmentRepos('env-1') + + expect(store.getState().worktreesByRepo.old).toEqual([]) + }) + + it('keeps the rows of a repo added while the catalog was in flight', async () => { + const store = createTestStore() + seedStore(store, { + repos: [serverRepo('kept')], + worktreesByRepo: { kept: [serverRow('kept')] } + }) + serverRepos = [serverRepo('kept')] + const { promise, resolve } = Promise.withResolvers() + repoListGate = promise + + const fetching = store.getState().fetchRuntimeEnvironmentRepos('env-1') + // The add lands after the request went out, so the answer cannot list it. + store.setState((s) => ({ + repos: [...s.repos, serverRepo('added', '/srv/added')], + worktreesByRepo: { ...s.worktreesByRepo, added: [serverRow('added', '/srv/added')] } + })) + resolve() + await fetching + + expect(store.getState().worktreesByRepo.added?.map((worktree) => worktree.id)).toEqual([ + 'added::/srv/added' + ]) + }) +}) diff --git a/src/renderer/src/store/repos/dropped-repo-worktree-rows.ts b/src/renderer/src/store/repos/dropped-repo-worktree-rows.ts new file mode 100644 index 00000000000..c364c541706 --- /dev/null +++ b/src/renderer/src/store/repos/dropped-repo-worktree-rows.ts @@ -0,0 +1,93 @@ +import type { AppState } from '../types' +import type { Repo } from '../../../../shared/repo-types' +import { getRepoExecutionHostId, type ExecutionHostId } from '../../../../shared/execution-host' +import { getRepoHostIdentity } from '../slices/repo-host-identity' +import { worktreeMatchesHost } from '../slices/worktrees/listing/worktree-host-ownership' +import { selectDetectedWorktreesByRepo, selectWorktreesByRepo } from '../workspace-record-selectors' + +type HostedRow = { hostId?: ExecutionHostId; runtimeOwnerEnvironmentId?: string } +type RowState = Pick + +/** Each repo id whose registration on these hosts a catalog write removed. */ +function droppedHostsByRepo( + previousRepos: readonly Repo[], + nextRepos: readonly Repo[] +): Map { + const nextIdentities = new Set(nextRepos.map(getRepoHostIdentity)) + const dropped = new Map() + for (const repo of previousRepos) { + if (!nextIdentities.has(getRepoHostIdentity(repo))) { + dropped.set(repo.id, [...(dropped.get(repo.id) ?? []), getRepoExecutionHostId(repo)]) + } + } + return dropped +} + +function dropHostRows( + rowsByRepo: Record, + dropped: ReadonlyMap, + survivingRepoIds: ReadonlySet +): Record { + let next: Record | null = null + for (const [repoId, hostIds] of dropped) { + const rows = rowsByRepo[repoId] + if (!rows) { + continue + } + // Why: a pre-host row belongs to whichever owner had the id, so it goes only once none is left. + const unhostedWorktreesMatchHost = !survivingRepoIds.has(repoId) + const kept = rows.filter( + (row) => + !hostIds.some((hostId) => worktreeMatchesHost(row, hostId, { unhostedWorktreesMatchHost })) + ) + if (kept.length !== rows.length) { + next ??= { ...rowsByRepo } + next[repoId] = kept + } + } + return next ?? rowsByRepo +} + +/** + * Drops the worktree rows of every repo registration a catalog write removed. A host that relists + * its checkouts under new repo ids (a project copied into another profile, then switched to) must + * not leave the old rows beside the new ones with no project to group under. Rows only: tabs and + * selection stay, so a registration that comes back relists into them, as after any listing. + */ +export function dropWorktreeRowsOfDroppedRepos( + state: RowState, + previousRepos: readonly Repo[], + nextRepos: readonly Repo[] +): RowState { + const dropped = droppedHostsByRepo(previousRepos, nextRepos) + if (dropped.size === 0) { + return state + } + const survivingRepoIds = new Set(nextRepos.map((repo) => repo.id)) + const previousWorktrees = selectWorktreesByRepo(state) + const previousDetected = selectDetectedWorktreesByRepo(state) + const worktreesByRepo = dropHostRows(previousWorktrees, dropped, survivingRepoIds) + let detectedWorktreesByRepo = previousDetected + for (const repoId of dropped.keys()) { + const detected = detectedWorktreesByRepo[repoId] + if (!detected) { + continue + } + const worktrees = dropHostRows({ [repoId]: detected.worktrees }, dropped, survivingRepoIds)[ + repoId + ] + if (worktrees !== detected.worktrees) { + if (detectedWorktreesByRepo === previousDetected) { + detectedWorktreesByRepo = { ...detectedWorktreesByRepo } + } + detectedWorktreesByRepo[repoId] = { ...detected, worktrees } + } + } + const changed = + worktreesByRepo !== previousWorktrees || detectedWorktreesByRepo !== previousDetected + return { + worktreesByRepo, + detectedWorktreesByRepo, + sortEpoch: changed ? state.sortEpoch + 1 : state.sortEpoch + } +} diff --git a/src/renderer/src/store/repos/repo-catalog-actions.ts b/src/renderer/src/store/repos/repo-catalog-actions.ts index 10b65166d71..8fe6d6acb34 100644 --- a/src/renderer/src/store/repos/repo-catalog-actions.ts +++ b/src/renderer/src/store/repos/repo-catalog-actions.ts @@ -25,6 +25,7 @@ import { fetchRepoCatalogForTarget, filterSetupsForPrunedRepoRows, mergeFetchedRepoCatalog, + reconcileFetchedCatalogWorktreeState, reconcileReadoptedSshWorktreeState, reconcileSupersededSshRepos } from './repo-catalog-merge' @@ -95,7 +96,7 @@ export function createRepoCatalogActions( set((s) => ({ reposFetchGeneration: s.reposFetchGeneration + 1 })) const fence = claimHostCatalogFence(get, 'repos', target) try { - const catalog = await fetchRepoCatalogForTarget(target) + const catalog = await fetchRepoCatalogForTarget(target, get().repos) // A newer same-host fetch or a replaced connection superseded us — drop this stale result. if (!isHostCatalogFenceCurrent(get, fence)) { return @@ -131,7 +132,7 @@ export function createRepoCatalogActions( return { repos: prunedRepos, pendingSshRepoReadoptions: reconciliation.pendingReadoptions, - ...reconcileReadoptedSshWorktreeState(s, s.pendingSshRepoReadoptions), + ...reconcileFetchedCatalogWorktreeState(s, catalog, prunedRepos), ...mergedProjectCompatibility, ...(arrayElementsUnchanged(prunedRepos, s.repos) ? {} diff --git a/src/renderer/src/store/repos/repo-catalog-merge.ts b/src/renderer/src/store/repos/repo-catalog-merge.ts index 6f1a2fa7431..bcd72d655d9 100644 --- a/src/renderer/src/store/repos/repo-catalog-merge.ts +++ b/src/renderer/src/store/repos/repo-catalog-merge.ts @@ -13,15 +13,20 @@ import { adoptFromEndpoint } from '../adopt-from-endpoint' import { getRuntimeTargetHostId } from '../../runtime/runtime-client-target' import { fetchProjectHostSetupCompatibility } from '../projects/project-host-routing' import { mergeFetchedReposForHost } from './repo-catalog-identity' +import { dropWorktreeRowsOfDroppedRepos } from './dropped-repo-worktree-rows' +import { getRepoHostIdentity } from '../slices/repo-host-identity' export type FetchedRepoCatalog = { repos: readonly Repo[] projectHostSetupCompatibility: ProjectHostSetupProjection hostId: ReturnType + /** This store's repo rows when the request went out; a registration added since is not its to drop. */ + reposAtRequest: readonly Repo[] } export async function fetchRepoCatalogForTarget( - target: RuntimeClientTarget + target: RuntimeClientTarget, + reposAtRequest: readonly Repo[] ): Promise { const fetchedRepos = target.kind === 'local' @@ -36,7 +41,8 @@ export async function fetchRepoCatalogForTarget( return { repos, projectHostSetupCompatibility: await fetchProjectHostSetupCompatibility(target, repos), - hostId: getRuntimeTargetHostId(target) + hostId: getRuntimeTargetHostId(target), + reposAtRequest } } @@ -115,6 +121,26 @@ export function reconcileReadoptedSshWorktreeState( } } +/** The worktree rows a fetched catalog leaves: re-adopted SSH rows moved, dropped repos' rows gone. */ +export function reconcileFetchedCatalogWorktreeState( + state: Pick< + AppState, + | 'repos' + | 'pendingSshRepoReadoptions' + | 'worktreesByRepo' + | 'detectedWorktreesByRepo' + | 'sortEpoch' + >, + catalog: Pick, + nextRepos: readonly Repo[] +): Pick { + const readopted = reconcileReadoptedSshWorktreeState(state, state.pendingSshRepoReadoptions) + // Why: a catalog requested before an add may omit it; that add's rows are not this answer's to drop. + const requested = new Set(catalog.reposAtRequest.map(getRepoHostIdentity)) + const previous = state.repos.filter((repo) => requested.has(getRepoHostIdentity(repo))) + return dropWorktreeRowsOfDroppedRepos(readopted, previous, nextRepos) +} + export function filterTrustedOrcaHooksToValidRepos( trust: AppState['trustedOrcaHooks'], validRepoIds: Set diff --git a/src/renderer/src/store/repos/runtime-repo-catalog-actions.ts b/src/renderer/src/store/repos/runtime-repo-catalog-actions.ts index ed9ea109d46..0b1a8c6f8d5 100644 --- a/src/renderer/src/store/repos/runtime-repo-catalog-actions.ts +++ b/src/renderer/src/store/repos/runtime-repo-catalog-actions.ts @@ -15,7 +15,7 @@ import { fetchRepoCatalogForTarget, filterSetupsForPrunedRepoRows, mergeFetchedRepoCatalog, - reconcileReadoptedSshWorktreeState, + reconcileFetchedCatalogWorktreeState, reconcileSupersededSshRepos } from './repo-catalog-merge' import { getRuntimeTargetHostId } from '../../runtime/runtime-client-target' @@ -34,7 +34,7 @@ export function createRuntimeRepoCatalogActions( const fence = claimHostCatalogFence(get, 'repos', target) try { const [catalog, visibilitySnapshot] = await Promise.all([ - fetchRepoCatalogForTarget(target), + fetchRepoCatalogForTarget(target, get().repos), readRuntimeWorktreeVisibilitySnapshot(environmentId) ]) const visibilityDefaults = visibilitySnapshot.defaults @@ -105,7 +105,7 @@ export function createRuntimeRepoCatalogActions( : {} : {}), pendingSshRepoReadoptions: reconciliation.pendingReadoptions, - ...reconcileReadoptedSshWorktreeState(s, s.pendingSshRepoReadoptions), + ...reconcileFetchedCatalogWorktreeState(s, catalog, finalizedRepos), ...mergedProjectCompatibility, activeRepoId: s.activeRepoId && validRepoIds.has(s.activeRepoId) ? s.activeRepoId : null, diff --git a/tests/e2e/paired-client-host-repo-id-change-rows.spec.ts b/tests/e2e/paired-client-host-repo-id-change-rows.spec.ts new file mode 100644 index 00000000000..2bb5e0cc17f --- /dev/null +++ b/tests/e2e/paired-client-host-repo-id-change-rows.spec.ts @@ -0,0 +1,69 @@ +/** + * STA-12076: a server whose catalog relists the same checkouts under a new project id (a project + * copied into another profile, then that profile switched to) left the paired desktop showing each + * workspace twice, the old copies under no project. + * + * Run: + * ORCA_BACKGROUND_LAUNCH=1 pnpm exec playwright test \ + * tests/e2e/paired-client-host-repo-id-change-rows.spec.ts \ + * --config tests/playwright.config.ts --project electron-headless --workers=1 + */ +import type { Page } from '@stablyai/playwright-test' +import { expect, test } from './helpers/orca-app' +import { launchHeadlessPairedRuntimeHost } from './helpers/headless-paired-runtime-host' +import { launchPairedElectronClient } from './helpers/paired-electron-client' + +type ListedRow = { repoId: string; repoKnown: boolean } + +function readListedRows(page: Page, repoPath: string): Promise { + return page.evaluate((target) => { + const state = window.__store?.getState() + if (!state) { + return [] + } + const repoIds = new Set(state.repos.map((repo) => repo.id)) + return state + .allWorktrees() + .filter((worktree) => worktree.path === target) + .map((worktree) => ({ repoId: worktree.repoId, repoKnown: repoIds.has(worktree.repoId) })) + }, repoPath) +} + +test('a server that relists its checkout under a new project id shows the workspace once', async ({ + testRepoPath +}, testInfo) => { + test.setTimeout(240_000) + const host = await launchHeadlessPairedRuntimeHost() + try { + const projectKind = 'git' as const + const added = await host.client.call<{ repo: { id: string } }>('repo.add', { + path: testRepoPath, + kind: projectKind + }) + const oldRepoId = added.result.repo.id + const desktop = await launchPairedElectronClient(host.offer, testInfo, 'repo id change') + try { + const page = desktop.page + await expect + .poll(() => readListedRows(page, testRepoPath), { timeout: 60_000 }) + .toEqual([{ repoId: oldRepoId, repoKnown: true }]) + + // The switched-to profile's catalog: the same checkout under a project id the desktop never saw. + await host.client.call('repo.rm', { repo: `id:${oldRepoId}` }) + const readded = await host.client.call<{ repo: { id: string } }>('repo.add', { + path: testRepoPath, + kind: projectKind + }) + const newRepoId = readded.result.repo.id + expect(newRepoId).not.toBe(oldRepoId) + + await expect + .poll(() => readListedRows(page, testRepoPath), { timeout: 60_000 }) + .toEqual([{ repoId: newRepoId, repoKnown: true }]) + } finally { + await desktop.dispose() + } + } finally { + await host.dispose() + } +})