From 4fa91acb4fe3d861eef571e7c9c2bdf569e51602 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Sat, 24 Oct 2026 21:51:22 -0700 Subject: [PATCH] fix(repos): drop a paired server's old workspace rows when it relists them under new project ids (#33396) * fix(repos): drop a repo's worktree rows when its host's catalog no longer lists it A paired server that relists its checkouts under new repo ids (a project copied into another profile, then that profile switched to) left the desktop showing the old rows beside the new ones, the old under Unknown. Every catalog fetch now drops the rows of the repo registrations it removed, scoped to the dropped host. Refs STA-12076 * fix(repos): purge a dropped repo's workspaces like a vanished checkout Clears selection, tabs and per-workspace state of the dropped rows through the same bulk purge an authoritative scan uses, flushing delayed editor text first; the three catalog fetch paths share one finalize step. * fix(repos): drop rows only, never a registration added after the request Review: purging tabs and selection could race repo add/remove and lose state. Keep the change to rows, and fence it to registrations the store held when the catalog request went out. * test(e2e): drop the fixed settle wait from the repo id change repro --- ...worktree-list-groups-mobile-parity.test.ts | 2 +- .../repos/all-host-repo-catalog-actions.ts | 10 +- .../repos/dropped-repo-worktree-rows.test.ts | 141 ++++++++++++++++++ .../store/repos/dropped-repo-worktree-rows.ts | 93 ++++++++++++ .../src/store/repos/repo-catalog-actions.ts | 5 +- .../src/store/repos/repo-catalog-merge.ts | 30 +++- .../repos/runtime-repo-catalog-actions.ts | 6 +- ...ed-client-host-repo-id-change-rows.spec.ts | 69 +++++++++ 8 files changed, 344 insertions(+), 12 deletions(-) create mode 100644 src/renderer/src/store/repos/dropped-repo-worktree-rows.test.ts create mode 100644 src/renderer/src/store/repos/dropped-repo-worktree-rows.ts create mode 100644 tests/e2e/paired-client-host-repo-id-change-rows.spec.ts 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() + } +})