Browse Source

refactor(ui-settings-plugins): plugin tab derives served namespaces from the mirror

Yichen Jiang 2 months ago
parent
commit
232e4beeae

+ 3 - 5
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 —
  * 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
  * that second read closes the window where a document commit lands between
  * the eager read and the SSE subscription and its invalidation is lost.
  * 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 scaffold: WebScaffold
 let browser: Browser
 let browser: Browser

+ 4 - 13
packages/client/ui-settings-plugins/src/client/index.ts

@@ -72,26 +72,17 @@ export function apply(ctx: ClientContext): void {
     'ui-settings-plugins: credential invalidations',
     '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(
   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(() => () => { 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.
   // A card registered after the first read joins the list without a wire call.
   ctx.effect(
   ctx.effect(
     () => ctx.slots.subscribe('settings.plugin.item', () => { configurable.refresh() }),
     () => ctx.slots.subscribe('settings.plugin.item', () => { configurable.refresh() }),
     'ui-settings-plugins: card ledger',
     'ui-settings-plugins: card ledger',
   )
   )
-  void configurable.load()
 
 
   let tabsVersion = -1
   let tabsVersion = -1
   let tabsRevision = -1
   let tabsRevision = -1

+ 21 - 42
packages/client/ui-settings-plugins/src/client/tab-store.ts

@@ -10,7 +10,7 @@
  * trace and does not count toward the empty line.
  * 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 type { StoredEntry } from '@deepseek-ai/dsh-client-ui-slots'
 import { createSnapshotStore, type SnapshotStore } from '@deepseek-ai/dsh-client-runtime/client'
 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 {
 export class ConfigurablePluginsTabController {
   private readonly store = createSnapshotStore<ConfigurablePluginsTabState>({ loaded: false, namespaces: [] })
   private readonly store = createSnapshotStore<ConfigurablePluginsTabState>({ 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 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.
    * @param entries - reads the cards currently registered into the section's slot.
    */
    */
   constructor(
   constructor(
-    private readonly api: Pick<IApiClient, 'settings'>,
+    private readonly describeFace: SettingsDescribeFace,
     private readonly entries: () => readonly StoredEntry[],
     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<void> {
-    if (this.isDisposed()) return
-    const generation = ++this.generation
-    let response: Awaited<ReturnType<IApiClient['settings']['describe']>>
-    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()
     this.publish()
   }
   }
 
 
@@ -92,10 +68,10 @@ export class ConfigurablePluginsTabController {
     this.publish()
     this.publish()
   }
   }
 
 
-  /** Stop publishing; an in-flight read settles without touching the store. */
+  /** Stop publishing and stop following the mirror. */
   dispose(): void {
   dispose(): void {
     this.disposed = true
     this.disposed = true
-    this.generation += 1
+    this.unsubscribe()
   }
   }
 
 
   /**
   /**
@@ -107,17 +83,20 @@ export class ConfigurablePluginsTabController {
   }
   }
 
 
   private publish(): void {
   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 =>
     const namespaces = this.entries().flatMap(entry =>
       entry.options.key !== undefined && served.has(entry.options.key) ? [entry.options.key] : [])
       entry.options.key !== undefined && served.has(entry.options.key) ? [entry.options.key] : [])
     const previous = this.store.getSnapshot()
     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.length === namespaces.length
       && previous.namespaces.every((ns, index) => ns === namespaces[index])) return
       && previous.namespaces.every((ns, index) => ns === namespaces[index])) return
-    this.store.set({ loaded: this.loaded, namespaces })
+    this.store.set({ loaded, namespaces })
   }
   }
 }
 }

+ 19 - 41
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 { CardForm, numberField, textField } from '../src/client/card-form.ts'
 import { AgentLoopCardController, type AgentLoopSettings } from '../src/client/agent-loop-card-controller.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 { 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 { ConfigurablePluginsTabController } from '../src/client/tab-store.ts'
 import { WebSearchCardController, type WebSearchSettings } from '../src/client/web-search-card-controller.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. */
   /** 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 () => {
   it('dispatches the served namespaces a card claims, in card registration order', async () => {
     const settings = settingsApi(['bash', 'ui-theme', 'agent-loop'])
     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
     // 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
     // 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 () => {
   it('never dispatches a card whose namespace this deployment does not serve', async () => {
     const settings = settingsApi(['bash'])
     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'])
     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 () => {
   it('takes a card registered after the read without asking the Host again', async () => {
     const settings = settingsApi(['bash'])
     const settings = settingsApi(['bash'])
     let entries = ledger()
     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([])
     expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual([])
 
 
     entries = ledger('bash')
     entries = ledger('bash')
@@ -599,34 +600,33 @@ describe('ConfigurablePluginsTabController', () => {
     expect(settings.describe).toHaveBeenCalledOnce()
     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 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'))
     settings.describe.mockRejectedValueOnce(new Error('offline'))
 
 
-    await controller.load()
+    await settings.mirror.load()
 
 
     expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual(['bash'])
     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 settings = settingsApi(['bash'])
-    const controller = new ConfigurablePluginsTabController(settings.api, () => ledger('bash'))
+    const controller = new ConfigurablePluginsTabController(settings.mirror, () => ledger('bash'))
 
 
     controller.dispose()
     controller.dispose()
-    await controller.load()
+    await settings.mirror.load()
 
 
     expect(controller.inject().hooks.configurablePlugins.getSnapshot())
     expect(controller.inject().hooks.configurablePlugins.getSnapshot())
       .toEqual({ loaded: false, namespaces: [] })
       .toEqual({ loaded: false, namespaces: [] })
-    expect(settings.describe).not.toHaveBeenCalled()
   })
   })
 
 
   it('ignores a slot-ledger change that arrives after disposal', async () => {
   it('ignores a slot-ledger change that arrives after disposal', async () => {
     const settings = settingsApi(['bash'])
     const settings = settingsApi(['bash'])
     let entries = ledger()
     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()
     controller.dispose()
     entries = ledger('bash')
     entries = ledger('bash')
@@ -635,33 +635,11 @@ describe('ConfigurablePluginsTabController', () => {
     expect(controller.inject().hooks.configurablePlugins.getSnapshot().namespaces).toEqual([])
     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<unknown>()
-    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 () => {
   it('reports the Host answered even when it serves nothing this tab shows', async () => {
     const settings = settingsApi(['ui-theme'])
     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())
     expect(controller.inject().hooks.configurablePlugins.getSnapshot())
       .toEqual({ loaded: true, namespaces: [] })
       .toEqual({ loaded: true, namespaces: [] })

+ 1 - 1
packages/client/ui-settings/src/client/index.ts

@@ -27,7 +27,7 @@ export type {
 } from './contract/slots.ts'
 } from './contract/slots.ts'
 export { SettingsScopeController, SettingsScopeBinder } from './settings-scope.ts'
 export { SettingsScopeController, SettingsScopeBinder } from './settings-scope.ts'
 export { SettingsDescribeMirror } from './settings-mirror.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
  * Required services: the wire handle for the mirror's reads and the forwarded

+ 29 - 1
packages/client/ui-settings/src/client/settings-mirror.ts

@@ -38,12 +38,40 @@ export interface SettingsMirrorSnapshot {
   error: string | null
   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<void>
+  /**
+   * 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.
  * Serializes every Host `settings.describe` read behind one snapshot store.
  * Concurrent {@link load} calls fold into the in-flight read plus one rerun,
  * 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.
  * so an invalidation arriving mid-read is never lost and never duplicated.
  */
  */
-export class SettingsDescribeMirror {
+export class SettingsDescribeMirror implements SettingsDescribeFace {
   private readonly store: SnapshotStore<SettingsMirrorSnapshot>
   private readonly store: SnapshotStore<SettingsMirrorSnapshot>
   private inFlight: Promise<void> | undefined
   private inFlight: Promise<void> | undefined
   private rerun = false
   private rerun = false

+ 12 - 1
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
 // never — the owning package's client-safe, type-only subpath supplies the
 // cordis `Events` entry (and with it the branded `SettingsNamespace`).
 // cordis `Events` entry (and with it the branded `SettingsNamespace`).
 import type {} from '@deepseek-ai/dsh-settings/types'
 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<IApiClient, 'settings'>
 type SettingsFace = Pick<IApiClient, 'settings'>
 
 
@@ -256,6 +256,17 @@ export class SettingsScopeBinder extends Service {
    * @param spec - domain-owned namespace contract.
    * @param spec - domain-owned namespace contract.
    * @returns the bound scope consumed by the domain's services and rows.
    * @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<T>(spec: SettingsScopeSpec<T>): SettingsScope<T> {
   bind<T>(spec: SettingsScopeSpec<T>): SettingsScope<T> {
     const ctx = this.ctx
     const ctx = this.ctx
     const connection = ctx.get('connection') as ConnectionHandle
     const connection = ctx.get('connection') as ConnectionHandle