fix(sync): invalidate bootstrap context when a newer same-directory run starts
## 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.
This commit is contained in:
@@ -584,4 +584,29 @@ describe('ChildStoreManager bootstrap context liveness', () => {
|
|||||||
expect(captured?.isCurrent()).toBe(false);
|
expect(captured?.isCurrent()).toBe(false);
|
||||||
manager.disposeAll();
|
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();
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -307,6 +307,8 @@ export class ChildStoreManager {
|
|||||||
private bootstrapConcurrency = 2
|
private bootstrapConcurrency = 2
|
||||||
private bootstrapGeneration = 0
|
private bootstrapGeneration = 0
|
||||||
private bootstrapSequence = 0
|
private bootstrapSequence = 0
|
||||||
|
private bootstrapRunSequence = 0
|
||||||
|
private readonly directoryBootstrapRuns = new Map<string, number>()
|
||||||
private manualBootstrapDemandRevision = 0
|
private manualBootstrapDemandRevision = 0
|
||||||
private disposed = false
|
private disposed = false
|
||||||
|
|
||||||
@@ -600,14 +602,18 @@ export class ChildStoreManager {
|
|||||||
// token in `.finally()` as soon as onBootstrap settles, while
|
// token in `.finally()` as soon as onBootstrap settles, while
|
||||||
// bootstrapDirectory schedules deferred recovery pulls (permission.list
|
// bootstrapDirectory schedules deferred recovery pulls (permission.list
|
||||||
// and friends) from a `setTimeout(0)` that always runs after that
|
// and friends) from a `setTimeout(0)` that always runs after that
|
||||||
// cleanup — gating those on the token made them dead code. The pump
|
// cleanup — gating those on the token made them dead code. A per-
|
||||||
// never replaces a running entry for the same directory (queueBootstrap
|
// directory run sequence keeps the context current across settle (so
|
||||||
// defers via rerunRequested), so during the run itself this is
|
// deferred pulls commit) while invalidating it as soon as a newer run
|
||||||
// equivalent. Mirrors the isCurrent contract in session-message-loader.
|
// 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 store = this.children.get(next.directory)
|
||||||
const isCurrent = () => (
|
const isCurrent = () => (
|
||||||
!this.disposed
|
!this.disposed
|
||||||
&& this.bootstrapGeneration === running.generation
|
&& this.bootstrapGeneration === running.generation
|
||||||
|
&& this.directoryBootstrapRuns.get(next.directory) === runSequence
|
||||||
&& this.children.get(next.directory) === store
|
&& this.children.get(next.directory) === store
|
||||||
)
|
)
|
||||||
let bootstrapPromise: Promise<void>
|
let bootstrapPromise: Promise<void>
|
||||||
@@ -681,6 +687,7 @@ export class ChildStoreManager {
|
|||||||
this.manualBootstrapDemands.delete(directory)
|
this.manualBootstrapDemands.delete(directory)
|
||||||
this.bootstrapStates.delete(directory)
|
this.bootstrapStates.delete(directory)
|
||||||
this.bootstrapFailures.delete(directory)
|
this.bootstrapFailures.delete(directory)
|
||||||
|
this.directoryBootstrapRuns.delete(directory)
|
||||||
for (const demands of this.bootstrapDemandsByOwner.values()) demands.delete(directory)
|
for (const demands of this.bootstrapDemandsByOwner.values()) demands.delete(directory)
|
||||||
this.children.delete(directory)
|
this.children.delete(directory)
|
||||||
this.notifyRegistrySubscribers()
|
this.notifyRegistrySubscribers()
|
||||||
|
|||||||
Reference in New Issue
Block a user