diff --git a/.project-docs/30-worklog/tasks/20260827-plugin-ml07-remediation-r3-7f4a2d91.md b/.project-docs/30-worklog/tasks/20260827-plugin-ml07-remediation-r3-7f4a2d91.md new file mode 100644 index 0000000..1527aa9 --- /dev/null +++ b/.project-docs/30-worklog/tasks/20260827-plugin-ml07-remediation-r3-7f4a2d91.md @@ -0,0 +1,64 @@ +# Task: Remediate accepted MakeLore ML-07 R3 finding + +## Identity + +- Task ID: 20260827-plugin-ml07-remediation-r3-7f4a2d91 +- Mode: Feature +- Branch: codex/20260827-plugin-ml07-remediation-r3-7f4a2d91-plugin-ml07-remediation-r3 +- Worktree: D:\Datas\OthersProjects\makelore-plugin-ml07-remediation-r3-7f4a2d91 +- Base commit: 405b9f64fd6872f6b4bd4c4429e6aaff2fa0b84a +- Owner: ml07-remediator +- Status: Ready for Integration + +## Scope + +- Correct same-project load/mutation ordering in `src/stores/coding-plugins.ts` with one project-scoped operation epoch. +- Add focused red/green coverage in `tests/unit/coding-plugins-store.test.ts` for both load-before-mutation and refresh-during-mutation orderings, including every adjacent Data Service mutation path. +- Preserve the existing project A-to-B stale-response guards and avoid UI changes unless store correctness cannot stand alone. + +## Intent And Constraints + +- A successful mutation must be the latest authority for its project: an older or concurrently started read cannot overwrite it, and any required post-mutation reload must not coalesce with an invalidated read. +- Keep the existing store shape and dependency seams; do not add a new store layer, framework, or UI-only correctness guard. +- Work only in the fresh isolated task worktree from exact base `405b9f64fd6872f6b4bd4c4429e6aaff2fa0b84a`; do not touch root/coordinator worktrees, push, open a PR, or revert peers. + +## Project Context Loaded + +- Concurrent Task Gate and Planning Gate passed for this exact task ID, feature mode, branch, worktree, base, and owner. +- Read the memory index, project positioning, integrated current state, decision index, system overview, architecture/data-flow/domain/success criteria, ADR-006, evidence/reflection/commitment/stale indexes, coordinator record, and R3 reviewer records. +- Coordinator owns integration; R3 standards/spec tasks are read-only reviews at the same base. Other active tasks do not overlap these two owned files. Older planning-only reviewer records contain no competing semantic decision. +- Relevant boundary: project configuration and Plugin Center state remain local/project-scoped, while Pi/Main authority and plugin policy contracts are unchanged by this Renderer store ordering fix. + +## Plan + +1. Add red tests proving an older same-project load cannot overwrite a completed enable and that a refresh started during a delayed enable cannot suppress the successful mutation. +2. Add table coverage for configure, reset, remove-collection, and remove-project under the same delayed-mutation/refresh ordering. +3. Replace load-generation coupling inside same-project mutations with a project operation epoch: successful mutations advance the epoch; loads capture it; epoch-specific load coalescing permits required authoritative post-mutation reloads. Retain active-project checks and the global generation for A-to-B load selection. +4. Run the focused store tests, typecheck, scoped/full lint checks, drift gate, and return one clean commit with the exact sole parent. + +## Outcome + +- Added one project-scoped operation epoch to the existing store. Loads capture the current epoch and may commit only when both the global latest-load generation and their project epoch remain current. +- Successful enable/configure/reset/remove-collection/remove-project mutations advance the project epoch before committing. Same-project refresh no longer suppresses a mutation, while the latest requested project check retains the project A-to-B guard. +- Load flights use an epoch-specific internal key, so a required post-mutation authoritative load cannot coalesce with an invalidated older read. A small pending-key reference count preserves the existing public `load:` contract while overlapping stale/current flights settle. +- Enable/reset commit their authoritative results directly; configure/remove-project trigger a new authoritative projection load; remove-collection commits successful inspection or triggers a fresh load when post-removal inspection fails. +- No Plugin Center UI change was needed: store correctness is independent of button disabling and existing page pending keys remain unchanged. + +## Verification + +- Initial red run: `coding-plugins-store` had 6 failures / 12 tests. An older load overwrote a completed enable; refresh during delayed enable suppressed the success; configure/remove-project skipped reload; reset/remove-collection skipped their commits. +- Additional red run: successful collection removal followed by failed inspection did not request an authoritative reload (1 failure / 13 tests). +- Green focused store run: 1 file / 13 tests passed. +- Expanded Renderer/plugin regression: `coding-plugins-store`, `coding-plugins-client`, `project-plugins-page`, and `data-service-plugin-settings` — 4 files / 22 tests passed. +- `corepack pnpm run typecheck` passed. +- Scoped ESLint for both changed product/test files passed with no output. +- `corepack pnpm run lint:check` passed. +- No build was run because no UI, Main, Preload, packaging import, or asset file changed; the coordinator will run the full integration ledger. + +## Follow-ups + +- Coordinator should rerun the full suite after integrating the sole remediation commit. + +## Promotion Candidates + +- None recorded. diff --git a/src/stores/coding-plugins.ts b/src/stores/coding-plugins.ts index 13cf3f8..fafd380 100644 --- a/src/stores/coding-plugins.ts +++ b/src/stores/coding-plugins.ts @@ -44,21 +44,53 @@ export function createCodingPluginsStore(overrides: Partial = {}): removeProject: removeDataServiceProject, ...overrides, }; const flights = new Map>(); + const pendingCounts = new Map(); + const projectEpochs = new Map(); let loadGeneration = 0; - const operation = (key: string, run: () => Promise): Promise => { + let latestLoadProjectId: string | null = null; + const projectEpoch = (projectId: string): number => projectEpochs.get(projectId) ?? 0; + const advanceProjectEpoch = (projectId: string): void => { + projectEpochs.set(projectId, projectEpoch(projectId) + 1); + }; + const isCurrentProject = (projectId: string | null): projectId is string => ( + projectId !== null + && store.getState().projectId === projectId + && (latestLoadProjectId === null || latestLoadProjectId === projectId) + ); + const operation = ( + key: string, + run: () => Promise, + pendingKey = key, + ): Promise => { const existing = flights.get(key); if (existing) return existing; + const pendingCount = (pendingCounts.get(pendingKey) ?? 0) + 1; + pendingCounts.set(pendingKey, pendingCount); let flight: Promise; flight = run().finally(() => { - flights.delete(key); - const pending = { ...store.getState().pending }; delete pending[key]; store.setState({ pending }); + if (flights.get(key) === flight) flights.delete(key); + const remaining = (pendingCounts.get(pendingKey) ?? 1) - 1; + if (remaining > 0) { + pendingCounts.set(pendingKey, remaining); + } else { + pendingCounts.delete(pendingKey); + const pending = { ...store.getState().pending }; + delete pending[pendingKey]; + store.setState({ pending }); + } }); - flights.set(key, flight); store.setState((state) => ({ pending: { ...state.pending, [key]: true } })); + flights.set(key, flight); + if (pendingCount === 1) { + store.setState((state) => ({ pending: { ...state.pending, [pendingKey]: true } })); + } return flight; }; const store = createStore((set, get) => ({ projectId: null, projection: null, dataService: null, loadState: 'idle', error: null, pending: {}, load(projectId) { - const key = `load:${projectId}`; + latestLoadProjectId = projectId; + const epoch = projectEpoch(projectId); + const pendingKey = `load:${projectId}`; + const key = `${pendingKey}:${epoch}`; const existing = flights.get(key); if (existing) return existing; const generation = ++loadGeneration; @@ -72,78 +104,82 @@ export function createCodingPluginsStore(overrides: Partial = {}): const result = await deps.inspectDataService(); if (result.success && result.data) dataService = result.data; } - if (generation === loadGeneration) { + if (generation === loadGeneration && epoch === projectEpoch(projectId)) { set({ projectId, projection, dataService, loadState: 'ready', error: null }); } } catch (error) { - if (generation === loadGeneration) { + if (generation === loadGeneration && epoch === projectEpoch(projectId)) { set({ loadState: 'error', error: error instanceof Error ? error.message : String(error) }); } throw error; } - }); + }, pendingKey); }, setEnabled(projectId, pluginId, enabled) { - const generation = loadGeneration; return operation(`enabled:${projectId}:${pluginId}`, async () => { const projection = await deps.setEnabled(projectId, pluginId, enabled); - if (generation !== loadGeneration || get().projectId !== projectId) return; + if (!isCurrentProject(projectId)) return; let dataService = get().dataService; const item = projection.items.find(({ id }) => id === pluginId); if (!enabled) { dataService = null; } else if (item?.settingsSurface === 'data-service' && item.backend.status === 'ready') { const inspected = await deps.inspectDataService(); - if (generation !== loadGeneration || get().projectId !== projectId) return; + if (!isCurrentProject(projectId)) return; if (inspected.success && inspected.data) dataService = inspected.data; } + advanceProjectEpoch(projectId); set({ - projectId, projection, dataService, error: null, + projectId, projection, dataService, loadState: 'ready', error: null, }); }); }, configure(collections) { const projectId = get().projectId; - const generation = loadGeneration; return operation('data-service:configure', async () => { const result = await deps.configureDataService(collections); if (!result.success || !result.data) throw new Error(result.error || '开发数据空间创建失败'); - if (generation !== loadGeneration || get().projectId !== projectId) return; - set({ dataService: result.data, error: null }); - if (projectId) await get().load(projectId); + if (!isCurrentProject(projectId)) return; + advanceProjectEpoch(projectId); + set({ dataService: result.data, loadState: 'ready', error: null }); + await get().load(projectId); }); }, reset() { const projectId = get().projectId; - const generation = loadGeneration; return operation('data-service:reset', async () => { const result = await deps.resetDataService(); if (!result.success || !result.data) throw new Error(result.error || '开发数据重置失败'); - if (generation !== loadGeneration || get().projectId !== projectId) return; - set({ dataService: result.data }); + if (!isCurrentProject(projectId)) return; + advanceProjectEpoch(projectId); + set({ dataService: result.data, loadState: 'ready' }); }); }, removeCollection(collection) { const projectId = get().projectId; - const generation = loadGeneration; return operation(`data-service:collection:${collection}`, async () => { const result = await deps.removeCollection(collection); if (!result.success) throw new Error(result.error || '移除 collection 失败'); - if (generation !== loadGeneration || get().projectId !== projectId) return; + if (!isCurrentProject(projectId)) return; const inspected = await deps.inspectDataService(); - if (generation !== loadGeneration || get().projectId !== projectId) return; - if (inspected.success && inspected.data) set({ dataService: inspected.data }); + if (!isCurrentProject(projectId)) return; + advanceProjectEpoch(projectId); + if (inspected.success && inspected.data) { + set({ dataService: inspected.data, loadState: 'ready' }); + } else { + await get().load(projectId); + } }); }, removeProject() { const projectId = get().projectId; - const generation = loadGeneration; return operation('data-service:remove-project', async () => { const result = await deps.removeProject(); if (!result.success) throw new Error(result.error || '删除开发数据空间失败'); - if (generation !== loadGeneration || get().projectId !== projectId) return; - set({ dataService: null }); - if (projectId) await get().load(projectId); + if (!isCurrentProject(projectId)) return; + advanceProjectEpoch(projectId); + set({ dataService: null, loadState: 'ready' }); + await get().load(projectId); }); }, })); diff --git a/tests/unit/coding-plugins-store.test.ts b/tests/unit/coding-plugins-store.test.ts index 6366f60..5ead9d4 100644 --- a/tests/unit/coding-plugins-store.test.ts +++ b/tests/unit/coding-plugins-store.test.ts @@ -118,6 +118,162 @@ describe('coding plugins store', () => { }); }); + it('does not let an older same-project load overwrite a completed enable mutation', async () => { + const staleLoad = deferred>(); + const store = createCodingPluginsStore({ + list: vi.fn(() => staleLoad.promise), + setEnabled: vi.fn().mockResolvedValue(projection(true)), + }); + store.setState({ projectId: 'local-project', projection: projection(false) }); + + const load = store.getState().load('local-project'); + await store.getState().setEnabled('local-project', 'makelore.data-service', true); + staleLoad.resolve(projection(false)); + await load; + + expect(store.getState().projection?.items[0].enabled).toBe(true); + }); + + it('commits a delayed enable after a same-project refresh observes old state', async () => { + const enabled = deferred>(); + const store = createCodingPluginsStore({ + list: vi.fn().mockResolvedValue(projection(false)), + setEnabled: vi.fn(() => enabled.promise), + }); + store.setState({ projectId: 'local-project', projection: projection(false) }); + + const mutation = store.getState().setEnabled('local-project', 'makelore.data-service', true); + await store.getState().load('local-project'); + expect(store.getState().projection?.items[0].enabled).toBe(false); + enabled.resolve(projection(true)); + await mutation; + + expect(store.getState().projection?.items[0].enabled).toBe(true); + }); + + const adjacentMutationCases = [ + { + name: 'configure', + verify: async () => { + const configured = deferred>(); + let loadCount = 0; + const list = vi.fn(async () => projection(++loadCount > 1)); + const store = createCodingPluginsStore({ + list, + configureDataService: vi.fn(() => configured.promise), + }); + store.setState({ projectId: 'local-project', projection: projection(false) }); + const mutation = store.getState().configure(['todos']); + await store.getState().load('local-project'); + configured.resolve(success(dataServiceInstance('configured'))); + await mutation; + expect(list).toHaveBeenCalledTimes(2); + expect(store.getState()).toMatchObject({ + projection: { items: [{ enabled: true }] }, + dataService: { instance_id: 'configured' }, + }); + }, + }, + { + name: 'reset', + verify: async () => { + const reset = deferred>(); + const store = createCodingPluginsStore({ + list: vi.fn().mockResolvedValue(projection(false)), + resetDataService: vi.fn(() => reset.promise), + }); + store.setState({ + projectId: 'local-project', projection: projection(false), + dataService: dataServiceInstance('before-reset'), + }); + const mutation = store.getState().reset(); + await store.getState().load('local-project'); + reset.resolve(success(dataServiceInstance('after-reset'))); + await mutation; + expect(store.getState().dataService?.instance_id).toBe('after-reset'); + }, + }, + { + name: 'removeCollection', + verify: async () => { + const removed = deferred>(); + const inspectDataService = vi.fn().mockResolvedValue(success(dataServiceInstance('after-remove'))); + const store = createCodingPluginsStore({ + list: vi.fn().mockResolvedValue(projection(false)), + removeCollection: vi.fn(() => removed.promise), + inspectDataService, + }); + store.setState({ + projectId: 'local-project', projection: projection(false), + dataService: dataServiceInstance('before-remove'), + }); + const mutation = store.getState().removeCollection('todos'); + await store.getState().load('local-project'); + removed.resolve(success({ removed: true, usage: { document_count: 0, total_bytes: 0 } })); + await mutation; + expect(inspectDataService).toHaveBeenCalledOnce(); + expect(store.getState().dataService?.instance_id).toBe('after-remove'); + }, + }, + { + name: 'removeProject', + verify: async () => { + const removed = deferred>(); + const list = vi.fn().mockResolvedValue(projection(false)); + const store = createCodingPluginsStore({ + list, + removeProject: vi.fn(() => removed.promise), + }); + store.setState({ + projectId: 'local-project', projection: projection(false), + dataService: dataServiceInstance('before-remove-project'), + }); + const mutation = store.getState().removeProject(); + await store.getState().load('local-project'); + removed.resolve(success({ removed: true })); + await mutation; + expect(list).toHaveBeenCalledTimes(2); + expect(store.getState().dataService).toBeNull(); + }, + }, + ]; + + it.each(adjacentMutationCases)( + 'uses the same same-project ordering guard for $name', + async ({ verify }) => await verify(), + ); + + it('reloads the newest projection when collection removal succeeds but inspection fails', async () => { + const readyProjection = projection(true); + readyProjection.items[0] = { + ...readyProjection.items[0], state: 'ready', backend: { status: 'ready' }, + }; + const afterRemoval = dataServiceInstance('after-remove-reload'); + const inspectDataService = vi.fn() + .mockResolvedValueOnce({ + success: false, status: 503, code: 'offline', error: 'offline', retryable: true, data: null, + }) + .mockResolvedValueOnce(success(afterRemoval)); + const list = vi.fn().mockResolvedValue(readyProjection); + const store = createCodingPluginsStore({ + list, + inspectDataService, + removeCollection: vi.fn().mockResolvedValue(success({ + removed: true, usage: { document_count: 0, total_bytes: 0 }, + })), + }); + store.setState({ + projectId: 'local-project', projection: readyProjection, + dataService: dataServiceInstance('before-remove-reload'), + }); + + await store.getState().removeCollection('todos'); + + expect(list).toHaveBeenCalledOnce(); + expect(inspectDataService).toHaveBeenCalledTimes(2); + expect(store.getState().dataService).toEqual(afterRemoval); + }); + it('does not let a late project A enable mutation overwrite loaded project B', async () => { const enabledA = deferred>(); const inspectDataService = vi.fn();