diff --git a/packages/ui/src/sync/child-store.test.ts b/packages/ui/src/sync/child-store.test.ts index 2ee99f9b..01af0960 100644 --- a/packages/ui/src/sync/child-store.test.ts +++ b/packages/ui/src/sync/child-store.test.ts @@ -2,6 +2,7 @@ import { describe, expect, test } from 'bun:test'; import { ChildStoreManager, + type DirectoryBootstrapContext, markDirectorySessionPartChanged, subscribeDirectoryPermission, subscribeDirectoryQuestion, @@ -558,3 +559,54 @@ describe('ChildStoreManager directory bootstrap scheduler', () => { manager.disposeAll(); }); }); + +describe('ChildStoreManager bootstrap context liveness', () => { + test('isCurrent stays true after the run settles so deferred recovery work can commit', async () => { + const manager = new ChildStoreManager(); + let captured: DirectoryBootstrapContext | undefined; + const cleanup = manager.configure({ + onBootstrap: (context) => { + captured = context; + }, + }); + manager.requestBootstrap({ directory: '/workspace', priority: 'selected', reason: 'current-directory' }); + await settle(); + expect(manager.getBootstrapState('/workspace')).toBe('complete'); + + // bootstrapDirectory schedules deferred recovery pulls (permission.list + // and friends) from a setTimeout(0), which always runs after the pump's + // .finally() has cleaned up the run entry. isCurrent must remain true + // there, or those pulls and every commit they make get skipped. + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(captured?.isCurrent()).toBe(true); + + cleanup(); + 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 353b0a03..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 @@ -596,11 +598,23 @@ export class ChildStoreManager { queuedMs: Math.max(0, Date.now() - next.enqueuedAt), }) + // Store liveness, not run-token ownership. The pump deletes the run + // 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. 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.runningBootstraps.get(next.directory)?.token === token - && this.children.has(next.directory) + && this.directoryBootstrapRuns.get(next.directory) === runSequence + && this.children.get(next.directory) === store ) let bootstrapPromise: Promise try { @@ -673,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()