From 29d6066870fd35488b4bf60f9237939dd8b0def1 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 16:46:14 +0800 Subject: [PATCH 01/28] feat(ui-settings): add SettingsDescribeMirror single describe source --- .../ui-settings/src/client/settings-mirror.ts | 164 ++++++++++++++++++ .../tests/settings-mirror.client.spec.ts | 140 +++++++++++++++ 2 files changed, 304 insertions(+) create mode 100644 packages/client/ui-settings/src/client/settings-mirror.ts create mode 100644 packages/client/ui-settings/tests/settings-mirror.client.spec.ts diff --git a/packages/client/ui-settings/src/client/settings-mirror.ts b/packages/client/ui-settings/src/client/settings-mirror.ts new file mode 100644 index 0000000000..a1dfe8c5a8 --- /dev/null +++ b/packages/client/ui-settings/src/client/settings-mirror.ts @@ -0,0 +1,164 @@ +/** + * Client mirror of the Host settings document: the one `settings.describe` + * reader in the browser. Every settings consumer derives from this store — + * per-namespace scopes through `SettingsScopeBinder.bind`, cross-namespace + * surfaces through the binder's read-only describe face — so startup cost and + * freshness are properties of this class, not of how many features own a + * preference. The Host stays the fact source: the mirror re-reads on the + * invalidations its owning plugin subscribes to and folds write answers in + * through {@link SettingsDescribeMirror.acceptView}. + */ + +import type { IApiClient, SettingsNamespaceView } from '@deepseek-ai/dsh-api-remotes/client' +import { createSnapshotStore, type SnapshotStore } from '@deepseek-ai/dsh-client-runtime/client' + +type SettingsFace = Pick + +/** The full `settings.describe` answer the mirror serves. */ +export interface SettingsDescribeView { + /** Every namespace a live Host plugin registered, as the Host reported it. */ + namespaces: readonly SettingsNamespaceView[] + /** Whether the settings provider accepts writes. */ + writable: boolean + /** Whether a native settings document exists for the Host to open. */ + hasDocument: boolean +} + +/** Mirror state every derived settings surface renders from. */ +export interface SettingsMirrorSnapshot { + /** + * `unavailable` is the terminal non-loopback state; `ready` persists across + * later failed refreshes (the held view keeps serving); `idle` means no + * answer is held and no read is running, so `ensure` will start one. + */ + status: 'idle' | 'loading' | 'ready' | 'unavailable' + /** The last good answer; undefined until the first success. */ + view: SettingsDescribeView | undefined + /** The latest refresh failure message, cleared by the next success. */ + error: string | null +} + +/** + * Serializes every Host `settings.describe` read behind one snapshot store. + * Concurrent {@link load} calls fold into the in-flight read plus one rerun, + * so an invalidation arriving mid-read is never lost and never duplicated. + */ +export class SettingsDescribeMirror { + private readonly store: SnapshotStore + private inFlight: Promise | undefined + private rerun = false + private generation = 0 + + /** + * @param api - settings wire face. + * @param persistence - remote browsers stay process-local because settings RPCs are loopback-only. + */ + constructor( + private readonly api: SettingsFace, + private readonly persistence: 'host' | 'memory' = 'host', + ) { + this.store = createSnapshotStore({ + status: persistence === 'host' ? 'idle' : 'unavailable', + view: undefined, + error: null, + }) + } + + /** @returns the current sync snapshot (stable reference until the next change). */ + getSnapshot(): SettingsMirrorSnapshot { + return this.store.getSnapshot() + } + + /** + * Observe snapshot replacements. + * @param listener - invoked after each snapshot change. + * @returns the disposer removing this listener. + */ + subscribe(listener: () => void): () => void { + return this.store.subscribe(listener) + } + + /** + * Refresh from the Host. A call during an in-flight read marks one rerun + * after it settles instead of racing a second wire read. + * @returns settlement after this call's freshness is reflected. + */ + load(): Promise { + if (this.persistence === 'memory') return Promise.resolve() + if (this.inFlight !== undefined) { + this.rerun = true + return this.inFlight + } + const run = this.run().finally(() => { this.inFlight = undefined }) + this.inFlight = run + return run + } + + /** + * Resolve once an answer is held (or the mirror is terminally unavailable), + * reading only from `idle`. The cheap idempotent entry for surfaces that + * render on first use. + * @returns settlement of the current or newly started read, if any. + */ + ensure(): Promise { + if (this.persistence === 'memory') return Promise.resolve() + if (this.inFlight !== undefined) return this.inFlight + if (this.getSnapshot().status === 'idle') return this.load() + return Promise.resolve() + } + + /** + * Fold one write answer's namespace view into the held view without a wire + * read. A no-op until a first answer exists — a write cannot precede the + * read that supplied its `expectedRevision`. + * @param view - the namespace view a settings write answered with. + */ + acceptView(view: SettingsNamespaceView): void { + const before = this.store.getSnapshot() + if (before.view === undefined) return + const namespaces = before.view.namespaces.some(row => row.ns === view.ns) + ? before.view.namespaces.map(row => row.ns === view.ns ? view : row) + : [...before.view.namespaces, view] + this.store.set({ ...before, view: { ...before.view, namespaces } }) + } + + /** + * Convenience row lookup on the held view. + * @param ns - namespace identity. + * @returns the namespace view, or undefined while unanswered or unregistered. + */ + namespace(ns: string): SettingsNamespaceView | undefined { + return this.store.getSnapshot().view?.namespaces.find(row => row.ns === ns) + } + + 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) + } +} diff --git a/packages/client/ui-settings/tests/settings-mirror.client.spec.ts b/packages/client/ui-settings/tests/settings-mirror.client.spec.ts new file mode 100644 index 0000000000..20c0cc55ee --- /dev/null +++ b/packages/client/ui-settings/tests/settings-mirror.client.spec.ts @@ -0,0 +1,140 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcResponse, SettingsNamespaceView } from '@deepseek-ai/dsh-api-remotes/client' +import { SettingsDescribeMirror, type SettingsDescribeView } from '../src/client/settings-mirror.ts' + +let rpc = 0 + +function ok(value: T): RpcResponse { + return { rpcId: `mirror-${rpc++}` as never, result: { ok: true, value } } +} + +function rejected(message: string): RpcResponse { + return { + rpcId: `mirror-${rpc++}` as never, + result: { + ok: false, + error: { code: 'settings-rejected', message, details: {} }, + }, + } +} + +function view(ns: string, revision = 0): SettingsNamespaceView { + return { ns, schema: {}, value: { field: ns }, applies: 'live', secrets: [], revision } +} + +function described(namespaces: SettingsNamespaceView[]): RpcResponse { + return ok({ writable: true, hasDocument: true, namespaces }) +} + +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise((res) => { resolve = res }) + return { promise, resolve } +} + +describe('SettingsDescribeMirror', () => { + it('folds concurrent load calls into the in-flight read plus one rerun', async () => { + const gate = deferred>() + const describeCall = vi.fn() + .mockReturnValueOnce(gate.promise) + .mockResolvedValue(described([view('theme', 1)])) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + const first = mirror.load() + const second = mirror.load() + const third = mirror.load() + gate.resolve(described([view('theme', 0)])) + await Promise.all([first, second, third]) + expect(describeCall).toHaveBeenCalledTimes(2) + expect(mirror.getSnapshot().status).toBe('ready') + expect(mirror.namespace('theme')?.revision).toBe(1) + }) + + it('keeps the last good view when a later refresh fails, recording the failure', async () => { + const describeCall = vi.fn() + .mockResolvedValueOnce(described([view('theme', 2)])) + .mockRejectedValueOnce(new Error('host gone')) + .mockResolvedValueOnce(rejected('busy')) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + await mirror.load() + expect(mirror.getSnapshot()).toMatchObject({ status: 'ready', error: null }) + await mirror.load() + expect(mirror.getSnapshot()).toMatchObject({ status: 'ready', error: 'host gone' }) + expect(mirror.namespace('theme')?.revision).toBe(2) + await mirror.load() + expect(mirror.getSnapshot()).toMatchObject({ status: 'ready', error: 'busy' }) + expect(mirror.getSnapshot().view?.namespaces).toHaveLength(1) + }) + + it('returns to idle after a first read that never succeeded, so ensure retries', async () => { + const describeCall = vi.fn() + .mockRejectedValueOnce(new Error('offline')) + .mockResolvedValueOnce(described([view('theme', 1)])) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + await mirror.ensure() + expect(mirror.getSnapshot()).toMatchObject({ status: 'idle', view: undefined, error: 'offline' }) + await mirror.ensure() + expect(mirror.getSnapshot()).toMatchObject({ status: 'ready', error: null }) + expect(describeCall).toHaveBeenCalledTimes(2) + }) + + it('treats ensure as a no-op once ready', async () => { + const describeCall = vi.fn().mockResolvedValue(described([view('theme', 1)])) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + await mirror.ensure() + await mirror.ensure() + await mirror.ensure() + expect(describeCall).toHaveBeenCalledTimes(1) + }) + + it('memory persistence is terminally unavailable and never touches the wire', async () => { + const describeCall = vi.fn() + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never, 'memory') + await mirror.ensure() + await mirror.load() + expect(mirror.getSnapshot()).toEqual({ status: 'unavailable', view: undefined, error: null }) + expect(describeCall).not.toHaveBeenCalled() + }) + + it('acceptView folds one write answer into the held view without a wire read', async () => { + const describeCall = vi.fn() + .mockResolvedValueOnce(described([view('theme', 1), view('locale', 4)])) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + await mirror.load() + const seen: number[] = [] + mirror.subscribe(() => { seen.push(mirror.namespace('theme')?.revision ?? -1) }) + mirror.acceptView(view('theme', 9)) + expect(mirror.namespace('theme')?.revision).toBe(9) + expect(mirror.namespace('locale')?.revision).toBe(4) + expect(seen).toEqual([9]) + expect(describeCall).toHaveBeenCalledTimes(1) + }) + + it('acceptView before any answer is a no-op instead of inventing a document', () => { + const describeCall = vi.fn() + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + mirror.acceptView(view('theme', 1)) + expect(mirror.getSnapshot()).toEqual({ status: 'idle', view: undefined, error: null }) + }) + + it('acceptView appends a namespace the held view has not seen yet', async () => { + const describeCall = vi.fn().mockResolvedValueOnce(described([view('theme', 1)])) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + await mirror.load() + mirror.acceptView(view('fresh-ns', 0)) + expect(mirror.namespace('fresh-ns')).toBeDefined() + expect(mirror.getSnapshot().view?.namespaces).toHaveLength(2) + }) + + it('suppresses a stale answer that lost to a newer generation', async () => { + const slow = deferred>() + const describeCall = vi.fn() + .mockReturnValueOnce(slow.promise) + .mockResolvedValue(described([view('theme', 8)])) + const mirror = new SettingsDescribeMirror({ settings: { describe: describeCall } } as never) + const first = mirror.load() + const second = mirror.load() + slow.resolve(described([view('theme', 1)])) + await Promise.all([first, second]) + expect(mirror.namespace('theme')?.revision).toBe(8) + }) +}) From 4db77d398808005f3f6567e76aeb6d446ae16629 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 16:57:47 +0800 Subject: [PATCH 02/28] 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() From fd61fa889b697fae3e519863553985bfd79cc8d2 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 16:57:48 +0800 Subject: [PATCH 03/28] refactor(ui-settings): derive settings scopes from the describe mirror --- .../client/ui-settings/src/client/index.ts | 53 +++- .../ui-settings/src/client/settings-scope.ts | 138 ++++++----- .../ui-settings/tests/plugin.client.spec.ts | 45 +++- .../tests/settings-scope.client.spec.ts | 232 ++++++++---------- 4 files changed, 250 insertions(+), 218 deletions(-) diff --git a/packages/client/ui-settings/src/client/index.ts b/packages/client/ui-settings/src/client/index.ts index 2ace9e56b1..b3c149c938 100644 --- a/packages/client/ui-settings/src/client/index.ts +++ b/packages/client/ui-settings/src/client/index.ts @@ -1,35 +1,64 @@ /** * Settings domain base plugin, browser half. Provides `ctx.settingsScope`, the - * settings-namespace Host transport every preference row binds its durable - * section through, and owns the canonical slot-type contract for the settings - * surface. It depends on no `ui-*` presentation package, so any feature that - * owns a preference can reach it: the settings SHELL — the `sidebar.settings` - * occupant, its navigation, and the chrome — lives in ui-settings-general, - * because a shell dependency on ui-sidebar would close a reference cycle - * through ui-layout and ui-theme. Export discipline: packages/client/AGENTS.md. + * settings-namespace scope service every preference row binds its durable + * section through, and owns the one `settings.describe` reader in the browser: + * the describe mirror, whose invalidation subscriptions + * (`settings/document-updated`, `connection/reset`) live here so every derived + * surface refreshes from a single wire read. It depends on no `ui-*` + * presentation package, so any feature that owns a preference can reach it: + * the settings SHELL — the `sidebar.settings` occupant, its navigation, and + * the chrome — lives in ui-settings-general, because a shell dependency on + * ui-sidebar would close a reference cycle through ui-layout and ui-theme. + * Export discipline: packages/client/AGENTS.md. */ import type { ClientContext } from '@deepseek-ai/dsh-client-runtime/client' +import type { ConnectionHandle } from '@deepseek-ai/dsh-api-remotes/client' +// Type-only pair supplying `$on` and its key face without dragging a build +// artifact into the Host graph (rationale beside the same pair in +// settings-scope.ts). +import type {} from '@deepseek-ai/dsh-api-remotes/types' +import type {} from '@deepseek-ai/dsh-settings/types' import { SettingsScopeBinder } from './settings-scope.ts' +import { SettingsDescribeMirror } from './settings-mirror.ts' export type { SettingsGeneralItemOwnerProps, SettingsHeaderOwnerProps, SettingsOnboardingOwnerProps, SettingsPluginsTabOwnerProps, SettingsSectionOwnerProps, SettingsTriggerOwnerProps, } from './contract/slots.ts' export { SettingsScopeController, SettingsScopeBinder } from './settings-scope.ts' +export { SettingsDescribeMirror } from './settings-mirror.ts' +export type { SettingsDescribeView, SettingsMirrorSnapshot } from './settings-mirror.ts' /** - * Required services: none. The transport is resolved per caller through - * `this.ctx` at `bind` time, so this plugin waits for nothing. + * Required services: the wire handle for the mirror's reads and the forwarded + * settings invalidation the mirror refreshes on. */ -export const inject = [] +export const inject = ['connection', 'remote'] /** - * Provide the settings-namespace scope service. + * Provide the settings-namespace scope service over one shared describe + * mirror, and keep that mirror fresh on the two signals that can move the + * settings document: a document commit and a (re)connect. * * Constructing the service in this plugin's fiber keeps its traced methods * bound to each consuming plugin's context. * @param ctx - client root context. */ export function apply(ctx: ClientContext): void { - new SettingsScopeBinder(ctx) + const connection = ctx.get('connection') as ConnectionHandle + const mirror = new SettingsDescribeMirror( + connection.api, + connection.isLoopback ? 'host' : 'memory', + ) + ctx.effect(() => { + const disposers = [ + (ctx.get('remote') as ClientContext['remote']).$on('settings/document-updated', () => { void mirror.load() }), + ctx.on('connection/reset', () => { void mirror.load() }), + ] + // The first connection also emits connection/reset; the in-flight fold + // makes this eager read and that reset converge to one wire call. + void mirror.ensure() + return () => { for (const dispose of disposers) dispose() } + }, 'ui-settings: describe mirror invalidations') + new SettingsScopeBinder(ctx, { mirror }) } diff --git a/packages/client/ui-settings/src/client/settings-scope.ts b/packages/client/ui-settings/src/client/settings-scope.ts index 4668c4924b..47ebb3daa3 100644 --- a/packages/client/ui-settings/src/client/settings-scope.ts +++ b/packages/client/ui-settings/src/client/settings-scope.ts @@ -1,8 +1,11 @@ /** * Host transport for the settings-namespace scope contract. The contract types * live in `dsh-client-runtime` (the common dependency of every feature that - * owns a preference); this file owns the wire behavior and the invalidation - * subscription, both of which are Settings-surface concerns. + * owns a preference); this file owns the per-namespace derivation over the + * shared {@link SettingsDescribeMirror} and the serialized write path, both of + * which are Settings-surface concerns. Reads never touch the wire here: the + * mirror is the one `settings.describe` reader, and every scope is a selector + * over its snapshot. */ import { Service } from '@deepseek-ai/cordis' @@ -22,8 +25,8 @@ import { // Client half declares `ctx.remote` with no generated import, and the // allowlist's `types` subpath is a pure-type source file, so the pair supplies // `$on` and its key face without dragging a build artifact in. The runtime -// `remote` injection belongs to whoever calls bindSettingsScope: the -// subscription is registered on the caller's own context. +// `remote` injection belongs to the providing plugin's apply, which registers +// the mirror's invalidation subscriptions. import type {} from '@deepseek-ai/dsh-api-remotes/client' import type {} from '@deepseek-ai/dsh-api-remotes/types' // The forwarded event's own declaration: `$on`'s key face is @@ -31,29 +34,39 @@ import type {} from '@deepseek-ai/dsh-api-remotes/types' // never — the owning package's client-safe, type-only subpath supplies the // cordis `Events` entry (and with it the branded `SettingsNamespace`). import type {} from '@deepseek-ai/dsh-settings/types' +import { SettingsDescribeMirror } from './settings-mirror.ts' + type SettingsFace = Pick /** - * Serializes one namespace's Host reads and writes behind a snapshot store. - * Reads never block plugin activation; writes carry the latest known - * namespace revision and teardown waits for the operation already crossing - * the wire. + * One namespace's derived view over the shared describe mirror, plus that + * namespace's serialized Host writes. Writes carry the latest known namespace + * revision, fold their answers back into the mirror, and teardown waits for + * the operation already crossing the wire. */ export class SettingsScopeController implements SettingsScope { private readonly store: SnapshotStore> private tail: Promise = Promise.resolve() - private readGeneration = 0 private writeGeneration = 0 private disposed = false + private readonly unsubscribe: (() => void) | undefined + /** + * Revision answered by a superseded write still ahead of the mirror: the + * mirror only folds the LATEST settlement in, so a queued successor takes + * its fence from here first. + */ + private pendingRevision: number | undefined /** - * @param api - settings wire face. + * @param api - settings wire face (writes only; reads ride the mirror). * @param spec - namespace identity and optional narrowing decoder. + * @param mirror - the shared describe mirror this scope derives from. * @param persistence - remote browsers remain process-local because settings RPCs are loopback-only. */ constructor( private readonly api: SettingsFace, private readonly spec: SettingsScopeSpec, + private readonly mirror: SettingsDescribeMirror, private readonly persistence: 'host' | 'memory' = 'host', ) { this.store = createSnapshotStore>({ @@ -65,6 +78,10 @@ export class SettingsScopeController implements SettingsScope { writable: false, mode: persistence, }) + if (persistence === 'host') { + this.unsubscribe = mirror.subscribe(() => { this.derive() }) + this.derive() + } } /** @returns the current sync snapshot (stable reference until the next change). */ @@ -81,15 +98,6 @@ export class SettingsScopeController implements SettingsScope { return this.store.subscribe(listener) } - /** - * Queue a Host refresh; a newer read or user write suppresses stale publication. - * @returns settlement after the queued read completes or is skipped. - */ - load(): Promise { - const generation = ++this.readGeneration - return this.enqueue(() => this.read(generation)) - } - /** * Queue one field write; see {@link SettingsScope.set} for the ordering, * revision, and recovery contract. @@ -112,10 +120,9 @@ export class SettingsScopeController implements SettingsScope { } private write(op: SettingsPathOpView): Promise { - this.readGeneration += 1 const generation = ++this.writeGeneration return this.enqueue(async () => { - const revision = this.getSnapshot().revision + const revision = this.pendingRevision ?? this.getSnapshot().revision let response: Awaited> try { response = await this.api.settings.mutate({ @@ -124,25 +131,39 @@ export class SettingsScopeController implements SettingsScope { ...(revision === undefined ? {} : { expectedRevision: revision }), }) } catch (_settingsWriteFailure) { - if (!this.disposed && generation === this.writeGeneration) await this.read(++this.readGeneration) + await this.recover(generation) return } if (!response.result.ok) { - if (!this.disposed && generation === this.writeGeneration) await this.read(++this.readGeneration) + await this.recover(generation) return } - this.accept(response.result.value, generation === this.writeGeneration) + if (this.disposed) return + if (generation === this.writeGeneration) { + this.pendingRevision = undefined + this.mirror.acceptView(response.result.value) + } else { + this.pendingRevision = response.result.value.revision + } }) } + /** Reload Host state for the latest failed write; superseded failures leave recovery to it. */ + private async recover(generation: number): Promise { + if (this.disposed || generation !== this.writeGeneration) return + this.pendingRevision = undefined + await this.mirror.load() + } + /** - * Stop queued operations and wait for the current wire call to settle. + * Stop queued operations, stop deriving, and wait for the current wire call + * to settle. * @returns settlement after the controller reaches quiescence. */ async dispose(): Promise { this.disposed = true - this.readGeneration += 1 this.writeGeneration += 1 + this.unsubscribe?.() await this.tail } @@ -158,36 +179,25 @@ export class SettingsScopeController implements SettingsScope { return task } - private async read(generation: number): Promise { - let response: Awaited> - try { - response = await this.api.settings.describe({}) - } catch (_settingsReadFailure) { - return - } - if (!response.result.ok || this.disposed) return - const { namespaces, writable } = response.result.value - const view = namespaces.find(candidate => candidate.ns === this.spec.namespace) - const publish = generation === this.readGeneration + private derive(): void { + if (this.disposed) return + const mirrored = this.mirror.getSnapshot() + if (mirrored.view === undefined) return + const { writable } = mirrored.view + const view = mirrored.view.namespaces.find(candidate => candidate.ns === this.spec.namespace) if (view === undefined) { - if (publish) { - this.store.update((draft) => { - draft.status = 'unavailable' - draft.writable = writable - }) - } + this.store.update((draft) => { + draft.status = 'unavailable' + draft.writable = writable + }) return } - this.accept(view, publish, writable) - } - - private accept(view: SettingsNamespaceView, publish: boolean, writable?: boolean): void { - const decoded = publish ? this.decode(view) : undefined + const decoded = this.decode(view) this.store.update((draft) => { draft.revision = view.revision draft.base = view.base draft.user = view.user - if (writable !== undefined) draft.writable = writable + draft.writable = writable if (decoded === undefined) return draft.status = 'ready' draft.value = decoded @@ -225,20 +235,24 @@ declare module '@deepseek-ai/cordis' { * (`packages/client/tsdown.client.ts`). */ export class SettingsScopeBinder extends Service { + private readonly mirror: SettingsDescribeMirror + /** * @param ctx - the providing plugin's context. + * @param config - the shared describe mirror every bound scope derives from. */ - constructor(ctx: Context) { + constructor(ctx: Context, config: { mirror: SettingsDescribeMirror }) { super(ctx, 'settingsScope') + this.mirror = config.mirror } /** - * Bind one namespace scope to settings and connection invalidations on the - * CALLER's plugin lifecycle — the service proxy binds `this.ctx` to the - * caller at call time, so the scope's disposer belongs to the calling fiber. - * Listeners exist before the initial background read starts, so activation - * never blocks on the settings transport. The caller injects `connection` - * for the transport and `remote` for the forwarded settings invalidation. + * Bind one namespace scope on the CALLER's plugin lifecycle — the service + * proxy binds `this.ctx` to the caller at call time, so the scope's disposer + * belongs to the calling fiber. The scope derives from the shared mirror + * (whose invalidation subscriptions live with the providing plugin), so + * binding adds no wire read of its own and activation never blocks on the + * settings transport. * @param spec - domain-owned namespace contract. * @returns the bound scope consumed by the domain's services and rows. */ @@ -248,20 +262,12 @@ export class SettingsScopeBinder extends Service { const controller = new SettingsScopeController( connection.api, spec, + this.mirror, connection.isLoopback ? 'host' : 'memory', ) ctx.effect(() => { - const refresh = (namespace?: string): void => { - if (namespace !== undefined && namespace !== spec.namespace) return - void controller.load() - } - const disposers = [ - (ctx.get('remote') as Context['remote']).$on('settings/document-updated', refresh), - ctx.on('connection/reset', () => { refresh() }), - ] - void controller.load() + void this.mirror.ensure() return async () => { - for (const dispose of disposers) dispose() await controller.dispose() } }, `ui-settings: ${spec.namespace} settings scope`) diff --git a/packages/client/ui-settings/tests/plugin.client.spec.ts b/packages/client/ui-settings/tests/plugin.client.spec.ts index 1643e9580f..c63bf1fe00 100644 --- a/packages/client/ui-settings/tests/plugin.client.spec.ts +++ b/packages/client/ui-settings/tests/plugin.client.spec.ts @@ -1,29 +1,56 @@ /** * The settings domain base plugin's own mounting behavior: it stands up - * `ctx.settingsScope` for every feature that owns a preference row, and the - * service retires with its fiber. + * `ctx.settingsScope` over one shared describe mirror, keeps that mirror + * fresh on settings-document and connection-reset invalidations, and retires + * both the service and the subscriptions with its fiber. */ import { Context } from '@deepseek-ai/cordis' -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' +import { TestRemote } from '@deepseek-ai/dsh-client-test-runtime' import { apply, inject, SettingsScopeBinder } from '../src/client/index.ts' -/** Boot the browser half over a bare root context; it injects nothing. */ +/** Boot the browser half over a fake loopback connection and test remote. */ function bench() { + const describeCall = vi.fn().mockResolvedValue({ + rpcId: 'plugin-bench' as never, + result: { ok: true, value: { writable: true, hasDocument: true, namespaces: [] } }, + }) const ctx = new Context() - return { ctx, fiber: ctx.plugin({ inject: [...inject], apply }) } + ctx.provide('connection', { + api: { settings: { describe: describeCall } }, + isLoopback: true, + } as never) + new TestRemote(ctx) + return { ctx, describeCall, fiber: ctx.plugin({ inject: [...inject], apply }) } } describe('settings domain base plugin', () => { - it('mounts the scope service under settingsScope', async () => { - const { ctx, fiber } = bench() + it('mounts the scope service under settingsScope and reads once eagerly', async () => { + const { ctx, describeCall, fiber } = bench() await fiber.await() expect(ctx.get('settingsScope')).toBeInstanceOf(SettingsScopeBinder) + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(1) }) }) - it('fiber disposal retires the service', async () => { - const { ctx, fiber } = bench() + it('refreshes the mirror on document commits and connection resets, once each', async () => { + const { ctx, describeCall, fiber } = bench() await fiber.await() + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(1) }) + ctx.remote.$dispatch('settings/document-updated', ['ui-test', 0]) + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(2) }) + ctx.emit('connection/reset') + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(3) }) + }) + + it('fiber disposal retires the service and its invalidation subscriptions', async () => { + const { ctx, describeCall, fiber } = bench() + await fiber.await() + await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(1) }) await fiber.dispose() expect(ctx.get('settingsScope')).toBeUndefined() + ctx.remote.$dispatch('settings/document-updated', ['ui-test', 0]) + ctx.emit('connection/reset') + await Promise.resolve() + expect(describeCall).toHaveBeenCalledTimes(1) }) }) diff --git a/packages/client/ui-settings/tests/settings-scope.client.spec.ts b/packages/client/ui-settings/tests/settings-scope.client.spec.ts index 429002028a..1e4c473ef1 100644 --- a/packages/client/ui-settings/tests/settings-scope.client.spec.ts +++ b/packages/client/ui-settings/tests/settings-scope.client.spec.ts @@ -5,6 +5,7 @@ import type { RpcResponse, SettingsNamespaceView } from '@deepseek-ai/dsh-api-re import { TestRemote } from '@deepseek-ai/dsh-client-test-runtime' import type { SettingsScope } from '@deepseek-ai/dsh-client-runtime/client' import { SettingsScopeController, SettingsScopeBinder } from '../src/client/settings-scope.ts' +import { SettingsDescribeMirror } from '../src/client/settings-mirror.ts' interface UiTestSettings { preference: 'light' | 'dark' | 'system' @@ -52,6 +53,17 @@ function deferred() { return { promise, resolve, reject } } +/** A host-mode mirror plus a controller derived from it, over one fake wire. */ +function derivedScope( + api: { describe?: ReturnType; mutate?: ReturnType }, + spec: { namespace: string; decode?: (section: unknown) => UiTestSettings | undefined } = { namespace: 'ui-test' }, +) { + const wire = { settings: api } as never + const mirror = new SettingsDescribeMirror(wire) + const scope = new SettingsScopeController(wire, spec, mirror) + return { mirror, scope } +} + /** Record each distinct published section, starting from the current one. */ function trackValues(scope: SettingsScope): Array { const seen: Array = [scope.getSnapshot().value] @@ -63,16 +75,13 @@ function trackValues(scope: SettingsScope): Array { - it('starts loading and publishes a schema-valid section with revision and writability', async () => { + it('starts loading and derives a schema-valid section with revision and writability', async () => { const describeCall = vi.fn().mockResolvedValueOnce(described({ preference: 'dark' }, 3)) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall }) expect(scope.getSnapshot()).toEqual({ status: 'loading', value: undefined, revision: undefined, writable: false, mode: 'host', }) - await scope.load() + await mirror.load() expect(scope.getSnapshot()).toEqual({ status: 'ready', value: { preference: 'dark' }, revision: 3, writable: true, mode: 'host', }) @@ -87,12 +96,9 @@ describe('SettingsScopeController', () => { .mockResolvedValueOnce(described(['queue'], 7)) .mockResolvedValueOnce(rejected()) .mockRejectedValueOnce(new Error('offline')) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall }) const good = trackValues(scope) - for (let i = 0; i < 7; i++) await scope.load() + for (let i = 0; i < 7; i++) await mirror.load() expect(scope.getSnapshot()).toMatchObject({ status: 'ready', value: { preference: 'dark' }, revision: 7, }) @@ -103,45 +109,22 @@ describe('SettingsScopeController', () => { const broken = { ...view({ preference: 'dark' }, 2), schema: null } const describeCall = vi.fn() .mockResolvedValueOnce(ok({ writable: true, hasDocument: true, namespaces: [broken] })) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) - await scope.load() + const { mirror, scope } = derivedScope({ describe: describeCall }) + await mirror.load() expect(scope.getSnapshot()).toMatchObject({ status: 'loading', value: undefined, revision: 2 }) }) - it('suppresses a superseded read of an unexposed namespace', async () => { - const describeCall = vi.fn() - .mockResolvedValueOnce(ok({ writable: true, hasDocument: true, namespaces: [] })) - .mockResolvedValueOnce(described({ preference: 'dark' }, 1)) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) - const statuses: string[] = [] - scope.subscribe(() => { statuses.push(scope.getSnapshot().status) }) - const stale = scope.load() - const fresh = scope.load() - await Promise.all([stale, fresh]) - expect(statuses).not.toContain('unavailable') - expect(scope.getSnapshot()).toMatchObject({ status: 'ready', value: { preference: 'dark' } }) - }) - it('reports an unexposed namespace as unavailable and recovers when it reappears', async () => { const describeCall = vi.fn() .mockResolvedValueOnce(described({ preference: 'light' }, 1)) .mockResolvedValueOnce(ok({ writable: true, hasDocument: true, namespaces: [] })) .mockResolvedValueOnce(described({ preference: 'system' }, 2)) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) - await scope.load() + const { mirror, scope } = derivedScope({ describe: describeCall }) + await mirror.load() expect(scope.getSnapshot().status).toBe('ready') - await scope.load() + await mirror.load() expect(scope.getSnapshot()).toMatchObject({ status: 'unavailable', value: { preference: 'light' } }) - await scope.load() + await mirror.load() expect(scope.getSnapshot()).toMatchObject({ status: 'ready', value: { preference: 'system' }, revision: 2 }) }) @@ -149,18 +132,15 @@ describe('SettingsScopeController', () => { const describeCall = vi.fn() .mockResolvedValueOnce(described({ preference: 'light' }, 1)) .mockResolvedValueOnce(described({ preference: 'dark' }, 2)) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { - namespace: 'ui-test', - decode: section => (section as UiTestSettings).preference === 'dark' - ? section as UiTestSettings - : undefined, - }, - ) - await scope.load() + const { mirror, scope } = derivedScope({ describe: describeCall }, { + namespace: 'ui-test', + decode: section => (section as UiTestSettings).preference === 'dark' + ? section as UiTestSettings + : undefined, + }) + await mirror.load() expect(scope.getSnapshot()).toMatchObject({ status: 'loading', value: undefined, revision: 1 }) - await scope.load() + await mirror.load() expect(scope.getSnapshot()).toMatchObject({ status: 'ready', value: { preference: 'dark' }, revision: 2 }) }) @@ -170,12 +150,9 @@ describe('SettingsScopeController', () => { const mutate = vi.fn() .mockReturnValueOnce(first.promise) .mockResolvedValueOnce(ok(view({ preference: 'light' }, 6))) - const scope = new SettingsScopeController( - { settings: { describe: describeCall, mutate } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall, mutate }) const published = trackValues(scope) - await scope.load() + await mirror.load() const dark = scope.set('preference', 'dark') const light = scope.set('preference', 'light') await vi.waitFor(() => { expect(mutate).toHaveBeenCalledOnce() }) @@ -195,6 +172,19 @@ describe('SettingsScopeController', () => { }) }) + it('folds the latest write answer into the mirror so a sibling scope sees it', async () => { + const describeCall = vi.fn().mockResolvedValueOnce(described({ preference: 'system' }, 4)) + const mutate = vi.fn().mockResolvedValueOnce(ok(view({ preference: 'dark' }, 5))) + const wire = { settings: { describe: describeCall, mutate } } as never + const mirror = new SettingsDescribeMirror(wire) + const writer = new SettingsScopeController(wire, { namespace: 'ui-test' }, mirror) + const sibling = new SettingsScopeController(wire, { namespace: 'ui-test' }, mirror) + await mirror.load() + await writer.set('preference', 'dark') + expect(describeCall).toHaveBeenCalledTimes(1) + expect(sibling.getSnapshot()).toMatchObject({ value: { preference: 'dark' }, revision: 5 }) + }) + it('recovers the latest rejected or thrown write from Host state', async () => { const describeCall = vi.fn() .mockResolvedValueOnce(described({ preference: 'system' }, 2)) @@ -202,52 +192,45 @@ describe('SettingsScopeController', () => { const mutate = vi.fn() .mockResolvedValueOnce(rejected()) .mockRejectedValueOnce(new Error('offline')) - const scope = new SettingsScopeController( - { settings: { describe: describeCall, mutate } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall, mutate }) const published = trackValues(scope) + await mirror.load() await scope.set('preference', 'dark') await scope.set('preference', 'system') expect(published.map(section => section?.preference)).toEqual([undefined, 'system', 'light']) }) it('does not recover superseded rejected or thrown writes', async () => { - const describeCall = vi.fn() + const describeCall = vi.fn().mockResolvedValueOnce(described({ preference: 'system' }, 2)) const mutate = vi.fn() .mockResolvedValueOnce(rejected()) .mockRejectedValueOnce(new Error('offline')) .mockResolvedValueOnce(ok(view({ preference: 'light' }, 3))) - const scope = new SettingsScopeController( - { settings: { describe: describeCall, mutate } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall, mutate }) const published = trackValues(scope) + await mirror.load() await Promise.all([ scope.set('preference', 'dark'), scope.set('preference', 'system'), scope.set('preference', 'light'), ]) - expect(describeCall).not.toHaveBeenCalled() - expect(published.map(section => section?.preference)).toEqual([undefined, 'light']) + expect(describeCall).toHaveBeenCalledTimes(1) + expect(published.map(section => section?.preference)).toEqual([undefined, 'system', 'light']) }) it('keeps the write queue usable when a subscriber throws', async () => { const describeCall = vi.fn() .mockResolvedValueOnce(described({ preference: 'dark' }, 1)) .mockResolvedValueOnce(described({ preference: 'light' }, 2)) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall }) let thrown = false scope.subscribe(() => { if (thrown) return thrown = true throw new Error('subscriber failed') }) - await expect(scope.load()).rejects.toThrow('subscriber failed') - await expect(scope.load()).resolves.toBeUndefined() + await expect(mirror.load()).rejects.toThrow('subscriber failed') + await expect(mirror.load()).resolves.toBeUndefined() expect(scope.getSnapshot()).toMatchObject({ value: { preference: 'light' }, revision: 2 }) }) @@ -255,10 +238,7 @@ describe('SettingsScopeController', () => { const first = deferred>() const mutate = vi.fn().mockReturnValue(first.promise) const describeCall = vi.fn() - const scope = new SettingsScopeController( - { settings: { describe: describeCall, mutate } } as never, - { namespace: 'ui-test' }, - ) + const { scope } = derivedScope({ describe: describeCall, mutate }) const published = trackValues(scope) const dark = scope.set('preference', 'dark') await vi.waitFor(() => { expect(mutate).toHaveBeenCalledOnce() }) @@ -270,24 +250,34 @@ describe('SettingsScopeController', () => { first.resolve(ok(view({ preference: 'dark' }, 1))) await Promise.all([dark, light, stop]) await scope.set('preference', 'system') - await scope.load() expect(mutate).toHaveBeenCalledOnce() expect(describeCall).not.toHaveBeenCalled() expect(published).toEqual([undefined]) }) + it('stops deriving from the mirror after dispose', async () => { + const describeCall = vi.fn() + .mockResolvedValueOnce(described({ preference: 'dark' }, 1)) + .mockResolvedValueOnce(described({ preference: 'light' }, 2)) + const { mirror, scope } = derivedScope({ describe: describeCall }) + await mirror.load() + expect(scope.getSnapshot()).toMatchObject({ value: { preference: 'dark' } }) + await scope.dispose() + await mirror.load() + expect(scope.getSnapshot()).toMatchObject({ value: { preference: 'dark' }, revision: 1 }) + }) + it('keeps a remote browser in memory mode without Host calls', async () => { const describeCall = vi.fn() const mutate = vi.fn() + const wire = { settings: { describe: describeCall, mutate } } as never + const mirror = new SettingsDescribeMirror(wire, 'memory') const scope = new SettingsScopeController( - { settings: { describe: describeCall, mutate } } as never, - { namespace: 'ui-test' }, - 'memory', - ) + wire, { namespace: 'ui-test' }, mirror, 'memory') expect(scope.getSnapshot()).toEqual({ status: 'unavailable', value: undefined, revision: undefined, writable: false, mode: 'memory', }) - await scope.load() + await mirror.load() await scope.set('preference', 'dark') await scope.dispose() expect(describeCall).not.toHaveBeenCalled() @@ -302,12 +292,9 @@ describe('SettingsScopeController', () => { } const describeCall = vi.fn() .mockResolvedValueOnce(ok({ writable: true, hasDocument: true, namespaces: [layered] })) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall }) - await scope.load() + await mirror.load() expect(scope.getSnapshot()).toMatchObject({ value: { preference: 'dark' }, @@ -320,12 +307,9 @@ describe('SettingsScopeController', () => { const inherited: SettingsNamespaceView = { ...view({ preference: 'system' }, 1), base: { preference: 'system' } } const describeCall = vi.fn() .mockResolvedValueOnce(ok({ writable: true, hasDocument: true, namespaces: [inherited] })) - const scope = new SettingsScopeController( - { settings: { describe: describeCall } } as never, - { namespace: 'ui-test' }, - ) + const { mirror, scope } = derivedScope({ describe: describeCall }) - await scope.load() + await mirror.load() expect(scope.getSnapshot().user).toBeUndefined() }) @@ -333,11 +317,8 @@ describe('SettingsScopeController', () => { it('clears one field through an unset op fenced by the held revision', async () => { const mutate = vi.fn().mockResolvedValueOnce(ok(view({ preference: 'system' }, 4))) const describeCall = vi.fn().mockResolvedValueOnce(described({ preference: 'dark' }, 3)) - const scope = new SettingsScopeController( - { settings: { describe: describeCall, mutate } } as never, - { namespace: 'ui-test' }, - ) - await scope.load() + const { mirror, scope } = derivedScope({ describe: describeCall, mutate }) + await mirror.load() await scope.unset('preference') @@ -354,64 +335,53 @@ describe('SettingsScopeController', () => { const describeCall = vi.fn() .mockResolvedValueOnce(described({ preference: 'dark' }, 3)) .mockResolvedValueOnce(described({ preference: 'light' }, 5)) - const scope = new SettingsScopeController( - { settings: { describe: describeCall, mutate } } as never, - { namespace: 'ui-test' }, - ) - await scope.load() + const { mirror, scope } = derivedScope({ describe: describeCall, mutate }) + await mirror.load() await scope.unset('preference') expect(scope.getSnapshot()).toMatchObject({ value: { preference: 'light' }, revision: 5 }) }) }) + describe('SettingsScopeBinder.bind', () => { - it('subscribes before the initial read and converges to the latest queued invalidation', async () => { - const initial = deferred>() - const describeCall = vi.fn() - .mockReturnValueOnce(initial.promise) - .mockResolvedValueOnce(described({ preference: 'light' }, 2)) - .mockResolvedValueOnce(described({ preference: 'system' }, 3)) + it('shares one mirror read across bound scopes and disposes each with its fiber', async () => { + const describeCall = vi.fn().mockResolvedValue(described({ preference: 'dark' }, 1)) + const wire = { settings: { describe: describeCall } } + const mirror = new SettingsDescribeMirror(wire as never) const ctx = new Context() - ctx.provide('connection', { - api: { settings: { describe: describeCall } }, - isLoopback: true, - } as never) - let scope!: SettingsScope + ctx.provide('connection', { api: wire, isLoopback: true } as never) + let theme!: SettingsScope + let locale!: SettingsScope new TestRemote(ctx) - await ctx.plugin(SettingsScopeBinder).await() + await ctx.plugin(SettingsScopeBinder, { mirror }).await() const fiber = ctx.plugin({ inject: ['connection', 'remote', 'settingsScope'], apply: (plugin: Context) => { - scope = plugin.settingsScope.bind({ namespace: 'ui-test' }) + theme = plugin.settingsScope.bind({ namespace: 'ui-test' }) + locale = plugin.settingsScope.bind({ namespace: 'ui-test' }) }, }) await fiber.await() - await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledOnce() }) - ctx.remote.$dispatch('settings/document-updated', ['unrelated', 0]) - ctx.remote.$dispatch('settings/document-updated', ['ui-test', 0]) - ctx.emit('connection/reset') - initial.resolve(described({ preference: 'dark' }, 1)) - await vi.waitFor(() => { expect(describeCall).toHaveBeenCalledTimes(3) }) await vi.waitFor(() => { - expect(scope.getSnapshot()).toMatchObject({ value: { preference: 'system' }, revision: 3 }) + expect(theme.getSnapshot()).toMatchObject({ status: 'ready', value: { preference: 'dark' } }) + expect(locale.getSnapshot()).toMatchObject({ status: 'ready', value: { preference: 'dark' } }) }) + expect(describeCall).toHaveBeenCalledTimes(1) await fiber.dispose() - ctx.remote.$dispatch('settings/document-updated', ['ui-test', 0]) - await Promise.resolve() - expect(describeCall).toHaveBeenCalledTimes(3) + await mirror.load() + expect(theme.getSnapshot()).toMatchObject({ revision: 1 }) }) it('binds a remote browser in memory mode without starting a settings read', async () => { const describeCall = vi.fn() + const wire = { settings: { describe: describeCall } } + const mirror = new SettingsDescribeMirror(wire as never, 'memory') const ctx = new Context() - ctx.provide('connection', { - api: { settings: { describe: describeCall } }, - isLoopback: false, - } as never) + ctx.provide('connection', { api: wire, isLoopback: false } as never) let scope!: SettingsScope new TestRemote(ctx) - await ctx.plugin(SettingsScopeBinder).await() + await ctx.plugin(SettingsScopeBinder, { mirror }).await() const fiber = ctx.plugin({ inject: ['connection', 'remote', 'settingsScope'], apply: (plugin: Context) => { From a2c001eb3e35e2e819bd263cf2476734f9d1a349 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 17:02:32 +0800 Subject: [PATCH 04/28] test(client): bench downstream settings consumers on the real ui-settings apply --- .../client/locale/tests/apply.client.spec.ts | 9 ++++++--- .../tests/apply.client.spec.ts | 4 ++-- .../tests/settings-mirror.client.spec.ts | 2 +- .../ui-theme/tests/apply.client.spec.ts | 20 ++++++++++++++----- 4 files changed, 24 insertions(+), 11 deletions(-) diff --git a/packages/client/locale/tests/apply.client.spec.ts b/packages/client/locale/tests/apply.client.spec.ts index dd38786073..2378ae796a 100644 --- a/packages/client/locale/tests/apply.client.spec.ts +++ b/packages/client/locale/tests/apply.client.spec.ts @@ -4,7 +4,7 @@ import { Context } from '@deepseek-ai/cordis' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { SlotRegistry } from '@deepseek-ai/dsh-client-runtime/client' -import { SettingsScopeBinder } from '@deepseek-ai/dsh-client-ui-settings/client' +import { apply as settingsApply, inject as settingsInject } from '@deepseek-ai/dsh-client-ui-settings/client' import { TestRemote } from '@deepseek-ai/dsh-client-test-runtime' import { apply, inject, SETTINGS_NS, @@ -47,7 +47,7 @@ async function bench() { ctx.provide('connection', { api: { settings: { describe, mutate } }, isLoopback: true } as never) // The settings transport and the forwarded-event port the plugin injects. new TestRemote(ctx) - await ctx.plugin(SettingsScopeBinder).await() + await ctx.plugin({ inject: [...settingsInject], apply: settingsApply }).await() return { ctx, slots: ctx.get('slots') as SlotRegistry, describe, mutate, setHostPreference: (next: string | undefined) => { preference = next; revision += 1 }, @@ -133,7 +133,10 @@ describe('locale apply', () => { it('loads and refreshes the explicit Host preference after nonblocking activation', async () => { const b = await bench() + // The shared mirror read once at bench time; a Host-side change reaches it + // through the document invalidation, exactly as production announces one. b.setHostPreference('en') + b.ctx.remote.$dispatch('settings/document-updated', [LOCALE_SETTINGS_NAMESPACE, 0]) declareItems(b.slots) await b.ctx.plugin({ inject: [...inject], apply }).await() const locale = b.ctx.get('locale') as LocaleRuntime @@ -144,7 +147,7 @@ describe('locale apply', () => { b.setHostPreference('en') b.ctx.remote.$dispatch('settings/document-updated', [LOCALE_SETTINGS_NAMESPACE, 0]) await vi.waitFor(() => { expect(locale.getLocale().active).toBe('en') }) - expect(b.describe).toHaveBeenCalledTimes(3) + expect(b.describe).toHaveBeenCalledTimes(4) }) it('recovers after an HMR collapse of the declaring entry (stale disposer must not block)', async () => { diff --git a/packages/client/ui-settings-plugins/tests/apply.client.spec.ts b/packages/client/ui-settings-plugins/tests/apply.client.spec.ts index 2934097b94..c5516ff4bc 100644 --- a/packages/client/ui-settings-plugins/tests/apply.client.spec.ts +++ b/packages/client/ui-settings-plugins/tests/apply.client.spec.ts @@ -6,7 +6,7 @@ import { resolveSlotLabel } from '@deepseek-ai/dsh-client-ui-slots' import { SlotRegistry } from '@deepseek-ai/dsh-client-runtime/client' import { LocaleRuntime } from '@deepseek-ai/dsh-client-locale/client' import { TestRemote, usePinnedBrowserLanguages } from '@deepseek-ai/dsh-client-test-runtime' -import { SettingsScopeBinder } from '@deepseek-ai/dsh-client-ui-settings/client' +import { apply as settingsApply, inject as settingsInject } from '@deepseek-ai/dsh-client-ui-settings/client' import { apply, inject } from '@deepseek-ai/dsh-client-ui-settings-plugins/client' import type { ConfigurablePluginsTabFace, PluginsSettingsSectionInjected, @@ -52,7 +52,7 @@ async function bench(served?: string[]) { credentials: { describe: describeCredentials }, }, } as never) - await ctx.plugin(SettingsScopeBinder).await() + await ctx.plugin({ inject: [...settingsInject], apply: settingsApply }).await() return { ctx, slots: ctx.get('slots') as SlotRegistry, describeCredentials, describeSettings } } 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 058c401d5e..972b3cea73 100644 --- a/packages/client/ui-settings/tests/settings-mirror.client.spec.ts +++ b/packages/client/ui-settings/tests/settings-mirror.client.spec.ts @@ -13,7 +13,7 @@ function rejected(message: string): RpcResponse { rpcId: `mirror-${rpc++}` as never, result: { ok: false, - error: { code: 'settings-rejected', message, details: {} }, + error: { code: 'settings-rejected', message, details: { ns: 'theme' } }, }, } } diff --git a/packages/client/ui-theme/tests/apply.client.spec.ts b/packages/client/ui-theme/tests/apply.client.spec.ts index fb84c9860d..1629ebe342 100644 --- a/packages/client/ui-theme/tests/apply.client.spec.ts +++ b/packages/client/ui-theme/tests/apply.client.spec.ts @@ -6,7 +6,7 @@ import { describe, expect, it, vi } from 'vitest' import { SlotRegistry } from '@deepseek-ai/dsh-client-runtime/client' import { LocaleRuntime } from '@deepseek-ai/dsh-client-locale/client' import { TestRemote, usePinnedBrowserLanguages } from '@deepseek-ai/dsh-client-test-runtime' -import { SettingsScopeBinder } from '@deepseek-ai/dsh-client-ui-settings/client' +import { apply as settingsApply, inject as settingsInject } from '@deepseek-ai/dsh-client-ui-settings/client' import { apply, inject, SETTINGS_NS } from '@deepseek-ai/dsh-client-ui-theme/client' import type { AppearanceRowInjected, ThemeRuntime } from '@deepseek-ai/dsh-client-ui-theme/client' import { THEME_SETTINGS_NAMESPACE, ThemeSettingsSchema } from '../src/theme-settings.ts' @@ -56,7 +56,7 @@ async function bench(isLoopback = true) { ctx.provide('connection', { api: { settings: { describe, mutate } }, isLoopback } as never) // The settings transport and the forwarded-event port the plugin injects. new TestRemote(ctx) - await ctx.plugin(SettingsScopeBinder).await() + await ctx.plugin({ inject: [...settingsInject], apply: settingsApply }).await() return { ctx, slots: ctx.get('slots') as SlotRegistry, locale, describe, mutate, setHostPreference: (next: string) => { preference = next }, @@ -127,13 +127,19 @@ describe('ui-theme apply', () => { it('loads Host settings at boot, refreshes its namespace, and keeps remote browsers process-local', async () => { const b = await bench() + // The shared mirror read once at bench time; a Host-side change reaches it + // through the document invalidation, exactly as production announces one. b.setHostPreference('dark') + b.ctx.remote.$dispatch('settings/document-updated', [THEME_SETTINGS_NAMESPACE, 0]) declareItems(b.slots) await b.ctx.plugin({ inject: [...inject], apply }).await() const theme = b.ctx.get('theme') as ThemeRuntime await vi.waitFor(() => { expect(theme.getTheme().preference).toBe('dark') }) + // The mirror refreshes on every document commit (ns-agnostic); the scope's + // derived value only moves when its own namespace changed. b.ctx.remote.$dispatch('settings/document-updated', ['unrelated', 0]) - expect(b.describe).toHaveBeenCalledOnce() + await vi.waitFor(() => { expect(b.describe).toHaveBeenCalledTimes(3) }) + expect(theme.getTheme().preference).toBe('dark') b.setHostPreference('light') b.ctx.remote.$dispatch('settings/document-updated', [THEME_SETTINGS_NAMESPACE, 0]) await vi.waitFor(() => { expect(theme.getTheme().preference).toBe('light') }) @@ -151,12 +157,15 @@ describe('ui-theme apply', () => { expect(remote.mutate).not.toHaveBeenCalled() }) - it('activates before a slow initial settings read and converges when it settles', async () => { + it('activates before a slow settings refresh and converges when it settles', async () => { const b = await bench() b.setHostPreference('dark') const describe = b.describe.getMockImplementation()! const pending = deferred>>() b.describe.mockImplementationOnce(() => pending.promise) + // The refresh hangs on the wire; the mirror keeps serving the last good + // answer, so activation never blocks on the settings transport. + b.ctx.remote.$dispatch('settings/document-updated', [THEME_SETTINGS_NAMESPACE, 0]) const fiber = b.ctx.plugin({ inject: [...inject], apply }) await fiber.await() const theme = b.ctx.get('theme') as ThemeRuntime @@ -169,9 +178,10 @@ describe('ui-theme apply', () => { it('ignores an invalid preference crossing the settings wire', async () => { const b = await bench() b.setHostPreference('sepia') + b.ctx.remote.$dispatch('settings/document-updated', [THEME_SETTINGS_NAMESPACE, 0]) await b.ctx.plugin({ inject: [...inject], apply }).await() const theme = b.ctx.get('theme') as ThemeRuntime - await vi.waitFor(() => { expect(b.describe).toHaveBeenCalledOnce() }) + await vi.waitFor(() => { expect(b.describe).toHaveBeenCalledTimes(2) }) expect(theme.getTheme().preference).toBe('system') }) From 941e5c506178681f33b4192dadfc0d622a101201 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Mon, 17 Aug 2026 17:03:48 +0800 Subject: [PATCH 05/28] feat(workflow): let users control run and phase disclosures --- ...low-run-status-driven-disclosure.i18n.yaml | 4 +- ...1-workflow-run-status-driven-disclosure.md | 30 +- ...orkflow-run-status-driven-disclosure.zh.md | 30 +- .../workflow-run/ui-live.expected.md | 23 ++ .../snapshots/workflow-run/ui.expected.md | 6 +- apps/web/tests/workflow-run.e2e.ts | 37 +- .../client/ui-workflow-run/README.i18n.yaml | 4 +- packages/client/ui-workflow-run/README.md | 2 +- packages/client/ui-workflow-run/README.zh.md | 2 +- .../src/client/WorkflowRunPanel.tsx | 275 +++++++++++--- .../tests/workflow-run.client.spec.tsx | 339 ++++++++++++++---- 11 files changed, 592 insertions(+), 160 deletions(-) create mode 100644 apps/web/tests/snapshots/workflow-run/ui-live.expected.md diff --git a/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.i18n.yaml b/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.i18n.yaml index 1f7ecb98d3..a9e09de830 100644 --- a/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.i18n.yaml +++ b/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write .agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.md -2026-08-11-workflow-run-status-driven-disclosure.md: 2f452d25a8922bb6c275419af55e8af155dd2781 -2026-08-11-workflow-run-status-driven-disclosure.zh.md: 12cc106fea274a1681ee5615906ae6df266d567b +2026-08-11-workflow-run-status-driven-disclosure.md: a783ede82442cfb28ed4c0a2fd394677728949d8 +2026-08-11-workflow-run-status-driven-disclosure.zh.md: 0d047bf7383f68ff7ec23d3602789e776c85d2cc diff --git a/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.md b/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.md index 2f452d25a8..a783ede824 100644 --- a/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.md +++ b/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.md @@ -6,38 +6,40 @@ English | [中文](2026-08-11-workflow-run-status-driven-disclosure.zh.md) ## Problem -A durable workflow Chat node updates in place from its running prefix to a terminal record. A disclosure choice initialized only at mount can hide a newly running phase, leave completed work occupying the conversation, or bury a failed, cancelled, or interrupted member behind two collapsed levels. Making openness a pure function of completion avoids those failures but also prevents users from reopening clean history for review. +A durable workflow Chat node updates in place from its running prefix to a terminal record. The renderer must draw attention to new work, abnormal outcomes, and normal completion without repeatedly overriding a user's decision to reclaim conversation space. -The renderer already receives every required lifecycle fact from the workflow Conversation Node. Visibility therefore needs a component-local lifecycle that gives current execution and attention states priority without adding another durable fact or taking ownership of workflow outcomes. +The renderer already receives every durable lifecycle fact from the workflow Conversation Node. Disclosure choice therefore belongs to the mounted presentation, but its lifecycle must also preserve nested phase choices when the outer run is hidden and avoid removing content that still contains keyboard focus. ## Decision -Each phase derives one visibility requirement from its current members. A running, failed, cancelled, or interrupted member forces that phase open; a phase whose members are all completed is clean. The workflow forces itself open when its own status requires attention or any phase is forced open, so an abnormal member remains visible even when the workflow outcome is recorded as completed. A completed sibling phase remains independently collapsible. +`WorkflowRunPanel` owns one local disclosure state for the run and a map keyed by the existing phase key. A phase is clean when every member completed, abnormal when any member failed, was cancelled, or was interrupted, and running otherwise. The run is abnormal when its own status or any phase is abnormal, running when its own status or any phase is running, and clean only when the run and every phase completed normally. A mount opens running and abnormal levels and closes clean levels. -A forced-open level renders as an expanded static row. It exposes no button role, focus target, keyboard toggle, or `aria-expanded` value because collapsing cannot change the result. This keeps the visual hierarchy and status summaries while making the interaction promise match the available action. +Each level records its current mode, append-only member count, open choice, and any pending clean close. Ordinary updates within a running or abnormal interval preserve the user's choice. A phase transition from clean to activity opens that phase and the outer run once, the first transition into abnormal opens once, and a transition into clean closes once. A member-count change while a phase remains clean represents a complete activity cycle delivered in one render and closes an open review without adding an activity epoch or durable field. After an automatic action, mouse, Enter, and Space control the level until another defined edge occurs. -A clean level mounts an ordinary controlled disclosure in the closed state. Its local choice survives rerenders for the same continuous clean interval. New running or abnormal data replaces that manual interval with forced expansion; the next transition back to clean mounts a fresh closed disclosure, which produces one automatic fold per activity cycle. Closing the workflow naturally unmounts its phase controls, and a Session remount reconstructs every level from the current durable status rather than restoring an earlier choice. +Phase state remains in `WorkflowRunPanel` while the outer disclosure hides its children, so closing and reopening the run restores each phase choice. Removing a phase deletes its entry; a renderer remount reconstructs every level from current durable facts rather than restoring an earlier choice. -For example, a running workflow exposes its active phase and member without clicks. When that phase completes, only the phase folds while the workflow remains open; when the workflow and every phase complete, the workflow also folds. The user can then reopen both levels for review. If another member starts under the same phase key, both affected levels immediately return to forced expansion and fold again only after the new activity completes. +Normal completion checks whether focus is inside the content before closing. Focused content remains mounted with current completed status and closes after focus leaves. When a navigable member becomes terminal while its button holds focus, `MemberRow` keeps the same button mounted as `aria-disabled` until blur; later terminal review renders the ordinary non-interactive row. This preserves the active DOM target without allowing terminal navigation or adding a focus manager. -The renderer owns only this visibility lifecycle. It does not add Session events, stores, settings, acknowledgement state, timers, focus movement, automatic scrolling, or cross-remount persistence. It does not change workflow status derivation, phase grouping, member order, navigation eligibility, copy, or the shared `DisclosureRow` API. Shared `data-expandable` styling owns pointer cursors, so forced-open static rows do not advertise an unavailable action. An interrupted durable prefix remains an attention state and therefore stays visible until the underlying facts change. +The renderer adds no Session events, store, setting, acknowledgement, timer, automatic scrolling, persistent activity identity, or `DisclosureRow` API. It does not change workflow status derivation, phase grouping, member order, navigation eligibility, copy, or visual tokens. ## Verification -Component tests drive the same keyed workflow and phase through running, clean completion, manual review, renewed activity, repeated clean completion, zero-member completion, and each abnormal status. They also verify abnormal-member propagation, clean-sibling independence, mouse and keyboard review, continuous-clean choice retention, and the absence of false button and ARIA semantics while expansion is mandatory. +Component tests drive one keyed run and its phases through initial running controls, mouse and keyboard choices, ordinary running updates, outer hide and restore, phase completion, run completion, clean review, same-key renewed activity, a fully batched clean cycle, every abnormal status, first-abnormal escalation, later abnormal updates, zero-member completion, focused-member completion, sibling independence, and renderer remount. They also verify terminal navigation remains absent after the deferred focus path settles. -The shipped Web replay observes the real workflow, worker, Session log, browser plugin graph, and child navigation. It requires the live workflow and active phase to be visible without disclosure controls, the normally settled workflow and phase to fold, manual review to retain the terminal member without navigation, and a reload to reconstruct the folded history from durable facts. +The shipped Web replay exercises the real workflow, worker, Session log, browser plugin graph, and child navigation. It collapses and reopens live run and phase controls, records the live collapsed status summary and ARIA state, verifies normal settlement folds both levels, confirms terminal review cannot navigate the member, and records the folded history reconstructed after reload. ## Alternatives considered -**Keep one manual state initialized from the first render.** Rejected because later lifecycle updates cannot reopen newly active or abnormal content and cannot fold normally settled work. +**Force every running or abnormal level open as a static row.** Rejected because it makes the attention state impossible to dismiss and removes truthful mouse, keyboard, and ARIA disclosure semantics. -**Derive `open` directly from whether a level is clean.** Rejected because completed history would remain permanently closed and could not be reopened for review. +**Keep one manual state initialized from the first render.** Rejected because later activity, abnormal escalation, and normal completion cannot perform their one-time automatic actions. -**Persist expansion, acknowledgement, or read state.** Rejected because current lifecycle facts already determine mandatory visibility, while review choice belongs only to the mounted presentation. Persistence would add a second state owner and require semantics for stale choices, abnormal acknowledgement, replay, and synchronization that the user result does not need. +**Let each phase own state inside its disclosure content.** Rejected because hiding the outer run unmounts that content and discards independent phase choices during the same mounted workflow record. + +**Persist expansion, acknowledgement, or an activity epoch.** Rejected because current workflow facts and the append-only member count provide every required edge. Persistence adds a second durable owner and synchronization semantics that this presentation choice does not need. ## Consequences -Workflow records expose current work and abnormal outcomes without preparatory clicks, then reclaim conversation space after normal completion without sacrificing review. Interaction semantics remain truthful during automatic control, and the same durable record produces the same initial state during live rendering, refresh, and history reconstruction. +Workflow records call attention to lifecycle changes while remaining dismissible in every status. Normal completion reclaims space, current focus remains safe, nested phase choices survive outer hiding, and the same durable record reconstructs a deterministic initial state on refresh or history replay. -The trade-off is deliberate local reset behavior. A phase choice disappears when its parent workflow closes or the component unmounts, and abnormal records cannot be manually hidden because the product has no acknowledgement state. Supporting either behavior later requires a separate ownership and persistence decision rather than extending this local lifecycle implicitly. +The local lifecycle deliberately resets on renderer remount and cannot remember a choice across refresh, devices, or users. Adding that behavior requires a separate persistence and stale-choice decision rather than extending this presentation state implicitly. diff --git a/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.zh.md b/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.zh.md index 12cc106fea..0d047bf738 100644 --- a/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.zh.md +++ b/.agents/notes/implemented/feature/2026-08-11-workflow-run-status-driven-disclosure.zh.md @@ -6,38 +6,40 @@ Status: implemented ## 问题 -持久工作流 Chat 节点会在同一位置从运行前缀更新为终态记录。只在挂载时初始化的 disclosure 选择可能隐藏新开始运行的阶段,让已完成工作继续占据对话空间,或者把失败、已取消或已中断成员埋在两层折叠内容之后。若只把开合状态作为完成状态的纯派生结果,虽然能避免这些问题,却也会阻止用户重新打开干净历史进行复盘。 +持久工作流 Chat 节点会在同一位置从运行前缀更新为终态记录。renderer 必须提示新工作、异常结果和正常完成,同时不能在普通更新中反复覆盖用户回收对话空间的选择。 -renderer 已经从工作流 Conversation Node 收到全部所需生命周期事实。因此,可见性需要一个组件本地生命周期:让当前执行与需注意状态优先,同时不增加另一项持久事实,也不取得工作流结果的所有权。 +renderer 已经从工作流 Conversation Node 收到全部持久生命周期事实。因此,disclosure 选择属于已挂载的展示层,但它的生命周期还必须在外层运行隐藏时保留嵌套 Phase 选择,并避免移除仍含键盘焦点的内容。 ## 决策 -每个阶段从当前成员派生一项可见性要求。存在运行中、失败、已取消或已中断成员时,该阶段强制展开;全部成员均已完成时,该阶段处于干净状态。工作流自身状态需要注意或任一阶段强制展开时,工作流也强制展开,因此即使工作流结果记录为已完成,异常成员仍保持可见。已完成的兄弟阶段继续可以独立折叠。 +`WorkflowRunPanel` 持有一项运行 disclosure 本地状态,以及一张按现有 phase key 索引的 Phase 状态表。全部成员都已完成时,Phase 为干净状态;任一成员失败、已取消或已中断时为异常状态;其余情况为运行状态。运行自身或任一 Phase 异常时,运行处于异常状态;运行自身或任一 Phase 正在运行时,运行处于运行状态;只有运行与全部 Phase 都正常完成时才处于干净状态。挂载时,运行和异常层级默认展开,干净层级默认折叠。 -强制展开层级渲染为静态展开行。它不提供按钮 role、焦点目标、键盘切换或 `aria-expanded` 值,因为折叠操作无法改变结果。这样既保留视觉层级与状态摘要,也让交互承诺与实际可执行动作一致。 +每个层级记录当前模式、仅追加成员数、开合选择和待执行的干净折叠。Phase 从干净状态进入新活动时,该 Phase 与外层运行自动展开一次;连续运行或异常区间内的普通更新保留用户选择,首次进入异常状态时自动展开一次,进入干净状态时自动折叠一次。若新增成员及其正常完成在同一次渲染中送达,Phase 会保持干净但成员数改变;该变化会折叠已打开的复盘,而无需增加 activity epoch 或持久字段。自动动作完成后,鼠标、Enter 和 Space 控制该层级,直到出现下一项约定边沿。 -干净层级会以关闭状态挂载普通受控 disclosure。它的本地选择在同一段连续干净状态的 rerender 中保持。新的运行中或异常数据会用强制展开替代该手动区间;下一次回到干净状态时会挂载新的关闭 disclosure,从而让每个活动周期只自动折叠一次。关闭工作流会自然卸载其阶段控件;Session remount 会从当前持久状态重建每个层级,而不恢复更早的选择。 +外层 disclosure 隐藏子内容时,Phase 状态仍留在 `WorkflowRunPanel`,因此关闭并重新打开运行会恢复各 Phase 选择。Phase 被移除时,其表项同时清理;renderer remount 会从当前持久事实重建每个层级,而不恢复更早选择。 -例如,运行中的工作流无需点击即可展示活跃阶段与成员。该阶段完成时,只有阶段折叠,工作流继续展开;工作流自身和全部阶段均完成时,工作流也会折叠。用户随后可以重新打开两个层级复盘。若同一阶段 key 下又开始新成员,受影响的两个层级会立即恢复强制展开,并且只在新活动完成后再次折叠。 +正常完成会在折叠前检查焦点是否位于内容内。仍含焦点的内容保持挂载并立即显示完成状态,焦点离开后再折叠。可导航成员的按钮持有焦点并变为终态时,`MemberRow` 会把同一个按钮以 `aria-disabled` 形式保留到 blur;之后的终态复盘渲染普通不可交互行。这样既保留当前 DOM 目标,也不允许终态导航,并且无需增加焦点管理器。 -renderer 只拥有这项可见性生命周期。它不增加 Session 事件、store、设置、确认状态、计时器、焦点迁移、自动滚动或跨 remount 持久化。它不改变工作流状态派生、阶段分组、成员顺序、导航准入、文案或共享 `DisclosureRow` API。pointer 光标由共享的 `data-expandable` 样式拥有,因此强制展开的静态行不会提示无法执行的操作。持久记录中的中断前缀仍属于需注意状态,因此在底层事实改变前始终可见。 +renderer 不增加 Session 事件、store、设置、确认状态、计时器、自动滚动、持久活动身份或 `DisclosureRow` API。它不改变工作流状态派生、Phase 分组、成员顺序、导航准入、文案或视觉 token。 ## 验证 -组件测试驱动同一个 keyed 工作流与阶段依次经过运行、干净完成、手动复盘、新活动、再次干净完成、零成员完成以及每种异常状态。测试还验证异常成员向上展开、干净兄弟阶段独立、鼠标和键盘复盘、连续干净状态中的选择保持,以及强制展开时不存在虚假按钮和 ARIA 语义。 +组件测试驱动同一个 keyed 运行及其 Phase,覆盖初始运行控件、鼠标和键盘选择、普通运行更新、外层隐藏与恢复、Phase 完成、运行完成、干净复盘、同 key 新活动、同次渲染送达的完整干净周期、每种异常状态、首次异常升级、后续异常更新、零成员完成、成员持焦点时完成、兄弟 Phase 独立以及 renderer remount。测试还确认延后焦点路径结算后,终态导航仍不存在。 -shipped Web 回放观察真实工作流、worker、Session 日志、浏览器插件图和子级导航。它要求实时工作流与活跃阶段无需 disclosure 控件即可见,正常结算的工作流与阶段会折叠,手动复盘仍能看到不再可导航的终态成员,并且刷新会从持久事实重建折叠历史。 +shipped Web 回放经过真实工作流、worker、Session 日志、浏览器插件图和子级导航。它折叠并重新打开实时运行与 Phase 控件,记录实时折叠标题的状态摘要和 ARIA 状态,验证正常结算会折叠两个层级,确认终态复盘不能导航成员,并记录刷新后从历史重建的折叠记录。 ## 曾考虑的替代方案 -**保留一项从首次渲染初始化的手动状态。** 拒绝,因为后续生命周期更新无法重新打开新活动或异常内容,也无法折叠正常结算的工作。 +**把每个运行中或异常层级强制展开为静态行。** 拒绝,因为需注意状态将无法收起,也不会提供真实的鼠标、键盘和 ARIA disclosure 语义。 -**只根据层级是否干净来派生 `open`。** 拒绝,因为已完成历史会永久保持关闭,无法重新打开复盘。 +**保留一项从首次渲染初始化的手动状态。** 拒绝,因为后续活动、异常升级和正常完成无法执行各自的一次性自动动作。 -**持久化展开、确认或已读状态。** 拒绝,因为当前生命周期事实已经决定强制可见性,而复盘选择只属于已挂载的展示层。持久化会增加第二个状态归属方,并要求定义陈旧选择、异常确认、回放和同步语义,而用户结果不需要这些机制。 +**让每个 Phase 在自身 disclosure 内容中持有状态。** 拒绝,因为隐藏外层运行会卸载这些内容,并在同一条已挂载工作流记录中丢失独立的 Phase 选择。 + +**持久化展开、确认或 activity epoch。** 拒绝,因为当前工作流事实与仅追加成员数已经提供全部所需边沿。持久化会增加第二个持久归属方以及本展示选择不需要的同步语义。 ## 后果 -工作流记录无需预备点击即可展示当前工作与异常结果,并在正常完成后回收对话空间,同时不牺牲复盘能力。自动控制期间的交互语义保持真实,同一份持久记录在实时渲染、刷新和历史重建时得到相同初始状态。 +工作流记录会提示生命周期变化,同时在所有状态下都允许用户收起。正常完成会回收空间,当前焦点保持安全,嵌套 Phase 选择在外层隐藏期间保留;同一份持久记录在刷新或历史回放时会重建确定性的初始状态。 -代价是有意保留的本地重置行为。父工作流关闭或组件卸载时,阶段选择会消失;由于产品没有确认状态,异常记录不能手动隐藏。以后若要支持任一行为,需要单独决定所有权与持久化,而不能隐式扩展这项本地生命周期。 +这项本地生命周期会在 renderer remount 时重置,无法跨刷新、设备或用户记住选择。若要增加该行为,需要单独决定持久化与陈旧选择语义,而不能隐式扩展这项展示状态。 diff --git a/apps/web/tests/snapshots/workflow-run/ui-live.expected.md b/apps/web/tests/snapshots/workflow-run/ui-live.expected.md new file mode 100644 index 0000000000..9e7f7b0ddc --- /dev/null +++ b/apps/web/tests/snapshots/workflow-run/ui-live.expected.md @@ -0,0 +1,23 @@ +- text: "Use the workflow tool exactly once, with args omitted, meta set to { \"name\": \"snapshot-flow\", \"description\": \"one child for the snapshot\" }, and this EXACT script body (copy it verbatim): phase('Run') const reply = await agent('Reply with exactly the word WF_CHILD_OK and nothing else.') return { reply } After the workflow returns, reply with the single word WORKFLOW_DONE and stop. Do not use any other tool. {{clock}}" +- button "Copy": + - img +- button "Context injection @deepseek-ai/dsh-system-prompt": + - img + - img + - text: Context injection @deepseek-ai/dsh-system-prompt +- button "Think The user wants me to use the workflow tool exactly once with specific parameters. Let me carefully follow the instructions:": + - img + - img + - text: "Think The user wants me to use the workflow tool exactly once with specific parameters. Let me carefully follow the instructions:" +- text: Running +- button "Tool call workflow ·": + - img + - img + - text: Tool call workflow · +- button "snapshot-flow 1 member Running" [expanded]: + - img + - text: snapshot-flow 1 member Running +- button "Run 1 member Running 1": + - img + - text: Run 1 member Running 1 +- status: Deep diving... diff --git a/apps/web/tests/snapshots/workflow-run/ui.expected.md b/apps/web/tests/snapshots/workflow-run/ui.expected.md index d9fe664863..5ad87e217b 100644 --- a/apps/web/tests/snapshots/workflow-run/ui.expected.md +++ b/apps/web/tests/snapshots/workflow-run/ui.expected.md @@ -13,13 +13,9 @@ - img - img - text: Tool call workflow · -- button "snapshot-flow 1 member Completed" [expanded]: +- button "snapshot-flow 1 member Completed": - img - text: snapshot-flow 1 member Completed -- button "Run 1 member Completed 1" [expanded]: - - img - - text: Run 1 member Completed 1 -- text: Reply with exactly the word WF_CHILD_OK and not… Completed - button "Think The workflow returned successfully with the reply \"WF_CHILD_OK\". Now I need to reply with exactly \"WORKFLOW_DONE\" and stop.": - img - img diff --git a/apps/web/tests/workflow-run.e2e.ts b/apps/web/tests/workflow-run.e2e.ts index 4cbae8e6e2..04f79cccd4 100644 --- a/apps/web/tests/workflow-run.e2e.ts +++ b/apps/web/tests/workflow-run.e2e.ts @@ -20,6 +20,7 @@ import { const MODE = webSnapshotMode() const SNAPSHOT_DIR = fileURLToPath(new URL('./snapshots/workflow-run', import.meta.url)) +const UI_LIVE_EXPECTED = join(SNAPSHOT_DIR, 'ui-live.expected.md') const UI_EXPECTED = join(SNAPSHOT_DIR, 'ui.expected.md') const PARENT_FIXTURE = join(REPO_ROOT, 'examples/acp-agent/tests/snapshots/workflow-run/session.jsonl') const CHILD_FIXTURE = join(REPO_ROOT, 'examples/acp-agent/tests/snapshots/workflow-run/session.1.jsonl') @@ -52,7 +53,7 @@ describe.skipIf(MODE === 'record')('web e2e: durable workflow run in Chat', () = scaffold = await launchWebScaffold({ replayFixture: PARENT_FIXTURE, replayChildFixtures: [CHILD_FIXTURE], - paceMs: 25, + paceMs: 50, }) browser = await chromium.launch() page = await newEnglishPage(browser) @@ -78,14 +79,30 @@ describe.skipIf(MODE === 'record')('web e2e: durable workflow run in Chat', () = await workflow.waitFor({ timeout: 30_000 }) const disclosures = workflow.locator('[data-disclosure-row]') await disclosures.nth(1).waitFor({ timeout: 15_000 }) - expect(await disclosures.nth(0).getAttribute('role')).toBeNull() - expect(await disclosures.nth(0).getAttribute('aria-expanded')).toBeNull() - expect(await disclosures.nth(1).getAttribute('role')).toBeNull() - expect(await disclosures.nth(1).getAttribute('aria-expanded')).toBeNull() - expect(await disclosures.nth(0).evaluate(element => getComputedStyle(element).cursor)).not.toBe('pointer') - expect(await disclosures.nth(1).evaluate(element => getComputedStyle(element).cursor)).not.toBe('pointer') + const runDisclosure = disclosures.nth(0) + const phaseDisclosure = disclosures.nth(1) + expect(await runDisclosure.getAttribute('role')).toBe('button') + expect(await runDisclosure.getAttribute('aria-expanded')).toBe('true') + expect(await phaseDisclosure.getAttribute('role')).toBe('button') + expect(await phaseDisclosure.getAttribute('aria-expanded')).toBe('true') + expect(await runDisclosure.evaluate(element => getComputedStyle(element).cursor)).toBe('pointer') + expect(await phaseDisclosure.evaluate(element => getComputedStyle(element).cursor)).toBe('pointer') const member = page.getByRole('button', { name: /^Open Reply with exactly the word/ }) await member.waitFor({ timeout: 15_000 }) + + await phaseDisclosure.click() + expect(await phaseDisclosure.getAttribute('aria-expanded')).toBe('false') + expect(await member.count()).toBe(0) + const liveSnapshot = await captureStableAria(page, '[data-chat-flow]', scaffold.workspaceCwd) + await compareOrRefreshGolden(UI_LIVE_EXPECTED, liveSnapshot, MODE) + await phaseDisclosure.press('Enter') + await member.waitFor() + await runDisclosure.click() + expect(await runDisclosure.getAttribute('aria-expanded')).toBe('false') + expect(await disclosures.count()).toBe(1) + await runDisclosure.press('Space') + expect(await disclosures.count()).toBe(2) + expect(await phaseDisclosure.getAttribute('aria-expanded')).toBe('true') await member.focus() const lightColor = await member.locator('[data-member-label]').evaluate(element => getComputedStyle(element).color) @@ -171,6 +188,8 @@ describe.skipIf(MODE === 'record')('web e2e: durable workflow run in Chat', () = const workflow = page.getByRole('button', { name: /^snapshot-flow/ }) await workflow.waitFor({ timeout: 15_000 }) expect(await workflow.getAttribute('aria-expanded')).toBe('false') + const snapshot = await captureStableAria(page, '[data-chat-flow]', scaffold.workspaceCwd) + await compareOrRefreshGolden(UI_EXPECTED, snapshot, MODE) await workflow.click() const phase = page.getByRole('button', { name: /^Run/ }) await phase.waitFor() @@ -179,13 +198,11 @@ describe.skipIf(MODE === 'record')('web e2e: durable workflow run in Chat', () = await page.getByText(CHILD_PROMPT, { exact: false }).waitFor() expect(await page.getByRole('button', { name: /^Open Reply with exactly the word/ }).count()).toBe(0) - const snapshot = await captureStableAria(page, '[data-chat-flow]', scaffold.workspaceCwd) - await compareOrRefreshGolden(UI_EXPECTED, snapshot, MODE) }, 60_000) it('stays clean and owns only its one golden', async () => { expect(tripwire.pageErrors).toEqual([]) expect(tripwire.warnings).toEqual([]) - await assertFixtureInventory(SNAPSHOT_DIR, ['ui.expected.md']) + await assertFixtureInventory(SNAPSHOT_DIR, ['ui-live.expected.md', 'ui.expected.md']) }) }) diff --git a/packages/client/ui-workflow-run/README.i18n.yaml b/packages/client/ui-workflow-run/README.i18n.yaml index ed480dc9c8..7f4dd23638 100644 --- a/packages/client/ui-workflow-run/README.i18n.yaml +++ b/packages/client/ui-workflow-run/README.i18n.yaml @@ -2,5 +2,5 @@ # side as of the last confirmed-consistent state. Both languages carry equal authority; # after editing either side, bring the other along and re-record with: # pnpm run verify-translation-pairing --write packages/client/ui-workflow-run/README.md -README.md: 3fcdc632b54be7d1f8f452c6f8b3b2e1525953aa -README.zh.md: 5f0ba0c8f8a251d0ce5977d469ee60637f0454db +README.md: 837ae237258430d942ebdd28a0f4d6efd2876de8 +README.zh.md: dfec61005f8f06637602722041f3233219b7216b diff --git a/packages/client/ui-workflow-run/README.md b/packages/client/ui-workflow-run/README.md index 3fcdc632b5..837ae23725 100644 --- a/packages/client/ui-workflow-run/README.md +++ b/packages/client/ui-workflow-run/README.md @@ -12,7 +12,7 @@ Phase groups come only from members that actually started. Exact phase strings s ## Presentation and navigation -The run and each phase derive disclosure control from their current lifecycle facts. The run stays expanded while its own status is running, failed, cancelled, or interrupted, or while any phase contains such a member; each affected phase also stays expanded. Forced-open headers are static expanded rows without button, keyboard, or `aria-expanded` promises. A phase folds once when every member completes, and the run folds once when it and every phase complete. Each clean layer then exposes an ordinary disclosure control whose local choice survives clean rerenders; new activity takes control again, and a remount derives the initial state from current data. The run uses a 32-pixel `--dsw-alias-bg-module-platform` row with persistent right/down chevrons and an inline state dot plus status text, without a badge. Phases use 32-pixel disclosure rows with title and member count in the flexible main area and a fixed precise aggregate-status tail, without another dot. Members use a 16-pixel dot slot, a truncating name area, and a fixed 64-pixel status column. +The run and each phase are controlled disclosures in every status. A mount opens running, failed, cancelled, and interrupted levels and closes fully completed levels; users can then toggle either level with the full row, Enter, or Space. Ordinary running updates preserve the current choice, the first abnormal edge opens once, normal completion closes once, and a completed phase plus the outer run open again when a new running member starts under the same phase key. Completion updates the visible status immediately but delays its automatic close while focus remains inside the content. `WorkflowRunPanel` owns the phase choices, so closing and reopening the outer run does not reset them; a renderer remount reconstructs every initial choice from durable facts. The run uses a 32-pixel `--dsw-alias-bg-module-platform` row with persistent right/down chevrons and an inline state dot plus status text, without a badge. Phases use 32-pixel disclosure rows with title and member count in the flexible main area and a fixed precise aggregate-status tail, without another dot. Members use a 16-pixel dot slot, a truncating name area, and a fixed 64-pixel status column. A member opens a child Session only while every current fact agrees: the member is running, the child id is in the ordinary Session list, the row has `origin: 'subagent'`, its `parentId` is the current Session, and the list row is still running. Underlined member text is the only visible navigation affordance; keyboard focus draws a two-pixel business-primary ring around the name area, while status copy remains `Running`. The component calls only the injected ordinary `sessions.open(id)` action; remote, addressed-only, wrong-parent, or terminal rows remain non-interactive. diff --git a/packages/client/ui-workflow-run/README.zh.md b/packages/client/ui-workflow-run/README.zh.md index 5f0ba0c8f8..dfec61005f 100644 --- a/packages/client/ui-workflow-run/README.zh.md +++ b/packages/client/ui-workflow-run/README.zh.md @@ -12,7 +12,7 @@ ## 展示与导航 -运行和每个阶段都从当前生命周期事实派生 disclosure 控制。运行自身处于运行中、失败、已取消或已中断,或者任一阶段包含这些状态的成员时,运行保持展开;受影响的阶段也保持展开。强制展开的标题行只是静态展开行,不承诺按钮、键盘操作或 `aria-expanded`。阶段在全部成员完成时折叠一次;运行在自身和全部阶段都完成时折叠一次。每个干净层级随后恢复普通 disclosure 控件,其本地选择在干净状态的 rerender 中保持;新活动会重新取得控制,remount 则从当前数据派生初始状态。运行使用 32 像素 `--dsw-alias-bg-module-platform` 背景行,常驻向右/向下 chevron,并以内联状态点加状态文字表达结局,不使用胶囊。阶段使用 32 像素 disclosure 行,在可伸缩主区显示标题与成员数,在固定尾部精确显示聚合状态且不重复状态点。成员使用 16 像素状态点槽、可省略名称区和固定 64 像素状态列。 +运行和每个阶段在所有状态下都是受控 disclosure。挂载时,运行中、失败、已取消和已中断层级默认展开,全部完成的层级默认折叠;此后用户可以点击整行,或按 Enter、Space 切换任一层级。普通运行更新保留当前选择,首次异常边沿只自动展开一次,正常完成只自动折叠一次;已完成阶段在同一 phase key 下开始新的运行成员时,该 Phase 与外层运行会再次自动展开。完成状态会立即更新,但只要焦点仍位于展开内容内,自动折叠就会等待焦点离开。Phase 选择由 `WorkflowRunPanel` 持有,因此关闭并重新打开外层运行不会重置它们;renderer remount 会从持久事实重建每层的初始选择。运行使用 32 像素 `--dsw-alias-bg-module-platform` 背景行,常驻向右/向下 chevron,并以内联状态点加状态文字表达结局,不使用胶囊。阶段使用 32 像素 disclosure 行,在可伸缩主区显示标题与成员数,在固定尾部精确显示聚合状态且不重复状态点。成员使用 16 像素状态点槽、可省略名称区和固定 64 像素状态列。 只有所有实时事实同时成立时,成员才可打开子 Session:成员仍在运行、子 id 位于普通 Session 列表、列表行为 `origin: 'subagent'`、`parentId` 等于当前 Session,且列表行仍标记运行。带下划线的成员文字是唯一可见导航提示;键盘聚焦时,名称区显示 2 像素 business-primary 焦点环,右侧状态仍只显示“运行中”。组件只调用注入的普通 `sessions.open(id)`;远程、仅地址化、父级不符或终态的行都不可交互。 diff --git a/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx b/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx index 5e24a16717..a839403266 100644 --- a/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx +++ b/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx @@ -1,4 +1,7 @@ -import { useState, type ReactNode } from 'react' +import { + useLayoutEffect, useMemo, useRef, useState, + type FocusEvent, type ReactNode, +} from 'react' import { DisclosureRow, IconChevronRightOutline14, StateDot, type DisclosureRowProps, type StateDotState, @@ -63,34 +66,85 @@ function memberCount(count: number, t: WorkflowRunPanelProps['t']): string { return t(count === 1 ? 'run.members.one' : 'run.members.other', { count }) } -function phaseRequiresExpansion(phase: WorkflowRunPhaseData): boolean { - return phase.members.some(member => member.status !== 'completed') +type DisclosureMode = 'clean' | 'running' | 'abnormal' + +interface DisclosureFacts { + readonly mode: DisclosureMode + readonly activityCount: number } -type StatusDisclosureProps = Omit - -/* v8 ignore next -- DisclosureRow requires the callback but cannot invoke it when expandable is false. */ -const forcedOpenToggle = (): void => {} - -function ManualDisclosure(props: StatusDisclosureProps) { - const [open, setOpen] = useState(false) - return ( - { setOpen(value => !value) }} - /> - ) +interface DisclosureState extends DisclosureFacts { + readonly open: boolean + readonly pendingCleanCollapse: boolean } -function StatusDisclosure({ cleanCycleKey, requiresExpansion, ...props }: StatusDisclosureProps & { - /** Remount a clean Phase when its append-only member count changes between batched renders. */ - readonly cleanCycleKey?: number | undefined - readonly requiresExpansion: boolean -}) { - if (!requiresExpansion) return - return +interface WorkflowDisclosureState { + readonly run: DisclosureState + readonly phases: ReadonlyMap +} + +type StatusDisclosureProps = Omit + +function StatusDisclosure(props: StatusDisclosureProps) { + return +} + +function abnormal(status: WorkflowRunStatus): boolean { + return status === 'failed' || status === 'cancelled' || status === 'interrupted' +} + +function phaseDisclosureFacts(phase: WorkflowRunPhaseData): DisclosureFacts { + const mode = phase.members.some(member => abnormal(member.status)) + ? 'abnormal' + : phase.members.some(member => member.status === 'running') ? 'running' : 'clean' + return { mode, activityCount: phase.members.length } +} + +function runDisclosureFacts( + status: WorkflowRunStatus, + phases: readonly (readonly [string, DisclosureFacts])[], +): DisclosureFacts { + const mode = abnormal(status) || phases.some(([, facts]) => facts.mode === 'abnormal') + ? 'abnormal' + : status === 'running' || phases.some(([, facts]) => facts.mode === 'running') + ? 'running' + : 'clean' + const activityCount = phases.reduce((count, [, facts]) => count + facts.activityCount, 0) + return { mode, activityCount } +} + +function initialDisclosureState(facts: DisclosureFacts): DisclosureState { + return { ...facts, open: facts.mode !== 'clean', pendingCleanCollapse: false } +} + +function advanceDisclosureState( + current: DisclosureState, + facts: DisclosureFacts, + focusWithin: boolean, +): DisclosureState { + const sameFacts = current.mode === facts.mode && current.activityCount === facts.activityCount + if (sameFacts) { + if (!current.pendingCleanCollapse || focusWithin) return current + return { ...current, open: false, pendingCleanCollapse: false } + } + if (facts.mode === 'clean') { + const deferCollapse = current.open && focusWithin + return { ...facts, open: deferCollapse, pendingCleanCollapse: deferCollapse } + } + if (current.mode === 'clean' || (facts.mode === 'abnormal' && current.mode !== 'abnormal')) { + return { ...facts, open: true, pendingCleanCollapse: false } + } + return { ...facts, open: current.open, pendingCleanCollapse: false } +} + +function focusIsWithin(element: HTMLElement | null | undefined): boolean { + if (element === null || element === undefined) return false + return element.contains(element.ownerDocument.activeElement) +} + +function collapsePending(state: DisclosureState): DisclosureState { + if (!state.pendingCleanCollapse) return state + return { ...state, open: false, pendingCleanCollapse: false } } function phaseStatusSummary(members: readonly WorkflowRunMemberData[], t: WorkflowRunPanelProps['t']): string { @@ -128,11 +182,12 @@ function navigableMembers( return result } -function RunHeader({ children, count, name, requiresExpansion, status, t }: { +function RunHeader({ children, count, name, onToggle, open, status, t }: { readonly children: ReactNode readonly count: number readonly name: string - readonly requiresExpansion: boolean + readonly onToggle: () => void + readonly open: boolean readonly status: WorkflowRunStatus readonly t: WorkflowRunPanelProps['t'] }) { @@ -140,7 +195,8 @@ function RunHeader({ children, count, name, requiresExpansion, status, t }: { } title={t('run.title', { name })} - requiresExpansion={requiresExpansion} + open={open} + onToggle={onToggle} expandOnRowClick previewChevron={false} keepContentWhenOpen @@ -170,6 +226,21 @@ function MemberRow({ member, navigable, openSession, t }: { readonly t: WorkflowRunPanelProps['t'] }) { const name = readableMember(member.label, t) + const buttonRef = useRef(null) + const [keepFocusedButton, setKeepFocusedButton] = useState(navigable) + const renderButton = navigable || keepFocusedButton + + useLayoutEffect(() => { + if (navigable) { + if (!keepFocusedButton) setKeepFocusedButton(true) + return + } + const button = buttonRef.current + if (button === null || button.ownerDocument.activeElement !== button) { + if (keepFocusedButton) setKeepFocusedButton(false) + } + }, [keepFocusedButton, navigable]) + const content = ( <> @@ -177,23 +248,33 @@ function MemberRow({ member, navigable, openSession, t }: { {t(STATUS_KEYS[member.status])} ) - if (!navigable) { + if (!renderButton) { return
{content}
} return ( ) } -function PhaseSection({ phase, navigable, openSession, t }: { +function PhaseSection({ + contentRef, onContentBlur, onToggle, open, phase, navigable, openSession, t, +}: { + readonly contentRef: (element: HTMLDivElement | null) => void + readonly onContentBlur: (event: FocusEvent) => void + readonly onToggle: () => void + readonly open: boolean readonly phase: WorkflowRunPhaseData readonly navigable: readonly SessionId[] readonly openSession: WorkflowRunInjected['openSession'] @@ -203,8 +284,8 @@ function PhaseSection({ phase, navigable, openSession, t }: { } title={readablePhase(phase.phase, t)} - cleanCycleKey={phase.members.length} - requiresExpansion={phaseRequiresExpansion(phase)} + open={open} + onToggle={onToggle} expandOnRowClick previewChevron={false} keepContentWhenOpen @@ -220,7 +301,7 @@ function PhaseSection({ phase, navigable, openSession, t }: { )} > -
+
{phase.members.map(member => ( count + phase.members.length, 0) - const requiresExpansion = node.data.status !== 'completed' - || node.data.phases.some(phaseRequiresExpansion) + const phaseFacts = useMemo(() => node.data.phases.map(phase => ( + [phase.key, phaseDisclosureFacts(phase)] as const + )), [node.data.phases]) + const runFacts = useMemo( + () => runDisclosureFacts(node.data.status, phaseFacts), + [node.data.status, phaseFacts], + ) + const totalMembers = runFacts.activityCount + const [disclosures, setDisclosures] = useState(() => ({ + run: initialDisclosureState(runFacts), + phases: new Map(phaseFacts.map(([key, facts]) => [key, initialDisclosureState(facts)])), + })) + const runContentRef = useRef(null) + const phaseContentRefs = useRef(new Map()) const navigable = useSessions( sessions => navigableMembers(sessions, node.data.phases, sessionId), shallowEqual, ) + + useLayoutEffect(() => { + setDisclosures((current) => { + const phases = new Map() + let phasesChanged = current.phases.size !== phaseFacts.length + let phaseBecameActive = false + for (const [key, facts] of phaseFacts) { + const previous = current.phases.get(key) + const next = previous === undefined + ? initialDisclosureState(facts) + : advanceDisclosureState(previous, facts, focusIsWithin(phaseContentRefs.current.get(key))) + phases.set(key, next) + if (next !== previous) phasesChanged = true + if (previous?.mode === 'clean' && facts.mode !== 'clean') phaseBecameActive = true + } + const advancedRun = advanceDisclosureState( + current.run, + runFacts, + focusIsWithin(runContentRef.current), + ) + const run = phaseBecameActive && !advancedRun.open + ? { ...advancedRun, open: true, pendingCleanCollapse: false } + : advancedRun + return run !== current.run || phasesChanged ? { run, phases } : current + }) + }, [disclosures.run.open, phaseFacts, runFacts]) + + const toggleRun = (): void => { + setDisclosures(current => ({ + ...current, + run: { + ...current.run, + open: !current.run.open, + pendingCleanCollapse: false, + }, + })) + } + const togglePhase = (key: string, facts: DisclosureFacts): void => { + setDisclosures((current) => { + const phases = new Map(current.phases) + /* v8 ignore next -- layout effects insert every rendered phase before user input can toggle it. */ + const phase = phases.get(key) ?? initialDisclosureState(facts) + phases.set(key, { + ...phase, + open: !phase.open, + pendingCleanCollapse: false, + }) + return { ...current, phases } + }) + } + const settleRunBlur = (event: FocusEvent): void => { + if (event.currentTarget.contains(event.relatedTarget)) return + setDisclosures((current) => { + const run = collapsePending(current.run) + return run === current.run ? current : { ...current, run } + }) + } + const settlePhaseBlur = (key: string, event: FocusEvent): void => { + if (event.currentTarget.contains(event.relatedTarget)) return + setDisclosures((current) => { + const phase = current.phases.get(key) + /* v8 ignore next -- the blur handler unmounts with the phase whose state it addresses. */ + if (phase === undefined) return current + const next = collapsePending(phase) + if (next === phase) return current + const phases = new Map(current.phases) + phases.set(key, next) + return { ...current, phases } + }) + } + return (
-
+
{node.data.phases.length === 0 ? {t('run.empty')} - : node.data.phases.map(phase => ( - - ))} + : node.data.phases.map((phase) => { + const facts = phaseDisclosureFacts(phase) + const disclosure = disclosures.phases.get(phase.key) ?? initialDisclosureState(facts) + return ( + { + if (element === null) phaseContentRefs.current.delete(phase.key) + else phaseContentRefs.current.set(phase.key, element) + }} + onContentBlur={(event) => { settlePhaseBlur(phase.key, event) }} + onToggle={() => { togglePhase(phase.key, facts) }} + open={disclosure.open} + phase={phase} + navigable={navigable} + openSession={openSession} + t={t} + /> + ) + })}
diff --git a/packages/client/ui-workflow-run/tests/workflow-run.client.spec.tsx b/packages/client/ui-workflow-run/tests/workflow-run.client.spec.tsx index 8be62e8209..e14d203421 100644 --- a/packages/client/ui-workflow-run/tests/workflow-run.client.spec.tsx +++ b/packages/client/ui-workflow-run/tests/workflow-run.client.spec.tsx @@ -27,6 +27,7 @@ afterEach(cleanup) const PARENT_ID = 'parent' as SessionId const CHILD_ID = 'child-1' as SessionId +const SECOND_ID = 'child-2' as SessionId interface ChatSnapshot { readonly nodes: ReadonlyMap @@ -301,28 +302,62 @@ function panelProps(data: WorkflowRunChatData, sessions = listState(), openSessi } describe('WorkflowRunPanel', () => { - it('forces running run and phase content open without false disclosure controls', () => { - const view = render( { + const running: WorkflowRunChatData = { name: 'audit', status: 'running', phases: [phase({ key: 'research', phase: 'Research' })], - })} />) - expect(screen.getByText('worker')).toBeTruthy() - expect(screen.queryByRole('button', { name: /^audit/ })).toBeNull() - expect(screen.queryByRole('button', { name: /Research/ })).toBeNull() - const rows = [...view.container.querySelectorAll('[data-disclosure-row]')] - expect(rows).toHaveLength(2) - for (const row of rows) { - expect(row.getAttribute('role')).toBeNull() - expect(row.getAttribute('tabindex')).toBeNull() - expect(row.getAttribute('aria-expanded')).toBeNull() - expect(row.getAttribute('data-expandable')).toBeNull() } + const view = render() + const runHeader = screen.getByRole('button', { name: /^audit/ }) + const phaseHeader = screen.getByRole('button', { name: /Research/ }) + expect(runHeader.getAttribute('aria-expanded')).toBe('true') + expect(phaseHeader.getAttribute('aria-expanded')).toBe('true') + expect(screen.getByText('worker')).toBeTruthy() + + fireEvent.click(phaseHeader) + expect(phaseHeader.getAttribute('aria-expanded')).toBe('false') + expect(screen.queryByText('worker')).toBeNull() + fireEvent.click(runHeader) + expect(runHeader.getAttribute('aria-expanded')).toBe('false') + + view.rerender() + expect(runHeader.getAttribute('aria-expanded')).toBe('false') + expect(screen.queryByRole('button', { name: /Research/ })).toBeNull() + fireEvent.keyDown(runHeader, { key: 'ArrowDown' }) + expect(runHeader.getAttribute('aria-expanded')).toBe('false') + fireEvent.keyDown(runHeader, { key: ' ' }) + const updatedPhase = screen.getByRole('button', { name: /Research/ }) + expect(updatedPhase.getAttribute('aria-expanded')).toBe('false') + expect(screen.getByText('运行中 2')).toBeTruthy() + fireEvent.keyDown(updatedPhase, { key: 'Enter' }) + expect(screen.getByText('worker')).toBeTruthy() + expect(screen.getByText('second')).toBeTruthy() + + fireEvent.click(runHeader) + expect(runHeader.getAttribute('aria-expanded')).toBe('false') + expect(screen.queryByRole('button', { name: /Research/ })).toBeNull() + fireEvent.keyDown(runHeader, { key: ' ' }) + expect(screen.getByRole('button', { name: /Research/ }).getAttribute('aria-expanded')).toBe('true') }) - it('folds each clean transition once and preserves review choices until activity returns', () => { + it('folds each normal completion once and opens a new same-key activity cycle', () => { const running: WorkflowRunChatData = { name: 'audit', status: 'running', phases: [phase()], } const view = render() + const runningPhase = screen.getByRole('button', { name: /未分阶段/ }) + fireEvent.click(runningPhase) + fireEvent.keyDown(runningPhase, { key: 'Enter' }) + expect(screen.getByText('worker')).toBeTruthy() + const phaseCompleted: WorkflowRunChatData = { ...running, phases: [phase({ @@ -338,27 +373,8 @@ describe('WorkflowRunPanel', () => { fireEvent.click(phaseHeader) expect(screen.getByText('done')).toBeTruthy() - const completed: WorkflowRunChatData = { ...phaseCompleted, status: 'completed' } - view.rerender() - const runHeader = screen.getByRole('button', { name: /^audit/ }) - expect(runHeader.getAttribute('aria-expanded')).toBe('false') - expect(screen.queryByText('未分阶段')).toBeNull() - fireEvent.keyDown(runHeader, { key: 'ArrowDown' }) - expect(runHeader.getAttribute('aria-expanded')).toBe('false') - fireEvent.keyDown(runHeader, { key: 'Enter' }) - expect(runHeader.getAttribute('aria-expanded')).toBe('true') - const completedPhase = screen.getByRole('button', { name: /未分阶段/ }) - fireEvent.keyDown(completedPhase, { key: 'Enter' }) - expect(screen.getByText('done')).toBeTruthy() - fireEvent.keyDown(runHeader, { key: ' ' }) - expect(runHeader.getAttribute('aria-expanded')).toBe('false') - fireEvent.keyDown(runHeader, { key: ' ' }) - expect(runHeader.getAttribute('aria-expanded')).toBe('true') - fireEvent.click(screen.getByRole('button', { name: /未分阶段/ })) - expect(screen.getByText('done')).toBeTruthy() - const cleanUpdate: WorkflowRunChatData = { - ...completed, + ...phaseCompleted, phases: [phase({ members: [{ seq: 1, label: 'reviewed', childId: 'child-1' as SessionId, status: 'completed', @@ -368,13 +384,44 @@ describe('WorkflowRunPanel', () => { view.rerender() expect(screen.getByText('reviewed')).toBeTruthy() - view.rerender() - expect(screen.queryByRole('button', { name: /^audit/ })).toBeNull() - expect(screen.queryByRole('button', { name: /未分阶段/ })).toBeNull() - expect(screen.getByText('worker')).toBeTruthy() - view.rerender() + fireEvent.click(screen.getByRole('button', { name: /未分阶段/ })) + const runHeader = screen.getByRole('button', { name: /^audit/ }) + fireEvent.click(runHeader) + const renewed: WorkflowRunChatData = { + name: 'audit', status: 'running', + phases: [phase({ + members: [ + { seq: 1, label: 'reviewed', childId: CHILD_ID, status: 'completed' }, + { seq: 2, label: 'new', childId: 'child-2' as SessionId, status: 'running' }, + ], + })], + } + view.rerender() + expect(screen.getByRole('button', { name: /^audit/ }).getAttribute('aria-expanded')).toBe('true') + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('true') + expect(screen.getByText('new')).toBeTruthy() + + const renewedPhaseCompleted: WorkflowRunChatData = { + ...renewed, + phases: [phase({ + members: [ + { seq: 1, label: 'reviewed', childId: CHILD_ID, status: 'completed' }, + { seq: 2, label: 'new', childId: 'child-2' as SessionId, status: 'completed' }, + ], + })], + } + view.rerender() + expect(screen.getByRole('button', { name: /^audit/ }).getAttribute('aria-expanded')).toBe('true') + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('false') + fireEvent.click(screen.getByRole('button', { name: /未分阶段/ })) + + view.rerender() expect(screen.getByRole('button', { name: /^audit/ }).getAttribute('aria-expanded')).toBe('false') - expect(screen.queryByText('未分阶段')).toBeNull() + fireEvent.click(screen.getByRole('button', { name: /^audit/ })) + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('true') }) it('refolds a phase when a complete activity cycle arrives as one clean update', () => { @@ -399,10 +446,32 @@ describe('WorkflowRunPanel', () => { expect(screen.queryByText('second')).toBeNull() }) + it('initializes a newly observed phase before it becomes interactive', () => { + const running: WorkflowRunChatData = { + name: 'dynamic-phase', status: 'running', + phases: [phase({ key: 'research', phase: 'Research' })], + } + const view = render() + view.rerender() + const build = screen.getByRole('button', { name: /Build/ }) + expect(build.getAttribute('aria-expanded')).toBe('true') + fireEvent.click(build) + expect(build.getAttribute('aria-expanded')).toBe('false') + }) + it('derives the zero-member running and completed states from the current run status', () => { const running: WorkflowRunChatData = { name: 'empty', status: 'running', phases: [] } const view = render() - expect(screen.queryByRole('button', { name: /^empty/ })).toBeNull() + expect(screen.getByRole('button', { name: /^empty/ }).getAttribute('aria-expanded')).toBe('true') expect(screen.getByText('没有启动成员')).toBeTruthy() view.rerender() const header = screen.getByRole('button', { name: /^empty/ }) @@ -413,31 +482,60 @@ describe('WorkflowRunPanel', () => { }) it.each(['failed', 'cancelled', 'interrupted'] as const)( - 'bubbles a %s member to the run and keeps a matching run outcome open', + 'initializes %s attention as an expanded disclosure that remains manually collapsible', (status) => { - const memberView = render() - expect(screen.queryByRole('button', { name: /^member-outcome/ })).toBeNull() - expect(screen.queryByRole('button', { name: /未分阶段/ })).toBeNull() + const runHeader = screen.getByRole('button', { name: /^member-outcome/ }) + const phaseHeader = screen.getByRole('button', { name: /未分阶段/ }) + expect(runHeader.getAttribute('aria-expanded')).toBe('true') + expect(phaseHeader.getAttribute('aria-expanded')).toBe('true') expect(screen.getByText(status)).toBeTruthy() - memberView.unmount() - - render() - expect(screen.queryByRole('button', { name: /^run-outcome/ })).toBeNull() - expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('false') - expect(screen.queryByText('done')).toBeNull() + fireEvent.click(phaseHeader) + fireEvent.click(runHeader) + expect(runHeader.getAttribute('aria-expanded')).toBe('false') }, ) + it('opens the first abnormal edge once and preserves later abnormal choices', () => { + const running: WorkflowRunChatData = { + name: 'audit', status: 'running', phases: [phase()], + } + const view = render() + fireEvent.click(screen.getByRole('button', { name: /未分阶段/ })) + fireEvent.click(screen.getByRole('button', { name: /^audit/ })) + + const failed: WorkflowRunChatData = { + name: 'audit', status: 'failed', + phases: [phase({ + members: [{ seq: 1, label: 'failed', childId: CHILD_ID, status: 'failed' }], + })], + } + view.rerender() + expect(screen.getByRole('button', { name: /^audit/ }).getAttribute('aria-expanded')).toBe('true') + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('true') + fireEvent.click(screen.getByRole('button', { name: /未分阶段/ })) + fireEvent.click(screen.getByRole('button', { name: /^audit/ })) + + view.rerender() + expect(screen.getByRole('button', { name: /^audit/ }).getAttribute('aria-expanded')).toBe('false') + fireEvent.click(screen.getByRole('button', { name: /^audit/ })) + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('false') + expect(screen.getByText('失败 1 · 已取消 1')).toBeTruthy() + }) + it('keeps clean sibling phases independent and preserves empty versus absent names', () => { render( { }] }), ], })} />) - expect(screen.queryByRole('button', { name: /^audit/ })).toBeNull() + const runHeader = screen.getByRole('button', { name: /^audit/ }) + expect(runHeader.getAttribute('aria-expanded')).toBe('true') const cleanPhase = screen.getByRole('button', { name: /空阶段名/ }) expect(cleanPhase.getAttribute('aria-expanded')).toBe('false') - expect(screen.queryByRole('button', { name: /未分阶段/ })).toBeNull() + const activePhase = screen.getByRole('button', { name: /未分阶段/ }) + expect(activePhase.getAttribute('aria-expanded')).toBe('true') expect(screen.queryByText('空成员名')).toBeNull() expect(screen.getByText('second')).toBeTruthy() fireEvent.click(cleanPhase) expect(screen.getByText('空成员名')).toBeTruthy() expect(screen.getByText('second')).toBeTruthy() - fireEvent.click(cleanPhase) + fireEvent.click(activePhase) + expect(screen.queryByText('second')).toBeNull() + expect(screen.getByText('空成员名')).toBeTruthy() + fireEvent.click(runHeader) + fireEvent.click(runHeader) + expect(screen.getByRole('button', { name: /空阶段名/ }).getAttribute('aria-expanded')).toBe('true') + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('false') + fireEvent.click(screen.getByRole('button', { name: /空阶段名/ })) expect(screen.queryByText('空成员名')).toBeNull() - expect(screen.getByText('second')).toBeTruthy() }) it('renders mixed and interrupted aggregate status while attention stays visible', () => { @@ -496,6 +602,107 @@ describe('WorkflowRunPanel', () => { expect(interruptedView.container.querySelectorAll('[data-state="warning"]')).toHaveLength(2) }) + it('defers normal completion collapse until focused member content loses focus', () => { + const sessions = listState({ + ids: [PARENT_ID, CHILD_ID, SECOND_ID], + byId: { + ...listState().byId, + [SECOND_ID]: { + id: SECOND_ID, displayTitle: 'second', parentId: PARENT_ID, origin: 'subagent', + running: true, blank: false, updatedAt: 0, + }, + }, + }) + const running: WorkflowRunChatData = { + name: 'audit', status: 'running', phases: [phase({ + members: [ + { seq: 1, label: 'worker', childId: CHILD_ID, status: 'running' }, + { seq: 2, label: 'second', childId: SECOND_ID, status: 'running' }, + ], + })], + } + const view = render() + const member = screen.getByRole('button', { name: '打开 worker' }) + const second = screen.getByRole('button', { name: '打开 second' }) + const runHeader = screen.getByRole('button', { name: /^audit/ }) + const phaseHeader = screen.getByRole('button', { name: /未分阶段/ }) + member.focus() + expect(document.activeElement).toBe(member) + fireEvent.blur(member, { relatedTarget: second }) + second.focus() + expect(runHeader.getAttribute('aria-expanded')).toBe('true') + expect(phaseHeader.getAttribute('aria-expanded')).toBe('true') + + const outside = document.createElement('button') + document.body.append(outside) + fireEvent.blur(second, { relatedTarget: outside }) + outside.focus() + expect(runHeader.getAttribute('aria-expanded')).toBe('true') + expect(phaseHeader.getAttribute('aria-expanded')).toBe('true') + member.focus() + + view.rerender() + expect(runHeader.getAttribute('aria-expanded')).toBe('true') + expect(phaseHeader.getAttribute('aria-expanded')).toBe('true') + const retained = screen.getByRole('button', { name: 'worker' }) + expect(retained.getAttribute('aria-disabled')).toBe('true') + expect(document.activeElement).toBe(retained) + + fireEvent.blur(retained, { relatedTarget: outside }) + outside.focus() + expect(document.activeElement).toBe(outside) + expect(runHeader.getAttribute('aria-expanded')).toBe('false') + fireEvent.click(runHeader) + const completedPhase = screen.getByRole('button', { name: /未分阶段/ }) + expect(completedPhase.getAttribute('aria-expanded')).toBe('false') + fireEvent.click(completedPhase) + expect(screen.queryByRole('button', { name: '打开 worker' })).toBeNull() + expect(screen.getByText('worker')).toBeTruthy() + outside.remove() + }) + + it('settles a deferred phase close when the user hides the outer run', () => { + const running: WorkflowRunChatData = { + name: 'audit', status: 'running', phases: [phase()], + } + const view = render() + const member = screen.getByRole('button', { name: '打开 worker' }) + member.focus() + view.rerender() + const runHeader = screen.getByRole('button', { name: /^audit/ }) + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('true') + fireEvent.click(runHeader) + expect(runHeader.getAttribute('aria-expanded')).toBe('false') + fireEvent.click(runHeader) + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('false') + }) + + it('reinitializes manual choices from durable facts after a renderer remount', () => { + const data: WorkflowRunChatData = { + name: 'audit', status: 'running', phases: [phase()], + } + const view = render() + fireEvent.click(screen.getByRole('button', { name: /未分阶段/ })) + fireEvent.click(screen.getByRole('button', { name: /^audit/ })) + view.unmount() + render() + expect(screen.getByRole('button', { name: /^audit/ }).getAttribute('aria-expanded')).toBe('true') + expect(screen.getByRole('button', { name: /未分阶段/ }).getAttribute('aria-expanded')).toBe('true') + }) + it('opens only a running ordinary-list subagent proven to have this parent', () => { const data: WorkflowRunChatData = { name: 'audit', status: 'running', phases: [phase()], @@ -506,6 +713,16 @@ describe('WorkflowRunPanel', () => { expect(openSession).toHaveBeenCalledWith('child-1') }) + it('promotes a running member when its ordinary Session row arrives', () => { + const data: WorkflowRunChatData = { + name: 'audit', status: 'running', phases: [phase()], + } + const view = render() + expect(screen.queryByRole('button', { name: '打开 worker' })).toBeNull() + view.rerender() + expect(screen.getByRole('button', { name: '打开 worker' })).toBeTruthy() + }) + it.each([ ['not in ordinary list', listState({ ids: [PARENT_ID] }), 'running'], ['remote row', listState({ byId: { From 85616ec627a37334607b8499e3329b0d785328d7 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 17:04:16 +0800 Subject: [PATCH 06/28] test(web): pin the cold-boot settings.describe budget --- apps/web/tests/startup-rpc-budget.e2e.ts | 58 ++++++++++++++++++++++++ apps/web/tsconfig.json | 1 + tsconfig.host.json | 1 + 3 files changed, 60 insertions(+) create mode 100644 apps/web/tests/startup-rpc-budget.e2e.ts diff --git a/apps/web/tests/startup-rpc-budget.e2e.ts b/apps/web/tests/startup-rpc-budget.e2e.ts new file mode 100644 index 0000000000..5f8d7dd0e1 --- /dev/null +++ b/apps/web/tests/startup-rpc-budget.e2e.ts @@ -0,0 +1,58 @@ +// Cold-boot RPC budget. The describe mirror (packages/client/ui-settings) is +// the one `settings.describe` reader in the browser, so startup describe +// traffic stays bounded no matter how many client plugins own a preference. +// A regression here means a consumer bypassed the mirror — grep for +// `settings.describe(` outside ui-settings' client sources. +// +// Zero model calls: the lane only boots chrome, so no replay fixture mounts. +import type { Browser, Page } from 'playwright' +import { chromium } from 'playwright' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { launchWebScaffold, watchConsole, type WebScaffold } from './scaffold.ts' +import { newEnglishPage } from './support.ts' + +/** + * Itemized so the budget stays explainable. The mirror reads twice: once + * eagerly at bind time over HTTP, and once on the first-connection reset — + * that second read closes the window where a document commit lands between + * the eager read and the SSE subscription and its invalidation is lost. + * Beside it, the direct callers not yet migrated: welcome notice (1) + models + * onboarding (1) + plugin-directory tab at bind and at reset (2) + + * agent-preset settings row on reset (1). Batch 2 migrates those onto the + * mirror and tightens this to 2. + */ +const DESCRIBE_BUDGET = 7 + +let scaffold: WebScaffold +let browser: Browser +let page: Page + +beforeAll(async () => { + scaffold = await launchWebScaffold() + browser = await chromium.launch() +}) + +afterAll(async () => { + await page?.close() + await browser?.close() + await scaffold?.close() +}) + +describe('startup RPC budget', () => { + it('keeps cold-boot settings.describe within the mirror budget', async () => { + page = await newEnglishPage(browser) + watchConsole(page) + const calls: string[] = [] + page.on('request', (request) => { + const url = new URL(request.url()) + if (url.pathname.startsWith('/api/')) calls.push(url.pathname.slice('/api/'.length)) + }) + await page.goto(scaffold.baseUrl) + // Boot settles when the workspace picker is interactive; the trailing wait + // absorbs the first-connection reset wave the budget must include. + await page.getByRole('textbox', { name: 'Choose workspace' }).waitFor({ timeout: 30_000 }) + await page.waitForTimeout(3000) + const describeCount = calls.filter(method => method === 'settings.describe').length + expect(describeCount, `startup /api calls:\n${calls.join('\n')}`).toBeLessThanOrEqual(DESCRIBE_BUDGET) + }) +}) diff --git a/apps/web/tsconfig.json b/apps/web/tsconfig.json index 6e706c7123..11cd1ec807 100644 --- a/apps/web/tsconfig.json +++ b/apps/web/tsconfig.json @@ -24,6 +24,7 @@ "exclude": [ "tests/scaffold.ts", "tests/scaffold-hermetic.e2e.ts", + "tests/startup-rpc-budget.e2e.ts", "tests/minimal-preset.snapshot.ts", "tests/message-feedback-protocol.snapshot.ts", "tests/live-interactions.e2e.ts", diff --git a/tsconfig.host.json b/tsconfig.host.json index 459036247a..80c91c2a14 100644 --- a/tsconfig.host.json +++ b/tsconfig.host.json @@ -13,6 +13,7 @@ "apps/web/tests/declared-reasoning.e2e.ts", "apps/web/tests/support.ts", "apps/web/tests/scaffold-hermetic.e2e.ts", + "apps/web/tests/startup-rpc-budget.e2e.ts", "apps/web/tests/minimal-preset.snapshot.ts", "apps/web/tests/message-feedback-protocol.snapshot.ts", "apps/web/tests/live-interactions.e2e.ts", From bb3128f266268c9918a35b75cca2f7b9203841d7 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 17:11:03 +0800 Subject: [PATCH 07/28] refactor(ui-settings-models): welcome notice reads through the settings scope --- .../ui-settings-models/src/client/index.ts | 34 +-- .../src/client/welcome-store.ts | 166 +++++++------ .../tests/apply.client.spec.ts | 60 +++-- .../tests/welcome-notice.client.spec.tsx | 52 ++-- .../tests/welcome-store.client.spec.ts | 235 ++++++++---------- 5 files changed, 283 insertions(+), 264 deletions(-) diff --git a/packages/client/ui-settings-models/src/client/index.ts b/packages/client/ui-settings-models/src/client/index.ts index dc7f32e370..e45e52a4e8 100644 --- a/packages/client/ui-settings-models/src/client/index.ts +++ b/packages/client/ui-settings-models/src/client/index.ts @@ -22,7 +22,7 @@ import { DeepSeekOnboardingDialog } from './DeepSeekOnboardingDialog.tsx' import type { DeepSeekOnboardingInjected } from './DeepSeekOnboardingDialog.tsx' import { WelcomeNotice } from './WelcomeNotice.tsx' import type { WelcomeNoticeInjected } from './WelcomeNotice.tsx' -import { refreshWelcomeIfLoaded, WelcomeNoticeStore } from './welcome-store.ts' +import { decodeWelcomeSection, WelcomeNoticeStore } from './welcome-store.ts' import { ModelsSettingsStore } from './store.ts' import { en, zh, type ModelsKey } from './locales.ts' import { WELCOME_NOTICE_SETTINGS_NAMESPACE } from '../onboarding-copy.ts' @@ -56,7 +56,7 @@ export function refreshIfLoaded(controller: ModelsSettingsStore): void { * ui-settings' apply, whose activation order relative to this one is NOT * constrained; registration depends on each slot through `slots.inject()`. */ -export const inject = ['slots', 'locale', 'connection', 'remote'] +export const inject = ['slots', 'locale', 'connection', 'remote', 'settingsScope'] /** * Register the Models section once the `settings.section` declaration is on @@ -85,10 +85,12 @@ export function apply(ctx: ClientContext): void { api: connection.api, t, }) - const welcomeController = new WelcomeNoticeStore( - connection.api, - connection.isLoopback ? 'host' : 'memory', - ) + // The scope's own memory mode is what keeps a remote browser process-local, + // so the store needs no isLoopback branch of its own. + const welcomeController = new WelcomeNoticeStore(ctx.settingsScope.bind({ + namespace: WELCOME_NOTICE_SETTINGS_NAMESPACE, + decode: decodeWelcomeSection, + })) const welcomeInjected = (): WelcomeNoticeInjected => ({ controller: welcomeController, hooks: { welcome: welcomeController.store }, @@ -96,23 +98,21 @@ export function apply(ctx: ClientContext): void { }) // Pushed invalidations converge every open surface without polling: any - // settings/credentials/topology change refetches once the page loaded. + // settings/credentials/topology change refetches once the page loaded. The + // welcome notice follows its settings scope, so the shared mirror already + // keeps it fresh without a subscription here. ctx.effect(() => { const refreshModels = (): void => { refreshIfLoaded(controller) } - const refreshAll = (): void => { - refreshModels() - refreshWelcomeIfLoaded(welcomeController) - } const disposers = [ - ctx.remote.$on('settings/document-updated', (ns) => { - refreshModels() - if (ns === WELCOME_NOTICE_SETTINGS_NAMESPACE) refreshWelcomeIfLoaded(welcomeController) - }), + ctx.remote.$on('settings/document-updated', () => { refreshModels() }), ctx.remote.$on('credentials/updated', refreshModels), ctx.remote.$on('llm/adapters-updated', refreshModels), - ctx.on('connection/reset', refreshAll), + ctx.on('connection/reset', refreshModels), ] - return () => { for (const dispose of disposers) dispose() } + return () => { + welcomeController.dispose() + for (const dispose of disposers) dispose() + } }, 'ui-settings-models: pushed invalidations') ctx.slots.inject('settings.section', () => ctx.slots.register({ diff --git a/packages/client/ui-settings-models/src/client/welcome-store.ts b/packages/client/ui-settings-models/src/client/welcome-store.ts index 6e139f1a43..9edd54a9cb 100644 --- a/packages/client/ui-settings-models/src/client/welcome-store.ts +++ b/packages/client/ui-settings-models/src/client/welcome-store.ts @@ -1,10 +1,14 @@ -/** Welcome-notice state, durable when the browser may use Host settings. */ +/** + * Welcome-notice state derived from the welcome settings scope. The scope is + * the transport: a loopback browser follows the durable Host section, while a + * remote browser's memory-mode scope never answers and the acknowledgement + * stays process-local here. + */ -import type { IApiClient, SettingsNamespaceView } from '@deepseek-ai/dsh-api-remotes/client' -import type { SnapshotStore } from '@deepseek-ai/dsh-client-runtime/client' +import type { SettingsScope, SnapshotStore } from '@deepseek-ai/dsh-client-runtime/client' import { createSnapshotStore } from '@deepseek-ai/dsh-client-runtime/client' import { - WELCOME_NOTICE_ACK_FIELD, WELCOME_NOTICE_SETTINGS_NAMESPACE, WELCOME_NOTICE_VERSION, + WELCOME_NOTICE_ACK_FIELD, WELCOME_NOTICE_VERSION, } from '../onboarding-copy.ts' /** State rendered by the welcome step. */ @@ -14,113 +18,111 @@ export interface WelcomeNoticeState { error: string | null } -function messageOf(error: unknown): string { - return error instanceof Error ? error.message : String(error) -} +/** The welcome section as the notice reads it. */ +export type WelcomeSection = Record -function acknowledgementOf(view: SettingsNamespaceView): string | undefined { - if (typeof view.value !== 'object' || view.value === null) return undefined - const value = (view.value as Record)[WELCOME_NOTICE_ACK_FIELD] - return typeof value === 'string' ? value : undefined +/** + * Accept any object section verbatim; a malformed durable value reads as an + * empty section, so the notice treats it as unacknowledged instead of leaving + * the scope stuck on its previous value. + * @param section - the wire section value. + * @returns the section object, or an empty one for non-object values. + */ +export function decodeWelcomeSection(section: unknown): WelcomeSection { + return typeof section === 'object' && section !== null && !Array.isArray(section) + ? section as WelcomeSection + : {} } /** Coordinates durable Host acknowledgement or a process-local remote fallback. */ export class WelcomeNoticeStore { /** uSES-safe state source shared by the registered welcome step. */ readonly store: SnapshotStore = createSnapshotStore({ - status: 'idle', acknowledged: false, error: null, + status: 'idle' as const, acknowledged: false, error: null, }) - private generation = 0 + private localAcknowledged = false + private saving = false + private following: (() => void) | undefined /** - * @param api - settings wire face used for durable reads and writes. - * @param persistence - remote browsers use memory because settings is loopback-only. + * @param scope - the welcome settings namespace scope; its memory mode is + * what keeps a remote browser process-local. */ - constructor( - private readonly api: Pick, - private readonly persistence: 'host' | 'memory' = 'host', - ) {} + constructor(private readonly scope: SettingsScope) {} - /** Load the acknowledgement from Host settings or initialize process-local state. */ + /** Begin following the bound scope (idempotent) and publish its current answer. */ async load(): Promise { - const generation = ++this.generation - if (this.persistence === 'memory') { - this.store.update((state) => { state.status = 'ready'; state.error = null }) - return - } - this.store.update((state) => { state.status = 'loading'; state.error = null }) - try { - const response = await this.api.settings.describe({}) - if (!response.result.ok) throw new Error(response.result.error.message) - const view = response.result.value.namespaces.find( - candidate => candidate.ns === WELCOME_NOTICE_SETTINGS_NAMESPACE, - ) - if (view === undefined) throw new Error('welcome acknowledgement settings are unavailable') - if (generation !== this.generation) return - this.store.update((state) => { - state.status = 'ready' - state.acknowledged = acknowledgementOf(view) === WELCOME_NOTICE_VERSION - state.error = null - }) - } catch (error) { - if (generation !== this.generation) return - this.store.update((state) => { - state.status = 'error' - state.acknowledged = false - state.error = messageOf(error) - }) - } + this.following ??= this.scope.subscribe(() => { this.derive() }) + this.derive() } /** - * Persist this copy version, or advance only this process for a remote browser. - * @returns true when the selected persistence mode accepted the acknowledgement. + * Persist this copy version, or advance only this process for a remote + * browser. Success is judged against the state the write left behind, so a + * refused or failed write reports false after its recovery read settles. + * @returns true when the selected persistence mode holds the acknowledgement. */ async acknowledge(): Promise { - const generation = ++this.generation - if (this.persistence === 'memory') { - this.store.update((state) => { - state.status = 'ready' - state.acknowledged = true - state.error = null - }) + if (this.scope.getSnapshot().mode === 'memory') { + this.localAcknowledged = true + this.derive() return true } + this.saving = true this.store.update((state) => { state.status = 'saving'; state.error = null }) try { - const response = await this.api.settings.mutate({ - ns: WELCOME_NOTICE_SETTINGS_NAMESPACE, - ops: [{ op: 'set', path: [WELCOME_NOTICE_ACK_FIELD], value: WELCOME_NOTICE_VERSION }], + await this.scope.set(WELCOME_NOTICE_ACK_FIELD, WELCOME_NOTICE_VERSION) + } finally { + this.saving = false + } + this.derive() + const { acknowledged } = this.store.getSnapshot() + if (!acknowledged) { + this.store.update((state) => { + state.status = 'error' + state.error = 'the acknowledgement did not persist' }) - if (!response.result.ok) throw new Error(response.result.error.message) - if (generation === this.generation) { - this.store.update((state) => { - state.status = 'ready' - state.acknowledged = true - state.error = null - }) - } - return true - } catch (error) { - if (generation === this.generation) { + } + return acknowledged + } + + /** Stop following the scope. */ + dispose(): void { + this.following?.() + this.following = undefined + } + + private derive(): void { + if (this.saving) return + const scope = this.scope.getSnapshot() + if (scope.mode === 'memory') { + this.store.update((state) => { + state.status = 'ready' + state.acknowledged = this.localAcknowledged + state.error = null + }) + return + } + switch (scope.status) { + case 'loading': + this.store.update((state) => { state.status = 'loading'; state.error = null }) + return + case 'unavailable': this.store.update((state) => { state.status = 'error' state.acknowledged = false - state.error = messageOf(error) + state.error = 'welcome acknowledgement settings are unavailable' + }) + return + case 'ready': { + const acknowledged = scope.value?.[WELCOME_NOTICE_ACK_FIELD] === WELCOME_NOTICE_VERSION + this.store.update((state) => { + state.status = 'ready' + state.acknowledged = acknowledged + state.error = null }) } - return false } } } - -/** - * Refresh only after welcome state has left idle. A memory-mode load retains - * acknowledgement so reconnect does not reopen a process-local notice. - * @param controller - welcome state owner whose current status decides whether to load. - */ -export function refreshWelcomeIfLoaded(controller: WelcomeNoticeStore): void { - if (controller.store.getSnapshot().status === 'idle') return - void controller.load() -} diff --git a/packages/client/ui-settings-models/tests/apply.client.spec.ts b/packages/client/ui-settings-models/tests/apply.client.spec.ts index 39ba3e4b65..5267f72974 100644 --- a/packages/client/ui-settings-models/tests/apply.client.spec.ts +++ b/packages/client/ui-settings-models/tests/apply.client.spec.ts @@ -5,7 +5,11 @@ import { resolveSlotLabel } from '@deepseek-ai/dsh-client-ui-slots' import { SlotRegistry } from '@deepseek-ai/dsh-client-runtime/client' import { LocaleRuntime } from '@deepseek-ai/dsh-client-locale/client' import { TestRemote, usePinnedBrowserLanguages } from '@deepseek-ai/dsh-client-test-runtime' +import { apply as settingsApply, inject as settingsInject } from '@deepseek-ai/dsh-client-ui-settings/client' import { apply, inject, refreshIfLoaded } from '@deepseek-ai/dsh-client-ui-settings-models/client' +import { + WELCOME_NOTICE_ACK_FIELD, WELCOME_NOTICE_SETTINGS_NAMESPACE, WELCOME_NOTICE_VERSION, +} from '../src/onboarding-copy.ts' import { ModelsSection } from '../src/client/ModelsSection.tsx' import { DeepSeekOnboardingDialog } from '../src/client/DeepSeekOnboardingDialog.tsx' import { WelcomeNotice } from '../src/client/WelcomeNotice.tsx' @@ -14,7 +18,7 @@ import { WelcomeNotice } from '../src/client/WelcomeNotice.tsx' // the shipped Chinese copy, so they state the browser they assume. usePinnedBrowserLanguages('zh-CN') -async function bench(isLoopback = true) { +async function bench(isLoopback = true, settings?: object) { const ctx = new Context() await ctx.plugin(SlotRegistry).await() const locale = new LocaleRuntime(ctx) @@ -22,9 +26,10 @@ async function bench(isLoopback = true) { // The plugins inject `remote`; forwarded events reach them through the // same `$dispatch` handoff the connection sink makes. new TestRemote(ctx) - // The apply path only captures the wire face; no call leaves this fake - // until a section actually loads. - ctx.provide('connection', { api: {}, isLoopback } as never) + // Without a settings face the mirror's reads fail and stay contained; the + // Models join itself never fetches until a section actually loads. + ctx.provide('connection', { api: settings === undefined ? {} : { settings }, isLoopback } as never) + await ctx.plugin({ inject: [...settingsInject], apply: settingsApply }).await() return { ctx, slots: ctx.get('slots') as SlotRegistry, locale } } @@ -43,7 +48,7 @@ function declare(slots: SlotRegistry): () => void { describe('ui-settings-models apply', () => { it('declares the services it uses', () => { - expect(inject).toEqual(['slots', 'locale', 'connection', 'remote']) + expect(inject).toEqual(['slots', 'locale', 'connection', 'remote', 'settingsScope']) }) it('registers the models nav entry for declarations before or after apply', async () => { @@ -204,8 +209,31 @@ describe('pushed invalidations', () => { expect(load).toHaveBeenCalledTimes(1) }) - it('routes only the onboarding namespace invalidation into welcome state', async () => { - const b = await bench() + it('welcome state follows the shared mirror across document commits', async () => { + // The welcome notice derives from its settings scope: a document commit + // reaches it through the mirror's one refresh, with no routing here. + const acknowledgement = { current: undefined as string | undefined } + const settings = { + describe: vi.fn(() => Promise.resolve({ + rpcId: 'apply-welcome' as never, + result: { + ok: true as const, + value: { + writable: true, + hasDocument: false, + namespaces: [{ + ns: WELCOME_NOTICE_SETTINGS_NAMESPACE, + schema: {}, + value: acknowledgement.current === undefined ? {} : { [WELCOME_NOTICE_ACK_FIELD]: acknowledgement.current }, + applies: 'live' as const, + secrets: [], + revision: 0, + }], + }, + }, + })), + } + const b = await bench(true, settings) declare(b.slots) await b.ctx.plugin({ inject: [...inject], apply }).await() const entry = b.slots.entries('settings.onboarding') @@ -214,14 +242,14 @@ describe('pushed invalidations', () => { entry.inject as unknown as () => import('../src/client/WelcomeNotice.tsx').WelcomeNoticeInjected )() - injected.hooks.welcome.update((state) => { state.status = 'ready' }) - const load = vi.spyOn(injected.controller, 'load').mockResolvedValue() - - b.ctx.remote.$dispatch('settings/document-updated', ['llm-deepseek', 1]) - expect(load).not.toHaveBeenCalled() - b.ctx.remote.$dispatch('settings/document-updated', ['ui-onboarding', 2]) - expect(load).toHaveBeenCalledOnce() - b.ctx.emit('connection/reset') - expect(load).toHaveBeenCalledTimes(2) + await injected.controller.load() + await vi.waitFor(() => { + expect(injected.hooks.welcome.getSnapshot()).toMatchObject({ status: 'ready', acknowledged: false }) + }) + acknowledgement.current = WELCOME_NOTICE_VERSION + b.ctx.remote.$dispatch('settings/document-updated', ['ui-onboarding', 1]) + await vi.waitFor(() => { + expect(injected.hooks.welcome.getSnapshot()).toMatchObject({ status: 'ready', acknowledged: true }) + }) }) }) diff --git a/packages/client/ui-settings-models/tests/welcome-notice.client.spec.tsx b/packages/client/ui-settings-models/tests/welcome-notice.client.spec.tsx index 8b8858c64a..7554e1b77f 100644 --- a/packages/client/ui-settings-models/tests/welcome-notice.client.spec.tsx +++ b/packages/client/ui-settings-models/tests/welcome-notice.client.spec.tsx @@ -2,9 +2,13 @@ import { act, cleanup, fireEvent, render, screen } from '@testing-library/react' import { afterEach, describe, expect, it, vi } from 'vitest' import { bindSnapshotSelector } from '@deepseek-ai/dsh-client-web-react' +import { + SettingsDescribeMirror, SettingsScopeController, +} from '@deepseek-ai/dsh-client-ui-settings/client' import { WelcomeNotice } from '../src/client/WelcomeNotice.tsx' import type { WelcomeNoticeProps } from '../src/client/WelcomeNotice.tsx' -import { WelcomeNoticeStore } from '../src/client/welcome-store.ts' +import { decodeWelcomeSection, WelcomeNoticeStore } from '../src/client/welcome-store.ts' +import type { WelcomeSection } from '../src/client/welcome-store.ts' import { en, zh } from '../src/client/locales.ts' import { WELCOME_NOTICE_ACK_FIELD, WELCOME_NOTICE_COPY, WELCOME_NOTICE_SETTINGS_NAMESPACE, @@ -20,7 +24,24 @@ function response(value: T) { return { rpcId: 'welcome-rpc' as never, result: { ok: true as const, value } } } -function mount(version?: string, mutateImpl: () => Promise = () => Promise.resolve(response({}))) { +function welcomeView(value: unknown, revision = 0) { + return { + ns: WELCOME_NOTICE_SETTINGS_NAMESPACE, + schema: {}, + value, + base: {}, + user: {}, + applies: 'live' as const, + secrets: [], + revision, + } +} + +function mount( + version?: string, + mutateImpl: () => Promise = () => + Promise.resolve(response(welcomeView({ [WELCOME_NOTICE_ACK_FIELD]: WELCOME_NOTICE_VERSION }, 1))), +) { const appRoot = document.createElement('div') appRoot.id = 'root' document.body.append(appRoot) @@ -30,21 +51,19 @@ function mount(version?: string, mutateImpl: () => Promise = () => Prom describe: () => Promise.resolve(response({ writable: true, hasDocument: false, - namespaces: [{ - ns: WELCOME_NOTICE_SETTINGS_NAMESPACE, - schema: {}, - value: version === undefined ? {} : { [WELCOME_NOTICE_ACK_FIELD]: version }, - base: {}, - user: {}, - applies: 'live' as const, - secrets: [], - revision: 0, - }], + namespaces: [welcomeView(version === undefined ? {} : { [WELCOME_NOTICE_ACK_FIELD]: version })], })), mutate, }, } - const controller = new WelcomeNoticeStore(api as never) + const mirror = new SettingsDescribeMirror(api as never) + const scope = new SettingsScopeController( + api as never, + { namespace: WELCOME_NOTICE_SETTINGS_NAMESPACE, decode: decodeWelcomeSection }, + mirror, + ) + const controller = new WelcomeNoticeStore(scope) + void mirror.load() const complete = vi.fn() const unusedHook = (() => { throw new Error('unused standard hook') }) as never const props: WelcomeNoticeProps = { @@ -57,7 +76,7 @@ function mount(version?: string, mutateImpl: () => Promise = () => Prom useWelcome: bindSnapshotSelector(controller.store), t: key => zh[key], } - return { ...render(), complete, controller, mutate, appRoot } + return { ...render(), complete, controller, mirror, mutate, appRoot } } describe('WelcomeNotice', () => { @@ -100,7 +119,10 @@ describe('WelcomeNotice', () => { it('skips itself when this exact version was already acknowledged', async () => { const h = mount(WELCOME_NOTICE_VERSION) - await act(async () => { await h.controller.load() }) + await act(async () => { + await h.mirror.load() + await h.controller.load() + }) expect(screen.queryByRole('dialog')).toBeNull() expect(h.complete).toHaveBeenCalledOnce() }) diff --git a/packages/client/ui-settings-models/tests/welcome-store.client.spec.ts b/packages/client/ui-settings-models/tests/welcome-store.client.spec.ts index e1fa7572c3..7eee8ae300 100644 --- a/packages/client/ui-settings-models/tests/welcome-store.client.spec.ts +++ b/packages/client/ui-settings-models/tests/welcome-store.client.spec.ts @@ -1,6 +1,9 @@ import { describe, expect, it, vi } from 'vitest' import type { RpcResponse } from '@deepseek-ai/dsh-api-remotes/client' -import { refreshWelcomeIfLoaded, WelcomeNoticeStore } from '../src/client/welcome-store.ts' +import { + SettingsDescribeMirror, SettingsScopeController, +} from '@deepseek-ai/dsh-client-ui-settings/client' +import { decodeWelcomeSection, WelcomeNoticeStore } from '../src/client/welcome-store.ts' import { WELCOME_NOTICE_ACK_FIELD, WELCOME_NOTICE_SETTINGS_NAMESPACE, WELCOME_NOTICE_VERSION, } from '../src/onboarding-copy.ts' @@ -10,31 +13,42 @@ function ok(value: T): RpcResponse { return { rpcId: `welcome-${rpc++}` as never, result: { ok: true, value } } } -function namespace(version?: string) { +function namespace(value: unknown = {}, revision = 0) { return { ns: WELCOME_NOTICE_SETTINGS_NAMESPACE, schema: {}, - value: version === undefined ? {} : { [WELCOME_NOTICE_ACK_FIELD]: version }, - base: {}, - user: {}, + value, applies: 'live' as const, secrets: [], - revision: 0, + revision, } } -function deferred() { - let resolve!: (value: T) => void - let reject!: (reason: unknown) => void - const promise = new Promise((res, rej) => { resolve = res; reject = rej }) - return { promise, resolve, reject } +function acknowledgedNamespace(version: string, revision = 1) { + return namespace({ [WELCOME_NOTICE_ACK_FIELD]: version }, revision) +} + +/** The welcome store over a real mirror-derived scope and a fake wire. */ +function buildWelcome( + api: { describe?: ReturnType; mutate?: ReturnType }, + persistence: 'host' | 'memory' = 'host', +) { + const wire = { settings: api } as never + const mirror = new SettingsDescribeMirror(wire, persistence) + const scope = new SettingsScopeController( + wire, + { namespace: WELCOME_NOTICE_SETTINGS_NAMESPACE, decode: decodeWelcomeSection }, + mirror, + persistence, + ) + return { mirror, controller: new WelcomeNoticeStore(scope) } } describe('WelcomeNoticeStore', () => { it('acknowledges in memory without calling loopback-only settings APIs', async () => { - const describe = vi.fn() + const describeCall = vi.fn() const mutate = vi.fn() - const controller = new WelcomeNoticeStore({ settings: { describe, mutate } } as never, 'memory') + const { controller } = buildWelcome({ describe: describeCall, mutate }, 'memory') await controller.load() expect(controller.store.getSnapshot()).toEqual({ status: 'ready', acknowledged: false, error: null }) @@ -42,7 +56,7 @@ describe('WelcomeNoticeStore', () => { expect(controller.store.getSnapshot()).toEqual({ status: 'ready', acknowledged: true, error: null }) await controller.load() expect(controller.store.getSnapshot()).toEqual({ status: 'ready', acknowledged: true, error: null }) - expect(describe).not.toHaveBeenCalled() + expect(describeCall).not.toHaveBeenCalled() expect(mutate).not.toHaveBeenCalled() }) @@ -52,148 +66,101 @@ describe('WelcomeNoticeStore', () => { ['older-copy', false], [WELCOME_NOTICE_VERSION, true], ] as const) { - const api = { - settings: { - describe: vi.fn(() => Promise.resolve(ok({ - writable: true, hasDocument: false, namespaces: [namespace(version)], - }))), - }, - } - const controller = new WelcomeNoticeStore(api as never) + const describeCall = vi.fn(() => Promise.resolve(ok({ + writable: true, + hasDocument: false, + namespaces: [version === undefined ? namespace() : acknowledgedNamespace(version)], + }))) + const { mirror, controller } = buildWelcome({ describe: describeCall }) + await mirror.load() await controller.load() expect(controller.store.getSnapshot()).toMatchObject({ status: 'ready', acknowledged }) } }) - it('persists the owner version through one idempotent path mutation', async () => { - const mutate = vi.fn(() => Promise.resolve(ok(namespace(WELCOME_NOTICE_VERSION)))) - const controller = new WelcomeNoticeStore({ settings: { mutate } } as never) + it('persists the owner version through one revision-fenced mutation', async () => { + const describeCall = vi.fn(() => Promise.resolve(ok({ + writable: true, hasDocument: false, namespaces: [namespace({}, 3)], + }))) + const mutate = vi.fn(() => Promise.resolve(ok(acknowledgedNamespace(WELCOME_NOTICE_VERSION, 4)))) + const { mirror, controller } = buildWelcome({ describe: describeCall, mutate }) + await mirror.load() + await controller.load() await expect(controller.acknowledge()).resolves.toBe(true) expect(mutate).toHaveBeenCalledWith({ ns: WELCOME_NOTICE_SETTINGS_NAMESPACE, ops: [{ op: 'set', path: [WELCOME_NOTICE_ACK_FIELD], value: WELCOME_NOTICE_VERSION }], + expectedRevision: 3, }) expect(controller.store.getSnapshot()).toMatchObject({ status: 'ready', acknowledged: true }) + // The write answer folded into the mirror; no re-read followed. + expect(describeCall).toHaveBeenCalledTimes(1) }) - it('keeps the notice pending when loading or persistence fails', async () => { - const load = new WelcomeNoticeStore({ - settings: { describe: () => Promise.reject(new Error('offline')) }, - } as never) - await load.load() - expect(load.store.getSnapshot()).toEqual({ status: 'error', acknowledged: false, error: 'offline' }) - - const save = new WelcomeNoticeStore({ - settings: { mutate: () => Promise.reject(new Error('disk full')) }, - } as never) - await expect(save.acknowledge()).resolves.toBe(false) - expect(save.store.getSnapshot()).toEqual({ status: 'error', acknowledged: false, error: 'disk full' }) - - const nonError = new WelcomeNoticeStore({ - // Durable/wire failures are unknown; exercise containment of a non-Error rejection. - settings: { describe: () => Promise.reject(new Error('offline string')) }, - } as never) - await nonError.load() - expect(nonError.store.getSnapshot().error).toBe('offline string') + it('keeps the notice pending while the settings read has not answered', async () => { + const describeCall = vi.fn(() => Promise.reject(new Error('offline'))) + const { mirror, controller } = buildWelcome({ describe: describeCall }) + await mirror.load() + await controller.load() + // No answer stands, so the step renders nothing and never acknowledges. + expect(controller.store.getSnapshot()).toEqual({ status: 'loading', acknowledged: false, error: null }) }) - it('reports business failures, missing namespaces, and malformed durable values', async () => { - for (const describe of [ - () => Promise.resolve({ - rpcId: 'failed' as never, - result: { ok: false as const, error: { code: 'internal' as const, message: 'denied', details: {} } }, - }), - () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [] })), - ]) { - const controller = new WelcomeNoticeStore({ settings: { describe } } as never) - await controller.load() - expect(controller.store.getSnapshot().status).toBe('error') - } + it('reports a failed or refused persistence attempt after its recovery read', async () => { + const describeCall = vi.fn(() => Promise.resolve(ok({ + writable: true, hasDocument: false, namespaces: [namespace()], + }))) + const mutate = vi.fn(() => Promise.reject(new Error('disk full'))) + const { mirror, controller } = buildWelcome({ describe: describeCall, mutate }) + await mirror.load() + await controller.load() + await expect(controller.acknowledge()).resolves.toBe(false) + expect(controller.store.getSnapshot()).toMatchObject({ + status: 'error', + acknowledged: false, + error: 'the acknowledgement did not persist', + }) + // The failed latest write triggered one mirror recovery read. + expect(describeCall).toHaveBeenCalledTimes(2) + }) + it('reports a missing namespace as an error instead of a silent skip', async () => { + const describeCall = vi.fn(() => Promise.resolve(ok({ + writable: true, hasDocument: false, namespaces: [], + }))) + const { mirror, controller } = buildWelcome({ describe: describeCall }) + await mirror.load() + await controller.load() + expect(controller.store.getSnapshot()).toMatchObject({ + status: 'error', + error: 'welcome acknowledgement settings are unavailable', + }) + }) + + it('reads malformed durable values as unacknowledged', async () => { for (const value of [null, 42, { [WELCOME_NOTICE_ACK_FIELD]: 42 }]) { - const controller = new WelcomeNoticeStore({ - settings: { describe: () => Promise.resolve(ok({ - writable: true, - hasDocument: false, - namespaces: [{ ...namespace(), value }], - })) }, - } as never) + const describeCall = vi.fn(() => Promise.resolve(ok({ + writable: true, hasDocument: false, namespaces: [namespace(value)], + }))) + const { mirror, controller } = buildWelcome({ describe: describeCall }) + await mirror.load() await controller.load() expect(controller.store.getSnapshot()).toMatchObject({ status: 'ready', acknowledged: false }) } - - const save = new WelcomeNoticeStore({ - settings: { mutate: () => Promise.resolve({ - rpcId: 'failed-save' as never, - result: { - ok: false, - error: { - code: 'settings-rejected', - message: 'denied', - details: { ns: WELCOME_NOTICE_SETTINGS_NAMESPACE }, - }, - }, - }) }, - } as never) - await expect(save.acknowledge()).resolves.toBe(false) - expect(save.store.getSnapshot().error).toBe('denied') }) - it('lets the latest load win over stale success and failure', async () => { - const first = deferred>() - const describe = vi.fn() - .mockImplementationOnce(() => first.promise) - .mockImplementationOnce(() => Promise.resolve(ok({ - writable: true, hasDocument: false, namespaces: [namespace()], - }))) - const controller = new WelcomeNoticeStore({ settings: { describe } } as never) - const stale = controller.load() + it('follows a later document change without an own read', async () => { + const describeCall = vi.fn() + .mockResolvedValueOnce(ok({ writable: true, hasDocument: false, namespaces: [namespace()] })) + .mockResolvedValueOnce(ok({ + writable: true, hasDocument: false, + namespaces: [acknowledgedNamespace(WELCOME_NOTICE_VERSION)], + })) + const { mirror, controller } = buildWelcome({ describe: describeCall }) + await mirror.load() await controller.load() - first.resolve(ok({ - writable: true, hasDocument: false, namespaces: [namespace(WELCOME_NOTICE_VERSION)], - })) - await stale - expect(controller.store.getSnapshot().acknowledged).toBe(false) - - const failed = deferred>() - describe - .mockImplementationOnce(() => failed.promise) - .mockImplementationOnce(() => Promise.resolve(ok({ - writable: true, hasDocument: false, namespaces: [namespace(WELCOME_NOTICE_VERSION)], - }))) - const staleFailure = controller.load() - await controller.load() - failed.reject('stale failure') - await staleFailure - expect(controller.store.getSnapshot()).toMatchObject({ status: 'ready', acknowledged: true, error: null }) - }) - - it('contains stale acknowledgement settlements and refreshes only a loaded store', async () => { - const write = deferred>() - const describe = vi.fn(() => Promise.resolve(ok({ - writable: true, hasDocument: false, namespaces: [namespace()], - }))) - const controller = new WelcomeNoticeStore({ - settings: { mutate: () => write.promise, describe }, - } as never) - refreshWelcomeIfLoaded(controller) - expect(describe).not.toHaveBeenCalled() - const staleWrite = controller.acknowledge() - await controller.load() - write.resolve(ok(namespace(WELCOME_NOTICE_VERSION))) - await expect(staleWrite).resolves.toBe(true) - expect(controller.store.getSnapshot().acknowledged).toBe(false) - refreshWelcomeIfLoaded(controller) - await vi.waitFor(() => { expect(describe).toHaveBeenCalledTimes(2) }) - - const failedWrite = deferred>() - const staleFailure = new WelcomeNoticeStore({ - settings: { mutate: () => failedWrite.promise, describe }, - } as never) - const pending = staleFailure.acknowledge() - await staleFailure.load() - failedWrite.reject('late failure') - await expect(pending).resolves.toBe(false) - expect(staleFailure.store.getSnapshot().status).toBe('ready') + expect(controller.store.getSnapshot()).toMatchObject({ acknowledged: false }) + await mirror.load() + expect(controller.store.getSnapshot()).toMatchObject({ status: 'ready', acknowledged: true }) }) }) From 232e4beeaeef9ae4d4514f596e721ba358a8052e Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 17:15:10 +0800 Subject: [PATCH 08/28] refactor(ui-settings-plugins): plugin tab derives served namespaces from the mirror --- apps/web/tests/startup-rpc-budget.e2e.ts | 8 +-- .../ui-settings-plugins/src/client/index.ts | 17 ++--- .../src/client/tab-store.ts | 63 +++++++------------ .../tests/stores.client.spec.ts | 60 ++++++------------ .../client/ui-settings/src/client/index.ts | 2 +- .../ui-settings/src/client/settings-mirror.ts | 30 ++++++++- .../ui-settings/src/client/settings-scope.ts | 13 +++- 7 files changed, 89 insertions(+), 104 deletions(-) diff --git a/apps/web/tests/startup-rpc-budget.e2e.ts b/apps/web/tests/startup-rpc-budget.e2e.ts index 5f8d7dd0e1..33938d6088 100644 --- a/apps/web/tests/startup-rpc-budget.e2e.ts +++ b/apps/web/tests/startup-rpc-budget.e2e.ts @@ -16,12 +16,10 @@ import { newEnglishPage } from './support.ts' * eagerly at bind time over HTTP, and once on the first-connection reset — * that second read closes the window where a document commit lands between * the eager read and the SSE subscription and its invalidation is lost. - * Beside it, the direct callers not yet migrated: welcome notice (1) + models - * onboarding (1) + plugin-directory tab at bind and at reset (2) + - * agent-preset settings row on reset (1). Batch 2 migrates those onto the - * mirror and tightens this to 2. + * Beside it, the direct callers not yet migrated: models onboarding (1) + + * agent-preset settings row on reset (1). Their migration tightens this to 2. */ -const DESCRIBE_BUDGET = 7 +const DESCRIBE_BUDGET = 4 let scaffold: WebScaffold let browser: Browser diff --git a/packages/client/ui-settings-plugins/src/client/index.ts b/packages/client/ui-settings-plugins/src/client/index.ts index 82dea6d796..184511ead1 100644 --- a/packages/client/ui-settings-plugins/src/client/index.ts +++ b/packages/client/ui-settings-plugins/src/client/index.ts @@ -72,26 +72,17 @@ export function apply(ctx: ClientContext): void { 'ui-settings-plugins: credential invalidations', ) - // Which namespaces the Host serves is a registration fact the wire does not - // announce, so the directory re-reads on the two signals that can carry a - // changed composition: a settings document commit and a reconnect. + // Which namespaces the Host serves comes from the shared describe mirror, + // whose owning plugin already refreshes it on document commits and + // reconnects — the tab only derives. const configurable = new ConfigurablePluginsTabController( - api, () => ctx.slots.entries('settings.plugin.item')) + ctx.settingsScope.describe(), () => ctx.slots.entries('settings.plugin.item')) ctx.effect(() => () => { configurable.dispose() }, 'ui-settings-plugins: tab directory') - ctx.effect( - () => ctx.remote.$on('settings/document-updated', () => { void configurable.load() }), - 'ui-settings-plugins: served-namespace invalidations', - ) - ctx.effect( - () => ctx.on('connection/reset', () => { void configurable.load() }), - 'ui-settings-plugins: served-namespace reconnect', - ) // A card registered after the first read joins the list without a wire call. ctx.effect( () => ctx.slots.subscribe('settings.plugin.item', () => { configurable.refresh() }), 'ui-settings-plugins: card ledger', ) - void configurable.load() let tabsVersion = -1 let tabsRevision = -1 diff --git a/packages/client/ui-settings-plugins/src/client/tab-store.ts b/packages/client/ui-settings-plugins/src/client/tab-store.ts index a4ed4439f2..ff9d7b4b74 100644 --- a/packages/client/ui-settings-plugins/src/client/tab-store.ts +++ b/packages/client/ui-settings-plugins/src/client/tab-store.ts @@ -10,7 +10,7 @@ * trace and does not count toward the empty line. */ -import type { IApiClient } from '@deepseek-ai/dsh-client-connection/client' +import type { SettingsDescribeFace } from '@deepseek-ai/dsh-client-ui-settings/client' import type { StoredEntry } from '@deepseek-ai/dsh-client-ui-slots' import { createSnapshotStore, type SnapshotStore } from '@deepseek-ai/dsh-client-runtime/client' @@ -42,47 +42,23 @@ export interface ConfigurablePluginsTabFace { } } -/** Reads the served namespaces and pairs them with the cards that claim them. */ +/** Derives the served namespaces from the shared describe mirror and pairs them with the cards that claim them. */ export class ConfigurablePluginsTabController { private readonly store = createSnapshotStore({ loaded: false, namespaces: [] }) - /** Last Host answer; kept so a slot mutation republishes without a wire read. */ - private served: readonly string[] = [] - private loaded = false - private generation = 0 private disposed = false + private readonly unsubscribe: () => void /** - * @param api - settings wire face. + * @param describeFace - the shared mirror's read-only face; its refreshes + * (document commits, reconnects) are what keep the served set current. * @param entries - reads the cards currently registered into the section's slot. */ constructor( - private readonly api: Pick, + private readonly describeFace: SettingsDescribeFace, private readonly entries: () => readonly StoredEntry[], - ) {} - - /** Opaque read of {@link disposed}: control flow cannot narrow it across awaits. */ - private isDisposed(): boolean { - return this.disposed - } - - /** - * Re-read the served namespaces from the Host and republish. - * @returns settlement after the read, or immediately once disposed. - */ - async load(): Promise { - if (this.isDisposed()) return - const generation = ++this.generation - let response: Awaited> - try { - response = await this.api.settings.describe({}) - } catch (_settingsReadFailure) { - // The tab keeps the namespaces it last knew; the next invalidation - // or reconnect reads again. - return - } - if (this.isDisposed() || generation !== this.generation || !response.result.ok) return - this.served = response.result.value.namespaces.map(view => view.ns) - this.loaded = true + ) { + this.unsubscribe = describeFace.subscribe(() => { this.publish() }) + void describeFace.ensure() this.publish() } @@ -92,10 +68,10 @@ export class ConfigurablePluginsTabController { this.publish() } - /** Stop publishing; an in-flight read settles without touching the store. */ + /** Stop publishing and stop following the mirror. */ dispose(): void { this.disposed = true - this.generation += 1 + this.unsubscribe() } /** @@ -107,17 +83,20 @@ export class ConfigurablePluginsTabController { } private publish(): void { - const served = new Set(this.served) + if (this.disposed) return + const mirrored = this.describeFace.getSnapshot() + const loaded = mirrored.view !== undefined + const served = new Set(mirrored.view?.namespaces.map(view => view.ns) ?? []) const namespaces = this.entries().flatMap(entry => entry.options.key !== undefined && served.has(entry.options.key) ? [entry.options.key] : []) const previous = this.store.getSnapshot() - // Every settings-document commit re-reads, and most of them change nothing - // this section shows. An observable source must keep its snapshot - // reference until the fact moves, or each unrelated save re-renders the - // whole card list (packages/client/AGENTS.md reactive rule 5). - if (previous.loaded === this.loaded + // Every settings-document commit refreshes the mirror, and most commits + // change nothing this section shows. An observable source must keep its + // snapshot reference until the fact moves, or each unrelated save + // re-renders the whole card list (packages/client/AGENTS.md reactive rule 5). + if (previous.loaded === loaded && previous.namespaces.length === namespaces.length && previous.namespaces.every((ns, index) => ns === namespaces[index])) return - this.store.set({ loaded: this.loaded, namespaces }) + this.store.set({ loaded, namespaces }) } } diff --git a/packages/client/ui-settings-plugins/tests/stores.client.spec.ts b/packages/client/ui-settings-plugins/tests/stores.client.spec.ts index 9901bc5eb1..481c6e3679 100644 --- a/packages/client/ui-settings-plugins/tests/stores.client.spec.ts +++ b/packages/client/ui-settings-plugins/tests/stores.client.spec.ts @@ -8,6 +8,7 @@ import { stubSettingsScope, type StubSettingsScope } from '@deepseek-ai/dsh-clie import { CardForm, numberField, textField } from '../src/client/card-form.ts' import { AgentLoopCardController, type AgentLoopSettings } from '../src/client/agent-loop-card-controller.ts' import { BashCardController, type BashSettings } from '../src/client/bash-card-controller.ts' +import { SettingsDescribeMirror } from '@deepseek-ai/dsh-client-ui-settings/client' import { ConfigurablePluginsTabController } from '../src/client/tab-store.ts' import { WebSearchCardController, type WebSearchSettings } from '../src/client/web-search-card-controller.ts' @@ -555,7 +556,7 @@ describe('ConfigurablePluginsTabController', () => { }, }, })) - return { api: { settings: { describe } } as never, describe } + return { mirror: new SettingsDescribeMirror({ settings: { describe } } as never), describe } } /** Slot ledger stand-in: one stored entry per registered card key. */ @@ -565,9 +566,9 @@ describe('ConfigurablePluginsTabController', () => { it('dispatches the served namespaces a card claims, in card registration order', async () => { const settings = settingsApi(['bash', 'ui-theme', 'agent-loop']) - const controller = new ConfigurablePluginsTabController(settings.api, () => ledger('agent-loop', 'bash')) + const controller = new ConfigurablePluginsTabController(settings.mirror, () => ledger('agent-loop', 'bash')) - await controller.load() + await settings.mirror.ensure() // ui-theme is served but claimed by no card here — another surface owns // it. The order is the cards', not the Host's: plugin activation can @@ -578,9 +579,9 @@ describe('ConfigurablePluginsTabController', () => { it('never dispatches a card whose namespace this deployment does not serve', async () => { const settings = settingsApi(['bash']) - const controller = new ConfigurablePluginsTabController(settings.api, () => ledger('bash', 'web-search-deepseek')) + const controller = new ConfigurablePluginsTabController(settings.mirror, () => ledger('bash', 'web-search-deepseek')) - await controller.load() + await settings.mirror.ensure() expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual(['bash']) }) @@ -588,8 +589,8 @@ describe('ConfigurablePluginsTabController', () => { it('takes a card registered after the read without asking the Host again', async () => { const settings = settingsApi(['bash']) let entries = ledger() - const controller = new ConfigurablePluginsTabController(settings.api, () => entries) - await controller.load() + const controller = new ConfigurablePluginsTabController(settings.mirror, () => entries) + await settings.mirror.ensure() expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual([]) entries = ledger('bash') @@ -599,34 +600,33 @@ describe('ConfigurablePluginsTabController', () => { expect(settings.describe).toHaveBeenCalledOnce() }) - it('keeps the namespaces it knew when a read fails', async () => { + it('keeps the namespaces it knew when a refresh fails', async () => { const settings = settingsApi(['bash']) - const controller = new ConfigurablePluginsTabController(settings.api, () => ledger('bash')) - await controller.load() + const controller = new ConfigurablePluginsTabController(settings.mirror, () => ledger('bash')) + await settings.mirror.ensure() settings.describe.mockRejectedValueOnce(new Error('offline')) - await controller.load() + await settings.mirror.load() expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual(['bash']) }) - it('publishes nothing once disposed, and never claims it was answered', async () => { + it('stops following the mirror once disposed, and never claims it was answered', async () => { const settings = settingsApi(['bash']) - const controller = new ConfigurablePluginsTabController(settings.api, () => ledger('bash')) + const controller = new ConfigurablePluginsTabController(settings.mirror, () => ledger('bash')) controller.dispose() - await controller.load() + await settings.mirror.load() expect(controller.inject().hooks.configurablePlugins.getSnapshot()) .toEqual({ loaded: false, namespaces: [] }) - expect(settings.describe).not.toHaveBeenCalled() }) it('ignores a slot-ledger change that arrives after disposal', async () => { const settings = settingsApi(['bash']) let entries = ledger() - const controller = new ConfigurablePluginsTabController(settings.api, () => entries) - await controller.load() + const controller = new ConfigurablePluginsTabController(settings.mirror, () => entries) + await settings.mirror.ensure() controller.dispose() entries = ledger('bash') @@ -635,33 +635,11 @@ describe('ConfigurablePluginsTabController', () => { expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual([]) }) - it('drops a read a newer one superseded', async () => { - // The section re-reads on every settings-document invalidation, so a slow - // first answer must not overwrite the newer one that already landed. - const settings = settingsApi(['bash']) - const controller = new ConfigurablePluginsTabController(settings.api, () => ledger('bash', 'agent-loop')) - const slow = Promise.withResolvers() - settings.describe.mockReturnValueOnce(slow.promise as never) - const stale = controller.load() - - await controller.load() - expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual(['bash']) - slow.resolve({ - rpcId: 's-0', - result: { ok: true, value: { writable: true, hasDocument: true, namespaces: [ - { ns: 'agent-loop', schema: {}, value: {}, applies: 'live', secrets: [], revision: 0 }, - ] } }, - }) - await stale - - expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual(['bash']) - }) - it('reports the Host answered even when it serves nothing this tab shows', async () => { const settings = settingsApi(['ui-theme']) - const controller = new ConfigurablePluginsTabController(settings.api, () => ledger('bash')) + const controller = new ConfigurablePluginsTabController(settings.mirror, () => ledger('bash')) - await controller.load() + await settings.mirror.ensure() expect(controller.inject().hooks.configurablePlugins.getSnapshot()) .toEqual({ loaded: true, namespaces: [] }) diff --git a/packages/client/ui-settings/src/client/index.ts b/packages/client/ui-settings/src/client/index.ts index b3c149c938..f1e968174f 100644 --- a/packages/client/ui-settings/src/client/index.ts +++ b/packages/client/ui-settings/src/client/index.ts @@ -27,7 +27,7 @@ export type { } from './contract/slots.ts' export { SettingsScopeController, SettingsScopeBinder } from './settings-scope.ts' export { SettingsDescribeMirror } from './settings-mirror.ts' -export type { SettingsDescribeView, SettingsMirrorSnapshot } from './settings-mirror.ts' +export type { SettingsDescribeFace, SettingsDescribeView, SettingsMirrorSnapshot } from './settings-mirror.ts' /** * Required services: the wire handle for the mirror's reads and the forwarded diff --git a/packages/client/ui-settings/src/client/settings-mirror.ts b/packages/client/ui-settings/src/client/settings-mirror.ts index 398895c9cb..61dc21e287 100644 --- a/packages/client/ui-settings/src/client/settings-mirror.ts +++ b/packages/client/ui-settings/src/client/settings-mirror.ts @@ -38,12 +38,40 @@ export interface SettingsMirrorSnapshot { error: string | null } +/** + * The mirror as cross-namespace surfaces consume it: current answer, + * subscription, first-use read, and the write-answer fold. `load` stays off + * this face — invalidation refreshes belong to the mirror's owning plugin. + */ +export interface SettingsDescribeFace { + /** @returns the current sync snapshot (stable reference until the next change). */ + getSnapshot(): SettingsMirrorSnapshot + /** + * Observe snapshot replacements. + * @param listener - invoked after each snapshot change. + * @returns the disposer removing this listener. + */ + subscribe(listener: () => void): () => void + /** + * Resolve once an answer is held (or the mirror is terminally unavailable), + * reading only from `idle`. + * @returns settlement of the current or newly started read, if any. + */ + ensure(): Promise + /** + * Fold one write answer's namespace view into the held view without a wire + * read. + * @param view - the namespace view a settings write answered with. + */ + acceptView(view: SettingsNamespaceView): void +} + /** * Serializes every Host `settings.describe` read behind one snapshot store. * Concurrent {@link load} calls fold into the in-flight read plus one rerun, * so an invalidation arriving mid-read is never lost and never duplicated. */ -export class SettingsDescribeMirror { +export class SettingsDescribeMirror implements SettingsDescribeFace { private readonly store: SnapshotStore private inFlight: Promise | undefined private rerun = false diff --git a/packages/client/ui-settings/src/client/settings-scope.ts b/packages/client/ui-settings/src/client/settings-scope.ts index 47ebb3daa3..72a472a162 100644 --- a/packages/client/ui-settings/src/client/settings-scope.ts +++ b/packages/client/ui-settings/src/client/settings-scope.ts @@ -34,7 +34,7 @@ import type {} from '@deepseek-ai/dsh-api-remotes/types' // never — the owning package's client-safe, type-only subpath supplies the // cordis `Events` entry (and with it the branded `SettingsNamespace`). import type {} from '@deepseek-ai/dsh-settings/types' -import { SettingsDescribeMirror } from './settings-mirror.ts' +import { SettingsDescribeMirror, type SettingsDescribeFace } from './settings-mirror.ts' type SettingsFace = Pick @@ -256,6 +256,17 @@ export class SettingsScopeBinder extends Service { * @param spec - domain-owned namespace contract. * @returns the bound scope consumed by the domain's services and rows. */ + /** + * The shared mirror's read-only face for cross-namespace surfaces (schema + * introspection, the served-namespace directory). Per-namespace consumers + * use {@link bind}; both derive from the same snapshot, so they can never + * disagree about the document. + * @returns the describe face over the shared mirror. + */ + describe(): SettingsDescribeFace { + return this.mirror + } + bind(spec: SettingsScopeSpec): SettingsScope { const ctx = this.ctx const connection = ctx.get('connection') as ConnectionHandle From e3f484f62ffa71259d7b4b368ee901b5cdb8ac9c Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Mon, 17 Aug 2026 17:19:10 +0800 Subject: [PATCH 09/28] refactor(ui-permission-presets): permission row derives from the describe mirror --- .../ui-permission-presets/src/client/index.ts | 26 +-- .../src/client/settings-store.ts | 134 +++++++------- .../tests/browser-plugin.client.spec.ts | 2 + .../permission-presets-row.client.spec.tsx | 17 +- .../tests/settings-store.client.spec.ts | 163 ++++++++---------- 5 files changed, 167 insertions(+), 175 deletions(-) diff --git a/packages/client/ui-permission-presets/src/client/index.ts b/packages/client/ui-permission-presets/src/client/index.ts index aec82bf9d9..ce6ecfc32a 100644 --- a/packages/client/ui-permission-presets/src/client/index.ts +++ b/packages/client/ui-permission-presets/src/client/index.ts @@ -33,9 +33,7 @@ import { import { displayPermissionPreset, FULL_ACCESS_PRESET, } from './presentation.ts' -import { - PERMISSION_SETTINGS_NS, PermissionPresetSettingsController, refreshPermissionIfLoaded, -} from './settings-store.ts' +import { PermissionPresetSettingsController } from './settings-store.ts' export type { PermissionRowInjected, PermissionRowProps } from './PermissionRow.tsx' export type { @@ -43,7 +41,7 @@ export type { } from './settings-store.ts' /** Required services (cordis fiber inject). */ -export const inject = ['commandUi', 'sessions', 'slots', 'locale', 'connection', 'remote'] +export const inject = ['commandUi', 'sessions', 'slots', 'locale', 'connection', 'remote', 'settingsScope'] const ACCESS_NS = 'permission.access' @@ -113,7 +111,10 @@ export function apply(ctx: ClientContext): void { ctx.effect(() => ctx.locale.register('settings.permission', { zh, en }), 'ui-permission: settings row dictionaries') const connection = ctx.get('connection') as ConnectionHandle - const controller = new PermissionPresetSettingsController(connection.api) + // The row follows the shared describe mirror, whose owning plugin already + // refreshes it on document commits and reconnects. + const controller = new PermissionPresetSettingsController( + ctx.settingsScope.describe(), connection.api) const load = (): Promise => controller.load() const select = (preset: string): Promise => controller.select(preset) const injected = (): PermissionRowInjected => ({ @@ -122,20 +123,7 @@ export function apply(ctx: ClientContext): void { select, }) - ctx.effect(() => { - const refresh = (): void => { refreshPermissionIfLoaded(controller) } - const disposers = [ - ctx.remote.$on('settings/document-updated', (ns) => { - if (ns !== PERMISSION_SETTINGS_NS) return - refresh() - }), - ctx.on('connection/reset', () => { refresh() }), - ] - return () => { - controller.dispose() - for (const dispose of disposers) dispose() - } - }, 'ui-permission: settings invalidations') + ctx.effect(() => () => { controller.dispose() }, 'ui-permission: settings row directory') ctx.slots.inject('settings.general.item', () => ctx.slots.register({ name: 'settings.general.item', diff --git a/packages/client/ui-permission-presets/src/client/settings-store.ts b/packages/client/ui-permission-presets/src/client/settings-store.ts index 6e7199f1be..5f61ed595a 100644 --- a/packages/client/ui-permission-presets/src/client/settings-store.ts +++ b/packages/client/ui-permission-presets/src/client/settings-store.ts @@ -1,7 +1,9 @@ /** - * Permission default-settings controller. The host descriptor supplies the - * current value and the dynamic preset enum; writes target only - * `defaultPreset` and carry the descriptor revision. + * Permission default-settings controller. The permission descriptor comes + * from the shared describe mirror (the dynamic preset enum lives in the + * namespace schema, which per-namespace scopes do not carry); writes target + * only `defaultPreset`, carry the descriptor revision, and fold their answer + * back into the mirror. */ import type { @@ -10,6 +12,7 @@ import type { import { createSnapshotStore, type SnapshotStore, } from '@deepseek-ai/dsh-client-runtime/client' +import type { SettingsDescribeFace } from '@deepseek-ai/dsh-client-ui-settings/client' import { nodeAtPath, rehydrateSchema, type SchemaNode, } from '@deepseek-ai/dsh-client-schema-form' @@ -75,7 +78,7 @@ export function permissionDefaultOf(view: SettingsNamespaceView): { return { currentValue: value, options } } -/** Controller joining Settings reads, writes, and pushed invalidations. */ +/** Controller deriving the row from the shared mirror and writing the default through it. */ export class PermissionPresetSettingsController { /** Row snapshot consumed through a bound selector hook. */ readonly store: SnapshotStore = createSnapshotStore({ @@ -87,42 +90,32 @@ export class PermissionPresetSettingsController { revision: 0, }) - private generation = 0 - private view: SettingsNamespaceView | undefined - - /** @param api - Settings wire face. */ - constructor(private readonly api: Pick) {} + private following: (() => void) | undefined + private saving = false + private disposed = false /** - * Refresh the permission descriptor. Latest request wins. - * @returns nothing; {@link store} carries success or failure. + * @param describeFace - the shared mirror's read-only face (descriptor and schema source). + * @param api - settings wire face for the `defaultPreset` write. + */ + constructor( + private readonly describeFace: SettingsDescribeFace, + private readonly api: Pick, + ) {} + + /** + * Begin following the mirror (idempotent) and reflect its current answer. + * @returns settlement once the snapshot reflects the mirror. */ async load(): Promise { - const generation = ++this.generation + if (this.disposed) return + this.following ??= this.describeFace.subscribe(() => { this.derive() }) this.store.update((state) => { state.status = 'loading' state.error = null }) - try { - const response = await this.api.settings.describe({}) - if (!response.result.ok) throw new Error(response.result.error.message) - if (generation !== this.generation) return - const view = response.result.value.namespaces.find(entry => entry.ns === PERMISSION_SETTINGS_NS) - if (view === undefined) { - this.view = undefined - this.store.update((state) => { - state.status = 'unavailable' - state.writable = false - state.currentValue = '' - state.options = [] - }) - return - } - this.accept(view, response.result.value.writable) - } catch (error) { - if (generation !== this.generation) return - this.fail(error) - } + await this.describeFace.ensure() + this.derive() } /** @@ -131,10 +124,11 @@ export class PermissionPresetSettingsController { * @returns nothing; {@link store} carries success or failure. */ async select(preset: string): Promise { - const view = this.view const state = this.store.getSnapshot() - if (view === undefined || !state.writable) return - const generation = ++this.generation + const view = this.describeFace.getSnapshot().view?.namespaces + .find(entry => entry.ns === PERMISSION_SETTINGS_NS) + if (view === undefined || !state.writable || this.saving) return + this.saving = true this.store.update((draft) => { draft.status = 'saving' draft.error = null @@ -145,32 +139,59 @@ export class PermissionPresetSettingsController { ops: [{ op: 'set', path: ['defaultPreset'], value: preset }], expectedRevision: view.revision, }) - if (generation !== this.generation) return if (!response.result.ok) throw new Error(response.result.error.message) - this.accept(response.result.value, true) + this.saving = false + if (this.disposed) return + // The mirror publish reaches this row's own subscription, so the fold + // is also what republishes the accepted value here. + this.describeFace.acceptView(response.result.value) } catch (error) { - if (generation !== this.generation) return + this.saving = false + if (this.disposed) return this.fail(error) } } - /** Stop in-flight responses from publishing after plugin disposal. */ + /** Stop following the mirror; later publishes leave the snapshot alone. */ dispose(): void { - this.generation += 1 - this.view = undefined + this.disposed = true + this.following?.() + this.following = undefined } - private accept(view: SettingsNamespaceView, writable: boolean): void { - const resolved = permissionDefaultOf(view) - this.view = view - this.store.update((state) => { - state.status = 'ready' - state.error = null - state.writable = writable - state.currentValue = resolved.currentValue - state.options = resolved.options - state.revision = view.revision - }) + private derive(): void { + if (this.disposed || this.saving) return + const mirrored = this.describeFace.getSnapshot() + if (mirrored.view === undefined) { + // A held failure with no answer is a failed row; without one the read + // is still in flight and the row keeps its loading state. + if (mirrored.error !== null) this.fail(new Error(mirrored.error)) + return + } + const view = mirrored.view.namespaces.find(entry => entry.ns === PERMISSION_SETTINGS_NS) + if (view === undefined) { + this.store.update((state) => { + state.status = 'unavailable' + state.writable = false + state.currentValue = '' + state.options = [] + }) + return + } + try { + const resolved = permissionDefaultOf(view) + const { writable } = mirrored.view + this.store.update((state) => { + state.status = 'ready' + state.error = null + state.writable = writable + state.currentValue = resolved.currentValue + state.options = resolved.options + state.revision = view.revision + }) + } catch (error) { + this.fail(error) + } } private fail(error: unknown): void { @@ -180,12 +201,3 @@ export class PermissionPresetSettingsController { }) } } - -/** - * Refetch only after the row has opened once. - * @param controller - permission settings controller. - */ -export function refreshPermissionIfLoaded(controller: PermissionPresetSettingsController): void { - if (controller.store.getSnapshot().status === 'idle') return - void controller.load() -} diff --git a/packages/client/ui-permission-presets/tests/browser-plugin.client.spec.ts b/packages/client/ui-permission-presets/tests/browser-plugin.client.spec.ts index e3968cf002..19ae476a37 100644 --- a/packages/client/ui-permission-presets/tests/browser-plugin.client.spec.ts +++ b/packages/client/ui-permission-presets/tests/browser-plugin.client.spec.ts @@ -13,6 +13,7 @@ import { describe, expect, it } from 'vitest' import { SlotRegistry, type SessionId } from '@deepseek-ai/dsh-client-runtime/client' import { LocaleRuntime } from '@deepseek-ai/dsh-client-locale/client' import { TestRemote } from '@deepseek-ai/dsh-client-test-runtime' +import { apply as settingsApply, inject as settingsInject } from '@deepseek-ai/dsh-client-ui-settings/client' import type { CommandDecoration } from '@deepseek-ai/dsh-client-ui-commands/client' import type { PermissionSelect } from '@deepseek-ai/dsh-permission-presets/client' import { @@ -58,6 +59,7 @@ async function bench() { }, }, } as never) + await ctx.plugin({ inject: [...settingsInject], apply: settingsApply }).await() let decoration: CommandDecoration | undefined ctx.provide('commandUi', { decorate(c: CommandDecoration) { diff --git a/packages/client/ui-permission-presets/tests/permission-presets-row.client.spec.tsx b/packages/client/ui-permission-presets/tests/permission-presets-row.client.spec.tsx index 9df3920bd5..e4cbedbf6b 100644 --- a/packages/client/ui-permission-presets/tests/permission-presets-row.client.spec.tsx +++ b/packages/client/ui-permission-presets/tests/permission-presets-row.client.spec.tsx @@ -5,8 +5,15 @@ import { bindSnapshotSelector } from '@deepseek-ai/dsh-client-web-react' import type { SettingsNamespaceView } from '@deepseek-ai/dsh-api-remotes/client' import { PermissionRow, type PermissionRowProps } from '../src/client/PermissionRow.tsx' import { en } from '../src/client/locales.ts' +import { SettingsDescribeMirror } from '@deepseek-ai/dsh-client-ui-settings/client' import { PermissionPresetSettingsController } from '../src/client/settings-store.ts' +/** Controller over a real mirror derived from the same fake wire. */ +function derivedController(api: { settings: object }) { + const wire = api as never + return new PermissionPresetSettingsController(new SettingsDescribeMirror(wire), wire) +} + afterEach(cleanup) const SCHEMA = { @@ -58,7 +65,7 @@ function mount(controller: PermissionPresetSettingsController) { describe('PermissionRow', () => { it('loads the descriptor, opens the menu, and selects a new default', async () => { const mutate = vi.fn(() => Promise.resolve(ok(view('workspace-write', 1)))) - const controller = new PermissionPresetSettingsController({ + const controller = derivedController({ settings: { describe: () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('read-only')] })), mutate, @@ -85,7 +92,7 @@ describe('PermissionRow', () => { it('requires explicit acknowledgement before saving Full access', async () => { const mutate = vi.fn(() => Promise.resolve(ok(view('danger-full-access', 1)))) - const controller = new PermissionPresetSettingsController({ + const controller = derivedController({ settings: { describe: () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('read-only')] })), mutate, @@ -109,7 +116,7 @@ describe('PermissionRow', () => { }) it('hides an unavailable namespace and disables a read-only provider', async () => { - const absent = new PermissionPresetSettingsController({ + const absent = derivedController({ settings: { describe: () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [] })), mutate: vi.fn(), @@ -119,7 +126,7 @@ describe('PermissionRow', () => { await waitFor(() => { expect(rendered.container.textContent).toBe('') }) rendered.unmount() - const readonly = new PermissionPresetSettingsController({ + const readonly = derivedController({ settings: { describe: () => Promise.resolve(ok({ writable: false, hasDocument: false, namespaces: [view('read-only')] })), mutate: vi.fn(), @@ -134,7 +141,7 @@ describe('PermissionRow', () => { writable: boolean namespaces: SettingsNamespaceView[] }>>>() - const controller = new PermissionPresetSettingsController({ + const controller = derivedController({ settings: { describe: () => describe.promise, mutate: () => Promise.resolve({ diff --git a/packages/client/ui-permission-presets/tests/settings-store.client.spec.ts b/packages/client/ui-permission-presets/tests/settings-store.client.spec.ts index e4e218fe86..b8a4725a5c 100644 --- a/packages/client/ui-permission-presets/tests/settings-store.client.spec.ts +++ b/packages/client/ui-permission-presets/tests/settings-store.client.spec.ts @@ -1,7 +1,8 @@ import { describe, expect, it, vi } from 'vitest' import type { SettingsNamespaceView } from '@deepseek-ai/dsh-api-remotes/client' +import { SettingsDescribeMirror } from '@deepseek-ai/dsh-client-ui-settings/client' import { - PermissionPresetSettingsController, permissionDefaultOf, refreshPermissionIfLoaded, + PermissionPresetSettingsController, permissionDefaultOf, } from '../src/client/settings-store.ts' const SCHEMA = { @@ -30,6 +31,13 @@ function ok(value: T) { return { rpcId: 'test', result: { ok: true as const, value } } } +/** The permission controller over a real mirror and one fake wire. */ +function permissionController(api: object) { + const wire = { settings: api } as never + const mirror = new SettingsDescribeMirror(wire) + return { mirror, controller: new PermissionPresetSettingsController(mirror, wire) } +} + describe('permission settings store', () => { it('derives dynamic options and host labels from the descriptor schema', () => { expect(permissionDefaultOf(view('read-only'))).toEqual({ @@ -92,9 +100,7 @@ describe('permission settings store', () => { namespaces: [view('read-only', 4)], }))) const mutate = vi.fn(() => Promise.resolve(ok(view('workspace-write', 5)))) - const controller = new PermissionPresetSettingsController({ - settings: { describe, mutate } as never, - }) + const { controller } = permissionController({ describe, mutate }) await controller.load() expect(controller.store.getSnapshot()).toMatchObject({ status: 'ready', @@ -113,126 +119,105 @@ describe('permission settings store', () => { currentValue: 'workspace-write', revision: 5, }) + // The write answer folded into the mirror; no re-read followed. + expect(describe).toHaveBeenCalledTimes(1) }) it('hides the row when the namespace is absent and contains write failures', async () => { const describe = vi.fn(() => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [] }))) - const controller = new PermissionPresetSettingsController({ - settings: { describe, mutate: vi.fn() } as never, - }) + const { controller } = permissionController({ describe, mutate: vi.fn() }) await controller.load() expect(controller.store.getSnapshot().status).toBe('unavailable') - const failing = new PermissionPresetSettingsController({ - settings: { - describe: () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('read-only')] })), - mutate: () => Promise.resolve({ - rpcId: 'test', - result: { - ok: false as const, - error: { code: 'settings-conflict', message: 'stale', details: {} }, - }, - }), - } as never, - }) + const failing = permissionController({ + describe: () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('read-only')] })), + mutate: () => Promise.resolve({ + rpcId: 'test', + result: { + ok: false as const, + error: { code: 'settings-conflict', message: 'stale', details: {} }, + }, + }), + }).controller await failing.load() await failing.select('workspace-write') expect(failing.store.getSnapshot()).toMatchObject({ status: 'error', error: 'stale' }) }) - it('contains read failures, no-ops without a writable view, and ignores stale responses', async () => { - const first = Promise.withResolvers>>() - const describe = vi.fn() - .mockImplementationOnce(() => first.promise) - .mockResolvedValueOnce(ok({ writable: false, hasDocument: false, namespaces: [view('read-only', 2)] })) + it('contains read failures and no-ops without a writable view', async () => { const mutate = vi.fn() - const controller = new PermissionPresetSettingsController({ - settings: { describe, mutate } as never, - }) - const stale = controller.load() - await controller.load() - first.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('workspace-write', 1)] })) - await stale - expect(controller.store.getSnapshot()).toMatchObject({ + const readOnly = permissionController({ + describe: () => Promise.resolve(ok({ + writable: false, hasDocument: false, namespaces: [view('read-only', 2)], + })), + mutate, + }).controller + await readOnly.load() + expect(readOnly.store.getSnapshot()).toMatchObject({ currentValue: 'read-only', writable: false, revision: 2, }) - await controller.select('workspace-write') + await readOnly.select('workspace-write') expect(mutate).not.toHaveBeenCalled() - const rejected = new PermissionPresetSettingsController({ - settings: { - describe: () => Promise.resolve({ - rpcId: 'test', - result: { ok: false as const, error: { code: 'internal', message: 'offline', details: {} } }, - }), - mutate, - } as never, - }) + const rejected = permissionController({ + describe: () => Promise.resolve({ + rpcId: 'test', + result: { ok: false as const, error: { code: 'internal', message: 'offline', details: {} } }, + }), + mutate, + }).controller await rejected.select('workspace-write') await rejected.load() expect(rejected.store.getSnapshot()).toMatchObject({ status: 'error', error: 'offline' }) + expect(mutate).not.toHaveBeenCalled() - const thrown = new PermissionPresetSettingsController({ - settings: { - // Promise consumers must contain unknown rejection values from a - // transport implementation, including non-Error legacy clients. - // oxlint-disable-next-line typescript/prefer-promise-reject-errors - describe: () => Promise.reject('disconnected'), - mutate, - } as never, - }) + const thrown = permissionController({ + // Promise consumers must contain unknown rejection values from a + // transport implementation, including non-Error legacy clients. + describe: () => Promise.reject('disconnected' as never), + mutate, + }).controller await thrown.load() expect(thrown.store.getSnapshot()).toMatchObject({ status: 'error', error: 'disconnected' }) }) - it('disposal suppresses in-flight reads and writes, and loaded invalidations refetch', async () => { + it('follows a mirror refresh without an own read once loaded', async () => { + const describe = vi.fn() + .mockResolvedValueOnce(ok({ writable: true, hasDocument: false, namespaces: [view('read-only', 1)] })) + .mockResolvedValueOnce(ok({ writable: true, hasDocument: false, namespaces: [view('workspace-write', 2)] })) + const { mirror, controller } = permissionController({ describe, mutate: vi.fn() }) + await controller.load() + expect(controller.store.getSnapshot()).toMatchObject({ currentValue: 'read-only' }) + + await mirror.load() + + expect(controller.store.getSnapshot()).toMatchObject({ currentValue: 'workspace-write', revision: 2 }) + }) + + it('disposal stops deriving and suppresses in-flight writes', async () => { const read = Promise.withResolvers>>() - const describe = vi.fn(() => read.promise) - const idle = new PermissionPresetSettingsController({ settings: { describe, mutate: vi.fn() } as never }) - refreshPermissionIfLoaded(idle) - expect(describe).not.toHaveBeenCalled() + const { mirror, controller: idle } = permissionController({ describe: () => read.promise, mutate: vi.fn() }) const loading = idle.load() idle.dispose() read.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('read-only')] })) - await loading + await Promise.all([loading, mirror.load()]) expect(idle.store.getSnapshot().status).toBe('loading') - const rejectedRead = Promise.withResolvers>>() - const disposedRead = new PermissionPresetSettingsController({ - settings: { describe: () => rejectedRead.promise, mutate: vi.fn() } as never, - }) - const reading = disposedRead.load() - disposedRead.dispose() - rejectedRead.reject(new Error('late read')) - await reading - expect(disposedRead.store.getSnapshot().status).toBe('loading') - const mutation = Promise.withResolvers>>() - const activeDescribe = vi.fn(() => Promise.resolve(ok({ - writable: true, - hasDocument: false, - namespaces: [view('read-only')], - }))) - const active = new PermissionPresetSettingsController({ - settings: { - describe: activeDescribe, - mutate: () => mutation.promise, - } as never, + const { controller: active } = permissionController({ + describe: () => Promise.resolve(ok({ + writable: true, + hasDocument: false, + namespaces: [view('read-only')], + })), + mutate: () => mutation.promise, }) await active.load() - refreshPermissionIfLoaded(active) - await vi.waitFor(() => { expect(activeDescribe).toHaveBeenCalledTimes(2) }) const saving = active.select('workspace-write') active.dispose() mutation.resolve(ok(view('workspace-write', 1))) @@ -240,11 +225,9 @@ describe('permission settings store', () => { expect(active.store.getSnapshot().status).toBe('saving') const rejectedMutation = Promise.withResolvers>>() - const disposedWrite = new PermissionPresetSettingsController({ - settings: { - describe: () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('read-only')] })), - mutate: () => rejectedMutation.promise, - } as never, + const { controller: disposedWrite } = permissionController({ + describe: () => Promise.resolve(ok({ writable: true, hasDocument: false, namespaces: [view('read-only')] })), + mutate: () => rejectedMutation.promise, }) await disposedWrite.load() const writing = disposedWrite.select('workspace-write') From 380030c48f564c3e49611e7e70c4407a27efdcf6 Mon Sep 17 00:00:00 2001 From: pku-xht Date: Mon, 17 Aug 2026 17:31:00 +0800 Subject: [PATCH 10/28] fix(workflow): preserve disclosure intent across completion --- .../workflow-run/ui-live.expected.md | 17 --- apps/web/tests/workflow-run.e2e.ts | 2 +- .../src/client/WorkflowRunPanel.tsx | 134 +++++++++--------- .../tests/workflow-run.client.spec.tsx | 35 ++++- 4 files changed, 104 insertions(+), 84 deletions(-) diff --git a/apps/web/tests/snapshots/workflow-run/ui-live.expected.md b/apps/web/tests/snapshots/workflow-run/ui-live.expected.md index 9e7f7b0ddc..52ac5c75fc 100644 --- a/apps/web/tests/snapshots/workflow-run/ui-live.expected.md +++ b/apps/web/tests/snapshots/workflow-run/ui-live.expected.md @@ -1,23 +1,6 @@ -- text: "Use the workflow tool exactly once, with args omitted, meta set to { \"name\": \"snapshot-flow\", \"description\": \"one child for the snapshot\" }, and this EXACT script body (copy it verbatim): phase('Run') const reply = await agent('Reply with exactly the word WF_CHILD_OK and nothing else.') return { reply } After the workflow returns, reply with the single word WORKFLOW_DONE and stop. Do not use any other tool. {{clock}}" -- button "Copy": - - img -- button "Context injection @deepseek-ai/dsh-system-prompt": - - img - - img - - text: Context injection @deepseek-ai/dsh-system-prompt -- button "Think The user wants me to use the workflow tool exactly once with specific parameters. Let me carefully follow the instructions:": - - img - - img - - text: "Think The user wants me to use the workflow tool exactly once with specific parameters. Let me carefully follow the instructions:" -- text: Running -- button "Tool call workflow ·": - - img - - img - - text: Tool call workflow · - button "snapshot-flow 1 member Running" [expanded]: - img - text: snapshot-flow 1 member Running - button "Run 1 member Running 1": - img - text: Run 1 member Running 1 -- status: Deep diving... diff --git a/apps/web/tests/workflow-run.e2e.ts b/apps/web/tests/workflow-run.e2e.ts index 04f79cccd4..ef787a60da 100644 --- a/apps/web/tests/workflow-run.e2e.ts +++ b/apps/web/tests/workflow-run.e2e.ts @@ -93,7 +93,7 @@ describe.skipIf(MODE === 'record')('web e2e: durable workflow run in Chat', () = await phaseDisclosure.click() expect(await phaseDisclosure.getAttribute('aria-expanded')).toBe('false') expect(await member.count()).toBe(0) - const liveSnapshot = await captureStableAria(page, '[data-chat-flow]', scaffold.workspaceCwd) + const liveSnapshot = await captureStableAria(page, '[data-workflow-run]', scaffold.workspaceCwd) await compareOrRefreshGolden(UI_LIVE_EXPECTED, liveSnapshot, MODE) await phaseDisclosure.press('Enter') await member.waitFor() diff --git a/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx b/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx index a839403266..9df3452775 100644 --- a/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx +++ b/packages/client/ui-workflow-run/src/client/WorkflowRunPanel.tsx @@ -147,6 +147,16 @@ function collapsePending(state: DisclosureState): DisclosureState { return { ...state, open: false, pendingCleanCollapse: false } } +function existingPhaseState( + phases: ReadonlyMap, + key: string, +): DisclosureState { + const phase = phases.get(key) + /* v8 ignore next -- mounted phase callbacks are created from this owner map. */ + if (phase === undefined) throw new Error(`Missing disclosure state for phase ${key}`) + return phase +} + function phaseStatusSummary(members: readonly WorkflowRunMemberData[], t: WorkflowRunPanelProps['t']): string { const counts = new Map() for (const member of members) counts.set(member.status, (counts.get(member.status) ?? 0) + 1) @@ -226,20 +236,8 @@ function MemberRow({ member, navigable, openSession, t }: { readonly t: WorkflowRunPanelProps['t'] }) { const name = readableMember(member.label, t) - const buttonRef = useRef(null) - const [keepFocusedButton, setKeepFocusedButton] = useState(navigable) - const renderButton = navigable || keepFocusedButton - - useLayoutEffect(() => { - if (navigable) { - if (!keepFocusedButton) setKeepFocusedButton(true) - return - } - const button = buttonRef.current - if (button === null || button.ownerDocument.activeElement !== button) { - if (keepFocusedButton) setKeepFocusedButton(false) - } - }, [keepFocusedButton, navigable]) + const [focused, setFocused] = useState(false) + const renderButton = navigable || focused const content = ( <> @@ -253,14 +251,14 @@ function MemberRow({ member, navigable, openSession, t }: { } return (