From 02ab4136de13952dd9667651b6ac41df42cf7d70 Mon Sep 17 00:00:00 2001 From: yangyaofei Date: Thu, 27 Aug 2026 00:30:39 +0800 Subject: [PATCH] fix(sync): invalidate bootstrap context when a newer same-directory run starts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Review on #3151: with isCurrent reduced to store liveness, a completed run's context stays current while a forced same-directory rerun starts, so a late deferred response could commit over the newer run's state. ## Fix Track a per-directory run sequence. isCurrent captures the sequence at run start and stays true across settle (deferred recovery pulls still commit — the original bug) but flips false the moment a newer run for the same directory begins. Entries are cleared on directory disposal. ## Validation - New test: after a forced rerun the first context is retired and the second is current. 20 pass in child-store.test.ts. - bun test src/sync/: 540 pass / 15 fail — same 15 pre-existing on main. - type-check and oxlint clean on authored lines. --- packages/ui/src/sync/child-store.test.ts | 25 ++++++++++++++++++++++++ packages/ui/src/sync/child-store.ts | 15 ++++++++++---- 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/packages/ui/src/sync/child-store.test.ts b/packages/ui/src/sync/child-store.test.ts index 7572eea0..01af0960 100644 --- a/packages/ui/src/sync/child-store.test.ts +++ b/packages/ui/src/sync/child-store.test.ts @@ -584,4 +584,29 @@ describe('ChildStoreManager bootstrap context liveness', () => { expect(captured?.isCurrent()).toBe(false); manager.disposeAll(); }); + + test('a newer same-directory run invalidates the previous context', async () => { + const manager = new ChildStoreManager(); + const contexts: DirectoryBootstrapContext[] = []; + const cleanup = manager.configure({ + onBootstrap: (context) => { + contexts.push(context); + }, + }); + manager.requestBootstrap({ directory: '/workspace', priority: 'selected', reason: 'current-directory' }); + await settle(); + expect(contexts[0]?.isCurrent()).toBe(true); + + // A forced rerun for the same directory must retire the previous + // context: its in-flight deferred responses may no longer commit over + // whatever the newer run synchronizes. + manager.requestBootstrap({ directory: '/workspace', priority: 'selected', reason: 'server-connected', force: true }); + await settle(); + expect(contexts).toHaveLength(2); + expect(contexts[0]?.isCurrent()).toBe(false); + expect(contexts[1]?.isCurrent()).toBe(true); + + cleanup(); + manager.disposeAll(); + }); }); diff --git a/packages/ui/src/sync/child-store.ts b/packages/ui/src/sync/child-store.ts index 8508c3a1..6a7fa9c6 100644 --- a/packages/ui/src/sync/child-store.ts +++ b/packages/ui/src/sync/child-store.ts @@ -307,6 +307,8 @@ export class ChildStoreManager { private bootstrapConcurrency = 2 private bootstrapGeneration = 0 private bootstrapSequence = 0 + private bootstrapRunSequence = 0 + private readonly directoryBootstrapRuns = new Map() private manualBootstrapDemandRevision = 0 private disposed = false @@ -600,14 +602,18 @@ export class ChildStoreManager { // token in `.finally()` as soon as onBootstrap settles, while // bootstrapDirectory schedules deferred recovery pulls (permission.list // and friends) from a `setTimeout(0)` that always runs after that - // cleanup — gating those on the token made them dead code. The pump - // never replaces a running entry for the same directory (queueBootstrap - // defers via rerunRequested), so during the run itself this is - // equivalent. Mirrors the isCurrent contract in session-message-loader. + // cleanup — gating those on the token made them dead code. A per- + // directory run sequence keeps the context current across settle (so + // deferred pulls commit) while invalidating it as soon as a newer run + // starts, so a late deferred response cannot overwrite a newer run's + // state. Mirrors the isCurrent contract in session-message-loader. + const runSequence = ++this.bootstrapRunSequence + this.directoryBootstrapRuns.set(next.directory, runSequence) const store = this.children.get(next.directory) const isCurrent = () => ( !this.disposed && this.bootstrapGeneration === running.generation + && this.directoryBootstrapRuns.get(next.directory) === runSequence && this.children.get(next.directory) === store ) let bootstrapPromise: Promise @@ -681,6 +687,7 @@ export class ChildStoreManager { this.manualBootstrapDemands.delete(directory) this.bootstrapStates.delete(directory) this.bootstrapFailures.delete(directory) + this.directoryBootstrapRuns.delete(directory) for (const demands of this.bootstrapDemandsByOwner.values()) demands.delete(directory) this.children.delete(directory) this.notifyRegistrySubscribers()