Merge pull request #3151 from yangyaofei/fix-deferred-permissions

fix(sync): keep bootstrap context live for deferred recovery pulls

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();
+  });
+});

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<string, number>()
   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<void>
       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()