diff --git a/packages/client/ui-agent-preset/src/client/settings-store.ts b/packages/client/ui-agent-preset/src/client/settings-store.ts index fcc092d179..6c35dcdccb 100644 --- a/packages/client/ui-agent-preset/src/client/settings-store.ts +++ b/packages/client/ui-agent-preset/src/client/settings-store.ts @@ -25,6 +25,12 @@ export interface AgentPresetOption { export interface AgentPresetSettingsState { status: 'idle' | 'loading' | 'ready' | 'saving' | 'unavailable' | 'error' error: string | null + /** + * Whether this browser may persist the choice at all. `settings.describe` is + * loopback-only and reports a read-only provider as `writable: false`; the + * row then shows the current default and disables the control rather than + * offering a write the gateway will refuse. + */ writable: boolean currentValue: string options: readonly AgentPresetOption[] @@ -33,6 +39,8 @@ export interface AgentPresetSettingsState { const INITIAL: AgentPresetSettingsState = { status: 'idle', error: null, + // Assumed until `load()` asks; a row that has not read yet renders nothing + // interactive anyway (status 'idle'). writable: true, currentValue: '', options: [], @@ -69,9 +77,15 @@ export class AgentPresetSettingsController { this.set({ status: 'unavailable', options: [], currentValue: '' }) return } + // The roster says what may be chosen; `settings.describe` says whether + // this browser may write the choice down. A non-loopback browser reaches + // neither method, so a refused describe leaves the row read-only rather + // than offering a control whose write answers `settings-not-exposed`. + const described = await this.api.settings.describe({}) this.set({ status: 'ready', error: null, + writable: described.result.ok && described.result.value.writable, options: presets.map(preset => ({ id: preset.id, trust: preset.trust })), currentValue: presets.find(preset => preset.isDefault)?.id ?? presets[0]?.id ?? '', }) diff --git a/packages/client/ui-agent-preset/tests/settings-store.spec.ts b/packages/client/ui-agent-preset/tests/settings-store.spec.ts index e8d8c19d62..16c7f8cc99 100644 --- a/packages/client/ui-agent-preset/tests/settings-store.spec.ts +++ b/packages/client/ui-agent-preset/tests/settings-store.spec.ts @@ -17,7 +17,7 @@ interface Recorded { ns: string; patch: unknown } /** A client whose roster and write outcome the test controls. */ function fakeApi( presets: { id: string; trust: 'system' | 'user'; isDefault: boolean }[], - options: { writes?: Recorded[]; failWrite?: string; failList?: string } = {}, + options: { writes?: Recorded[]; failWrite?: string; failList?: string; readOnly?: boolean } = {}, ): IApiClient { return { agentPresets: { @@ -26,6 +26,15 @@ function fakeApi( : { rpcId: 'r', result: { ok: false as const, error: { code: 'internal', message: options.failList, details: {} } } }), }, settings: { + // Loopback-only in production; a read-only provider answers writable:false + // and the row disables its control instead of offering a refused write. + describe: () => Promise.resolve({ + rpcId: 'r', + result: { + ok: true as const, + value: { writable: options.readOnly !== true, hasDocument: true, namespaces: [] }, + }, + }), update: (payload: { ns: string; patch: unknown }) => { options.writes?.push({ ns: payload.ns, patch: payload.patch }) if (options.failWrite !== undefined) { @@ -42,6 +51,20 @@ function fakeApi( } describe('the agent-preset settings controller', () => { + it('disables the control when this browser may not write settings', async () => { + const controller = new AgentPresetSettingsController(fakeApi([ + { id: 'standard', trust: 'system', isDefault: true }, + ], { readOnly: true })) + + await controller.load() + + // `settings.describe` is loopback-only and reports a read-only provider; + // offering a control whose write answers `settings-not-exposed` would + // promise a switch the host refuses. + expect(controller.store.getSnapshot().writable).toBe(false) + expect(controller.store.getSnapshot().currentValue).toBe('standard') + }) + it('derives options and the current default from one roster call', async () => { const controller = new AgentPresetSettingsController(fakeApi([ { id: 'standard', trust: 'system', isDefault: true }, diff --git a/packages/host/apiproxy/src/api-proxy.ts b/packages/host/apiproxy/src/api-proxy.ts index dbeffdb608..ba0e2e43be 100644 --- a/packages/host/apiproxy/src/api-proxy.ts +++ b/packages/host/apiproxy/src/api-proxy.ts @@ -319,12 +319,14 @@ async function summarizeCold( // a cold log to check for turns would defeat the index read, so a listed // cold session is served as not-blank (its log holds its conversation). blank: false, - ...meta.parentSession === undefined ? {} : { parentSessionId: meta.parentSession }, - ...meta.origin === undefined ? {} : { origin: meta.origin }, - /* v8 ignore next -- the empty arm needs a cwd-less meta, but list() - filters those out (legacy logs are not served); the conditional mirrors - summarize() shape. */ - ...meta.cwd === undefined ? {} : { cwd: meta.cwd }, + // The same projection the attached path uses. Hand-copying the header here + // is how `agentPreset` went missing from cold rows while `summarize()` + // served it — a restored session then read as preset-less and the picker + // showed the deployment default instead of what the session runs. With no + // events to read (a cold row never loads its log, see `blank` above), this + // resolves to the header's value; a switch recorded while blank surfaces + // once the session attaches. + ...sessionListFields(meta), } } @@ -745,6 +747,14 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro const targets = new WeakMap() /** Implicit resume of cold sessions, deduplicating concurrent calls (follows the jsonrpc sessionCreations precedent). */ const resumes = new Map>() + /** + * Serializes `agentPreset.select` per session. Two concurrent selects both + * pass the blank check, and the second `unmountPresetFor` then finds nothing + * to unmount because the first already removed the record — leaving two + * compositions registered into one agent layer. The client's `busy` flag is + * not enforcement: the wire is reachable directly. + */ + const presetSwitches = new Map>() /** Client-chosen identity creation/resume, deduplicated across concurrent retries. */ const sessionCreations = new Map>() /** Serializes path ownership and explicit title checks with Workspace mutations. */ @@ -1119,7 +1129,14 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro const handle = await ctx.agents.resume({ resumeSessionId: sessionId, agentOptions, - setup: (await composeAgent(inspected.meta.agentPreset)).setup, + // Resolved from the LOG, not the header: a session that switched + // while blank ran its turns under the newer composition, and the + // header is written once at creation. Reading the header here + // would silently undo the switch on the next restart and restore + // that history under the old tool set. + setup: (await composeAgent( + resolveSessionPreset({ header: inspected.meta, events: inspected.events }), + )).setup, }) return handle.agent } finally { @@ -2528,39 +2545,51 @@ export function createApiProxy(ctx: Context, defaults: ApiProxyDefaults): ApiPro const found = await agentFor(sessionId) if ('error' in found) return err(request, found.error) const { agent } = found - if (!sessionBlank(agent.session)) { - return err(request, { - code: 'agent-preset-locked', - message: `session "${sessionId}" has already started; its agent preset is fixed`, - details: { sessionId, agentPreset }, - }) + const swap = async (): Promise> => { + // Re-read inside the queue: an earlier switch may have run, and a + // conversation may have started, since this request arrived. + if (!sessionBlank(agent.session)) { + return err(request, { + code: 'agent-preset-locked', + message: `session "${sessionId}" has already started; its agent preset is fixed`, + details: { sessionId, agentPreset }, + }) + } + try { + const preset = await presets.recompose(agent.ctx, agentPreset) + // Recorded only after the swap committed: the log states what the + // agent runs, and a rejected mount leaves the previous composition. + agent.session.append('agent-preset/selected', { agentPreset: preset.id }) + return ok(request, { agentPreset: preset.id }) + } catch (error: unknown) { + if (error instanceof UnknownPresetError) { + return err(request, { + code: 'agent-preset-not-found', + message: error.message, + details: { agentPreset: error.presetId, available: [...error.available] }, + }) + } + if (error instanceof PresetMountError) { + return err(request, { + code: 'agent-preset-invalid', + message: error.message, + details: { agentPreset: error.presetId, reason: error.reason }, + }) + } + return err(request, { + code: 'internal', + message: `failed to select agent preset "${agentPreset}": ${String(error)}`, + details: {}, + }) + } } + const queued = presetSwitches.get(sessionId) ?? Promise.resolve() + const turn = queued.then(swap) + presetSwitches.set(sessionId, turn.catch(() => undefined)) try { - const preset = await presets.recompose(agent.ctx, agentPreset) - // Recorded only after the swap committed: the log states what the - // agent runs, and a rejected mount leaves the previous composition. - agent.session.append('agent-preset/selected', { agentPreset: preset.id }) - return ok(request, { agentPreset: preset.id }) - } catch (error: unknown) { - if (error instanceof UnknownPresetError) { - return err(request, { - code: 'agent-preset-not-found', - message: error.message, - details: { agentPreset: error.presetId, available: [...error.available] }, - }) - } - if (error instanceof PresetMountError) { - return err(request, { - code: 'agent-preset-invalid', - message: error.message, - details: { agentPreset: error.presetId, reason: error.reason }, - }) - } - return err(request, { - code: 'internal', - message: `failed to select agent preset "${agentPreset}": ${String(error)}`, - details: {}, - }) + return await turn + } finally { + if (presetSwitches.get(sessionId) === turn) presetSwitches.delete(sessionId) } }, }, diff --git a/packages/host/apiproxy/src/api/agent-presets.ts b/packages/host/apiproxy/src/api/agent-presets.ts index 83630d1dc3..3471c06f6d 100644 --- a/packages/host/apiproxy/src/api/agent-presets.ts +++ b/packages/host/apiproxy/src/api/agent-presets.ts @@ -24,7 +24,11 @@ export interface AgentPresetEntry { /** agent-preset-domain unary methods (the map key agentPreset.* of RpcMethodMap). */ export interface AgentPresetsApi { /** - * Lists every preset the deployment currently supplies, ordered by id. + * Lists every preset the deployment currently supplies, in root-precedence + * order — the roots as configured, each root's own presets sorted by id, + * and the first root to supply an id wins. The order is not globally + * sorted: a user root's preset sits in that root's block, not among the + * shipped ids. * An empty roster means the deployment composes no presets at all, and * every session shares the host composition. */ diff --git a/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts b/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts index 5473d3ec63..95810a6c7c 100644 --- a/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts +++ b/packages/host/apiproxy/tests/api-proxy-agent-preset.spec.ts @@ -14,7 +14,7 @@ import type { Agent } from '@deepseek-ai/dsh-agent' import SessionStore, { SessionId, type Session } from '@deepseek-ai/dsh-session' import UserInteractionService from '@deepseek-ai/dsh-user-interaction' import { RpcId, type RpcRequest } from '../src/api/rpc.ts' -import { UnknownPresetError } from '@deepseek-ai/dsh-agent-presets' +import { resolveSessionPreset, UnknownPresetError } from '@deepseek-ai/dsh-agent-presets' import { GoalId } from '@deepseek-ai/dsh-goal' import { createApiProxy } from '../src/api-proxy.ts' import { describe, expect, it } from 'vitest' @@ -280,6 +280,46 @@ describe('agentPreset.select', () => { expect(response.result.value.agentPreset).toBe('core-web') }) + it('records the switch in the log, and the list reads it back', async () => { + const { api, ctx } = await harness(['standard', 'core-web']) + await api.sessions.create(request({ sessionId: SessionId('sel-log'), agentPreset: 'standard' })) + + await api.agentPresets.select( + request({ sessionId: SessionId('sel-log'), agentPreset: 'core-web' })) + + // The header is written once at creation, so the switch lives in the log — + // this is what a restart replays and what every projection resolves from. + // Asserting only the RPC's echo would miss a switch that never persisted. + const session = ctx.sessions.get(SessionId('sel-log')) + if (session === undefined) throw new Error('unreachable') + expect(session.header.agentPreset).toBe('standard') + expect(resolveSessionPreset(session)).toBe('core-web') + const listed = await api.sessions.list(request({})) + if (!listed.result.ok) throw new Error('unreachable') + expect(listed.result.value.items.find(item => item.sessionId === 'sel-log')?.agentPreset) + .toBe('core-web') + }) + + it('serializes two concurrent selects on one session', async () => { + const { api, ctx } = await harness(['standard', 'core-web']) + await api.sessions.create(request({ sessionId: SessionId('sel-race'), agentPreset: 'standard' })) + + // Both pass the blank check; unserialized, the second unmount finds no + // record because the first already removed it, and two compositions end up + // in one agent layer. The client's busy flag is not enforcement. + const [first, second] = await Promise.all([ + api.agentPresets.select(request({ sessionId: SessionId('sel-race'), agentPreset: 'core-web' })), + api.agentPresets.select(request({ sessionId: SessionId('sel-race'), agentPreset: 'standard' })), + ]) + + expect(first.result.ok).toBe(true) + expect(second.result.ok).toBe(true) + const session = ctx.sessions.get(SessionId('sel-race')) + if (session === undefined) throw new Error('unreachable') + // One winner, and the log agrees with it: the last committed switch. + expect(resolveSessionPreset(session)).toBe('standard') + }) + it('refuses once the conversation has started', async () => { const { api, ctx } = await harness(['standard', 'core-web']) await api.sessions.create(request({ sessionId: SessionId('sel-2'), agentPreset: 'standard' })) diff --git a/packages/preset/agent-presets/src/index.ts b/packages/preset/agent-presets/src/index.ts index dbc9b1d9ba..f2bc932769 100644 --- a/packages/preset/agent-presets/src/index.ts +++ b/packages/preset/agent-presets/src/index.ts @@ -190,11 +190,16 @@ export class AgentPresets extends Service { try { await mountPreset(agentCtx, preset) } catch (error) { - if (previous !== undefined && previous !== preset.id) { + if (previous !== undefined) { + // Restored unconditionally, same id included: the roster is a live + // directory, so "the same inputs that worked a moment ago" does not + // hold — the file may have changed between the original mount and + // this one, which is exactly how a same-id reselect fails. Skipping + // the restore there left the agent with no composition at all. await this.mount(agentCtx, previous).catch(() => { // The agent now has no composition, but the switch failure below is - // the actionable diagnostic and the restore had the same inputs that - // worked a moment ago; reporting its failure instead would hide why. + // the actionable diagnostic; reporting the restore's instead would + // hide why the switch was attempted and what the operator must fix. }) } throw error