From cab2f9adf340876edb20de87644d5d16e4c6bcb7 Mon Sep 17 00:00:00 2001 From: yangyaofei Date: Thu, 27 Aug 2026 00:21:43 +0800 Subject: [PATCH 1/2] fix(sync): keep bootstrap context live for deferred recovery pulls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem Pending permission requests (and questions, MCP/LSP/VCS status) vanish permanently after a page reload. The deferred recovery phase in bootstrapDirectory — the code that re-pulls permission.list after load — never executed, so the UI had no way to re-render a card for a request the opencode server still holds. Fixes #3150. ## Root cause The deferred phase is scheduled via setTimeout(0) guarded by isStale(), which maps to the pump's isCurrent(). isCurrent required the run token to still be present in runningBootstraps, but the pump deletes that token in .finally() as soon as onBootstrap settles — always before the setTimeout macrotask fires. The guard was therefore deterministically stale and the phase was dead code (introduced by 85400459). ## Fix Make isCurrent a store-liveness check — disposed, generation, and store identity — instead of run-token ownership. The pump never replaces a running entry for the same directory (queueBootstrap defers via rerunRequested), so during the run itself this is equivalent. This mirrors the existing isCurrent contract in session-message-loader.ts. ## Validation - New regression test in child-store.test.ts fails on main and passes with the fix (isCurrent stays true across the post-settle macrotask, flips false after teardown). - bun test src/sync/: 539 pass / 15 fail — the same 15 fail on pristine main; the only delta is the new passing test. - bun run type-check (packages/ui): pass. - bunx oxlint on both changed files: no findings on authored lines. --- packages/ui/src/sync/child-store.test.ts | 27 ++++++++++++++++++++++++ packages/ui/src/sync/child-store.ts | 12 +++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/packages/ui/src/sync/child-store.test.ts b/packages/ui/src/sync/child-store.test.ts index 2ee99f9b..7572eea0 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,29 @@ 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(); + }); +}); diff --git a/packages/ui/src/sync/child-store.ts b/packages/ui/src/sync/child-store.ts index 353b0a03..8508c3a1 100644 --- a/packages/ui/src/sync/child-store.ts +++ b/packages/ui/src/sync/child-store.ts @@ -596,11 +596,19 @@ 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. 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. + 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.children.get(next.directory) === store ) let bootstrapPromise: Promise try { From 02ab4136de13952dd9667651b6ac41df42cf7d70 Mon Sep 17 00:00:00 2001 From: yangyaofei Date: Thu, 27 Aug 2026 00:30:39 +0800 Subject: [PATCH 2/2] 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()