fix(context): guard project fetches by scope
This commit is contained in:
parent
636d121f5f
commit
2fdbf301b4
3 changed files with 240 additions and 0 deletions
|
|
@ -4,6 +4,10 @@
|
||||||
|
|
||||||
import { api } from '@renderer/api';
|
import { api } from '@renderer/api';
|
||||||
|
|
||||||
|
import {
|
||||||
|
captureContextScopedRequestEpoch,
|
||||||
|
isContextScopedRequestEpochCurrent,
|
||||||
|
} from '../utils/contextScopedRequestEpoch';
|
||||||
import { getSessionResetState } from '../utils/stateResetHelpers';
|
import { getSessionResetState } from '../utils/stateResetHelpers';
|
||||||
|
|
||||||
import type { AppState } from '../types';
|
import type { AppState } from '../types';
|
||||||
|
|
@ -43,15 +47,29 @@ export const createProjectSlice: StateCreator<AppState, [], [], ProjectSlice> =
|
||||||
fetchProjects: async () => {
|
fetchProjects: async () => {
|
||||||
// Guard: prevent concurrent fetches (component mount + centralized init chain)
|
// Guard: prevent concurrent fetches (component mount + centralized init chain)
|
||||||
if (get().projectsLoading) return;
|
if (get().projectsLoading) return;
|
||||||
|
const requestContextId = get().activeContextId;
|
||||||
|
const requestContextEpoch = captureContextScopedRequestEpoch();
|
||||||
set({ projectsLoading: true, projectsError: null });
|
set({ projectsLoading: true, projectsError: null });
|
||||||
try {
|
try {
|
||||||
const projects = await api.getProjects();
|
const projects = await api.getProjects();
|
||||||
|
if (
|
||||||
|
get().activeContextId !== requestContextId ||
|
||||||
|
!isContextScopedRequestEpochCurrent(requestContextEpoch)
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
// Sort by most recent session (descending)
|
// Sort by most recent session (descending)
|
||||||
const sorted = [...projects].sort(
|
const sorted = [...projects].sort(
|
||||||
(a, b) => (b.mostRecentSession ?? 0) - (a.mostRecentSession ?? 0)
|
(a, b) => (b.mostRecentSession ?? 0) - (a.mostRecentSession ?? 0)
|
||||||
);
|
);
|
||||||
set({ projects: sorted, projectsLoading: false, projectsInitialized: true });
|
set({ projects: sorted, projectsLoading: false, projectsInitialized: true });
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
if (
|
||||||
|
get().activeContextId !== requestContextId ||
|
||||||
|
!isContextScopedRequestEpochCurrent(requestContextEpoch)
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
set({
|
set({
|
||||||
projectsError: error instanceof Error ? error.message : 'Failed to fetch projects',
|
projectsError: error instanceof Error ? error.message : 'Failed to fetch projects',
|
||||||
projectsLoading: false,
|
projectsLoading: false,
|
||||||
|
|
|
||||||
|
|
@ -5,6 +5,10 @@
|
||||||
import { api } from '@renderer/api';
|
import { api } from '@renderer/api';
|
||||||
import { createLogger } from '@shared/utils/logger';
|
import { createLogger } from '@shared/utils/logger';
|
||||||
|
|
||||||
|
import {
|
||||||
|
captureContextScopedRequestEpoch,
|
||||||
|
isContextScopedRequestEpochCurrent,
|
||||||
|
} from '../utils/contextScopedRequestEpoch';
|
||||||
import { getSessionResetState } from '../utils/stateResetHelpers';
|
import { getSessionResetState } from '../utils/stateResetHelpers';
|
||||||
|
|
||||||
import type { AppState } from '../types';
|
import type { AppState } from '../types';
|
||||||
|
|
@ -71,6 +75,8 @@ export const createRepositorySlice: StateCreator<AppState, [], [], RepositorySli
|
||||||
fetchRepositoryGroups: async () => {
|
fetchRepositoryGroups: async () => {
|
||||||
// Guard: prevent concurrent fetches (component mount + centralized init chain)
|
// Guard: prevent concurrent fetches (component mount + centralized init chain)
|
||||||
if (get().repositoryGroupsLoading) return;
|
if (get().repositoryGroupsLoading) return;
|
||||||
|
const requestContextId = get().activeContextId;
|
||||||
|
const requestContextEpoch = captureContextScopedRequestEpoch();
|
||||||
const startedAt = Date.now();
|
const startedAt = Date.now();
|
||||||
set({ repositoryGroupsLoading: true, repositoryGroupsError: null });
|
set({ repositoryGroupsLoading: true, repositoryGroupsError: null });
|
||||||
try {
|
try {
|
||||||
|
|
@ -79,6 +85,12 @@ export const createRepositorySlice: StateCreator<AppState, [], [], RepositorySli
|
||||||
FETCH_REPOSITORY_GROUPS_TIMEOUT_MS,
|
FETCH_REPOSITORY_GROUPS_TIMEOUT_MS,
|
||||||
'get-repository-groups'
|
'get-repository-groups'
|
||||||
);
|
);
|
||||||
|
if (
|
||||||
|
get().activeContextId !== requestContextId ||
|
||||||
|
!isContextScopedRequestEpochCurrent(requestContextEpoch)
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
// Already sorted by most recent session in the scanner
|
// Already sorted by most recent session in the scanner
|
||||||
set({
|
set({
|
||||||
repositoryGroups: groups,
|
repositoryGroups: groups,
|
||||||
|
|
@ -90,6 +102,12 @@ export const createRepositorySlice: StateCreator<AppState, [], [], RepositorySli
|
||||||
logger.warn(`fetchRepositoryGroups slow ms=${ms} count=${groups.length}`);
|
logger.warn(`fetchRepositoryGroups slow ms=${ms} count=${groups.length}`);
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
if (
|
||||||
|
get().activeContextId !== requestContextId ||
|
||||||
|
!isContextScopedRequestEpochCurrent(requestContextEpoch)
|
||||||
|
) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
const ms = Date.now() - startedAt;
|
const ms = Date.now() - startedAt;
|
||||||
logger.warn(
|
logger.warn(
|
||||||
`fetchRepositoryGroups failed ms=${ms}: ${error instanceof Error ? error.message : String(error)}`
|
`fetchRepositoryGroups failed ms=${ms}: ${error instanceof Error ? error.message : String(error)}`
|
||||||
|
|
|
||||||
204
test/renderer/store/projectRepositoryContextRace.test.ts
Normal file
204
test/renderer/store/projectRepositoryContextRace.test.ts
Normal file
|
|
@ -0,0 +1,204 @@
|
||||||
|
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
import { create } from 'zustand';
|
||||||
|
|
||||||
|
import { createProjectSlice } from '../../../src/renderer/store/slices/projectSlice';
|
||||||
|
import { createRepositorySlice } from '../../../src/renderer/store/slices/repositorySlice';
|
||||||
|
import {
|
||||||
|
invalidateContextScopedRequestEpoch,
|
||||||
|
resetContextScopedRequestEpochForTests,
|
||||||
|
} from '../../../src/renderer/store/utils/contextScopedRequestEpoch';
|
||||||
|
|
||||||
|
import type { AppState } from '../../../src/renderer/store/types';
|
||||||
|
import type { Project, RepositoryGroup } from '../../../src/renderer/types/data';
|
||||||
|
|
||||||
|
const apiMock = vi.hoisted(() => ({
|
||||||
|
getProjects: vi.fn(),
|
||||||
|
getRepositoryGroups: vi.fn(),
|
||||||
|
}));
|
||||||
|
|
||||||
|
vi.mock('@renderer/api', () => ({
|
||||||
|
api: apiMock,
|
||||||
|
}));
|
||||||
|
|
||||||
|
function deferred<T>(): {
|
||||||
|
promise: Promise<T>;
|
||||||
|
resolve: (value: T) => void;
|
||||||
|
} {
|
||||||
|
let resolve!: (value: T) => void;
|
||||||
|
const promise = new Promise<T>((innerResolve) => {
|
||||||
|
resolve = innerResolve;
|
||||||
|
});
|
||||||
|
return { promise, resolve };
|
||||||
|
}
|
||||||
|
|
||||||
|
function project(id: string, path = `/${id}`): Project {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
path,
|
||||||
|
name: id,
|
||||||
|
sessions: [],
|
||||||
|
totalSessions: 0,
|
||||||
|
createdAt: 0,
|
||||||
|
mostRecentSession: 0,
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function repositoryGroup(id: string, path = `/${id}`): RepositoryGroup {
|
||||||
|
return {
|
||||||
|
id,
|
||||||
|
identity: null,
|
||||||
|
name: id,
|
||||||
|
totalSessions: 0,
|
||||||
|
mostRecentSession: 0,
|
||||||
|
worktrees: [
|
||||||
|
{
|
||||||
|
id: `${id}-worktree`,
|
||||||
|
path,
|
||||||
|
name: id,
|
||||||
|
isMainWorktree: true,
|
||||||
|
source: 'unknown',
|
||||||
|
sessions: [],
|
||||||
|
totalSessions: 0,
|
||||||
|
createdAt: 0,
|
||||||
|
mostRecentSession: 0,
|
||||||
|
},
|
||||||
|
],
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
function createProjectRepositoryStore() {
|
||||||
|
return create<AppState>()((set, get, store) =>
|
||||||
|
({
|
||||||
|
...createProjectSlice(set as never, get as never, store as never),
|
||||||
|
...createRepositorySlice(set as never, get as never, store as never),
|
||||||
|
activeContextId: 'local',
|
||||||
|
activeProjectId: null,
|
||||||
|
fetchSessionsInitial: vi.fn(async () => undefined),
|
||||||
|
}) as unknown as AppState
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('project and repository context races', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
resetContextScopedRequestEpochForTests();
|
||||||
|
apiMock.getProjects.mockReset();
|
||||||
|
apiMock.getRepositoryGroups.mockReset();
|
||||||
|
});
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
resetContextScopedRequestEpochForTests();
|
||||||
|
vi.restoreAllMocks();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('applies current-context project loads', async () => {
|
||||||
|
const store = createProjectRepositoryStore();
|
||||||
|
apiMock.getProjects.mockResolvedValue([project('current-project')]);
|
||||||
|
|
||||||
|
await store.getState().fetchProjects();
|
||||||
|
|
||||||
|
expect(store.getState().projects).toEqual([project('current-project')]);
|
||||||
|
expect(store.getState().projectsInitialized).toBe(true);
|
||||||
|
expect(store.getState().projectsLoading).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores project loads resolved for a previous context', async () => {
|
||||||
|
const store = createProjectRepositoryStore();
|
||||||
|
const localProjects = deferred<Project[]>();
|
||||||
|
const currentProjects = [project('ssh-project', '/ssh/project')];
|
||||||
|
apiMock.getProjects.mockReturnValueOnce(localProjects.promise);
|
||||||
|
|
||||||
|
const fetchPromise = store.getState().fetchProjects();
|
||||||
|
expect(store.getState().projectsLoading).toBe(true);
|
||||||
|
|
||||||
|
store.setState({
|
||||||
|
activeContextId: 'ssh-dev',
|
||||||
|
projects: currentProjects,
|
||||||
|
projectsLoading: false,
|
||||||
|
projectsInitialized: true,
|
||||||
|
});
|
||||||
|
localProjects.resolve([project('local-project', '/local/project')]);
|
||||||
|
await fetchPromise;
|
||||||
|
|
||||||
|
expect(store.getState().projects).toBe(currentProjects);
|
||||||
|
expect(store.getState().projectsLoading).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores project loads resolved before a same-context epoch reset', async () => {
|
||||||
|
const store = createProjectRepositoryStore();
|
||||||
|
const oldLocalProjects = deferred<Project[]>();
|
||||||
|
const currentProjects = [project('fresh-local-project', '/fresh-local/project')];
|
||||||
|
apiMock.getProjects.mockReturnValueOnce(oldLocalProjects.promise);
|
||||||
|
|
||||||
|
const fetchPromise = store.getState().fetchProjects();
|
||||||
|
expect(store.getState().projectsLoading).toBe(true);
|
||||||
|
|
||||||
|
invalidateContextScopedRequestEpoch();
|
||||||
|
store.setState({
|
||||||
|
activeContextId: 'local',
|
||||||
|
projects: currentProjects,
|
||||||
|
projectsLoading: false,
|
||||||
|
projectsInitialized: true,
|
||||||
|
});
|
||||||
|
oldLocalProjects.resolve([project('old-local-project', '/old-local/project')]);
|
||||||
|
await fetchPromise;
|
||||||
|
|
||||||
|
expect(store.getState().projects).toBe(currentProjects);
|
||||||
|
expect(store.getState().projectsLoading).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('applies current-context repository group loads', async () => {
|
||||||
|
const store = createProjectRepositoryStore();
|
||||||
|
apiMock.getRepositoryGroups.mockResolvedValue([repositoryGroup('current-repo')]);
|
||||||
|
|
||||||
|
await store.getState().fetchRepositoryGroups();
|
||||||
|
|
||||||
|
expect(store.getState().repositoryGroups).toEqual([repositoryGroup('current-repo')]);
|
||||||
|
expect(store.getState().repositoryGroupsInitialized).toBe(true);
|
||||||
|
expect(store.getState().repositoryGroupsLoading).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores repository group loads resolved for a previous context', async () => {
|
||||||
|
const store = createProjectRepositoryStore();
|
||||||
|
const localGroups = deferred<RepositoryGroup[]>();
|
||||||
|
const currentGroups = [repositoryGroup('ssh-repo', '/ssh/repo')];
|
||||||
|
apiMock.getRepositoryGroups.mockReturnValueOnce(localGroups.promise);
|
||||||
|
|
||||||
|
const fetchPromise = store.getState().fetchRepositoryGroups();
|
||||||
|
expect(store.getState().repositoryGroupsLoading).toBe(true);
|
||||||
|
|
||||||
|
store.setState({
|
||||||
|
activeContextId: 'ssh-dev',
|
||||||
|
repositoryGroups: currentGroups,
|
||||||
|
repositoryGroupsLoading: false,
|
||||||
|
repositoryGroupsInitialized: true,
|
||||||
|
});
|
||||||
|
localGroups.resolve([repositoryGroup('local-repo', '/local/repo')]);
|
||||||
|
await fetchPromise;
|
||||||
|
|
||||||
|
expect(store.getState().repositoryGroups).toBe(currentGroups);
|
||||||
|
expect(store.getState().repositoryGroupsLoading).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores repository group loads resolved before a same-context epoch reset', async () => {
|
||||||
|
const store = createProjectRepositoryStore();
|
||||||
|
const oldLocalGroups = deferred<RepositoryGroup[]>();
|
||||||
|
const currentGroups = [repositoryGroup('fresh-local-repo', '/fresh-local/repo')];
|
||||||
|
apiMock.getRepositoryGroups.mockReturnValueOnce(oldLocalGroups.promise);
|
||||||
|
|
||||||
|
const fetchPromise = store.getState().fetchRepositoryGroups();
|
||||||
|
expect(store.getState().repositoryGroupsLoading).toBe(true);
|
||||||
|
|
||||||
|
invalidateContextScopedRequestEpoch();
|
||||||
|
store.setState({
|
||||||
|
activeContextId: 'local',
|
||||||
|
repositoryGroups: currentGroups,
|
||||||
|
repositoryGroupsLoading: false,
|
||||||
|
repositoryGroupsInitialized: true,
|
||||||
|
});
|
||||||
|
oldLocalGroups.resolve([repositoryGroup('old-local-repo', '/old-local/repo')]);
|
||||||
|
await fetchPromise;
|
||||||
|
|
||||||
|
expect(store.getState().repositoryGroups).toBe(currentGroups);
|
||||||
|
expect(store.getState().repositoryGroupsLoading).toBe(false);
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue