From 97c30c5ccc79e7481d9cc6bf730aba0262f80e97 Mon Sep 17 00:00:00 2001 From: Anddy Sahir Agudelo <44873492+AnddyAgudelo@users.noreply.github.com> Date: Fri, 3 Jul 2026 15:40:17 -0500 Subject: [PATCH] fix: drop stale repos fetches so deleted projects don't reappear (#7020) (#7024) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: drop stale repos fetches so deleted projects don't reappear Deleting a project group with "Remove contained projects" could leave the removed projects as stale, unusable sidebar rows until an app restart (#7020). `repos:changed` fires once per removal and the renderer starts an unsequenced repos fetch per event; an earlier fetch that read pre-removal state could resolve last and overwrite the newer result, reintroducing the deleted repos. Guard fetchRepos and fetchReposForAllHosts with a monotonic token so a fetch drops its own result once a newer repos fetch has superseded it — only the latest fetch, which reads the final persisted state, applies. Add a regression test that a stale fetch resolving after a newer one can't resurrect a removed repo. * test: isolate stale-fetch race in a dedicated file; scope guard to fetchRepos Move the #7020 regression test out of repos.test.ts into a focused repos-stale-fetch.test.ts and add a reject-path case (a superseding fetch that later rejects must still block the older stale fetch). Scope the monotonic guard to fetchRepos only: the original shared-counter guard on fetchReposForAllHosts let an unrelated fetchRepos bump the counter and abort an in-flight all-host load, dropping every host's repos. Co-authored-by: Orca * chore: restore origin/main cdp-ws-proxy.test.ts (drop merge artifact) Co-authored-by: Orca --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Orca --- .../store/slices/repos-stale-fetch.test.ts | 77 +++++++++++++++++++ src/renderer/src/store/slices/repos.ts | 14 ++++ 2 files changed, 91 insertions(+) create mode 100644 src/renderer/src/store/slices/repos-stale-fetch.test.ts diff --git a/src/renderer/src/store/slices/repos-stale-fetch.test.ts b/src/renderer/src/store/slices/repos-stale-fetch.test.ts new file mode 100644 index 00000000000..a536224b860 --- /dev/null +++ b/src/renderer/src/store/slices/repos-stale-fetch.test.ts @@ -0,0 +1,77 @@ +import { describe, expect, it, vi, beforeEach } from 'vitest' +import { createTestStore } from './store-test-helpers' +import type { Repo } from '../../../../shared/types' +import { clearRuntimeCompatibilityCacheForTests } from '../../runtime/runtime-rpc-client' + +const localRepo: Repo = { + id: 'local-repo', + path: '/local', + displayName: 'Local', + badgeColor: '#000', + addedAt: 1 +} + +const remoteRepo: Repo = { + id: 'remote-repo', + path: '/remote', + displayName: 'Remote', + badgeColor: '#111', + addedAt: 2 +} + +const reposList = vi.fn() + +beforeEach(() => { + clearRuntimeCompatibilityCacheForTests() + reposList.mockReset() + // Only repos.list is exercised here — the missing projects API makes + // fetchProjectHostSetupCompatibility fall back to deriving from repos. + vi.stubGlobal('window', { api: { repos: { list: reposList } } }) +}) + +// A repos:changed burst (deleting a project group with contained projects) starts +// overlapping fetchRepos calls; the slice must keep the latest and drop superseded +// results so removed projects don't reappear until restart (#7020). +describe('repos slice stale-fetch race (#7020)', () => { + it('drops a stale repos fetch that resolves after a newer one', async () => { + const store = createTestStore() + let resolveStale!: (repos: Repo[]) => void + const stalePromise = new Promise((resolve) => { + resolveStale = resolve + }) + // Why: mirrors the delete-project-group burst — the first fetch reads + // pre-removal state (both repos) but resolves LAST; the second reads + // post-removal state (remoteRepo gone) and resolves first. + reposList.mockReturnValueOnce(stalePromise).mockResolvedValueOnce([localRepo]) + + const stale = store.getState().fetchRepos() + const fresh = store.getState().fetchRepos() + await fresh + expect(store.getState().repos.map((repo) => repo.id)).toEqual(['local-repo']) + + resolveStale([localRepo, remoteRepo]) + await stale + // The superseded fetch must not resurrect the removed repo. + expect(store.getState().repos.map((repo) => repo.id)).toEqual(['local-repo']) + }) + + it('a superseding fetch that later rejects still blocks the older stale fetch', async () => { + const store = createTestStore() + let resolveStale!: (repos: Repo[]) => void + const stalePromise = new Promise((resolve) => { + resolveStale = resolve + }) + // The stale fetch reads pre-removal state and resolves LAST; the superseding + // fetch reads post-removal state but REJECTS. Because the generation is + // claimed synchronously before the await, the failed fetch still supersedes + // the stale one, which must be dropped rather than resurrect remoteRepo. + reposList.mockReturnValueOnce(stalePromise).mockRejectedValueOnce(new Error('boom')) + + const stale = store.getState().fetchRepos() + await store.getState().fetchRepos() + + resolveStale([localRepo, remoteRepo]) + await stale + expect(store.getState().repos).toEqual([]) + }) +}) diff --git a/src/renderer/src/store/slices/repos.ts b/src/renderer/src/store/slices/repos.ts index 3b382cb1bef..b44f4e177d7 100644 --- a/src/renderer/src/store/slices/repos.ts +++ b/src/renderer/src/store/slices/repos.ts @@ -1152,6 +1152,8 @@ export type RepoSlice = { folderWorkspaces: FolderWorkspace[] folderWorkspacePathStatuses: Record activeRepoId: string | null + // Monotonic sequence so an overlapping fetchRepos can drop its own stale result (#7020). + reposFetchGeneration: number fetchRepos: () => Promise fetchReposForAllHosts: () => Promise fetchRuntimeEnvironmentRepos: (environmentId: string) => Promise @@ -1270,8 +1272,16 @@ export const createRepoSlice: StateCreator = (set, folderWorkspaces: [], folderWorkspacePathStatuses: {}, activeRepoId: null, + reposFetchGeneration: 0, fetchRepos: async () => { + // Why: overlapping repos:changed fetches can resolve out of order; an earlier + // one must not overwrite a newer result and resurrect deleted projects (#7020). + let generation = 0 + set((s) => { + generation = s.reposFetchGeneration + 1 + return { reposFetchGeneration: generation } + }) try { const target = getActiveRuntimeTarget(get().settings) const { @@ -1279,6 +1289,10 @@ export const createRepoSlice: StateCreator = (set, projectCompatibility, hostId } = await fetchReposForTarget(target, get().repos) + // A newer fetchRepos superseded us while we awaited — drop this stale result. + if (get().reposFetchGeneration !== generation) { + return + } set((s) => { const validRepoIds = new Set(reconciledRepos.map((repo) => repo.id)) const mergedProjectCompatibility = mergeFetchedProjectCompatibilityForHost({