From 9f996be8e32ba2d32478dadb8879a20e24fff06b Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Thu, 30 Jul 2026 18:30:15 +0800 Subject: [PATCH] fix(web-config): close the wire boundary, the redacted-replace data loss, and three P2s MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five findings from the #939 review, each reproduced before being fixed. **Configuration reads are as privileged as writes.** `settings.describe` returns every exposed namespace's configuration and `credentials.describe` reports whether an arbitrary environment-variable name is configured and from where — reconnaissance no anonymous caller should have. Both join PRIVILEGED_METHODS, so the whole configuration plane is loopback-only until real authentication exists; `trustedHosts` was never authentication. The model catalog stays reachable: it carries no endpoints or key state, and a LAN client's model picker legitimately needs it. Asserted over a real HTTP server, because the Host header a browser actually sends is what decides this. **The proxy serves only namespaces a registered model provider addresses.** The settings seam is general — any plugin may register one — but the Web configuration plane is the model-provider surface. Without the gate, every future `settings.register()` would silently become remotely readable and writable configuration. An unregistered namespace and an unexposed one answer identically, so no caller can enumerate the registry one probe at a time. **Path-addressed writes replace the redacted-document rebuild.** The editor reads the REDACTED descriptor, so rebuilding a section from it and replacing wholesale deleted every literal secret the wire never returned — reproduced as `{baseURL, reasoning}` in, stored `apiKey` gone out. `settings.mutate` applies set/unset ops to the section as it stands at the front of the seam's write queue, and the client names only fields it can see, so an unseen secret is untouched by construction rather than by care. P2s in the same pass: `llm/adapters-updated` now contains async listener rejections (an uncontained one escaped as unhandledRejection, contradicting the documented "observer failures are contained"); llm-deepseek's retry-policy swap uses the atomic `registration.replace` instead of dispose-then-register, which published `[]` then `["deepseek-official"]` so an observer saw the provider disappear and come back; and a transport rejection no longer strands the page in `loading` or a card in `busy`, with removal failures surfaced on the page banner instead of swallowed. --- packages/client/connection/src/client/api.ts | 2 +- .../client/connection/src/client/fixture.ts | 6 + .../client/connection/src/client/index.ts | 2 +- packages/client/connection/src/index.ts | 18 ++- packages/client/connection/tests/fake-api.ts | 1 + .../client/connection/tests/node-half.spec.ts | 80 ++++++++++- packages/client/runtime/tests/fake-api.ts | 1 + .../ui-models/src/client/ModelsSection.tsx | 38 ++++-- .../ui-models/src/client/ProviderEditor.tsx | 122 +++++++++-------- packages/client/ui-models/src/client/store.ts | 22 ++- .../ui-models/tests/components.spec.tsx | 125 +++++++++++------- packages/host/apiproxy/src/api-proxy.ts | 45 ++++++- packages/host/apiproxy/src/api/index.ts | 2 +- packages/host/apiproxy/src/api/rpc-map.ts | 1 + packages/host/apiproxy/src/api/rpc.schema.ts | 1 + packages/host/apiproxy/src/api/rpc.ts | 6 + .../host/apiproxy/src/api/settings.schema.ts | 17 ++- packages/host/apiproxy/src/api/settings.ts | 19 +++ packages/host/apiproxy/src/fetch/client.ts | 5 +- packages/host/apiproxy/src/fetch/handler.ts | 3 +- .../apiproxy/tests/api-proxy-config.spec.ts | 62 ++++++++- .../apiproxy/tests/client-handler.spec.ts | 2 + .../host/apiproxy/tests/fetch-carrier.spec.ts | 3 + packages/llm/llm-deepseek/src/index.ts | 11 +- .../llm-deepseek/tests/dynamic-config.spec.ts | 11 +- packages/llm/llm/src/index.ts | 19 ++- packages/llm/llm/tests/topology.spec.ts | 36 +++++ packages/settings/settings/src/index.ts | 86 +++++++++++- .../settings/settings/tests/settings.spec.ts | 86 ++++++++++++ 29 files changed, 676 insertions(+), 156 deletions(-) diff --git a/packages/client/connection/src/client/api.ts b/packages/client/connection/src/client/api.ts index 25507c1c08..cc9d2a1eb9 100644 --- a/packages/client/connection/src/client/api.ts +++ b/packages/client/connection/src/client/api.ts @@ -14,7 +14,7 @@ export type { ModelCatalogFailure, ModelCatalogModel, ModelProviderGroup, ModelReasoning, InboxItemId, ModelReasoningEffort, ModelTarget, QueueAction, QueuedInboxItem, SessionModels, GoalsApi, GoalRef, - SettingsApi, SettingsNamespaceView, SettingsSecretView, + SettingsApi, SettingsNamespaceView, SettingsPathOpView, SettingsSecretView, CredentialsApi, CredentialView, ConfigurableProviderView, LlmApi, } from '@deepseek-ai/dsh-host-apiproxy/api' export type { ToolCallView, ToolResultView } from '@deepseek-ai/dsh-tools/presentation' diff --git a/packages/client/connection/src/client/fixture.ts b/packages/client/connection/src/client/fixture.ts index 4e737e4e7f..d3106fef66 100644 --- a/packages/client/connection/src/client/fixture.ts +++ b/packages/client/connection/src/client/fixture.ts @@ -1545,6 +1545,11 @@ export function createFixtureApi(options: FixtureOptions = {}): ApiProxy { message: 'fixture: no settings namespaces are registered', details: { ns: request.payload.ns }, }), + mutate: request => err(request, { + code: 'settings-rejected', + message: 'fixture: no settings namespaces are registered', + details: { ns: request.payload.ns }, + }), }, credentials: { describe: request => ok(request, { @@ -1668,6 +1673,7 @@ export class FixtureApiClient extends AbstractApiClient { case 'settings.describe': return this.api.settings.describe(request) case 'settings.update': return this.api.settings.update(request) case 'settings.replace': return this.api.settings.replace(request) + case 'settings.mutate': return this.api.settings.mutate(request) case 'credentials.describe': return this.api.credentials.describe(request) case 'credentials.set': return this.api.credentials.set(request) case 'credentials.unset': return this.api.credentials.unset(request) diff --git a/packages/client/connection/src/client/index.ts b/packages/client/connection/src/client/index.ts index dabc85dc94..93f5153936 100644 --- a/packages/client/connection/src/client/index.ts +++ b/packages/client/connection/src/client/index.ts @@ -22,7 +22,7 @@ export type { ClientRequest, ServerResponse, ServerRequest, ClientResponse, RpcMessage, RpcReceipt, IApiClient, SessionId, SessionEvent, ContentBlock, StreamChunk, GoalsApi, GoalRef, - SettingsApi, SettingsNamespaceView, SettingsSecretView, + SettingsApi, SettingsNamespaceView, SettingsPathOpView, SettingsSecretView, CredentialsApi, CredentialView, ConfigurableProviderView, LlmApi, } from './api.ts' export { RpcId, AbstractApiClient, transportError } from './api.ts' diff --git a/packages/client/connection/src/index.ts b/packages/client/connection/src/index.ts index b14a591646..cedc7d86e7 100644 --- a/packages/client/connection/src/index.ts +++ b/packages/client/connection/src/index.ts @@ -35,16 +35,26 @@ export const Config: z = z.object({ /** * Methods gated to loopback even on a trusted-host deployment. Native dialogs - * act on the host machine; settings and credential writes mutate the user's - * configuration and secret store. A declared `trustedHosts` authority reaches - * every other method, but these stay loopback-same-origin until a real - * authentication layer exists. + * act on the host machine; the settings and credential domains mutate the + * user's configuration and secret store, and READING them is equally + * privileged — `settings.describe` returns every exposed namespace's + * configuration and `credentials.describe` reports whether an arbitrary + * environment-variable name is configured and where from, which is + * reconnaissance no anonymous caller should have. `trustedHosts` is a + * DNS-rebinding fence, explicitly not authentication, so the whole + * configuration plane stays loopback-same-origin until a real authentication + * layer exists. The model catalog (`llm.providers`, `llm.models`) is + * deliberately NOT here: it carries provider ids, display names, and model + * lists — no endpoints, keys, or key state — and a LAN client's model picker + * legitimately needs it. */ const PRIVILEGED_METHODS = new Set([ 'host.pickDirectory', 'host.openPath', + 'settings.describe', 'settings.update', 'settings.replace', + 'credentials.describe', 'credentials.set', 'credentials.unset', ]) diff --git a/packages/client/connection/tests/fake-api.ts b/packages/client/connection/tests/fake-api.ts index 56053009b6..2ed688d555 100644 --- a/packages/client/connection/tests/fake-api.ts +++ b/packages/client/connection/tests/fake-api.ts @@ -158,6 +158,7 @@ export class FakeApiClient implements IApiClient { describe: payload => this.record('settings.describe', payload, Promise.resolve(ok({ writable: true, namespaces: [] }))), update: payload => this.record('settings.update', payload, Promise.resolve(ok({ ns: 'fake', schema: {}, value: {}, applies: 'live' as const, secrets: [] }))), replace: payload => this.record('settings.replace', payload, Promise.resolve(ok({ ns: 'fake', schema: {}, value: {}, applies: 'live' as const, secrets: [] }))), + mutate: payload => this.record('settings.mutate', payload, Promise.resolve(ok({ ns: 'fake', schema: {}, value: {}, applies: 'live' as const, secrets: [] }))), } readonly credentials: IApiClient['credentials'] = { diff --git a/packages/client/connection/tests/node-half.spec.ts b/packages/client/connection/tests/node-half.spec.ts index db1c987575..6839d8b3ca 100644 --- a/packages/client/connection/tests/node-half.spec.ts +++ b/packages/client/connection/tests/node-half.spec.ts @@ -1,8 +1,10 @@ /** Node half: registers the /api prefix route bridging to the api gateway. */ import { EventEmitter } from 'node:events' +import { createServer, request as httpRequest } from 'node:http' import { Readable } from 'node:stream' import { Context } from 'cordis' import { describe, expect, it } from 'vitest' +import type { AddressInfo } from 'node:net' import type { IncomingMessage, ServerResponse } from 'node:http' import type { ApiProxy } from '@deepseek-ai/dsh-host-apiproxy/api' import type { HttpServerService, WebRoute } from '@deepseek-ai/dsh-host-webserver' @@ -99,14 +101,14 @@ describe('connection node half', () => { it('pins privileged methods to loopback even for a declared trusted authority', async () => { const { routes, dispose } = await mounted({ trustedHosts: ['harness.example'] }) - // The privileged set: native dialogs plus every settings/credential write. - // The same declared authority reaches ordinary reads (carrier-level 404 - // from the empty proxy proves the fence passed), but each privileged - // method stays loopback-only and short-circuits 403. + // The privileged set: native dialogs plus the whole settings/credential + // configuration plane, reads included. The same declared authority reaches + // ordinary reads (carrier-level 404 from the empty proxy proves the fence + // passed), but each privileged method stays loopback-only and 403s. for (const method of [ 'host.pickDirectory', 'host.openPath', - 'settings.update', 'settings.replace', - 'credentials.set', 'credentials.unset', + 'settings.describe', 'settings.update', 'settings.replace', + 'credentials.describe', 'credentials.set', 'credentials.unset', ]) { const denied = fakeResponse() await routes[0]!.handler( @@ -143,3 +145,69 @@ describe('connection node half', () => { await dispose() }) }) + +describe('connection node half over a real HTTP server', () => { + /** Serve the registered prefix route from a real server and return its port. */ + async function serve(routes: WebRoute[]): Promise<{ port: number; close: () => Promise }> { + const server = createServer((request, response) => { + void routes[0]!.handler(request, response) + }) + await new Promise(resolve => server.listen(0, '127.0.0.1', resolve)) + const address = server.address() as AddressInfo + return { + port: address.port, + close: () => new Promise((resolve, reject) => { + server.close((error) => { + if (error === undefined || error === null) resolve() + else reject(error) + }) + }), + } + } + + /** One real request; `host` spoofs the authority the way a LAN client's browser would send it. */ + function call(port: number, method: string, host: string): Promise { + return new Promise((resolve, reject) => { + const request = httpRequest( + { host: '127.0.0.1', port, path: `${API_PATH}/${method}`, method: 'GET', headers: { host } }, + (response) => { + response.resume() + response.on('end', () => { resolve(response.statusCode ?? 0) }) + }, + ) + request.on('error', reject) + request.end() + }) + } + + it('answers a declared LAN authority with 403 on every configuration method, over real HTTP', async () => { + // The fence's input is a real IncomingMessage parsed by Node from the + // wire, not a hand-assembled object: the Host header a LAN browser sends + // is exactly what decides loopback-only here, so the boundary is asserted + // against the parse the server actually performs. + const { routes, dispose } = await mounted({ trustedHosts: ['harness.example'] }) + const { port, close } = await serve(routes) + try { + // Reads are as privileged as writes: describe returns the exposed + // configuration, and credentials.describe probes arbitrary env-var names. + for (const method of [ + 'settings.describe', 'settings.update', 'settings.replace', + 'credentials.describe', 'credentials.set', 'credentials.unset', + 'host.pickDirectory', 'host.openPath', + ]) { + expect([method, await call(port, method, 'harness.example')]).toEqual([method, 403]) + } + // The model catalog stays reachable for the same authority: a LAN + // client's model picker needs it, and it carries no key or endpoint + // state (404 is the empty proxy's carrier answer — the fence passed). + for (const method of ['llm.providers', 'llm.models']) { + expect([method, await call(port, method, 'harness.example')]).toEqual([method, 404]) + } + // Loopback reaches everything, configuration included. + expect(await call(port, 'settings.describe', `127.0.0.1:${String(port)}`)).toBe(404) + } finally { + await close() + await dispose() + } + }) +}) diff --git a/packages/client/runtime/tests/fake-api.ts b/packages/client/runtime/tests/fake-api.ts index eaf215b216..480a6f034d 100644 --- a/packages/client/runtime/tests/fake-api.ts +++ b/packages/client/runtime/tests/fake-api.ts @@ -185,6 +185,7 @@ export class FakeApiClient implements IApiClient { describe: payload => this.record('settings.describe', payload, Promise.resolve(ok({ writable: true, namespaces: [] }))), update: payload => this.record('settings.update', payload, Promise.resolve(ok({ ns: 'fake', schema: {}, value: {}, applies: 'live' as const, secrets: [] }))), replace: payload => this.record('settings.replace', payload, Promise.resolve(ok({ ns: 'fake', schema: {}, value: {}, applies: 'live' as const, secrets: [] }))), + mutate: payload => this.record('settings.mutate', payload, Promise.resolve(ok({ ns: 'fake', schema: {}, value: {}, applies: 'live' as const, secrets: [] }))), } readonly credentials: IApiClient['credentials'] = { diff --git a/packages/client/ui-models/src/client/ModelsSection.tsx b/packages/client/ui-models/src/client/ModelsSection.tsx index d4f485b5cf..20ad4a4e01 100644 --- a/packages/client/ui-models/src/client/ModelsSection.tsx +++ b/packages/client/ui-models/src/client/ModelsSection.tsx @@ -11,7 +11,6 @@ import { useState } from 'react' import type { ReactNode } from 'react' import type { IApiClient, SettingsNamespaceView } from '@deepseek-ai/dsh-client-connection/client' -import { deletePath } from '@deepseek-ai/dsh-client-schema-form' import type { SnapshotSelectorHook } from '@deepseek-ai/dsh-client-web-react' import type { ModelsSettingsState, ModelsSettingsStore, ProviderRow } from './store.ts' import { ProviderEditor } from './ProviderEditor.tsx' @@ -45,24 +44,35 @@ interface EditorTarget { } /** - * Remove one user-added provider profile from its namespace's user section - * (wholesale replace — merge cannot express a removal) and reload on success. + * Remove one user-added provider profile by unsetting its path in the stored + * user section, then reload. The removal names the profile rather than + * rebuilding the section: this page only ever holds the redacted descriptor, + * so a rebuilt section would drop every literal secret stored elsewhere in + * the namespace along with the profile being removed. * @param api - settings wire face. * @param controller - the page store to refresh. * @param target - the provider's settings address. - * @param namespace - the owning namespace view. - * @returns settles when the write and any reload finished. + * @returns the failure message, or undefined once the write and reload landed. */ export async function removeProviderProfile( api: Pick, controller: ModelsSettingsStore, target: { settingsNs: string; settingsPath: readonly string[] }, - namespace: SettingsNamespaceView, -): Promise { - const user = structuredClone((namespace.user ?? {}) as Record) - const next = deletePath(user, [...target.settingsPath]) - const response = await api.settings.replace({ ns: target.settingsNs, section: next }) - if (response.result.ok) await controller.load() +): Promise { + let response + try { + response = await api.settings.mutate({ + ns: target.settingsNs, + ops: [{ op: 'unset', path: [...target.settingsPath] }], + }) + } catch (error) { + // The transport rejected rather than answering; the caller must be able + // to say so instead of the row silently staying put. + return error instanceof Error ? error.message : String(error) + } + if (!response.result.ok) return response.result.error.message + await controller.load() + return undefined } /** @@ -184,7 +194,11 @@ function Loaded({ injected }: { injected: ModelsSectionInjected }): ReactNode { type="button" className={styles['dangerButton']} disabled={!state.writable} - onClick={() => { void removeProviderProfile(api, controller, target, namespace) }} + onClick={() => { + void removeProviderProfile(api, controller, target).then((failure) => { + if (failure !== undefined) controller.fail(failure) + }) + }} > {t('remove')} diff --git a/packages/client/ui-models/src/client/ProviderEditor.tsx b/packages/client/ui-models/src/client/ProviderEditor.tsx index 8ea1fb22fe..7178a75de2 100644 --- a/packages/client/ui-models/src/client/ProviderEditor.tsx +++ b/packages/client/ui-models/src/client/ProviderEditor.tsx @@ -6,15 +6,15 @@ * has none, and the pi-ai profile records that derivation as `apiKeyEnv`); * the collapsed 自定义设置 area carries the per-family extras (`baseURL` for * both families, plus `reasoningEffort` for deepseek / `reasoning` for - * pi-ai). Everything else stays owned by `settings.yaml`. Profile edits land as a - * minimal `settings.update` merge patch; clearing a field back to inherited - * removes its key, so that apply replaces the user section (safe: the section - * stores references, never key values). + * pi-ai). Everything else stays owned by `settings.yaml`. Profile edits land as + * minimal `settings.mutate` path ops against the stored section — the card + * reads the redacted descriptor, so it names only the fields it can see and a + * stored literal secret is never collaterally removed. */ import { useEffect, useMemo, useState } from 'react' import type { ReactNode } from 'react' -import type { CredentialView, IApiClient, SettingsNamespaceView } from '@deepseek-ai/dsh-client-connection/client' +import type { CredentialView, IApiClient, SettingsNamespaceView, SettingsPathOpView } from '@deepseek-ai/dsh-client-connection/client' import { deletePath, getPath, nodeAtPath, rehydrateSchema, setPath, validateDraft, } from '@deepseek-ai/dsh-client-schema-form' @@ -70,21 +70,33 @@ function draftAt(namespace: SettingsNamespaceView, path: readonly string[]): Rec } /** - * Whether any key present in `before` is absent from `after` (a reset - * happened somewhere in the draft, so the apply must replace, not merge). - * @param before - the user-layer subtree the draft started from. - * @param after - the edited draft. - * @returns whether a removal exists at any depth. + * The minimal path ops carrying `after` over `before`, both as the card sees + * them (that is, redacted). Only keys the card observed are named: a stored + * `role('secret')` field appears in neither side, so it produces no op and + * survives the write — the whole reason edits are path-addressed rather than + * a rebuilt section. + * @param base - path of the edited subtree inside the user section. + * @param before - the subtree as loaded, or undefined when it is new. + * @param after - the subtree as edited. + * @returns ordered set/unset ops; empty when nothing changed. */ -export function removedAny(before: unknown, after: unknown): boolean { - if (typeof before !== 'object' || before === null) return false - /* v8 ignore next -- the editor edits containers in place; a container cannot become a primitive */ - if (typeof after !== 'object' || after === null) return true - for (const [key, value] of Object.entries(before)) { - if (!(key in (after as Record))) return true - if (removedAny(value, (after as Record)[key])) return true +export function pathOps( + base: readonly string[], + before: unknown, + after: Record, +): SettingsPathOpView[] { + const previous = typeof before === 'object' && before !== null && !Array.isArray(before) + ? before as Record + : {} + const ops: SettingsPathOpView[] = [] + for (const [key, value] of Object.entries(after)) { + if (JSON.stringify(previous[key]) === JSON.stringify(value)) continue + ops.push({ op: 'set', path: [...base, key], value }) } - return false + for (const key of Object.keys(previous)) { + if (!(key in after)) ops.push({ op: 'unset', path: [...base, key] }) + } + return ops } /** The editor layout the owning namespace selects. */ @@ -140,9 +152,14 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode { setDraft(current => next === undefined ? deletePath(current, [key]) : setPath(current, [key], next)) } - const apply = async (): Promise => { - setBusy(true) - setFailure(undefined) + /** + * The write for this card, or a failure message. Every edit travels as + * path ops against the STORED section: the draft comes from the redacted + * descriptor, so a wholesale replace rebuilt from it would delete the + * literal secrets the wire never returned. Ops name only the fields this + * card can see, so a stored secret is untouched by construction. + */ + const applyOnce = async (): Promise => { const ns = namespace.ns const original = getPath(namespace.user, settingsPath) // The pi-ai profile must name the reference the key stores under, so a @@ -151,45 +168,42 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode { && stringAt(fallback, 'apiKeyEnv') === undefined ? setPath(draft, ['apiKeyEnv'], keyRef) : draft - const settingsChanged = JSON.stringify(next) !== JSON.stringify(original ?? {}) - if (settingsChanged) { - const needsReplace = removedAny(original, next) - // Merge patches stay minimal (just this profile); a replace must carry - // the complete next user section because it lands wholesale. - const patch = settingsPath.length === 0 ? next : setPath({}, [...settingsPath], next) - /* v8 ignore next 3 -- a subtree apply implies the join served this namespace's user layer */ - const nextSection = settingsPath.length === 0 - ? next - : setPath(structuredClone((namespace.user ?? {}) as Record), [...settingsPath], next) - /* v8 ignore next -- apply is only reachable from the rendered card, which required a resolved node */ - if (node !== undefined) { - const sectionError = settingsPath.length === 0 ? validateDraft(node, next) : undefined - if (sectionError !== undefined) { - setBusy(false) - setFailure(sectionError) - return - } - } - const response = needsReplace - ? await api.settings.replace({ ns, section: nextSection }) - : await api.settings.update({ ns, patch }) - if (!response.result.ok) { - setBusy(false) - setFailure(response.result.error.message) - return - } + /* v8 ignore next -- apply is only reachable from the rendered card, which required a resolved node */ + if (node !== undefined && settingsPath.length === 0) { + const sectionError = validateDraft(node, next) + if (sectionError !== undefined) return sectionError + } + const ops = pathOps(settingsPath, original, next) + if (ops.length > 0) { + const response = await api.settings.mutate({ ns, ops }) + if (!response.result.ok) return response.result.error.message } if (keyDraft.length > 0) { const stored = await api.credentials.set({ ref: keyRef, value: keyDraft }) - if (!stored.result.ok) { - setBusy(false) - setFailure(stored.result.error.message) + if (!stored.result.ok) return stored.result.error.message + } + setKeyDraft('') + return undefined + } + + const apply = async (): Promise => { + setBusy(true) + setFailure(undefined) + try { + const failure = await applyOnce() + if (failure !== undefined) { + setFailure(failure) return } - setKeyDraft('') + props.onClose(true) + } catch (error) { + // A transport failure (disconnect, a request the host refuses) rejects + // rather than answering; without this the card would stay busy forever + // with no error shown. + setFailure(error instanceof Error ? error.message : String(error)) + } finally { + setBusy(false) } - setBusy(false) - props.onClose(true) } if (node === undefined) { diff --git a/packages/client/ui-models/src/client/store.ts b/packages/client/ui-models/src/client/store.ts index c84d857cad..89625228ea 100644 --- a/packages/client/ui-models/src/client/store.ts +++ b/packages/client/ui-models/src/client/store.ts @@ -75,6 +75,18 @@ export class ModelsSettingsStore { */ constructor(private readonly api: Pick) {} + /** + * Surface a failure from an operation the page ran outside {@link load} — + * a row removal — on the same banner a load failure uses. + * @param message - the failure text to show. + */ + fail(message: string): void { + this.store.update((s) => { + s.status = 'error' + s.error = message + }) + } + /** * Refresh the whole page snapshot: directory and namespaces in parallel, * then one batched credential describe over every referenced ref. A @@ -125,10 +137,12 @@ export class ModelsSettingsStore { const refs = [...new Set(rows.flatMap(row => row.apiKeyEnv === undefined ? [] : [row.apiKeyEnv]))] let credentials: Record = {} if (refs.length > 0) { - const response = await this.api.credentials.describe({ refs }) - // Credential state is an enrichment: rows render without it, so a - // missing credential provider degrades the badge, not the page. - if (response.result.ok) credentials = response.result.value.credentials + // Credential state is an enrichment: rows render without it, so neither + // a business rejection nor a transport failure (disconnect, a request + // the host refuses) may fail the load — an escaping rejection would + // leave the page stuck in `loading` with no error shown. + const response = await this.api.credentials.describe({ refs }).catch(() => undefined) + if (response?.result.ok === true) credentials = response.result.value.credentials } if (generation !== this.generation) return this.store.update((s) => { diff --git a/packages/client/ui-models/tests/components.spec.tsx b/packages/client/ui-models/tests/components.spec.tsx index 79bc4a7c88..24980bbccf 100644 --- a/packages/client/ui-models/tests/components.spec.tsx +++ b/packages/client/ui-models/tests/components.spec.tsx @@ -7,7 +7,7 @@ import { bindSnapshotSelector } from '@deepseek-ai/dsh-client-web-react' import type { RpcResponse, SettingsNamespaceView } from '@deepseek-ai/dsh-client-connection/client' import { ModelsSection, needsSetup, removeProviderProfile } from '../src/client/ModelsSection.tsx' import type { ModelsSectionInjected } from '../src/client/ModelsSection.tsx' -import { removedAny } from '../src/client/ProviderEditor.tsx' +import { pathOps } from '../src/client/ProviderEditor.tsx' import { deriveKeyRef, ModelsSettingsStore } from '../src/client/store.ts' import type { ProviderRow } from '../src/client/store.ts' import { en } from '../src/client/locales.ts' @@ -79,10 +79,12 @@ function fail(message: string, code = 'settings-rejected'): RpcResponse { function scriptedFace(overrides: { update?: ReturnType replace?: ReturnType + mutate?: ReturnType set?: ReturnType } = {}) { const update = overrides.update ?? vi.fn(() => Promise.resolve(ok(wireNamespaces()[2]))) const replace = overrides.replace ?? vi.fn(() => Promise.resolve(ok(wireNamespaces()[2]))) + const mutate = overrides.mutate ?? vi.fn(() => Promise.resolve(ok(wireNamespaces()[2]))) const set = overrides.set ?? vi.fn(() => Promise.resolve(ok({}))) const face = { llm: { @@ -102,6 +104,7 @@ function scriptedFace(overrides: { describe: vi.fn(() => Promise.resolve(ok({ writable: true, namespaces: wireNamespaces() }))), update, replace, + mutate, }, credentials: { describe: vi.fn((payload: { refs: string[] }) => Promise.resolve(ok({ @@ -115,13 +118,13 @@ function scriptedFace(overrides: { unset: vi.fn(() => Promise.resolve(ok({}))), }, } - return { face, update, replace, set } + return { face, update, replace, mutate, set } } type WireFace = ConstructorParameters[0] async function mountSection(overrides: Parameters[0] = {}) { - const { face, update, replace, set } = scriptedFace(overrides) + const { face, update, replace, mutate, set } = scriptedFace(overrides) const controller = new ModelsSettingsStore(face as unknown as WireFace) await controller.load() const injected: ModelsSectionInjected = { @@ -131,7 +134,7 @@ async function mountSection(overrides: Parameters[0] = {}) t, } const view = render() - return { view, face, update, replace, set, controller } + return { view, face, update, replace, mutate, set, controller } } describe('ModelsSection', () => { @@ -184,10 +187,15 @@ describe('ModelsSection', () => { expect(deriveKeyRef('minimax-cn')).toBe('MINIMAX_CN_API_KEY') }) - it('detects removals at any draft depth', () => { - expect(removedAny({ a: { b: 1, c: 2 } }, { a: { b: 1 } })).toBe(true) - expect(removedAny({ a: { b: 1 } }, { a: { b: 2 }, d: 3 })).toBe(false) - expect(removedAny(undefined, {})).toBe(false) + it('names only the fields the card can see, so an unseen secret survives', () => { + // `before` is the REDACTED subtree: a stored literal apiKey is in neither + // side, so no op mentions it and the seam leaves it alone. + expect(pathOps(['providers', 'openai'], { baseURL: 'https://old', reasoning: 'high' }, { reasoning: 'high' })) + .toEqual([{ op: 'unset', path: ['providers', 'openai', 'baseURL'] }]) + expect(pathOps([], { b: 1 }, { b: 2, d: 3 })) + .toEqual([{ op: 'set', path: ['b'], value: 2 }, { op: 'set', path: ['d'], value: 3 }]) + expect(pathOps([], undefined, {})).toEqual([]) + expect(pathOps([], { a: 1 }, { a: 1 })).toEqual([]) }) it('stores a typed key write-only from the setup card without touching settings', async () => { @@ -200,9 +208,9 @@ describe('ModelsSection', () => { await waitFor(() => { expect(face.settings.describe.mock.calls.length).toBeGreaterThan(1) }) }) - it('applies customized deepseek fields as a merge patch', async () => { - const { update } = await mountSection({ - update: vi.fn(() => Promise.resolve(ok(wireNamespaces()[0]))), + it('applies customized deepseek fields as path ops', async () => { + const { mutate } = await mountSection({ + mutate: vi.fn(() => Promise.resolve(ok(wireNamespaces()[0]))), }) fireEvent.click(screen.getByText(en.customized)) const baseURL = screen.getByLabelText(en.baseUrl) @@ -211,23 +219,31 @@ describe('ModelsSection', () => { expect(baseURL.placeholder).toBe('https://api.deepseek.com') fireEvent.change(baseURL, { target: { value: 'https://next2' } }) fireEvent.click(screen.getByText(en.apply)) - await waitFor(() => { expect(update).toHaveBeenCalledTimes(1) }) - expect(update.mock.calls[0]?.[0]).toEqual({ + await waitFor(() => { expect(mutate).toHaveBeenCalledTimes(1) }) + // Only the field that actually changed: reasoningEffort was already + // 'high' in the loaded profile, so it produces no op. + expect(mutate.mock.calls[0]?.[0]).toEqual({ ns: 'llm-deepseek', - patch: { reasoningEffort: 'high', baseURL: 'https://next2' }, + ops: [{ op: 'set', path: ['baseURL'], value: 'https://next2' }], }) }) - it('clears an inherited override through replace so the removal lands', async () => { - const { replace, update } = await mountSection() + it('clears an inherited override with an unset op, never a whole-section replace', async () => { + // The data-loss shape: the old path rebuilt the section from the REDACTED + // user layer and replaced it wholesale, deleting any stored literal key. + const { replace, update, mutate } = await mountSection() fireEvent.click(screen.getByText(en.customized)) const effort = screen.getByLabelText(en.effort) expect(effort.value).toBe('high') fireEvent.change(effort, { target: { value: '' } }) fireEvent.click(screen.getByText(en.apply)) - await waitFor(() => { expect(replace).toHaveBeenCalledTimes(1) }) + await waitFor(() => { expect(mutate).toHaveBeenCalledTimes(1) }) + expect(replace).not.toHaveBeenCalled() expect(update).not.toHaveBeenCalled() - expect(replace.mock.calls[0]?.[0]).toEqual({ ns: 'llm-deepseek', section: {} }) + expect(mutate.mock.calls[0]?.[0]).toEqual({ + ns: 'llm-deepseek', + ops: [{ op: 'unset', path: ['reasoningEffort'] }], + }) }) it('pins the deepseek placeholder and clears typed input back to inherited', async () => { @@ -269,7 +285,7 @@ describe('ModelsSection', () => { }) it('edits a pi-ai profile with the curated fields only', async () => { - const { update } = await mountSection() + const { mutate } = await mountSection() fireEvent.click(screen.getAllByText(en.edit)[0] as HTMLElement) // The configured credential shows as the stored placeholder. const keys = await screen.findAllByLabelText(en.keyInput) @@ -284,19 +300,19 @@ describe('ModelsSection', () => { const effort = screen.getAllByLabelText(en.effort) fireEvent.change(effort[effort.length - 1] as HTMLSelectElement, { target: { value: 'xhigh' } }) fireEvent.click(screen.getAllByText(en.apply)[1] as HTMLElement) - await waitFor(() => { expect(update).toHaveBeenCalledTimes(1) }) - expect(update.mock.calls[0]?.[0]).toEqual({ + await waitFor(() => { expect(mutate).toHaveBeenCalledTimes(1) }) + // Only the edited field travels: apiKeyEnv, baseURL and headers were + // already stored with these values, so no op restates them — and the + // profile's stored literal apiKey, absent from the redacted view the card + // read, is named by nothing at all. + expect(mutate.mock.calls[0]?.[0]).toEqual({ ns: 'llm-pi-ai', - patch: { - providers: { - openai: { apiKeyEnv: 'OPENAI_API_KEY', baseURL: 'https://proxy', headers: { 'X-Team': 'a' }, reasoning: 'xhigh' }, - }, - }, + ops: [{ op: 'set', path: ['providers', 'openai', 'reasoning'], value: 'xhigh' }], }) }) it('adds a dormant provider with a derived reference and stores its key', async () => { - const { update, set } = await mountSection() + const { mutate, set } = await mountSection() fireEvent.click(screen.getByText(`+ ${en.add}`)) const pick = await screen.findByLabelText(en.provider) expect([...pick.options].map(option => option.value)).toEqual(['anthropic', 'broken', 'plain']) @@ -310,10 +326,10 @@ describe('ModelsSection', () => { const addKey = keys[keys.length - 1] as HTMLInputElement fireEvent.change(addKey, { target: { value: 'sk-ant' } }) fireEvent.click(screen.getAllByText(en.apply)[1] as HTMLElement) - await waitFor(() => { expect(update).toHaveBeenCalledTimes(1) }) - expect(update.mock.calls[0]?.[0]).toEqual({ + await waitFor(() => { expect(mutate).toHaveBeenCalledTimes(1) }) + expect(mutate.mock.calls[0]?.[0]).toEqual({ ns: 'llm-pi-ai', - patch: { providers: { anthropic: { apiKeyEnv: 'ANTHROPIC_API_KEY' } } }, + ops: [{ op: 'set', path: ['providers', 'anthropic', 'apiKeyEnv'], value: 'ANTHROPIC_API_KEY' }], }) await waitFor(() => { expect(set).toHaveBeenCalledWith({ ref: 'ANTHROPIC_API_KEY', value: 'sk-ant' }) }) }) @@ -336,7 +352,7 @@ describe('ModelsSection', () => { it('surfaces a rejected settings write and never stores the key after it', async () => { const { set } = await mountSection({ - update: vi.fn(() => Promise.resolve(fail('llm-pi-ai: unknown pi-ai provider "bogus"'))), + mutate: vi.fn(() => Promise.resolve(fail('llm-pi-ai: unknown pi-ai provider "bogus"'))), }) fireEvent.click(screen.getByText(`+ ${en.add}`)) await screen.findByLabelText(en.provider) @@ -383,11 +399,15 @@ describe('ModelsSection', () => { await waitFor(() => { expect(set).toHaveBeenCalledTimes(1) }) }) - it('removes a user-added provider through replace', async () => { - const { replace } = await mountSection() + it('removes a user-added provider by unsetting its path', async () => { + const { replace, mutate } = await mountSection() fireEvent.click(screen.getAllByText(en.remove)[0] as HTMLElement) - await waitFor(() => { expect(replace).toHaveBeenCalledTimes(1) }) - expect(replace.mock.calls[0]?.[0]).toEqual({ ns: 'llm-pi-ai', section: { providers: { zombie: {} } } }) + await waitFor(() => { expect(mutate).toHaveBeenCalledTimes(1) }) + expect(replace).not.toHaveBeenCalled() + expect(mutate.mock.calls[0]?.[0]).toEqual({ + ns: 'llm-pi-ai', + ops: [{ op: 'unset', path: ['providers', 'openai'] }], + }) }) it('renders the load failure with a retry control', async () => { @@ -461,30 +481,45 @@ describe('ModelsSection', () => { await screen.findByText('DeepSeek') }) - it('removes against a namespace with no user layer as an empty-section replace', async () => { - const { face, replace, controller } = await mountSection() - const namespace = controller.store.getSnapshot().namespaces.get('llm-plain') + it('removes by unsetting the profile path, never by rebuilding the section', async () => { + // The section rebuild is what dropped stored literal secrets: this page + // only ever holds the redacted descriptor, so the removal names the path. + const { face, mutate, replace, controller } = await mountSection() await removeProviderProfile( face as unknown as Parameters[0], controller, { settingsNs: 'llm-plain', settingsPath: ['ghost-profile'] }, - namespace as NonNullable, ) - expect(replace.mock.calls[0]?.[0]).toEqual({ ns: 'llm-plain', section: {} }) + expect(mutate.mock.calls[0]?.[0]).toEqual({ + ns: 'llm-plain', + ops: [{ op: 'unset', path: ['ghost-profile'] }], + }) + expect(replace).not.toHaveBeenCalled() }) - it('keeps the snapshot untouched when a removal write is refused', async () => { + it('keeps the snapshot untouched and reports the message when a removal write is refused', async () => { const { face, controller } = await mountSection({ - replace: vi.fn(() => Promise.resolve(fail('read-only'))), + mutate: vi.fn(() => Promise.resolve(fail('read-only'))), }) - const namespace = controller.store.getSnapshot().namespaces.get('llm-pi-ai') const before = controller.store.getSnapshot().rows - await removeProviderProfile( + const failure = await removeProviderProfile( face as unknown as Parameters[0], controller, { settingsNs: 'llm-pi-ai', settingsPath: ['providers', 'openai'] }, - namespace as NonNullable, ) + expect(failure).toBe('read-only') expect(controller.store.getSnapshot().rows).toBe(before) }) + + it('reports a transport rejection instead of failing the removal silently', async () => { + const { face, controller } = await mountSection({ + mutate: vi.fn(() => Promise.reject(new Error('connection lost'))), + }) + const failure = await removeProviderProfile( + face as unknown as Parameters[0], + controller, + { settingsNs: 'llm-pi-ai', settingsPath: ['providers', 'openai'] }, + ) + expect(failure).toBe('connection lost') + }) }) diff --git a/packages/host/apiproxy/src/api-proxy.ts b/packages/host/apiproxy/src/api-proxy.ts index 3b919f79d2..c707f9fa71 100644 --- a/packages/host/apiproxy/src/api-proxy.ts +++ b/packages/host/apiproxy/src/api-proxy.ts @@ -42,7 +42,7 @@ import type {} from '@deepseek-ai/dsh-skill' // service reads stay optional (`ctx.get`) so a composition without either // provider still serves every other domain. import { settingsNamespace } from '@deepseek-ai/dsh-settings' -import type { SettingsDescriptor, SettingsNamespace } from '@deepseek-ai/dsh-settings' +import type { SettingsDescriptor, SettingsNamespace, SettingsPathOp } from '@deepseek-ai/dsh-settings' import { credentialRef } from '@deepseek-ai/dsh-credentials' // Value edge: the rename impl narrows the title service's validation failure; the import also resolves `ctx.get('sessionTitle')`. import { SessionTitleInvalidError } from '@deepseek-ai/dsh-session-title' @@ -1010,16 +1010,39 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro } } + /** + * The settings namespaces this proxy serves: exactly those a registered + * configurable provider addresses. The settings seam itself is general — + * any plugin may register a namespace for its own configuration — but the + * Web configuration plane is scoped to model providers, and that boundary + * has to be enforced here rather than assumed from the current plugin set. + * Without it, every future `settings.register()` would silently become + * remotely readable and writable configuration. + */ + function exposedNamespaces(): Set { + return new Set(ctx.llm.listConfigurableProviders().map(entry => entry.settingsNs)) + } + + /** Refuse a namespace outside the model-provider boundary, naming why. */ + function notExposed(request: RpcRequest, ns: string): RpcResponse { + return err(request, { + code: 'settings-not-exposed', + message: `settings namespace "${ns}" is not exposed to configuration clients; only a namespace a registered model provider addresses is`, + details: { ns }, + }) + } + /** * Run one settings write (merge or wholesale replace) and acknowledge with - * the namespace's new redacted view. Every seam refusal — unknown or - * invalid namespace, read-only provider, schema validation, storage — - * becomes one `settings-rejected` carrying the seam's own message. + * the namespace's new redacted view. A namespace outside the model-provider + * boundary is refused before the seam is touched; every seam refusal — + * unknown or invalid namespace, read-only provider, schema validation, + * storage — becomes one `settings-rejected` carrying the seam's own message. */ async function settingsWrite( request: RpcRequest, ns: string, - mode: 'update' | 'replace', + mode: 'update' | 'replace' | 'mutate', section: object, ): Promise> { const settings = ctx.get('settings') @@ -1033,11 +1056,15 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro try { branded = settingsNamespace(ns) } catch (error: unknown) { + // A malformed name is a client bug, reported as such; it could never be + // in the exposed set either, so naming the real fault costs no ground. return rejected(error) } + if (!exposedNamespaces().has(ns)) return notExposed(request, ns) try { if (mode === 'update') await settings.update(branded, section) - else await settings.replace(branded, section) + else if (mode === 'replace') await settings.replace(branded, section) + else await settings.mutate(branded, section as SettingsPathOp[]) } catch (error: unknown) { return rejected(error) } @@ -1632,13 +1659,17 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro describe(request) { const settings = ctx.get('settings') if (settings === undefined) return Promise.resolve(err(request, settingsAbsent())) + const exposed = exposedNamespaces() return Promise.resolve(ok(request, { writable: settings.writable, - namespaces: settings.describe({ redactSecrets: true }).map(namespaceView), + namespaces: settings.describe({ redactSecrets: true }) + .filter(descriptor => exposed.has(String(descriptor.ns))) + .map(namespaceView), })) }, update: request => settingsWrite(request, request.payload.ns, 'update', request.payload.patch), replace: request => settingsWrite(request, request.payload.ns, 'replace', request.payload.section), + mutate: request => settingsWrite(request, request.payload.ns, 'mutate', request.payload.ops), }, credentials: { diff --git a/packages/host/apiproxy/src/api/index.ts b/packages/host/apiproxy/src/api/index.ts index 9c5269840e..4eda35007d 100644 --- a/packages/host/apiproxy/src/api/index.ts +++ b/packages/host/apiproxy/src/api/index.ts @@ -43,7 +43,7 @@ export type { CommandsApi, CommandDescriptor } from './commands.ts' export type { SkillsApi, SkillEntry } from './skills.ts' export type { EventsApi, MuxFrame, HostFrame, QueuedInboxItem, ToolCallView, ToolEventView, ToolResultView } from './events.ts' export type { GoalsApi, GoalId, GoalRef } from './goals.ts' -export type { SettingsApi, SettingsNamespaceView, SettingsSecretView } from './settings.ts' +export type { SettingsApi, SettingsNamespaceView, SettingsPathOpView, SettingsSecretView } from './settings.ts' export type { CredentialsApi, CredentialView } from './credentials.ts' export type { ConfigurableProviderView, LlmApi } from './llm.ts' export type { ApprovalResponsePayload } from './approvals.ts' diff --git a/packages/host/apiproxy/src/api/rpc-map.ts b/packages/host/apiproxy/src/api/rpc-map.ts index 409f463dbb..911e3008eb 100644 --- a/packages/host/apiproxy/src/api/rpc-map.ts +++ b/packages/host/apiproxy/src/api/rpc-map.ts @@ -52,6 +52,7 @@ export interface RpcMethodMap { 'settings.describe': SettingsApi['describe'] 'settings.update': SettingsApi['update'] 'settings.replace': SettingsApi['replace'] + 'settings.mutate': SettingsApi['mutate'] 'credentials.describe': CredentialsApi['describe'] 'credentials.set': CredentialsApi['set'] 'credentials.unset': CredentialsApi['unset'] diff --git a/packages/host/apiproxy/src/api/rpc.schema.ts b/packages/host/apiproxy/src/api/rpc.schema.ts index 68e4b3fc66..85ecc1618b 100644 --- a/packages/host/apiproxy/src/api/rpc.schema.ts +++ b/packages/host/apiproxy/src/api/rpc.schema.ts @@ -51,6 +51,7 @@ export const rpcErrorSchema: z.ZodType = z.discriminatedUnion('code', z.object({ code: z.literal('command-error'), message: z.string(), details: z.object({}) }), z.object({ code: z.literal('unknown-command'), message: z.string(), details: z.object({}) }), z.object({ code: z.literal('settings-rejected'), message: z.string(), details: z.object({ ns: z.string() }) }), + z.object({ code: z.literal('settings-not-exposed'), message: z.string(), details: z.object({ ns: z.string() }) }), z.object({ code: z.literal('credential-rejected'), message: z.string(), details: z.object({ ref: z.string() }) }), z.object({ code: z.literal('title-invalid'), message: z.string(), details: z.object({ sessionId: z.string() }) }), z.object({ code: z.literal('internal'), message: z.string(), details: z.object({}) }), diff --git a/packages/host/apiproxy/src/api/rpc.ts b/packages/host/apiproxy/src/api/rpc.ts index 572ca66efd..d215f7f6fa 100644 --- a/packages/host/apiproxy/src/api/rpc.ts +++ b/packages/host/apiproxy/src/api/rpc.ts @@ -55,6 +55,12 @@ export interface RpcErrorDetailsMap { * read-only provider, or storage failure); the message is the seam's text. */ 'settings-rejected': { ns: string } + /** + * A settings namespace exists in the seam but is outside the configuration + * plane's model-provider boundary, so this proxy neither reads nor writes + * it; the message names the namespace. + */ + 'settings-not-exposed': { ns: string } /** A credential write was refused (read-only shadowing layer or storage failure); the message is the seam's own text. */ 'credential-rejected': { ref: string } 'title-invalid': { sessionId: SessionId } diff --git a/packages/host/apiproxy/src/api/settings.schema.ts b/packages/host/apiproxy/src/api/settings.schema.ts index 105573b109..2def419e1b 100644 --- a/packages/host/apiproxy/src/api/settings.schema.ts +++ b/packages/host/apiproxy/src/api/settings.schema.ts @@ -6,7 +6,7 @@ import { z } from 'zod' import type { RequestPayload, ResponseValue } from './rpc-map.ts' import type { Wire } from './rpc.schema.ts' -import type { SettingsNamespaceView, SettingsSecretView } from './settings.ts' +import type { SettingsNamespaceView, SettingsPathOpView, SettingsSecretView } from './settings.ts' /** One redacted secret slot. */ export const settingsSecretViewSchema = z.object({ @@ -49,5 +49,20 @@ export const settingsReplaceRequestSchema = z.object({ section: z.record(z.string(), z.unknown()), }) satisfies z.ZodType>> +/** One path-addressed edit of settings.mutate. */ +export const settingsPathOpSchema = z.discriminatedUnion('op', [ + z.object({ op: z.literal('set'), path: z.array(z.string()), value: z.unknown() }), + z.object({ op: z.literal('unset'), path: z.array(z.string()) }), +]) as unknown as z.ZodType> + +/** settings.mutate request payload. */ +export const settingsMutateRequestSchema = z.object({ + ns: z.string().min(1), + ops: z.array(settingsPathOpSchema), +}) satisfies z.ZodType>> + +/** settings.mutate response value: the namespace's new redacted view. */ +export const settingsMutateValueSchema = settingsNamespaceViewSchema satisfies z.ZodType>> + /** settings.replace response value. */ export const settingsReplaceValueSchema = settingsNamespaceViewSchema satisfies z.ZodType>> diff --git a/packages/host/apiproxy/src/api/settings.ts b/packages/host/apiproxy/src/api/settings.ts index 27e4d11156..7bad8c566e 100644 --- a/packages/host/apiproxy/src/api/settings.ts +++ b/packages/host/apiproxy/src/api/settings.ts @@ -34,6 +34,15 @@ export interface SettingsNamespaceView { secrets: SettingsSecretView[] } +/** + * One path-addressed edit carried by `settings.mutate`. `set` writes the + * value at the path (creating intermediate objects); `unset` removes it. The + * empty path addresses the section root. + */ +export type SettingsPathOpView = + | { op: 'set'; path: string[]; value: unknown } + | { op: 'unset'; path: string[] } + /** Settings-domain unary methods (the map keys settings.* of RpcMethodMap). */ export interface SettingsApi { /** @@ -60,4 +69,14 @@ export interface SettingsApi { * keep) or accept the reset. */ replace(request: RpcRequest<{ ns: string; section: object }>): Promise> + + /** + * Apply path-addressed edits to one namespace's user section, resolved + * against the section as stored — NOT against whatever the caller last + * read. This is the removal path for any client holding the redacted + * descriptor: it names the field it means, so a secret the wire never + * returned cannot be deleted as a side effect. `replace` remains the + * deliberate wholesale reset. + */ + mutate(request: RpcRequest<{ ns: string; ops: SettingsPathOpView[] }>): Promise> } diff --git a/packages/host/apiproxy/src/fetch/client.ts b/packages/host/apiproxy/src/fetch/client.ts index d833d462e5..6fc52ab3fe 100644 --- a/packages/host/apiproxy/src/fetch/client.ts +++ b/packages/host/apiproxy/src/fetch/client.ts @@ -46,7 +46,7 @@ import { goalClearValueSchema, } from '../api/goals.schema.ts' import { - settingsDescribeValueSchema, settingsReplaceValueSchema, settingsUpdateValueSchema, + settingsDescribeValueSchema, settingsMutateValueSchema, settingsReplaceValueSchema, settingsUpdateValueSchema, } from '../api/settings.schema.ts' import { credentialsDescribeValueSchema, credentialsSetValueSchema, credentialsUnsetValueSchema, @@ -117,6 +117,7 @@ export interface IApiClient { describe(payload: RequestPayload<'settings.describe'>, signal?: AbortSignal): Promise>> update(payload: RequestPayload<'settings.update'>, signal?: AbortSignal): Promise>> replace(payload: RequestPayload<'settings.replace'>, signal?: AbortSignal): Promise>> + mutate(payload: RequestPayload<'settings.mutate'>, signal?: AbortSignal): Promise>> } credentials: { describe(payload: RequestPayload<'credentials.describe'>, signal?: AbortSignal): Promise>> @@ -167,6 +168,7 @@ const UNARY_VALUE_SCHEMAS: { [K in keyof RpcMethodMap]: z.ZodType this.callUnary('settings.describe', payload, signal), update: (payload, signal) => this.callUnary('settings.update', payload, signal), replace: (payload, signal) => this.callUnary('settings.replace', payload, signal), + mutate: (payload, signal) => this.callUnary('settings.mutate', payload, signal), } readonly credentials: IApiClient['credentials'] = { diff --git a/packages/host/apiproxy/src/fetch/handler.ts b/packages/host/apiproxy/src/fetch/handler.ts index ef22a076d8..970cc16cfb 100644 --- a/packages/host/apiproxy/src/fetch/handler.ts +++ b/packages/host/apiproxy/src/fetch/handler.ts @@ -48,7 +48,7 @@ import { goalClearRequestSchema, } from '../api/goals.schema.ts' import { - settingsDescribeRequestSchema, settingsReplaceRequestSchema, settingsUpdateRequestSchema, + settingsDescribeRequestSchema, settingsMutateRequestSchema, settingsReplaceRequestSchema, settingsUpdateRequestSchema, } from '../api/settings.schema.ts' import { credentialsDescribeRequestSchema, credentialsSetRequestSchema, credentialsUnsetRequestSchema, @@ -103,6 +103,7 @@ const UNARY_ROUTES: UnaryRoutes = { 'settings.describe': { schema: settingsDescribeRequestSchema, invoke: (api, r) => api.settings.describe(r) }, 'settings.update': { schema: settingsUpdateRequestSchema, invoke: (api, r) => api.settings.update(r) }, 'settings.replace': { schema: settingsReplaceRequestSchema, invoke: (api, r) => api.settings.replace(r) }, + 'settings.mutate': { schema: settingsMutateRequestSchema, invoke: (api, r) => api.settings.mutate(r) }, 'credentials.describe': { schema: credentialsDescribeRequestSchema, invoke: (api, r) => api.credentials.describe(r) }, 'credentials.set': { schema: credentialsSetRequestSchema, invoke: (api, r) => api.credentials.set(r) }, 'credentials.unset': { schema: credentialsUnsetRequestSchema, invoke: (api, r) => api.credentials.unset(r) }, diff --git a/packages/host/apiproxy/tests/api-proxy-config.spec.ts b/packages/host/apiproxy/tests/api-proxy-config.spec.ts index 2562de5785..badfad76b3 100644 --- a/packages/host/apiproxy/tests/api-proxy-config.spec.ts +++ b/packages/host/apiproxy/tests/api-proxy-config.spec.ts @@ -148,6 +148,8 @@ const AdapterConfig = z.object({ async function harness(options?: { settings?: false | { doc?: Record; readOnly?: boolean } credentials?: false | { shadowed?: string[] } + /** Skip the directory registration to exercise a namespace the proxy does not expose. */ + configurableProviders?: false }): Promise { const ctx = new Context() await ctx.plugin(SessionStore) @@ -158,6 +160,13 @@ async function harness(options?: { await ctx.plugin(LlmService) if (options?.settings !== false) await ctx.plugin(MemorySettings, options?.settings) if (options?.credentials !== false) await ctx.plugin(MemoryCredentials, options?.credentials) + // The proxy serves only namespaces a configurable provider addresses, which + // is what the real LLM plugins declare at load; the tests mirror that. + if (options?.configurableProviders !== false) { + ctx.llm.registerConfigurableProviders([ + { provider: 'deepseek-official', displayName: 'DeepSeek', settingsNs: 'llm-deepseek', settingsPath: [] }, + ]) + } // Host-stream opener reads the committed-workspace baseline; the stub // suffices — the real workspace composition is api-proxy-workspace.spec's. ctx.provide('workspace', { list: () => [] } as never) @@ -213,6 +222,41 @@ describe('settings domain', () => { expect(JSON.stringify(value)).not.toContain('user-secret') }) + it('serves only namespaces a registered model provider addresses', async () => { + // The settings seam is general: any plugin may register a namespace for + // its own configuration. The Web configuration plane is not — it is the + // model-provider surface, and a namespace nothing in the provider + // directory addresses must be invisible and unwritable here, so a future + // plugin cannot become remotely configurable just by registering. + const ctx = await harness() + ctx.settings.register(NS, AdapterConfig) + ctx.settings.register(settingsNamespace('some-other-plugin'), z.object({ secretPath: z.string() })) + const api = createApiProxy(ctx, DEFAULTS) + + const value = expectOk(await api.settings.describe(request({}))) + expect(value.namespaces.map(view => view.ns)).toEqual(['llm-deepseek']) + + for (const response of [ + await api.settings.update(request({ ns: 'some-other-plugin', patch: { secretPath: '/etc/shadow' } })), + await api.settings.replace(request({ ns: 'some-other-plugin', section: {} })), + ]) { + const error = expectErr(response) + expect(error.code).toBe('settings-not-exposed') + expect(error.details).toEqual({ ns: 'some-other-plugin' }) + } + // The write never reached the seam. + expect(ctx.settings.describe().find(d => String(d.ns) === 'some-other-plugin')?.value).toEqual({}) + }) + + it('refuses even a model-provider namespace once its directory entry is gone', async () => { + const ctx = await harness({ configurableProviders: false }) + ctx.settings.register(NS, AdapterConfig) + const api = createApiProxy(ctx, DEFAULTS) + expect(expectOk(await api.settings.describe(request({}))).namespaces).toEqual([]) + expect(expectErr(await api.settings.update(request({ ns: 'llm-deepseek', patch: { baseURL: 'https://x' } }))).code) + .toBe('settings-not-exposed') + }) + it('updates the user layer, answers with the new redacted view, and broadcasts the frame', async () => { const ctx = await harness() ctx.settings.register(NS, AdapterConfig, { base: { baseURL: 'https://base' } }) @@ -238,7 +282,6 @@ describe('settings domain', () => { it.each([ ['an invalid namespace name', 'Not A Namespace', {}], - ['an unregistered namespace', 'unknown-ns', {}], ['a schema-invalid patch', 'llm-deepseek', { baseURL: 42 }], ])('rejects %s as settings-rejected', async (_case, ns, patch) => { const ctx = await harness() @@ -249,6 +292,21 @@ describe('settings domain', () => { expect(error.details).toEqual({ ns }) }) + it('answers an unregistered namespace exactly like an unexposed one', async () => { + // Deliberately indistinguishable: separating "does not exist" from + // "exists but is not yours to configure" would let a caller enumerate the + // registered namespaces one probe at a time. + const ctx = await harness() + ctx.settings.register(NS, AdapterConfig) + ctx.settings.register(settingsNamespace('some-other-plugin'), z.object({ secretPath: z.string() })) + const api = createApiProxy(ctx, DEFAULTS) + const unknown = expectErr(await api.settings.update(request({ ns: 'unknown-ns', patch: {} }))) + const unexposed = expectErr(await api.settings.update(request({ ns: 'some-other-plugin', patch: {} }))) + expect(unknown.code).toBe('settings-not-exposed') + expect(unexposed.code).toBe(unknown.code) + expect(unexposed.message.replace('some-other-plugin', 'unknown-ns')).toBe(unknown.message) + }) + it('maps a read-only provider refusal onto the same rejection', async () => { const ctx = await harness({ settings: { readOnly: true } }) ctx.settings.register(NS, AdapterConfig) @@ -303,7 +361,7 @@ describe('credentials domain', () => { describe('llm domain', () => { it('merges the configurable directory with live routes and appends undeclared ones', async () => { - const ctx = await harness() + const ctx = await harness({ configurableProviders: false }) ctx.llm.registerConfigurableProviders([ { provider: 'deepseek-official', displayName: 'DeepSeek', settingsNs: 'llm-deepseek', settingsPath: [] }, { provider: 'openai', displayName: 'openai', settingsNs: 'llm-pi-ai', settingsPath: ['providers', 'openai'] }, diff --git a/packages/host/apiproxy/tests/client-handler.spec.ts b/packages/host/apiproxy/tests/client-handler.spec.ts index e481ff8623..dec295ae13 100644 --- a/packages/host/apiproxy/tests/client-handler.spec.ts +++ b/packages/host/apiproxy/tests/client-handler.spec.ts @@ -89,6 +89,7 @@ function scriptedApi(overrides: { describe: r => ok(r, { writable: true, namespaces: [] }), update: err, replace: err, + mutate: err, ...overrides.settings, }, credentials: { @@ -615,6 +616,7 @@ describe('config unary surface', () => { describe: record('settings.describe', r => ok(r, { writable: true, namespaces: [view] })), update: record('settings.update', r => ok(r, view)), replace: record('settings.replace', r => ok(r, view)), + mutate: record('settings.mutate', r => ok(r, view)), }, credentials: { describe: record('credentials.describe', r => ok(r, { credentials: { OPENAI_API_KEY: { configured: true, source: 'file', writable: true } } })), diff --git a/packages/host/apiproxy/tests/fetch-carrier.spec.ts b/packages/host/apiproxy/tests/fetch-carrier.spec.ts index 24474ca4bc..351ed9b20a 100644 --- a/packages/host/apiproxy/tests/fetch-carrier.spec.ts +++ b/packages/host/apiproxy/tests/fetch-carrier.spec.ts @@ -177,6 +177,9 @@ function fakeApi(overrides: Partial<{ muxFrames: MuxFrame[]; hostFrames: HostFra async replace(request) { return { rpcId: request.rpcId, result: { ok: false, error: { code: 'settings-rejected', message: 'stub', details: { ns: request.payload.ns } } } } }, + async mutate(request) { + return { rpcId: request.rpcId, result: { ok: false, error: { code: 'settings-rejected', message: 'stub', details: { ns: request.payload.ns } } } } + }, }, credentials: { async describe(request) { diff --git a/packages/llm/llm-deepseek/src/index.ts b/packages/llm/llm-deepseek/src/index.ts index 56310437c8..16194955d1 100644 --- a/packages/llm/llm-deepseek/src/index.ts +++ b/packages/llm/llm-deepseek/src/index.ts @@ -220,16 +220,17 @@ export function apply(ctx: Context, config: Config): void { ]) // Route effects bind to this apply fiber via the stable `ctx` reference, // even when a swap runs inside the scoped settings callback below. - let disposeRoute = ctx.llm.registerAdapter([PROVIDER], adapter) + const registration = ctx.llm.registerAdapter([PROVIDER], adapter) let registeredPolicy = options().retryPolicy const ensureRegistrationFacts = (): void => { const policy = options().retryPolicy if (deepEqualJson(policy, registeredPolicy)) return // The registry captures the retry policy at registration, so it is the one - // fact per-request resolution cannot refresh: swap the registration in one - // synchronous section (same adapter instance, no NO_ADAPTER window). - disposeRoute() - disposeRoute = ctx.llm.registerAdapter([PROVIDER], adapter) + // fact per-request resolution cannot refresh. `replace` re-reads it in one + // synchronous registry section: disposing and re-registering instead would + // publish an empty route set between the two, and an observer that reacted + // to it would see this provider disappear and come back. + registration.replace([PROVIDER]) registeredPolicy = policy } diff --git a/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts b/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts index f5266f2d91..25cf4f293b 100644 --- a/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts +++ b/packages/llm/llm-deepseek/tests/dynamic-config.spec.ts @@ -113,10 +113,18 @@ describe('request-level dynamic configuration', () => { ]) }) - it('re-registers the route in place when the captured retry policy changes', async () => { + it('re-registers the route in place when the captured retry policy changes, without an empty-registry window', async () => { const dir = await home() const { ctx } = await boot(dir, { apiKey: 'k', baseURL: 'http://127.0.0.1:1' }) + // Observing the topology event, not just the end state: disposing and + // re-registering also lands on the right final registry, but publishes an + // empty route set in between, so an observer sees the provider disappear. + const observed: string[][] = [] + ctx.on('llm/adapters-updated', () => { + observed.push(ctx.llm.listProviders().map(provider => provider.id)) + }) + await ctx.settings.update(NS, { retryPolicy: { mode: 'always', backoff: { initialDelayMs: 25, maxDelayMs: 100, jitterRatio: 0.2 } }, }) @@ -127,6 +135,7 @@ describe('request-level dynamic configuration', () => { jitterRatio: 0.2, }) expect(ctx.llm.listProviders()).toEqual([{ id: 'deepseek-official', name: 'DeepSeek' }]) + expect(observed).toEqual([['deepseek-official']]) }) it('keeps the last good options when a settings snapshot fails beyond-schema validation', async () => { diff --git a/packages/llm/llm/src/index.ts b/packages/llm/llm/src/index.ts index 5c9b091ef0..9a96d2f3d7 100644 --- a/packages/llm/llm/src/index.ts +++ b/packages/llm/llm/src/index.ts @@ -236,19 +236,32 @@ export class LlmService extends Service { let invariantFailure: unknown for (const listener of this.ctx.events.dispatch('emit', ['llm/adapters-updated']) as Array<() => unknown>) { try { - listener() + const returned = listener() + if (returned != null && typeof (returned as PromiseLike).then === 'function') { + // An emit listener may still be an async function; its rejection + // cannot reach the synchronous INVARIANT rethrow below, so it is + // contained here instead of becoming an unhandled rejection. + void Promise.resolve(returned as PromiseLike).then(undefined, (error: unknown) => { + this.warnAdaptersListenerFailure(error) + }) + } } catch (error) { if ((error as { code?: unknown } | null)?.code === 'INVARIANT') { invariantFailure ??= error continue } - this.ctx.logger.warn('llm: an llm/adapters-updated listener failed') - this.ctx.logger.warn(error) + this.warnAdaptersListenerFailure(error) } } if (invariantFailure !== undefined) throw invariantFailure as Error } + /** Contained-listener diagnostic shared by the sync and async failure paths. */ + private warnAdaptersListenerFailure(error: unknown): void { + this.ctx.logger.warn('llm: an llm/adapters-updated listener failed') + this.ctx.logger.warn(error) + } + /** * Register an adapter for the given provider routes. Throws `LlmError` with code * `DUPLICATE_ADAPTER` if any provider already has an adapter (all-or-nothing). diff --git a/packages/llm/llm/tests/topology.spec.ts b/packages/llm/llm/tests/topology.spec.ts index f33cd69399..f07b33af7d 100644 --- a/packages/llm/llm/tests/topology.spec.ts +++ b/packages/llm/llm/tests/topology.spec.ts @@ -53,6 +53,42 @@ describe('llm/adapters-updated', () => { expect(warn).toHaveBeenCalledWith('llm: an llm/adapters-updated listener failed') }) + it('contains an ASYNC listener rejection instead of leaving it unhandled', async () => { + // An emit listener may be an async function; its rejection cannot reach + // the synchronous catch, so an uncontained one escapes the process as an + // unhandled rejection rather than a warned observer failure. + const ctx = await setup() + const warn = vi.spyOn(ctx.logger, 'warn').mockImplementation(() => undefined) + const unhandled = vi.fn() + process.on('unhandledRejection', unhandled) + try { + // Typed as returning unknown so the listener is not a Promise-returning + // function type: the point is exactly that an async one may slip in. + const rejecting = (): unknown => Promise.reject(new Error('async observer')) + ctx.on('llm/adapters-updated', rejecting) + ctx.llm.registerAdapter(['a'], new NoopAdapter()) + expect(ctx.llm.listProviders().map(provider => provider.id)).toEqual(['a']) + await new Promise(resolve => setTimeout(resolve, 10)) + expect(unhandled).not.toHaveBeenCalled() + expect(warn).toHaveBeenCalledWith('llm: an llm/adapters-updated listener failed') + } finally { + process.off('unhandledRejection', unhandled) + } + }) + + it('replaces a route set in one event, never publishing an empty registry between the two', async () => { + // The retry-policy swap in llm-deepseek: disposing and re-registering + // would let an observer see the provider disappear and come back. + const ctx = await setup() + const observed: string[][] = [] + const registration = ctx.llm.registerAdapter(['a'], new NoopAdapter()) + ctx.on('llm/adapters-updated', () => { + observed.push(ctx.llm.listProviders().map(provider => provider.id)) + }) + registration.replace(['a']) + expect(observed).toEqual([['a']]) + }) + it('rethrows the first INVARIANT-coded listener failure after notifying the rest', async () => { const ctx = await setup() const later = vi.fn() diff --git a/packages/settings/settings/src/index.ts b/packages/settings/settings/src/index.ts index f356057d69..2eaf32e9a8 100644 --- a/packages/settings/settings/src/index.ts +++ b/packages/settings/settings/src/index.ts @@ -162,6 +162,44 @@ function isPlainObject(value: unknown): value is Record { return proto === Object.prototype || proto === null } +/** + * One path-addressed edit to a namespace's user section. Path mutation exists + * for a caller holding an INCOMPLETE view of the section — a configuration UI + * reads the redacted descriptor, which by construction never received the + * `role('secret')` fields. Such a caller can name the field it means without + * restating the section: a wholesale `replace` rebuilt from a redacted + * document silently deletes every secret the wire never returned. + */ +export type SettingsPathOp = + | { op: 'set'; path: readonly string[]; value: unknown } + | { op: 'unset'; path: readonly string[] } + +/** Apply one path op to a detached section, returning the next section. */ +function applyPathOp(section: Record, op: SettingsPathOp): Record { + const [head, ...rest] = op.path + // The empty path addresses the section itself. + if (head === undefined) { + if (op.op === 'unset') return {} + if (!isPlainObject(op.value)) { + throw new TypeError('settings mutate: setting the section root requires a plain object') + } + return { ...op.value } + } + if (rest.length === 0) { + if (op.op === 'set') return { ...section, [head]: op.value } + const { [head]: _removed, ...kept } = section + return kept + } + const child = section[head] + if (!isPlainObject(child)) { + // Unsetting through an absent path is already satisfied; setting through + // one creates the intermediate objects it needs. + if (op.op === 'unset') return section + return { ...section, [head]: applyPathOp({}, { ...op, path: rest }) } + } + return { ...section, [head]: applyPathOp(child, { ...op, path: rest }) } +} + /** Human label for a value rejected by the JSON-shape boundary (numbers reject inline). */ function describeRejected(value: unknown): string { if (value === undefined) return 'undefined' @@ -443,9 +481,32 @@ export abstract class Settings extends Service { return this.write(ns, section, 'replace') } + /** + * Apply path-addressed edits to one registered namespace's user section, + * validate, persist, then commit and emit. The ops are applied to the + * section as it stands when the write reaches the front of the queue, so a + * caller never has to restate fields it did not touch — and, crucially, + * cannot delete fields it never saw. This is the write path for any caller + * holding a redacted view; `replace` remains the wholesale reset. + * @param ns - the registered namespace to edit. + * @param ops - ordered path edits; later ops observe earlier ones. + */ + async mutate(ns: SettingsNamespace, ops: readonly SettingsPathOp[]): Promise { + if (!Array.isArray(ops)) throw new TypeError(`settings mutate for "${ns}" must be an array of path ops`) + for (const op of ops) { + if (!isPlainObject(op) || (op['op'] !== 'set' && op['op'] !== 'unset')) { + throw new TypeError(`settings mutate for "${ns}" ops must be {op:'set'|'unset', path}`) + } + if (!Array.isArray(op['path']) || (op['path'] as unknown[]).some(part => typeof part !== 'string')) { + throw new TypeError(`settings mutate for "${ns}" op paths must be arrays of strings`) + } + } + return this.write(ns, ops, 'mutate') + } + /** Validate a write, then queue it on the namespace's serialized write chain. */ - private write(ns: SettingsNamespace, input: object, mode: 'merge' | 'replace'): Promise { - const verb = mode === 'merge' ? 'update' : 'replace' + private write(ns: SettingsNamespace, input: object, mode: 'merge' | 'replace' | 'mutate'): Promise { + const verb = mode === 'merge' ? 'update' : mode === 'replace' ? 'replace' : 'mutate' const registration = this.registrations.get(ns) if (registration === undefined) { throw new Error(`settings namespace "${ns}" is not registered`) @@ -456,13 +517,19 @@ export abstract class Settings extends Service { if (!this.writable) { throw new Error(`settings provider is read-only: "${ns}" cannot be updated in-process`) } - if (!isPlainObject(input)) { - throw new TypeError(`settings ${verb} for "${ns}" must be a plain object`) + // A mutate's ops array is wrapped so one JSON-shape walk covers both + // shapes; merge/replace carry the section itself. + let payload: Record + if (mode === 'mutate') { + payload = { ops: input } + } else { + if (!isPlainObject(input)) throw new TypeError(`settings ${verb} for "${ns}" must be a plain object`) + payload = input } // Snapshot at call time: the queue must never read a caller-owned object // the caller may keep mutating while the write waits its turn. The same // walk is the JSON-shape boundary check (see cloneJsonShaped). - const snapshot = cloneJsonShaped(input, (label, path) => + const snapshot = cloneJsonShaped(payload, (label, path) => new TypeError(`settings ${verb} for "${ns}" must be JSON-shaped data (found ${label} at ${path})`)) const previous = this.writeQueues.get(ns) ?? Promise.resolve() // Chain past a failed predecessor: one rejected write must not poison the @@ -474,9 +541,14 @@ export abstract class Settings extends Service { if (this.registrations.get(ns) !== registration) { throw new Error(`settings namespace "${ns}" registration was disposed before the queued ${verb} ran`) } + // Every mode derives from the section as it stands NOW, at the front of + // the queue — never from whatever the caller last saw. + const current = this.section(ns) ?? {} const section = mode === 'merge' - ? mergeLayers(this.section(ns) ?? {}, snapshot) as Record - : snapshot + ? mergeLayers(current, snapshot) as Record + : mode === 'replace' + ? snapshot + : (snapshot['ops'] as SettingsPathOp[]).reduce(applyPathOp, current) const next = deepFreeze(this.resolve(registration.schema, registration.base, section)) await this.persist(ns, section) // The write reached storage either way; the cache must say so. Commit diff --git a/packages/settings/settings/tests/settings.spec.ts b/packages/settings/settings/tests/settings.spec.ts index 5f9ac7dae7..9fcf841551 100644 --- a/packages/settings/settings/tests/settings.spec.ts +++ b/packages/settings/settings/tests/settings.spec.ts @@ -725,3 +725,89 @@ describe('installSettingsSection', () => { expect(changes).toEqual(['user']) }) }) + +describe('mutate (path-addressed writes)', () => { + interface KeyedConfig { + apiKey: string + baseURL: string + reasoning: string + } + + const KeyedSchema: z = z.object({ + apiKey: z.string().role('secret'), + baseURL: z.string(), + reasoning: z.string(), + }) + + const KEYED = settingsNamespace('keyed') + const NESTED = settingsNamespace('workspace') + + async function mounted(doc: Record) { + const ctx = new Context() + await ctx.plugin(BareProvider, { doc }) + ctx.settings.register(KEYED, KeyedSchema) + return ctx + } + + it('removes one field without touching a secret the caller never saw', async () => { + // The data-loss shape this exists to prevent: a configuration UI reads the + // REDACTED descriptor (no apiKey), the user resets baseURL, and the client + // rebuilds the section from what it holds. A wholesale replace of that + // rebuild deletes the stored literal key; a path unset cannot. + const ctx = await mounted({ keyed: { apiKey: 'sk-stored', baseURL: 'https://user', reasoning: 'high' } }) + const redacted = ctx.settings.describe({ redactSecrets: true }).find(d => d.ns === KEYED)! + expect(redacted.user).toEqual({ baseURL: 'https://user', reasoning: 'high' }) + + await ctx.settings.mutate(KEYED, [{ op: 'unset', path: ['baseURL'] }]) + + const raw = ctx.settings.describe().find(d => d.ns === KEYED)! + expect(raw.user).toEqual({ apiKey: 'sk-stored', reasoning: 'high' }) + }) + + it('applies set and unset in one write, in order', async () => { + const ctx = await mounted({ keyed: { apiKey: 'sk-stored', baseURL: 'https://old' } }) + await ctx.settings.mutate(KEYED, [ + { op: 'set', path: ['baseURL'], value: 'https://new' }, + { op: 'set', path: ['reasoning'], value: 'low' }, + { op: 'unset', path: ['reasoning'] }, + ]) + expect(ctx.settings.describe().find(d => d.ns === KEYED)!.user) + .toEqual({ apiKey: 'sk-stored', baseURL: 'https://new' }) + }) + + it('reads the section as it stands at the front of the queue, not at call time', async () => { + // Two concurrent writers: the mutate is issued against the pre-update + // section but must observe the update that ran before it. + const ctx = await mounted({ keyed: { apiKey: 'sk-stored' } }) + const first = ctx.settings.update(KEYED, { baseURL: 'https://first', reasoning: 'high' }) + const second = ctx.settings.mutate(KEYED, [{ op: 'unset', path: ['reasoning'] }]) + await Promise.all([first, second]) + expect(ctx.settings.describe().find(d => d.ns === KEYED)!.user) + .toEqual({ apiKey: 'sk-stored', baseURL: 'https://first' }) + }) + + it('creates intermediate objects for a nested set and leaves an absent unset alone', async () => { + const ctx = new Context() + await ctx.plugin(BareProvider, { doc: {} }) + ctx.settings.register(NESTED, NestedSchema) + await ctx.settings.mutate(NESTED, [{ op: 'set', path: ['retry', 'attempts'], value: 5 }]) + expect(ctx.settings.describe().find(d => d.ns === NESTED)!.user).toEqual({ retry: { attempts: 5 } }) + await ctx.settings.mutate(NESTED, [{ op: 'unset', path: ['missing', 'deep'] }]) + expect(ctx.settings.describe().find(d => d.ns === NESTED)!.user).toEqual({ retry: { attempts: 5 } }) + }) + + it('rejects a malformed op before anything is queued', async () => { + const ctx = await mounted({ keyed: { apiKey: 'sk-stored' } }) + await expect(ctx.settings.mutate(KEYED, [{ op: 'delete' } as never])) + .rejects.toThrow(/must be \{op:'set'\|'unset', path\}/) + await expect(ctx.settings.mutate(KEYED, [{ op: 'unset', path: ['a', 1] as never }])) + .rejects.toThrow(/op paths must be arrays of strings/) + expect(ctx.settings.describe().find(d => d.ns === KEYED)!.user).toEqual({ apiKey: 'sk-stored' }) + }) + + it('rejects a value the JSON-shape boundary refuses', async () => { + const ctx = await mounted({ keyed: {} }) + await expect(ctx.settings.mutate(KEYED, [{ op: 'set', path: ['baseURL'], value: new Date() }])) + .rejects.toThrow(/must be JSON-shaped data/) + }) +})