fix(plugin): serialize project refresh mutations

2026-08-27 21:42:33 +08:00
parent 405b9f64fd
commit e3e34d3803
3 changed files with 283 additions and 27 deletions
--- 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
@@ -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:<projectId>` 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.
--- a/src/stores/coding-plugins.ts
+++ b/src/stores/coding-plugins.ts
@@ -44,21 +44,53 @@ export function createCodingPluginsStore(overrides: Partial<Dependencies> = {}):
    removeProject: removeDataServiceProject, ...overrides,
  };
  const flights = new Map<string, Promise<void>>();
+  const pendingCounts = new Map<string, number>();
+  const projectEpochs = new Map<string, number>();
  let loadGeneration = 0;
-  const operation = (key: string, run: () => Promise<void>): Promise<void> => {
+  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<void>,
+    pendingKey = key,
+  ): Promise<void> => {
    const existing = flights.get(key); if (existing) return existing;
+    const pendingCount = (pendingCounts.get(pendingKey) ?? 0) + 1;
+    pendingCounts.set(pendingKey, pendingCount);
    let flight: Promise<void>;
    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<CodingPluginsState>((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<Dependencies> = {}):
            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);
      });
    },
  }));
--- 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<ReturnType<typeof projection>>();
+    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<ReturnType<typeof projection>>();
+    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<DataServiceHostResult<DataServiceInstanceState>>();
+        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<DataServiceHostResult<DataServiceInstanceState>>();
+        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<DataServiceHostResult<DataServiceCollectionRemoval>>();
+        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<DataServiceHostResult<DataServiceInstanceRemoval>>();
+        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<ReturnType<typeof projection>>();
    const inspectDataService = vi.fn();