From bf0db65bb0e9b6e080a0328a01622c92cc032c98 Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Wed, 26 Aug 2026 17:12:58 +0800 Subject: [PATCH] =?UTF-8?q?fix(ui-settings-models):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20derived=20key=20fact,=20required=20render=20seat,?= =?UTF-8?q?=20spec=20sync?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The provider-card seat's keyConfigured now derives from the reference the page would use — the profile's apiKeyEnv, or the page's derived _API_KEY while the profile names none — so the add-provider draft agrees with its own editor about an existing conventional credential (the store joins the derived describe in the same batched call, as ProviderRow.derivedCredential). ModelsSectionProps makes the renderSlot seat required so a direct render that forgets it fails to compile; the one such render in provider-form.client.spec regained a real mount and the test boilerplate collapsed to renderSlot={() => null}. The extension-slots Agent Note now states the keyed cell's real override rule (same priority throws, a different priority shadows), and docs/subsystems/slots.md carries the two new seats in its hierarchy, both languages. --- ...8-26-models-page-extension-slots.i18n.yaml | 4 +-- .../2026-08-26-models-page-extension-slots.md | 2 +- ...26-08-26-models-page-extension-slots.zh.md | 2 +- docs/subsystems/slots.i18n.yaml | 4 +-- docs/subsystems/slots.md | 2 ++ docs/subsystems/slots.zh.md | 2 ++ .../src/client/ModelsSection.tsx | 20 +++++++++---- .../ui-settings-models/src/client/store.ts | 24 ++++++++++----- .../tests/components.client.spec.tsx | 30 ++++++++++++++----- .../tests/invariant.client.spec.ts | 3 +- .../tests/provider-form.client.spec.tsx | 1 + .../tests/store.client.spec.ts | 18 ++++++++--- 12 files changed, 82 insertions(+), 30 deletions(-) diff --git a/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.i18n.yaml b/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.i18n.yaml index e31c3b95ce..e6e2dae0a2 100644 --- a/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.i18n.yaml +++ b/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.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/architecture/2026-08-26-models-page-extension-slots.md -2026-08-26-models-page-extension-slots.md: e1ad30fd7283470b76d302508404d7c1445d5e50 -2026-08-26-models-page-extension-slots.zh.md: 4e7aa7cef0acbcf2e8344f31d4d458fde30c889c +2026-08-26-models-page-extension-slots.md: 9983c669b7838de00d49a634a54e5d115ef4945a +2026-08-26-models-page-extension-slots.zh.md: 639654ea4b912d43a95f0a9295969523f4e1c5f8 diff --git a/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.md b/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.md index e1ad30fd72..9983c669b7 100644 --- a/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.md +++ b/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.md @@ -30,4 +30,4 @@ Without registrants both seats render nothing, so the shipped page is pixel-iden ## Consequences -An out-of-tree plugin can now integrate per-family card UI into the Models page with no product edits; `llm-pi-ai-oauth` is the first consumer. The cost is a public contract: `ProviderCardExtrasOwnerProps` exposes `ConfigurableProviderView` at the `./client` boundary, and the dispatch sites (saved card, setup posture, add draft, footer) become behavior extensions rely on. One extension owner per adapter family is a deliberate limit — a second plugin claiming the same namespace is refused by the keyed registry rather than silently merged. +An out-of-tree plugin can now integrate per-family card UI into the Models page with no product edits; `llm-pi-ai-oauth` is the first consumer. The cost is a public contract: `ProviderCardExtrasOwnerProps` exposes `ConfigurableProviderView` at the `./client` boundary, and the dispatch sites (saved card, setup posture, add draft, footer) become behavior extensions rely on. Per adapter family the keyed cell renders one owner at a time: a second registration under the same namespace at the same priority is refused by the registry, while a different priority deliberately shadows it (the lowest-priority entry renders) — the slot kit's standard override channel, never a silent merge. diff --git a/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.zh.md b/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.zh.md index 4e7aa7cef0..639654ea4b 100644 --- a/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.zh.md +++ b/.agents/notes/implemented/architecture/2026-08-26-models-page-extension-slots.zh.md @@ -30,4 +30,4 @@ Status: implemented ## Consequences -仓库外插件现在无需改动产品即可把按家族的卡片 UI 集成进 Models 页；`llm-pi-ai-oauth` 是首个消费者。代价是一份公开契约：`ProviderCardExtrasOwnerProps` 在 `./client` 边界暴露 `ConfigurableProviderView`，且各分发位点（已保存卡片、setup 形态、新增草稿、footer）成为扩展方依赖的行为。每个适配器家族只有一个扩展所有者是刻意的限制——第二个插件认领同一 namespace 会被 keyed 注册表拒绝，而不是被静默合并。 +仓库外插件现在无需改动产品即可把按家族的卡片 UI 集成进 Models 页；`llm-pi-ai-oauth` 是首个消费者。代价是一份公开契约：`ProviderCardExtrasOwnerProps` 在 `./client` 边界暴露 `ConfigurableProviderView`，且各分发位点（已保存卡片、setup 形态、新增草稿、footer）成为扩展方依赖的行为。每个适配器家族的 keyed 单元格同一时刻只渲染一个所有者：同一 namespace 下同 priority 的第二次注册会被注册表拒绝，不同 priority 则是刻意的遮蔽（最低 priority 的条目渲染）——这是 slot 套件的标准覆盖通道，绝非静默合并。 diff --git a/docs/subsystems/slots.i18n.yaml b/docs/subsystems/slots.i18n.yaml index cda920ea03..d0c94878b2 100644 --- a/docs/subsystems/slots.i18n.yaml +++ b/docs/subsystems/slots.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 docs/subsystems/slots.md -slots.md: 6eb61780ca2f06ebc38a0fcf2638a7fafcd5ceee -slots.zh.md: e5a25763d382c228525f3da24694e41dd09737e0 +slots.md: 8e115e30aed68e543eca2f1aac6e28ad9f57cf73 +slots.zh.md: 3894b69d8d020b4bb67ce325d389ab3f20cfcc9a diff --git a/docs/subsystems/slots.md b/docs/subsystems/slots.md index 6eb61780ca..8e115e30ae 100644 --- a/docs/subsystems/slots.md +++ b/docs/subsystems/slots.md @@ -123,6 +123,8 @@ root │ ├─ settings.onboarding │ └─ settings.section │ ├─ settings.general.item +│ ├─ settings.models.provider-card +│ ├─ settings.models.footer │ └─ settings.plugins.tab │ └─ settings.plugin.item ├─ conversation diff --git a/docs/subsystems/slots.zh.md b/docs/subsystems/slots.zh.md index e5a25763d3..3894b69d8d 100644 --- a/docs/subsystems/slots.zh.md +++ b/docs/subsystems/slots.zh.md @@ -123,6 +123,8 @@ root │ ├─ settings.onboarding │ └─ settings.section │ ├─ settings.general.item +│ ├─ settings.models.provider-card +│ ├─ settings.models.footer │ └─ settings.plugins.tab │ └─ settings.plugin.item ├─ conversation diff --git a/packages/client/ui-settings-models/src/client/ModelsSection.tsx b/packages/client/ui-settings-models/src/client/ModelsSection.tsx index 2e7b01899a..6776a097dd 100644 --- a/packages/client/ui-settings-models/src/client/ModelsSection.tsx +++ b/packages/client/ui-settings-models/src/client/ModelsSection.tsx @@ -53,9 +53,11 @@ type ModelsRenderSlot = PropsRenderSlots['renderSlot'] /** * Props delivered by the slot outlet: the inject face spread flat (the * renderer erases the share boundary at the render call) plus the child-slot - * dispatch seat. + * dispatch seat. The seat is required: the renderer binds it at the render + * call itself — unlike the inject face it is never absent at runtime — and a + * direct render that forgets it fails to compile instead of mounting nothing. */ -export type ModelsSectionProps = Partial> & Partial> +export type ModelsSectionProps = Partial> & PropsRenderSlots type ModelsSectionFace = InjectFace @@ -148,9 +150,17 @@ export function needsSetup(row: ProviderRow, anyUsable: boolean): boolean { return row.credential?.configured !== true } -/** The provider-card seat's credential fact: a referenced key confirmed configured. */ +/** + * The provider-card seat's credential fact: the reference this page would use + * for the row — the profile's `apiKeyEnv`, or the page's derived + * `_API_KEY` while the profile names none — confirmed configured. The + * derived half is what keeps the seat consistent with the editor on the + * add-provider draft, whose dormant row names no reference yet. + */ function keyConfiguredOf(row: ProviderRow): boolean { - return row.apiKeyEnv !== undefined && row.credential?.configured === true + return row.apiKeyEnv !== undefined + ? row.credential?.configured === true + : row.derivedCredential?.configured === true } function targetOf(row: ProviderRow): EditorTarget { @@ -192,7 +202,7 @@ export function ModelsSection(props: ModelsSectionProps): ReactNode { const { controller, useSnapshot, api, schema, t, renderSlot } = props if ( controller === undefined || useSnapshot === undefined || api === undefined - || schema === undefined || t === undefined || renderSlot === undefined + || schema === undefined || t === undefined ) return null return } diff --git a/packages/client/ui-settings-models/src/client/store.ts b/packages/client/ui-settings-models/src/client/store.ts index 2b17c9fc4b..df5bcb5dcd 100644 --- a/packages/client/ui-settings-models/src/client/store.ts +++ b/packages/client/ui-settings-models/src/client/store.ts @@ -32,6 +32,13 @@ export interface ProviderRow { apiKeyEnv: string | undefined /** Credential state for {@link apiKeyEnv}, once described. */ credential: CredentialView | undefined + /** + * Credential state for the page's derived `_API_KEY`, described only + * while the profile names no reference — the provider-card seat's + * `keyConfigured` fact for dormant and keyless rows, matching the editor's + * own derivation rule. + */ + derivedCredential?: CredentialView } /** Page snapshot. */ @@ -185,7 +192,7 @@ export class ModelsSettingsStore { credential: undefined, } }) - const refs = [...new Set(rows.flatMap(row => row.apiKeyEnv === undefined ? [] : [row.apiKeyEnv]))] + const refs = [...new Set(rows.map(row => row.apiKeyEnv ?? deriveKeyRef(row.entry.provider)))] let credentials: Record = {} let credentialError: string | null = null if (refs.length > 0) { @@ -206,12 +213,15 @@ export class ModelsSettingsStore { s.error = null s.credentialError = credentialError s.writable = writable - s.rows = rows.map(row => ({ - ...row, - ...row.apiKeyEnv !== undefined && credentials[row.apiKeyEnv] !== undefined - ? { credential: credentials[row.apiKeyEnv] } - : {}, - })) + s.rows = rows.map((row) => { + const named = row.apiKeyEnv === undefined ? undefined : credentials[row.apiKeyEnv] + const derived = row.apiKeyEnv !== undefined ? undefined : credentials[deriveKeyRef(row.entry.provider)] + return { + ...row, + ...named === undefined ? {} : { credential: named }, + ...derived === undefined ? {} : { derivedCredential: derived }, + } + }) s.namespaces = namespaces }) } diff --git a/packages/client/ui-settings-models/tests/components.client.spec.tsx b/packages/client/ui-settings-models/tests/components.client.spec.tsx index edfb6462ee..1a72d7c444 100644 --- a/packages/client/ui-settings-models/tests/components.client.spec.tsx +++ b/packages/client/ui-settings-models/tests/components.client.spec.tsx @@ -229,7 +229,7 @@ async function mountFace(scripted: ReturnType) { api: face as never, schema: settingsSchema, t, - renderSlot: renderSlot as unknown as NonNullable, + renderSlot: renderSlot as unknown as ModelsSectionProps['renderSlot'], } const view = render() return { view, face, update, replace, mutate, set, unset, controller, mirror, renderSlot } @@ -293,6 +293,22 @@ describe('ModelsSection', () => { expect(cardSeatCalls(renderSlot)).toContainEqual(['anthropic', false, false, 'llm-pi-ai']) }) + it('derives the draft seat\'s key fact from the page\'s conventional reference', async () => { + const scripted = scriptedFace() + scripted.face.credentials.describe.mockImplementation((payload: { refs: string[] }) => Promise.resolve(ok({ + credentials: Object.fromEntries(payload.refs.map(ref => [ref, { + configured: ref === 'OPENAI_API_KEY' || ref === 'ANTHROPIC_API_KEY', + writable: true, + }])), + }))) + const { renderSlot } = await mountFace(scripted) + renderSlot.mockClear() + fireEvent.click(screen.getByRole('button', { name: en.add })) + // The dormant row names no reference yet; the seat still reports the + // derived ANTHROPIC_API_KEY the editor itself displays as configured. + expect(cardSeatCalls(renderSlot)).toContainEqual(['anthropic', false, true, 'llm-pi-ai']) + }) + it('skips the draft seat when a refresh drops the dormant row', async () => { const { renderSlot, face, controller } = await mountSection() fireEvent.click(screen.getByRole('button', { name: en.add })) @@ -415,7 +431,7 @@ describe('ModelsSection', () => { api={face as never} schema={settingsSchema} t={t} - renderSlot={stubRenderSlot() as unknown as NonNullable} + renderSlot={() => null} />) const missing = screen.getByRole('img', { name: en.credentialMissing }) @@ -440,7 +456,7 @@ describe('ModelsSection', () => { api={face as never} schema={settingsSchema} t={t} - renderSlot={stubRenderSlot() as unknown as NonNullable} + renderSlot={() => null} />) // Now a row with an Edit button, not an open card. expect(screen.getAllByText(en.edit).length).toBeGreaterThan(1) @@ -1171,7 +1187,7 @@ describe('ModelsSection', () => { api={face as never} schema={settingsSchema} t={t} - renderSlot={stubRenderSlot() as unknown as NonNullable} + renderSlot={() => null} />) const key = await screen.findByLabelText(en.keyInput) expect(key.placeholder).toBe(en.keyPlaceholder) @@ -1311,7 +1327,7 @@ describe('ModelsSection', () => { api={face.face as never} schema={settingsSchema} t={t} - renderSlot={stubRenderSlot() as unknown as NonNullable} + renderSlot={() => null} />) expect(screen.getByText(/directory down/)).toBeTruthy() fireEvent.click(screen.getByText(en.retry)) @@ -1334,7 +1350,7 @@ describe('ModelsSection', () => { api={face as never} schema={settingsSchema} t={t} - renderSlot={stubRenderSlot() as unknown as NonNullable} + renderSlot={() => null} />) expect(screen.getByText(en.readOnly)).toBeTruthy() expect(screen.getAllByText(en.remove).every(button => button.disabled)).toBe(true) @@ -1396,7 +1412,7 @@ describe('ModelsSection', () => { api={face as never} schema={settingsSchema} t={t} - renderSlot={stubRenderSlot() as unknown as NonNullable} + renderSlot={() => null} />) await screen.findByText('DeepSeek') }) diff --git a/packages/client/ui-settings-models/tests/invariant.client.spec.ts b/packages/client/ui-settings-models/tests/invariant.client.spec.ts index ceb0b1f368..0d7a0578b7 100644 --- a/packages/client/ui-settings-models/tests/invariant.client.spec.ts +++ b/packages/client/ui-settings-models/tests/invariant.client.spec.ts @@ -3,6 +3,7 @@ import { Context } from '@deepseek-ai/cordis' import * as ModelsInvariant from '@deepseek-ai/dsh-client-ui-settings-models/invariant' import InvariantRegistry from '@deepseek-ai/dsh-invariants' import { ModelsSection } from '../src/client/ModelsSection.tsx' +import type { ModelsSectionProps } from '../src/client/ModelsSection.tsx' describe('invariant companion', () => { it('registers under the package name with an empty installer', async () => { @@ -18,6 +19,6 @@ describe('invariant companion', () => { }) it('renders null until the shell injects the section dependencies', () => { - expect(ModelsSection({})).toBeNull() + expect(ModelsSection({} as ModelsSectionProps)).toBeNull() }) }) diff --git a/packages/client/ui-settings-models/tests/provider-form.client.spec.tsx b/packages/client/ui-settings-models/tests/provider-form.client.spec.tsx index 892a1ccb73..89c30f710b 100644 --- a/packages/client/ui-settings-models/tests/provider-form.client.spec.tsx +++ b/packages/client/ui-settings-models/tests/provider-form.client.spec.tsx @@ -672,6 +672,7 @@ describe('provider rows', () => { api={scripted.face as never} schema={settingsSchema} t={t} + renderSlot={() => null} />) // Absent is "unknown", never "shipped": an adapter that answers nothing diff --git a/packages/client/ui-settings-models/tests/store.client.spec.ts b/packages/client/ui-settings-models/tests/store.client.spec.ts index c71457e474..c477c25993 100644 --- a/packages/client/ui-settings-models/tests/store.client.spec.ts +++ b/packages/client/ui-settings-models/tests/store.client.spec.ts @@ -81,7 +81,9 @@ describe('ModelsSettingsStore', () => { expect(state.status).toBe('ready') expect(state.writable).toBe(true) expect(state.credentialError).toBeNull() - expect(seenRefs).toEqual([['DEEPSEEK_API_KEY', 'OPENAI_API_KEY']]) + // Named references first (rows order), then the derived _API_KEY + // of every row whose profile names none — one batched describe. + expect(seenRefs).toEqual([['DEEPSEEK_API_KEY', 'OPENAI_API_KEY', 'ANTHROPIC_API_KEY', 'GHOST_API_KEY']]) const byProvider = new Map(state.rows.map(row => [row.entry.provider, row])) expect(byProvider.get('deepseek-official')).toMatchObject({ configured: true, @@ -196,7 +198,7 @@ describe('edge joins', () => { expect(state.rows[0]?.apiKeyEnv).toBeUndefined() }) - it('skips the credential describe entirely when no row names a reference', async () => { + it('describes the derived reference for a row whose profile names none', async () => { const { face, mirror, seenRefs } = api({ describeSettings: () => Promise.resolve(ok({ writable: true, @@ -208,11 +210,19 @@ describe('edge joins', () => { { provider: 'anthropic', displayName: 'anthropic', settingsNs: 'llm-pi-ai', settingsPath: ['providers', 'anthropic'], active: false }, ] as never, })), + describeCredentials: refs => Promise.resolve(ok({ + credentials: Object.fromEntries(refs.map(ref => [ref, { configured: true, writable: true }])), + })), }) const store = new ModelsSettingsStore(face, settingsSchema, mirror) await store.load() - expect(seenRefs).toEqual([]) - expect(store.store.getSnapshot().status).toBe('ready') + // The dormant row names no reference, so the join asks about the page's + // own derived _API_KEY — what the editor would display for it. + expect(seenRefs).toEqual([['ANTHROPIC_API_KEY']]) + const state = store.store.getSnapshot() + expect(state.status).toBe('ready') + expect(state.rows[0]?.credential).toBeUndefined() + expect(state.rows[0]?.derivedCredential).toMatchObject({ configured: true }) }) it('surfaces a settings describe failure', async () => {