From 4db77d398808005f3f6567e76aeb6d446ae16629 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 16:57:47 +0800 Subject: [PATCH] fix(ui-settings): clear the mirror in-flight slot in the rerun check's segment --- .../ui-settings/src/client/settings-mirror.ts | 66 +++++++++++-------- .../tests/settings-mirror.client.spec.ts | 14 ++++ 2 files changed, 51 insertions(+), 29 deletions(-) diff --git a/packages/client/ui-settings/src/client/settings-mirror.ts b/packages/client/ui-settings/src/client/settings-mirror.ts index a1dfe8c5a8..398895c9cb 100644 --- a/packages/client/ui-settings/src/client/settings-mirror.ts +++ b/packages/client/ui-settings/src/client/settings-mirror.ts @@ -89,7 +89,7 @@ export class SettingsDescribeMirror { this.rerun = true return this.inFlight } - const run = this.run().finally(() => { this.inFlight = undefined }) + const run = this.run() this.inFlight = run return run } @@ -132,33 +132,41 @@ export class SettingsDescribeMirror { } private async run(): Promise { - do { - this.rerun = false - const generation = ++this.generation - const before = this.store.getSnapshot() - if (before.status === 'idle') this.store.set({ ...before, status: 'loading' }) - let outcome: { view: SettingsDescribeView } | { failure: string } - try { - const response = await this.api.settings.describe({}) - outcome = response.result.ok - ? { view: response.result.value } - : { failure: response.result.error.message } - } catch (error) { - outcome = { failure: error instanceof Error ? error.message : String(error) } - } - if (generation !== this.generation) continue - if ('view' in outcome) { - this.store.set({ status: 'ready', view: outcome.view, error: null }) - } else { - const held = this.store.getSnapshot() - // No answer yet: fall back to idle so `ensure` retries; with one, the - // held view keeps serving and only the error field reports the miss. - this.store.set({ - status: held.view === undefined ? 'idle' : 'ready', - view: held.view, - error: outcome.failure, - }) - } - } while (this.rerun) + // The in-flight slot must clear in the same synchronous segment that + // observes `rerun` false (and on abrupt exit): a `.finally()` on the + // returned promise runs one microtask later, and a `load()` landing in + // that gap would mark a rerun nobody reads, losing the read. + try { + do { + this.rerun = false + const generation = ++this.generation + const before = this.store.getSnapshot() + if (before.status === 'idle') this.store.set({ ...before, status: 'loading' }) + let outcome: { view: SettingsDescribeView } | { failure: string } + try { + const response = await this.api.settings.describe({}) + outcome = response.result.ok + ? { view: response.result.value } + : { failure: response.result.error.message } + } catch (error) { + outcome = { failure: error instanceof Error ? error.message : String(error) } + } + if (generation !== this.generation) continue + if ('view' in outcome) { + this.store.set({ status: 'ready', view: outcome.view, error: null }) + } else { + const held = this.store.getSnapshot() + // No answer yet: fall back to idle so `ensure` retries; with one, the + // held view keeps serving and only the error field reports the miss. + this.store.set({ + status: held.view === undefined ? 'idle' : 'ready', + view: held.view, + error: outcome.failure, + }) + } + } while (this.rerun) + } finally { + this.inFlight = undefined + } } } diff --git a/packages/client/ui-settings/tests/settings-mirror.client.spec.ts b/packages/client/ui-settings/tests/settings-mirror.client.spec.ts index 20c0cc55ee..058c401d5e 100644 --- a/packages/client/ui-settings/tests/settings-mirror.client.spec.ts +++ b/packages/client/ui-settings/tests/settings-mirror.client.spec.ts @@ -125,6 +125,20 @@ describe('SettingsDescribeMirror', () => { expect(mirror.getSnapshot().view?.namespaces).toHaveLength(2) }) + it('never loses a load landing between a run settling and its slot clearing', async () => { + // Regression: with the in-flight slot cleared by a promise .finally(), + // a load() in the one-microtask gap after the rerun check marked a rerun + // nobody read, and that refresh never reached the wire. + const describeCall = vi.fn().mockResolvedValue(described([view('theme', 1)])) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + void mirror.load() + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(1) }) + void mirror.load() + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(2) }) + void mirror.load() + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(3) }) + }) + it('suppresses a stale answer that lost to a newer generation', async () => { const slow = deferred>() const describeCall = vi.fn()