mirror of
https://github.com/stablyai/orca.git
synced 2026-09-22 08:02:28 +00:00
* 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 <help@stably.ai> * chore: restore origin/main cdp-ws-proxy.test.ts (drop merge artifact) Co-authored-by: Orca <help@stably.ai> --------- Co-authored-by: Neil <4138956+nwparker@users.noreply.github.com> Co-authored-by: Orca <help@stably.ai>
This commit is contained in:
co-authored by
Orca
Neil
parent
2c0be62df6
commit
97c30c5ccc
@@ -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<Repo[]>((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<Repo[]>((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([])
|
||||
})
|
||||
})
|
||||
@@ -1152,6 +1152,8 @@ export type RepoSlice = {
|
||||
folderWorkspaces: FolderWorkspace[]
|
||||
folderWorkspacePathStatuses: Record<string, FolderWorkspacePathStatusCacheEntry>
|
||||
activeRepoId: string | null
|
||||
// Monotonic sequence so an overlapping fetchRepos can drop its own stale result (#7020).
|
||||
reposFetchGeneration: number
|
||||
fetchRepos: () => Promise<void>
|
||||
fetchReposForAllHosts: () => Promise<void>
|
||||
fetchRuntimeEnvironmentRepos: (environmentId: string) => Promise<Repo[]>
|
||||
@@ -1270,8 +1272,16 @@ export const createRepoSlice: StateCreator<AppState, [], [], RepoSlice> = (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<AppState, [], [], RepoSlice> = (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({
|
||||
|
||||
Reference in New Issue
Block a user