diff --git a/apps/cli/AGENTS.md b/apps/cli/AGENTS.md index af4c8cf4e..cb46bcf90 100644 --- a/apps/cli/AGENTS.md +++ b/apps/cli/AGENTS.md @@ -261,28 +261,189 @@ Two things the dev build does deliberately, both load-bearing: mapping onto each agent's advertised option ids (also the source of truth for the web selectors), `applyAgentRunConfigSelection` in `src/commands/session.ts` applies it once the target agent's cached capabilities are read, and - `validateSessionCreateOptions({ dispatchConfig })` rejects unsupported selections - before the Operation is accepted. `lody_session_create_options` publishes the valid + `validateSessionCreateOptions({ dispatchConfig })` resolves the effective dispatch + config before the Operation is accepted (it no longer rejects on capability + evidence — see the snapshot invariant below). `lody_session_create_options` publishes the valid values per agent config as `runConfig`. Its default response is sparse: online Machines, one default/current agent config, the current local project, and no GitHub repository fetch. Agent configs/local projects/repos expand only through their query inputs. Durable create acceptance stores each target's resolved effective dispatch config; recovery must use it instead of inheriting again from mutable requester history. -- INVARIANT: reasoning effort and fast mode are per MODEL. An ACP probe's - `configOptions` only describe the model that was current at probe time — agents - rebuild those options on every model switch and then REJECT a value the new model - does not support. `acp-capability-normalization.ts` recovers the model-independent - view into `AcpCapabilityCacheEntry.modelReasoningEfforts` from agents that also - publish the legacy `model[effort]` list (Codex); effort is validated against the - TARGET model and the ids so validated come back as `validatedConfigIds`, which - `validateTurnConfigOptionValues(..., skipIds)` must skip (the probed model's list - would wrongly reject them). What cannot be checked offline is dispatched as - requested. Runtime rejections remain in debug diagnostics; Codex/Claude mismatches - for model, reasoning effort, Fast, or Plan are not promoted to visible - `agent_warning` notices, while other rejected selections still are. Compatibility exception: Claude - Fable models omit the Fast mode option, so an explicit `fast=false` is skipped as - an already-effective no-op; `fast=true` must still be dispatched and retained in debug - diagnostics if rejected. +- INVARIANT: a capability snapshot never rejects a run config. `configOptions` + describes the model that was current when it was captured — and every created + session rewrites it (`scheduleCreatedSessionCapabilityUpdate`), so it describes + whichever model ran last. Neither a missing option, nor a value outside the list + it recorded, nor an unseen model/mode id is evidence about the model a turn + selects. `validateTurnConfigOptionValues` therefore rejects only what no model + could carry: a value the option's own declared TYPE forbids. Everything else is + dispatched and reconciled against the state the agent publishes. + Do not reintroduce a `validatedConfigIds`-style exemption set, nor an offline + classification of what "could not be confirmed": both only made sense while the + snapshot could reject. With rejection gone there is nothing to exempt, and a + classification nobody reads is not a diagnostic — the runtime divergence + comparison is the report. + INHERITANCE is the one place that still drops: + `filterInheritedTurnConfigOptionValues` keeps an uncataloged key only when + `isAcpPerModelConfigId` recognises it. A value carried forward from an older + turn is not a request — nobody asked for it on this turn — so a removed or + renamed option would otherwise ride the Session lineage forever: every new + Session inherits it, the agent rejects it, and that config becomes the next + inheritance source, with no surface anywhere to clear it. The same rule applies + when there is no capability at all; an explicit request is always available, so + a missing snapshot must not become a licence to carry every historical key. + Explicit `--config-option` and frozen Operation requests keep going out + unchanged — that split is the whole point. + The one thing that still fails loudly is a missing wire BINDING — no snapshot + option and no agent convention for how to spell a control, so there is no + request to send and an invented id would be a silent no-op. That is a different + statement from "unsupported" and must be worded as such. + `acp-capability-normalization.ts` still recovers `modelReasoningEfforts` from the + legacy `model[effort]` list (Codex): a published per-model breakdown describes + models the snapshot itself does not. It never rejects one. + A DECLARED catalog is the same kind of evidence, said explicitly. An adapter may + attach `_meta.lody.modelCapabilities` (`{ version: 1, models: { : + { effortValues?, fastMode? } } }`) to its `session/new` response, which is what + lets a surface answer "does Luna support Fast?" while the probe ran on a model + that has no fast tier — `measuredForModelId` records which model the snapshot is + actually about. It is advisory in one direction only: it may report a control a + snapshot never carried, and it never grants permission, never authorizes a value, + and never rejects one. A model it does not name is UNKNOWN, not unsupported, so + the declaration is read whole or ignored whole (`readDeclaredModelCapabilities`): + half a catalog past the 64-model bound would answer "no fast mode" for models the + agent simply could not fit. It is freshness-gated — a TTL plus a `sourceVersion` + that must equal the entry's own, because a declaration heard from one adapter + build says nothing about the next one. Storage has two rules that are easy to + undo: a later probe that heard no declaration must NOT clear one already stored + for the same `sourceVersion` (an ordinary refresh does not re-elicit `_meta`), and + the write-dedup key must include the declared content, or the first declaration to + arrive alongside an otherwise unchanged snapshot is silently dropped. + Client side: a Role may be seeded only from `authoritative` capabilities — + `provisional` means the built-in static tables, and seeding from those persists a + guess as a durable promise. Nor may it be SAVED without one: a Role is its run + config and pins the permission mode, so `validateAgentRoleForm` refuses a new + Role while capabilities are unreported (`run_config_unavailable`), keyed on + CREATION rather than on the value being empty: the chat landing and the input + area both create a Role from what the composer currently shows, and under + `provisional` capabilities those are the static tables' own defaults — a + non-empty run config nobody chose. Editing an existing Role stays open, so its + owner can rename it while the machine is offline. The composer keeps stored keys its selector catalog + does not cover; only a present runtime table (the agent's live state) owns the + whole key set. Runtime rejections remain in debug diagnostics; what becomes a visible + `agent_warning` is DIVERGENCE — the state the agent publishes after applying the + turn's config contradicts what was requested. A rejection is not that signal in + either direction: Codex accepts `fast-mode` on a model with no fast speed tier and + then omits the option from its published state, so the state is the only evidence + Fast is off, while a rejected value that was already effective changed nothing and + must stay silent. Only where the agent published no config options at all (or for a + sensitive id, which the runtime state deliberately omits) does the failed call + remain the sole signal. Do not restore a per-agent suppression list: it silenced + exactly the per-model controls users most need told about. Compatibility exception: + Claude Fable models omit the Fast mode option, so an explicit `fast=false` is + skipped as an already-effective no-op and is judged for neither; `fast=true` must + still be dispatched and retained in debug diagnostics if rejected. +- INVARIANT: permission-bearing config (the mode option, plan/collaboration mode) + is applied LAST, after the model and the ordinary options. Claude rebuilds the + available permission modes on a model switch and downgrades the current one to + `default` when the new model lacks it, so a mode set before the model is + silently widened by the model that follows. `applyPromptConfig` runs before + `prompt`, so the state read after applying is still taken before the agent acts. + A successful `session/set_mode` is an acknowledgement, not proof of the resulting + state: it may only FILL a mode the agent's own state does not report, never + overwrite one — echoing the request back as the outcome makes every mode + divergence invisible. Divergence is reported, not blocked: no run-config + mismatch may prevent Session creation or prompt submission, and Agent Roles and + frozen Operations behave exactly like ordinary preferences at run time, + differing only in warning wording and follow-up marking. A Role that would run + diverged is surfaced, not refused — an upgrade must never turn a Role that used + to run into one that fails. + EXCEPTION, and the only one: when the agent's own reported final state says the + turn would run with MORE permission than it asked for, the turn fails before + `prompt` (`AcpPermissionNotAppliedError` → `permission_not_applied`). By the + time a warning about that is readable the agent may already have edited files, + so this is the one divergence a notice cannot cover. It fires only on a live + contradiction. Permission arrives in THREE shapes and all three are checked: + the legacy `session/set_mode` selector, a `category: 'mode'` config option, and + an explicit `category: '_permission'` one (Grok's `permission_mode`, values + `ask`/`auto`/`always-approve`). Matching only the first two let a requested + `ask` run as `always-approve` with nothing but a warning. Each requested + permission-bearing value is compared against the agent's reported value for + THAT control; `isAcpPermissionWiderThanRequested` requires BOTH values to be + ranked among the builtin ones and the effective one to be strictly wider, and + the effective value is read from the agent's published state — a snapshot, a + stale cache, an unranked third-party mode, an unconfirmed request, or a + NARROWER outcome must never stop a turn. The rank table covers only values the + repo adapts (Codex, Claude, Grok `ask`/`auto`/`always-approve`, DeepSeek + Harness `read-only`/`workspace-write`/`danger-full-access`); adding an agent + means adding its values there, or its escalations go unseen. The way out is explicit and + per-turn: `SessionTurnInputConfig.acceptWiderPermission` is informed + acceptance carried by one resend, never inherited and never a default, and it + suppresses the stop while still reporting the mismatch. It NAMES the exact + difference that was disclosed — `{ controlId, requestedModeId, + effectiveModeId }`, written from the failure notice's own meta — because a + bare boolean also accepts differences the user never saw: the agent may have + moved further by the time the turn re-runs (`plan → auto` accepted, `plan → + always-approve` live), and a second permission control may have widened + alongside the one in the notice (Grok carries both a mode selector and an + explicit `_permission` one). The applier skips ONLY exact triple matches and + keeps scanning the remaining permission selections, so anything undisclosed + stops the turn again with its own accurate notice. + It is a LIST (`acceptWiderPermissions`) because those differences are + disclosed one stop at a time: two controls widening at once produce two + notices, and a replay carrying only the newest acceptance would drop the + previous one and land back on the first — the user alternates between two + notices with no way through. A retry therefore inherits the acceptances + already on the turn it is replaying, re-validated, and appends the current + notice's triple. Accumulating grants nothing extra, because every entry is + still matched exactly. An ordinary send, edit-and-resend and any prompt-derived + turn clear the whole set; a same-turn transport retry keeps it. Do not match on the values + alone, and do not treat a rank ceiling as equivalent across different + permission controls. The notice meta and the acceptance share ONE schema + (`AcceptedWiderPermissionSchema`): the client reads the meta and writes it + straight back, and Zod strips undeclared keys, so a second declaration drops a + field on every history read — the action then disappears from a failure the + daemon reported correctly. A malformed or pre-triple `permission` degrades to + absent rather than failing the parse, so the notice still renders and simply + offers no acceptance. That resend replays the + STOPPED turn: prompt, mode, model, config values, Role, `mcpServerIds` + (including an explicit empty selection), `taskToolsEnabled` and + `issuePRMentions` all come from its frozen `inputConfig`, never from the + composer — pairing an old prompt with tool reach the user has since changed + would hand it permissions that turn never had. Those fields travel as ONE + required `TurnScopedOverrides` carrier through every send route (direct, + queue, guide), because each route rebuilds the turn config from composer + state: a route that forwarded three of four silently produced a turn with no + acceptance, which the daemon stopped again. Required, not optional, so a new + route fails to compile rather than dropping it. The same field must survive + every REBUILD on the way to the daemon, and there are three hand-written ones: + `normalizeSessionTurnInputConfig` (which direct RPC, `session/dispatch-turn`, + steer, the Loro history readback and queue promotion all run the config + through), the dispatch watcher's two `acpSessionConfig` constructions, and + queue promotion's `buildSessionTurnInputConfig` call. A rebuild that omits it + stops the very turn the user just accepted. Each copies ONLY an explicit + `true`: `false` and absent are the same answer, and neither may be written + back as something a later turn could read. `buildCliHistoryInputConfig` is the + deliberate exception — CLI and MCP turns never carry an acceptance, and it + must not become inheritable there. + The mirror image is just as load-bearing: making the field survive rebuilds + also made it copyable. Any derivation that mints a new `userTurnId` or changes + the prompt/input blocks — edit-and-resend, queue-item editing, the Operation + completion turn, history replay import — must drop it, through + `deriveTurnInputConfigForNewTurn` wherever the type allows. The acceptance was + given for ONE prompt; carrying it onto another is a permission bypass built + out of a spread, and `{...original, ...replacement}` is that spread: a + replacement that merely LACKS the field does not overwrite a present one. The + DAEMON enforces this, not the client — edit-and-resend rebuilds server-side, + so a correct client is not what makes it safe. Same-turn rewrites (status, + `_lodyDeliveryKind`, transport retry) are not copies and keep it. The + `overrides` parameter cannot re-add the field, and speculative preparation + configs do not carry it at all. + The notice names the turn it stopped (`permission.userTurnId`) and the client + matches by that id. Reading "the nearest user entry above the notice" instead + attaches the acceptance to whatever landed last — another client's turn, or an + edit-and-resend that rewrote history between the failure and the notice — and + if that prompt makes the same permission selection it runs with an acceptance + nobody gave it. Adjacency remains only as the fallback for notices written + before the field existed. - MCP `session_list` defaults to 20 (maximum 100), and `session_history` defaults to 10 (maximum 50 and 128 KiB). Keep the MCP surface bounded even though the human CLI retains `session history --all`. `session_list` and `session_status_many` derive busy/idle from diff --git a/apps/cli/src/agent/acp-capability-normalization.ts b/apps/cli/src/agent/acp-capability-normalization.ts index 7809aac72..d1cd2d6f4 100644 --- a/apps/cli/src/agent/acp-capability-normalization.ts +++ b/apps/cli/src/agent/acp-capability-normalization.ts @@ -2,11 +2,80 @@ import { deriveModelReasoningEffortsFromLegacyModelIds, type AcpCommandSummary, type AcpConfigOptionSummary, + type DeclaredModelCapabilities, } from '@lody/shared'; import type { SessionConfigOption, SessionConfigSelectGroup } from '@agentclientprotocol/sdk'; import { z } from 'zod'; import { filterAcpConfigOptions } from '@/agent/acp-config-option-filter'; +/** + * Bounds for the agent's self-declared model catalog. + * + * `_meta` is whatever the other side put there, and this one gets persisted and + * fanned out to every client of the workspace, so it is bounded before it is + * believed. Numbers are generous for a real catalog and small for a payload: + * an agent publishing more models than this is not describing itself. + */ +const DECLARED_MODEL_LIMITS = { + models: 64, + modelIdLength: 128, + effortValues: 16, + effortValueLength: 64, +} as const; + +const zDeclaredModelCapabilities = z.object({ + _meta: z + .object({ + lody: z + .object({ + modelCapabilities: z + .object({ + version: z.literal(1), + producerRevision: z.string().trim().min(1).max(128).optional(), + models: z + .record( + z.string().trim().min(1).max(DECLARED_MODEL_LIMITS.modelIdLength), + z.object({ + effortValues: z + .array(z.string().trim().min(1).max(DECLARED_MODEL_LIMITS.effortValueLength)) + .max(DECLARED_MODEL_LIMITS.effortValues) + .optional(), + fastMode: z.boolean().optional(), + }) + ) + .refine((models) => Object.keys(models).length <= DECLARED_MODEL_LIMITS.models), + }) + .nullish(), + }) + .nullish(), + }) + .nullish(), +}); + +/** + * Reads the agent's own per-model statement, or nothing. + * + * An unknown `version`, a shape that does not parse, or a catalog past the + * bounds is ignored WHOLE rather than partially: half a catalog would answer + * "this model has no fast mode" for models the agent simply could not fit. + */ +export function readDeclaredModelCapabilities( + sessionResponse: unknown, + receivedAt: number +): DeclaredModelCapabilities | undefined { + const parsed = zDeclaredModelCapabilities.safeParse(sessionResponse); + const declared = parsed.success ? parsed.data._meta?.lody?.modelCapabilities : undefined; + if (!declared || Object.keys(declared.models).length === 0) { + return undefined; + } + return { + version: 1, + models: declared.models, + receivedAt, + ...(declared.producerRevision ? { producerRevision: declared.producerRevision } : {}), + }; +} + export type AcpCapabilitiesResult = { modes: Array<{ id: string; name: string; description?: string }>; models: Array<{ modelId: string; name?: string; description?: string }>; @@ -15,6 +84,8 @@ export type AcpCapabilitiesResult = { sessionFork: boolean; acknowledgedSteer: boolean; modelReasoningEfforts?: Record; + measuredForModelId?: string; + declaredModelCapabilities?: DeclaredModelCapabilities; }; function isSelectGroup(item: unknown): item is SessionConfigSelectGroup { @@ -151,7 +222,11 @@ type AcpSessionCapabilitiesResponse = { /** Extract cacheable capabilities from a real ACP new/load/resume session response. */ export function normalizeAcpSessionCapabilities( sessionResponse: AcpSessionCapabilitiesResponse, - lifecycleCapabilities: { sessionFork?: boolean; acknowledgedSteer?: boolean } = {} + lifecycleCapabilities: { + sessionFork?: boolean; + acknowledgedSteer?: boolean; + receivedAt?: number; + } = {} ): AcpCapabilitiesResult { const modes = (sessionResponse.modes?.availableModes ?? []).map((mode) => ({ id: mode.id, @@ -176,6 +251,18 @@ export function normalizeAcpSessionCapabilities( legacyModels.map((model) => model.modelId) ); + // What the snapshot is a snapshot OF, stored rather than left to each reader + // to infer from the model option's `currentValue`. + const modelOptionValue = modelOption?.currentValue; + const measuredForModelId = + typeof modelOptionValue === 'string' + ? modelOptionValue + : (readLegacySessionModelState(sessionResponse)?.currentModelId ?? undefined); + const declaredModelCapabilities = readDeclaredModelCapabilities( + sessionResponse, + lifecycleCapabilities.receivedAt ?? Date.now() + ); + return { modes, models, @@ -184,5 +271,7 @@ export function normalizeAcpSessionCapabilities( sessionFork: lifecycleCapabilities.sessionFork === true, acknowledgedSteer: lifecycleCapabilities.acknowledgedSteer === true, ...(modelReasoningEfforts ? { modelReasoningEfforts } : {}), + ...(measuredForModelId ? { measuredForModelId } : {}), + ...(declaredModelCapabilities ? { declaredModelCapabilities } : {}), }; } diff --git a/apps/cli/src/agent/acp-declared-model-capabilities.test.ts b/apps/cli/src/agent/acp-declared-model-capabilities.test.ts new file mode 100644 index 000000000..1cc1f2d31 --- /dev/null +++ b/apps/cli/src/agent/acp-declared-model-capabilities.test.ts @@ -0,0 +1,129 @@ +import { describe, expect, it } from 'vitest'; +import { + findDeclaredEffortValues, + findDeclaredFastModeSupport, + summarizeAgentRunConfigCapabilities, + DECLARED_MODEL_CAPABILITIES_TTL_MS, + type AcpCapabilityCacheEntry, +} from '@lody/shared'; + +import { normalizeAcpSessionCapabilities } from './acp-capability-normalization'; + +/** A `session/new` response shaped like Codex's, carrying the Lody declaration. */ +const sessionResponse = (models: Record, extra: Record = {}) => ({ + configOptions: [ + { + id: 'model', + name: 'Model', + category: 'model', + type: 'select' as const, + currentValue: 'gpt-5.2', + options: [ + { value: 'gpt-5.2', name: 'GPT-5.2' }, + { value: 'gpt-5.6-luna', name: 'Luna' }, + ], + }, + { + id: 'reasoning_effort', + name: 'Reasoning effort', + category: 'thought_level', + type: 'select' as const, + currentValue: 'medium', + options: [ + { value: 'low', name: 'Low' }, + { value: 'medium', name: 'Medium' }, + ], + }, + ], + _meta: { lody: { modelCapabilities: { version: 1, models, ...extra } } }, +}); + +const entryOf = ( + response: ReturnType, + receivedAt = 1_000 +): AcpCapabilityCacheEntry => ({ + cliType: 'builtin', + agentType: 'codex', + modes: [], + models: [], + fetchedAt: receivedAt, + ...normalizeAcpSessionCapabilities(response, { receivedAt }), +}); + +describe('declared model capabilities', () => { + const declared = { + 'gpt-5.2': { effortValues: ['low', 'medium'], fastMode: false }, + 'gpt-5.6-luna': { effortValues: ['low', 'medium', 'high', 'xhigh'], fastMode: true }, + }; + + it('answers for a model the snapshot never described', () => { + // The probe ran on gpt-5.2, which has no fast tier, so `configOptions` + // carries no fast toggle at all. That is the exact case where the snapshot + // knows nothing and the declaration does. + const entry = entryOf(sessionResponse(declared)); + + expect(entry.measuredForModelId).toBe('gpt-5.2'); + expect(findDeclaredFastModeSupport(entry, 'gpt-5.6-luna', 1_000)).toBe(true); + expect(findDeclaredFastModeSupport(entry, 'gpt-5.2', 1_000)).toBe(false); + expect(findDeclaredEffortValues(entry, 'gpt-5.6-luna', 1_000)).toEqual([ + 'low', + 'medium', + 'high', + 'xhigh', + ]); + }); + + it('says nothing about a model the agent did not name', () => { + const entry = entryOf(sessionResponse(declared)); + // Unknown, not unsupported: a declaration answers only for what it lists. + expect(findDeclaredFastModeSupport(entry, 'gpt-6-unreleased', 1_000)).toBeUndefined(); + }); + + it('stops speaking once stale or heard under another adapter version', () => { + const entry = entryOf(sessionResponse(declared)); + const stale = 1_000 + DECLARED_MODEL_CAPABILITIES_TTL_MS + 1; + expect(findDeclaredFastModeSupport(entry, 'gpt-5.6-luna', stale)).toBeUndefined(); + + // Same data, but the declaration was heard under a different adapter build. + const moved: AcpCapabilityCacheEntry = { + ...entry, + sourceVersion: 'codex@2', + declaredModelCapabilities: entry.declaredModelCapabilities + ? { ...entry.declaredModelCapabilities, sourceVersion: 'codex@1' } + : undefined, + }; + expect(findDeclaredFastModeSupport(moved, 'gpt-5.6-luna', 1_000)).toBeUndefined(); + }); + + it('reports fast mode for the agent once any model declares it', () => { + // The MCP create-options summary used to answer from the probed model's + // snapshot alone, so an agent whose default model lacks fast published + // `fastMode: false` for every model it has. + const entry = entryOf(sessionResponse(declared)); + const summary = summarizeAgentRunConfigCapabilities(entry, 1_000); + + expect(summary.fastMode).toBe(true); + expect(summary.measuredForModelId).toBe('gpt-5.2'); + expect( + summary.models.find((model) => model.id === 'gpt-5.6-luna')?.reasoningEffortValues + ).toEqual(['low', 'medium', 'high', 'xhigh']); + }); + + it('ignores a declaration it cannot trust, whole rather than in part', () => { + // Unknown version, and a catalog past the bound. Half a catalog would answer + // "no fast mode" for models the agent simply could not fit. + const wrongVersion = { + ...sessionResponse(declared), + _meta: { lody: { modelCapabilities: { version: 2, models: declared } } }, + }; + expect(entryOf(wrongVersion).declaredModelCapabilities).toBeUndefined(); + + const oversized = Object.fromEntries( + Array.from({ length: 65 }, (_unused, index) => [`model-${index}`, { fastMode: true }]) + ); + expect(entryOf(sessionResponse(oversized)).declaredModelCapabilities).toBeUndefined(); + + const noMeta = { ...sessionResponse(declared), _meta: undefined }; + expect(entryOf(noMeta).declaredModelCapabilities).toBeUndefined(); + }); +}); diff --git a/apps/cli/src/commands/session.test.ts b/apps/cli/src/commands/session.test.ts index 5d97136ca..9d3480a8f 100644 --- a/apps/cli/src/commands/session.test.ts +++ b/apps/cli/src/commands/session.test.ts @@ -31,7 +31,7 @@ import { filterAuthorizedMachineMetas, filterAuthorizedLocalProjectCandidates, filterCompatibleInheritedTurnConfig, - filterCompatibleTurnConfigOptionValues, + filterInheritedTurnConfigOptionValues, filterSessionMetas, hasNonPositionalPromptSource, listChildSessionIds, @@ -66,7 +66,6 @@ import { updateSessionActivityTimestamps, updateSessionActivityTimestampsBestEffort, validateTurnConfigOptionValues, - validateTurnModeAndModel, withBuiltinDefaultTurnMode, } from './session'; @@ -367,20 +366,24 @@ describe('session command helpers', () => { }); }); - it('validates ACP config option ids, types, and select values before dispatch', () => { + it('rejects only values the option type cannot carry, in any model', () => { const capability = createAcpCapability(); expect(() => validateTurnConfigOptionValues({ approval_policy: 'never', web_search: true }, capability) ).not.toThrow(); - expect(() => validateTurnConfigOptionValues({ unknown: true }, capability)).toThrow( - /Unknown ACP config option/ - ); + + // The snapshot describes one model. An id it does not carry, and a value + // outside the list it recorded, are both dispatched and reconciled against + // what the agent actually applies. + expect(() => validateTurnConfigOptionValues({ unknown: true }, capability)).not.toThrow(); + expect(() => + validateTurnConfigOptionValues({ approval_policy: 'invalid' }, capability) + ).not.toThrow(); + + // A boolean is not a select value under any model. expect(() => validateTurnConfigOptionValues({ approval_policy: false }, capability)).toThrow( /expects a select value/ ); - expect(() => - validateTurnConfigOptionValues({ approval_policy: 'invalid' }, capability) - ).toThrow(/Allowed values/); expect(() => validateTurnConfigOptionValues({ web_search: 'true' }, capability)).toThrow( /expects a boolean value/ ); @@ -409,20 +412,22 @@ describe('session command helpers', () => { runConfig: { modelId: 'model-a', reasoningEffort: 'high' }, }, capability - ).config + ) ).toEqual({ modelId: 'model-a', configOptionValues: { approval_policy: 'never', reasoning_effort: 'high' }, }); // No selection: the config passes through untouched, capability or not. - expect(applyAgentRunConfigSelection({ modeId: 'default' }, undefined).config).toEqual({ + expect(applyAgentRunConfigSelection({ modeId: 'default' }, undefined)).toEqual({ modeId: 'default', }); - expect(() => - applyAgentRunConfigSelection({ runConfig: { fastMode: true } }, capability) - ).toThrow(/does not offer a fast mode option/); + // The snapshot carries no fast toggle, which says nothing about the model + // this turn runs: it is dispatched on the agent's own binding. + expect( + applyAgentRunConfigSelection({ runConfig: { fastMode: true } }, capability).configOptionValues + ).toEqual({ 'fast-mode': true }); }); it('validates effort against the selected model and skips the probed-model snapshot check', () => { @@ -459,48 +464,106 @@ describe('session command helpers', () => { capability ); - expect(requested.config.configOptionValues).toEqual({ reasoning_effort: 'xhigh' }); - expect(requested.validatedConfigIds.has('reasoning_effort')).toBe(true); - // `xhigh` is absent from the probed model's option list, so the snapshot - // check must skip it rather than reject a value valid for model-b. + expect(requested.configOptionValues).toEqual({ reasoning_effort: 'xhigh' }); + // `xhigh` being absent from the probed model's option list is not a reason + // to reject a value the agent published for the model being run. + expect(() => + validateTurnConfigOptionValues(requested.configOptionValues, capability) + ).not.toThrow(); + }); + + it('accepts a stored per-model config option dispatched with another model', () => { + // Codex publishes `fast-mode` only while the current model has a fast speed + // tier, so a probe under a model without one produces this snapshot. An + // Agent Role (or a frozen Operation replay) then dispatches the concrete id + // with no semantic selection to resolve. + const capability: AcpCapabilityCacheEntry = { + ...createAcpCapability(), + configOptions: [ + { + id: 'model', + name: 'Model', + category: 'model', + type: 'select', + currentValue: 'model-a', + options: [ + { value: 'model-a', name: 'Model A' }, + { value: 'model-b', name: 'Model B' }, + ], + }, + ], + models: [], + }; + const roleRunConfig = { modelId: 'model-b', configOptionValues: { 'fast-mode': true } }; + + const requested = applyAgentRunConfigSelection(roleRunConfig, capability); + + expect(() => + validateTurnConfigOptionValues(requested.configOptionValues, capability) + ).not.toThrow(); + + // Same for the probed model: a snapshot that never carried the toggle is + // still only a snapshot, and the runtime settles whether Fast is on. expect(() => validateTurnConfigOptionValues( - requested.config.configOptionValues, - capability, - requested.validatedConfigIds + applyAgentRunConfigSelection( + { modelId: 'model-a', configOptionValues: { 'fast-mode': true } }, + capability + ).configOptionValues, + capability ) ).not.toThrow(); - expect(() => - validateTurnConfigOptionValues(requested.config.configOptionValues, capability) - ).toThrow(/Allowed values/); }); - it('drops inherited ACP config options that are no longer compatible', () => { + it('inherits only what the catalog knows, plus the per-model controls it cannot', () => { expect( - filterCompatibleTurnConfigOptionValues( - { approval_policy: 'never', web_search: 'true', removed: false }, + filterInheritedTurnConfigOptionValues( + { + approval_policy: 'never', + web_search: 'true', + removed_option: false, + 'fast-mode': true, + reasoning_effort: 'high', + thinking: 'on', + }, createAcpCapability() ) - ).toEqual({ approval_policy: 'never' }); + ).toEqual({ + approval_policy: 'never', + // Absent from the snapshot because the captured model lacked them, which + // says nothing about the model a new Session runs. + 'fast-mode': true, + reasoning_effort: 'high', + thinking: 'on', + }); + // `web_search` is a boolean option handed a string: undispatchable under any + // model. `removed_option` is an id nobody asked for on this turn and the + // catalog has never heard of — inheriting it forever is how a deleted + // option keeps being resent down the Session lineage. + + // With no capability at all the same rule applies: inheritance is a + // convenience, and an explicit request is always available. + expect( + filterInheritedTurnConfigOptionValues({ removed_option: false, 'fast-mode': true }, undefined) + ).toEqual({ 'fast-mode': true }); }); - it('validates explicit mode and model selectors against agent capabilities', () => { - const capability = createAcpCapability(); + it('still dispatches an explicitly requested option the catalog does not know', () => { + // The other half of the rule: an explicit `--config-option` or a frozen + // Operation request is a request, and a snapshot of one model does not get + // to refuse it. + const requested = applyAgentRunConfigSelection( + { configOptionValues: { removed_option: false } }, + createAcpCapability() + ); + + expect(requested.configOptionValues).toEqual({ removed_option: false }); expect(() => - validateTurnModeAndModel({ modeId: 'default', modelId: 'model-a' }, capability) + validateTurnConfigOptionValues(requested.configOptionValues, createAcpCapability()) ).not.toThrow(); - expect(() => validateTurnModeAndModel({ modeId: 'plan' }, capability)).toThrow( - 'Unsupported ACP mode' - ); - expect(() => validateTurnModeAndModel({ modelId: 'model-b' }, capability)).toThrow( - 'Unsupported ACP model' - ); - expect(() => validateTurnModeAndModel({ modeId: 'default' }, undefined)).toThrow( - 'Unsupported ACP mode' - ); }); - it('drops incompatible inherited mode and model selectors', () => { + it('keeps inherited mode and model selectors the snapshot does not list', () => { expect( filterCompatibleInheritedTurnConfig( { @@ -511,40 +574,12 @@ describe('session command helpers', () => { createAcpCapability() ) ).toEqual({ + modeId: 'plan', modelId: 'model-a', configOptionValues: { approval_policy: 'never' }, }); }); - it('accepts mode and model selectors advertised as ACP config options', () => { - const capability: AcpCapabilityCacheEntry = { - ...createAcpCapability(), - modes: [], - models: [], - configOptions: [ - { - id: 'mode', - name: 'Mode', - category: 'mode', - type: 'select', - currentValue: 'plan', - options: [{ value: 'plan', name: 'Plan' }], - }, - { - id: 'model', - name: 'Model', - category: 'model', - type: 'select', - currentValue: 'model-b', - options: [{ value: 'model-b', name: 'Model B' }], - }, - ], - }; - expect(() => - validateTurnModeAndModel({ modeId: 'plan', modelId: 'model-b' }, capability) - ).not.toThrow(); - }); - it('sorts sessions with invalid createdAt timestamps deterministically', () => { const sessions = [ createSessionMeta({ diff --git a/apps/cli/src/commands/session.ts b/apps/cli/src/commands/session.ts index 198bf0d26..25fa80cb7 100644 --- a/apps/cli/src/commands/session.ts +++ b/apps/cli/src/commands/session.ts @@ -49,6 +49,7 @@ import { isMachineDocRoomId, isSessionDocRoomId, hasAgentRunConfigSelection, + isAcpPerModelConfigId, resolveAgentRunConfigSelection, resolveBaseBranchPreference, resolveProjectGitHubRepo, @@ -1346,36 +1347,28 @@ export type ResolvedTurnDispatchConfig = { * option values the target agent advertises. Explicit ids on the config win over * the semantic selection only where the selection produced nothing. * - * Returns the ids the resolver validated against the TARGET model so the caller - * can exclude them from the probed-model snapshot check, plus any selection that - * could not be verified offline. + * Nothing here blocks a turn: the snapshot describes the model it was captured + * under, and the runtime is what settles the rest. */ export function applyAgentRunConfigSelection( config: ResolvedTurnDispatchConfig, capability: AcpCapabilityCacheEntry | undefined -): { - config: ResolvedTurnDispatchConfig; - validatedConfigIds: ReadonlySet; - unverifiedSelections: readonly string[]; -} { +): ResolvedTurnDispatchConfig { const { runConfig, ...rest } = config; - if (!hasAgentRunConfigSelection(runConfig)) { - return { config: rest, validatedConfigIds: new Set(), unverifiedSelections: [] }; - } - const resolved = resolveAgentRunConfigSelection(runConfig, capability); + const resolved = hasAgentRunConfigSelection(runConfig) + ? resolveAgentRunConfigSelection(runConfig, capability) + : {}; const configOptionValues = { ...(rest.configOptionValues ?? {}), ...(resolved.configOptionValues ?? {}), }; + const modeId = resolved.modeId ?? rest.modeId; + const modelId = resolved.modelId ?? rest.modelId; return { - config: { - ...(rest.taskToolsEnabled !== undefined ? { taskToolsEnabled: rest.taskToolsEnabled } : {}), - ...((resolved.modeId ?? rest.modeId) ? { modeId: resolved.modeId ?? rest.modeId } : {}), - ...((resolved.modelId ?? rest.modelId) ? { modelId: resolved.modelId ?? rest.modelId } : {}), - ...(Object.keys(configOptionValues).length > 0 ? { configOptionValues } : {}), - }, - validatedConfigIds: new Set(resolved.validatedConfigIds ?? []), - unverifiedSelections: resolved.unverifiedSelections ?? [], + ...(rest.taskToolsEnabled !== undefined ? { taskToolsEnabled: rest.taskToolsEnabled } : {}), + ...(modeId ? { modeId } : {}), + ...(modelId ? { modelId } : {}), + ...(Object.keys(configOptionValues).length > 0 ? { configOptionValues } : {}), }; } @@ -1446,7 +1439,7 @@ function mergeTurnDispatchConfig( }; } -function validateConfigOptionValue( +function validateConfigOptionShape( option: AcpConfigOptionSummary, value: string | boolean ): string | undefined { @@ -1455,98 +1448,85 @@ function validateConfigOptionValue( ? undefined : `Config option "${option.id}" expects a boolean value.`; } - if (typeof value !== 'string') { - return `Config option "${option.id}" expects a select value.`; - } - if (!option.options.some((candidate) => candidate.value === value)) { - return `Invalid value for config option "${option.id}": ${value}. Allowed values: ${option.options - .map((candidate) => candidate.value) - .join(', ')}.`; - } - return undefined; + return typeof value === 'string' + ? undefined + : `Config option "${option.id}" expects a select value.`; } +/** + * Rejects only what cannot be dispatched at all. + * + * The capability snapshot describes ONE model — the one that was current when + * it was captured — so an id it omits, or a value outside the list it recorded, + * is not evidence about the model this turn runs. Those are dispatched and + * reconciled against what the agent actually applied. What stays a local error + * is a value the option's own declared TYPE cannot carry: a boolean toggle + * cannot take a string, and a select cannot take a boolean, in any model. + */ export function validateTurnConfigOptionValues( values: Record | undefined, - capability: AcpCapabilityCacheEntry | undefined, - /** - * Ids already validated against the model actually being selected. The - * capability's `configOptions` only describe the probed model, so re-checking - * them here would reject values that are valid for the target model. - */ - skipIds?: ReadonlySet + capability: AcpCapabilityCacheEntry | undefined ): void { - const entries = Object.entries(values ?? {}).filter(([id]) => !skipIds?.has(id)); - if (entries.length === 0) { + const entries = Object.entries(values ?? {}); + if (entries.length === 0 || !capability?.configOptions) { return; } - if (!capability?.configOptions) { - throw new Error('ACP config options are unavailable for the selected agent.'); - } const optionsById = new Map(capability.configOptions.map((option) => [option.id, option])); for (const [id, value] of entries) { const option = optionsById.get(id); if (!option) { - throw new Error(`Unknown ACP config option for the selected agent: ${id}.`); + continue; } - const error = validateConfigOptionValue(option, value); + const error = validateConfigOptionShape(option, value); if (error) { throw new Error(error); } } } -export function filterCompatibleTurnConfigOptionValues( +/** + * What a NEW Session may inherit from an older turn's config. + * + * Inheritance is not a request. Nobody asked for these values on this turn — + * they are carried forward as a convenience — so an id the capability catalog + * does not know gets no benefit of the doubt here, unlike an explicit + * `--config-option` or a frozen Operation request, which are dispatched as + * asked and reconciled at runtime. Without that distinction a removed or + * renamed option rides the Session lineage forever: every new Session inherits + * it, the agent rejects or warns about it, and the resulting config becomes the + * next inheritance source, with no surface anywhere to clear it. + * + * The one exception is an id Lody knows names a PER-MODEL control: those are + * absent from a snapshot whenever the captured model lacked them, which says + * nothing about the model a new Session will run. This mirrors what the + * composer keeps client-side. + * + * With no capability at all nothing can be cataloged, so the same rule applies + * and only per-model ids survive. Inheritance is a convenience and an explicit + * request is always available; carrying every historical key forward on a + * missing snapshot is how the accumulation starts. + */ +export function filterInheritedTurnConfigOptionValues( values: Record | undefined, capability: AcpCapabilityCacheEntry | undefined ): Record | undefined { - if (!values || !capability?.configOptions) { + if (!values) { return undefined; } - const optionsById = new Map(capability.configOptions.map((option) => [option.id, option])); + const optionsById = new Map( + (capability?.configOptions ?? []).map((option) => [option.id, option]) + ); const compatible = Object.fromEntries( Object.entries(values).filter(([id, value]) => { const option = optionsById.get(id); - return option !== undefined && validateConfigOptionValue(option, value) === undefined; + return option === undefined + ? isAcpPerModelConfigId(id) + : validateConfigOptionShape(option, value) === undefined; }) ); return Object.keys(compatible).length > 0 ? compatible : undefined; } -const getSupportedTurnSelectorIds = ( - capability: AcpCapabilityCacheEntry | undefined, - category: 'mode' | 'model' -): Set => { - const ids = new Set( - category === 'mode' - ? (capability?.modes ?? []).map((mode) => mode.id) - : (capability?.models ?? []).map((model) => model.modelId) - ); - for (const option of capability?.configOptions ?? []) { - if (option.category !== category || option.type !== 'select') { - continue; - } - for (const candidate of option.options) { - if (typeof candidate.value === 'string') { - ids.add(candidate.value); - } - } - } - return ids; -}; - -export function validateTurnModeAndModel( - config: Pick, - capability: AcpCapabilityCacheEntry | undefined -): void { - if (config.modeId && !getSupportedTurnSelectorIds(capability, 'mode').has(config.modeId)) { - throw new Error(`Unsupported ACP mode for the selected agent: ${config.modeId}.`); - } - if (config.modelId && !getSupportedTurnSelectorIds(capability, 'model').has(config.modelId)) { - throw new Error(`Unsupported ACP model for the selected agent: ${config.modelId}.`); - } -} - export function filterCompatibleInheritedTurnConfig( config: ResolvedTurnDispatchConfig | undefined, capability: AcpCapabilityCacheEntry | undefined @@ -1554,15 +1534,13 @@ export function filterCompatibleInheritedTurnConfig( if (!config) { return undefined; } - const supportedModes = getSupportedTurnSelectorIds(capability, 'mode'); - const supportedModels = getSupportedTurnSelectorIds(capability, 'model'); - const configOptionValues = filterCompatibleTurnConfigOptionValues( + const configOptionValues = filterInheritedTurnConfigOptionValues( config.configOptionValues, capability ); return { - ...(config.modeId && supportedModes.has(config.modeId) ? { modeId: config.modeId } : {}), - ...(config.modelId && supportedModels.has(config.modelId) ? { modelId: config.modelId } : {}), + ...(config.modeId ? { modeId: config.modeId } : {}), + ...(config.modelId ? { modelId: config.modelId } : {}), ...(configOptionValues ? { configOptionValues } : {}), ...(config.taskToolsEnabled !== undefined ? { taskToolsEnabled: config.taskToolsEnabled } : {}), }; @@ -2773,16 +2751,11 @@ async function resolveEffectiveSessionCreateDispatchConfig(args: { }) : undefined; const requested = applyAgentRunConfigSelection(dispatchConfig, capability); - validateTurnModeAndModel(requested.config, capability); - validateTurnConfigOptionValues( - requested.config.configOptionValues, - capability, - requested.validatedConfigIds - ); + validateTurnConfigOptionValues(requested.configOptionValues, capability); return { ...withBuiltinDefaultTurnMode( mergeTurnDispatchConfig( - requested.config, + requested, filterCompatibleInheritedTurnConfig(inheritedDispatchConfig, capability) ), args.agentConfig @@ -3152,7 +3125,6 @@ export async function sendSessionChatResult( machineId: session.machineId, agentConfigId: session.agentConfigId, }); - validateTurnModeAndModel(dispatchConfig, capability); validateTurnConfigOptionValues(dispatchConfig.configOptionValues, capability); } const effectiveDispatchConfig = withBuiltinDefaultTurnMode(dispatchConfig, session); diff --git a/apps/cli/src/lib/loro/doc.ts b/apps/cli/src/lib/loro/doc.ts index 9d6a9ce7f..aaa30e115 100644 --- a/apps/cli/src/lib/loro/doc.ts +++ b/apps/cli/src/lib/loro/doc.ts @@ -1522,7 +1522,11 @@ export class LoroDocumentManager { sourceVersion: string, modelReasoningEfforts?: Record, acknowledgedSteer = false, - options: { signal?: AbortSignal } = {} + options: { + signal?: AbortSignal; + measuredForModelId?: string; + declaredModelCapabilities?: AcpCapabilityCacheEntry['declaredModelCapabilities']; + } = {} ): Promise { options.signal?.throwIfAborted(); if (!this.machine) { @@ -3120,6 +3124,17 @@ const serializeAcpCapabilityWithoutFetchTime = (entry: AcpCapabilityCacheEntry): models: entry.models, configOptions: entry.configOptions, modelReasoningEfforts: entry.modelReasoningEfforts, + measuredForModelId: entry.measuredForModelId, + // Content, not freshness: `receivedAt` is deliberately excluded so a + // re-probe that learns nothing new does not rewrite the row, while a + // catalog that actually changed does. + declaredModelCapabilities: entry.declaredModelCapabilities + ? { + version: entry.declaredModelCapabilities.version, + models: entry.declaredModelCapabilities.models, + producerRevision: entry.declaredModelCapabilities.producerRevision, + } + : undefined, availableCommands: entry.availableCommands, sessionFork: entry.sessionFork, acknowledgedSteer: entry.acknowledgedSteer, @@ -3208,7 +3223,11 @@ export class MachineDocument implements LoroDocument<{}, MachineMeta> { sourceVersion: string, modelReasoningEfforts?: Record, acknowledgedSteer = false, - options: { signal?: AbortSignal } = {} + options: { + signal?: AbortSignal; + measuredForModelId?: string; + declaredModelCapabilities?: AcpCapabilityCacheEntry['declaredModelCapabilities']; + } = {} ): Promise { options.signal?.throwIfAborted(); const normalizedModes = modes.map((mode) => ({ @@ -3221,6 +3240,14 @@ export class MachineDocument implements LoroDocument<{}, MachineMeta> { name: model.name ?? model.modelId, description: model.description ?? undefined, })); + const handle = await this.openMachineFlockDoc(); + options.signal?.throwIfAborted(); + const capabilityKey = getAcpCapabilityCacheKey(configId); + const existing = getMachineFlockAcpCapabilities( + readMachineFlockRowsFromFlock(handle.flock, { families: ['acpCapability'] }) + )[capabilityKey]; + const existingDeclared = + existing?.sourceVersion === sourceVersion ? existing.declaredModelCapabilities : undefined; const entry: AcpCapabilityCacheEntry = { cliType, agentType, @@ -3238,14 +3265,17 @@ export class MachineDocument implements LoroDocument<{}, MachineMeta> { modelReasoningEfforts && Object.keys(modelReasoningEfforts).length > 0 ? modelReasoningEfforts : undefined, + ...(options.measuredForModelId ? { measuredForModelId: options.measuredForModelId } : {}), + // A refresh that carries no declaration keeps the one already stored: the + // agent has not retracted it, this probe simply did not hear it (an older + // adapter, a failed catalog fetch). Only a NEW declaration replaces it. + ...((options.declaredModelCapabilities ?? existingDeclared) + ? { + declaredModelCapabilities: options.declaredModelCapabilities ?? existingDeclared, + } + : {}), fetchedAt: getServerNow(), }; - const handle = await this.openMachineFlockDoc(); - options.signal?.throwIfAborted(); - const capabilityKey = getAcpCapabilityCacheKey(configId); - const existing = getMachineFlockAcpCapabilities( - readMachineFlockRowsFromFlock(handle.flock, { families: ['acpCapability'] }) - )[capabilityKey]; if ( existing && serializeAcpCapabilityWithoutFetchTime(existing) === diff --git a/apps/cli/src/lib/loro/machine-document-capabilities.test.ts b/apps/cli/src/lib/loro/machine-document-capabilities.test.ts index a9254d48f..63864b9cb 100644 --- a/apps/cli/src/lib/loro/machine-document-capabilities.test.ts +++ b/apps/cli/src/lib/loro/machine-document-capabilities.test.ts @@ -120,6 +120,72 @@ describe('MachineDocument ACP capabilities', () => { }); }); + it('keeps a declaration a later probe did not hear, and replaces one it did', async () => { + const flock = new FakeMachineFlock(); + const repo = { + openFlockDoc: vi.fn(async () => ({ flock, syncOnce: vi.fn(async () => undefined) })), + flush: vi.fn(async () => undefined), + } as unknown as LoroRepo; + const document = new MachineDocument( + repo, + 'workspace-1' as WorkspaceId, + 'machine-1' as MachineId, + vi.fn() + ); + const write = (options: Parameters[11]) => + document.updateAcpCapabilities( + 'config-1' as AgentConfigId, + 'builtin', + 'codex', + [{ id: 'agent', name: 'Agent' }], + [{ modelId: 'gpt-5', name: 'GPT-5' }], + undefined, + undefined, + false, + 'builtin:codex:test', + undefined, + false, + options + ); + const stored = () => + [...flock.rows.values()][0]?.value as { + declaredModelCapabilities?: { models: Record }; + }; + + await write({ + declaredModelCapabilities: { + version: 1, + models: { 'gpt-5.6-luna': { fastMode: true } }, + receivedAt: 1, + }, + }); + expect(stored()?.declaredModelCapabilities?.models).toEqual({ + 'gpt-5.6-luna': { fastMode: true }, + }); + + // A probe against an older adapter, or one whose catalog fetch failed, hears + // no declaration. That is not the agent retracting it. + await write({}); + expect(stored()?.declaredModelCapabilities?.models).toEqual({ + 'gpt-5.6-luna': { fastMode: true }, + }); + + // A new declaration is the agent speaking again, and replaces it whole — + // even when NOTHING else about the capabilities changed, which is the case + // the de-duplication key has to notice on its own. + await write({ + declaredModelCapabilities: { + version: 1, + models: { 'gpt-5.6-luna': { fastMode: true }, 'gpt-5.2': { fastMode: false } }, + receivedAt: 2, + }, + }); + expect(stored()?.declaredModelCapabilities?.models).toEqual({ + 'gpt-5.6-luna': { fastMode: true }, + 'gpt-5.2': { fastMode: false }, + }); + }); + it('does not write capabilities when cancelled while opening the Machine Flock', async () => { const flock = new FakeMachineFlock(); let markOpenStarted!: () => void; diff --git a/apps/cli/src/lib/message-handler.ts b/apps/cli/src/lib/message-handler.ts index 50d1170ca..574e76bd7 100644 --- a/apps/cli/src/lib/message-handler.ts +++ b/apps/cli/src/lib/message-handler.ts @@ -63,6 +63,7 @@ import { type SessionLegacyMetaFields, PERMISSION_REQUEST_TIMEOUT_MS, type ChatFailedCode, + type ChatFailedMeta, type ChatFailedReason, type ProjectRef, type SessionPreparationCancelSpec, @@ -318,6 +319,7 @@ import { AutoPromptRunner } from '@/session/auto-prompt-runner'; import { TurnPostProcessingService } from '@/session/turn-post-processing-service'; import { applyAcpSessionRunConfig, + AcpPermissionNotAppliedError, type AcpSessionRunConfig, } from '@/session/acp-session-config-applier'; import { @@ -1679,7 +1681,8 @@ export class MessageHandler { sessionDoc: SessionDocument, reason: ChatFailedReason, message?: string, - code?: ChatFailedCode + code?: ChatFailedCode, + permission?: ChatFailedMeta['permission'] ): Promise { // Failure notices append to the history list; order them after the user // turn entry for RPC fast-path turns (no-op when no gate is pending). @@ -1692,6 +1695,7 @@ export class MessageHandler { meta: { reason, ...(code ? { code } : {}), + ...(permission ? { permission } : {}), message, }, }; @@ -1721,11 +1725,12 @@ export class MessageHandler { basedOnUserTurnId?: string; } ): Promise { - const { runtimeConfigPatch, warningSelections } = await applyAcpSessionRunConfig({ - session, - config, - logger: this.logger, - }); + const { runtimeConfigPatch, warningSelections, permissionEscalation } = + await applyAcpSessionRunConfig({ + session, + config, + logger: this.logger, + }); if (runtimeConfigPatch && context.basedOnUserTurnId) { const basedOnUserTurnId = context.basedOnUserTurnId; @@ -1740,13 +1745,27 @@ export class MessageHandler { }); } + /* The one divergence that stops a turn. Everything else — model, effort, + fast — runs and reports, because the worst case is a slower or costlier + turn. Running with MORE permission than the user asked for is not that: + by the time a warning is readable the agent may already have edited + files. The turn fails here, before `prompt`, and the user re-sends with + an explicit one-time acceptance if they want it anyway. */ + if (permissionEscalation) { + throw new AcpPermissionNotAppliedError( + permissionEscalation.controlId, + permissionEscalation.requestedModeId, + permissionEscalation.effectiveModeId + ); + } + if (warningSelections.length > 0) { // Not awaited: this is reporting, and the prompt hot path must not block // on a history write. void this.recordAgentWarning(session.sessionId, { - message: `The agent rejected part of the requested run configuration (${warningSelections.join( + message: `The agent did not apply part of the requested run configuration (${warningSelections.join( ', ' - )}) and is using its own values instead. Reasoning effort and fast mode depend on the selected model.`, + )}) and is running with its own values instead. Reasoning effort and fast mode depend on the selected model.`, source: 'configWarning', }); } @@ -3119,8 +3138,8 @@ export class MessageHandler { notifySessionCompleted: async (sessionId, userId, occurrenceId) => await this.notifySessionCompleted(sessionId, userId, occurrenceId), }, - recordChatFailure: async (sessionDoc, reason, message, code) => - await this.recordChatFailure(sessionDoc, reason, message, code), + recordChatFailure: async (sessionDoc, reason, message, code, permission) => + await this.recordChatFailure(sessionDoc, reason, message, code, permission), maybeGenerateAndStoreSessionTitle: async ( sessionId, cliType, diff --git a/apps/cli/src/orchestration/operation-coordinator.ts b/apps/cli/src/orchestration/operation-coordinator.ts index f069af2ee..245efe601 100644 --- a/apps/cli/src/orchestration/operation-coordinator.ts +++ b/apps/cli/src/orchestration/operation-coordinator.ts @@ -6,6 +6,7 @@ import { performance } from 'node:perf_hooks'; import type { RepoWatchHandle } from 'loro-repo'; import { + deriveTurnInputConfigForNewTurn, buildMissingEmail, getServerNow, getSessionRoomId, @@ -1010,8 +1011,11 @@ export class LodyOperationCoordinator { items: [item], fileDiff: [], finished: true, + // New `systemTurnId`, new prompt: the frozen turn's one-time permission + // acceptance stays with the turn it was given for, even in history that + // never dispatches — a nested Operation would freeze and copy it again. inputConfig: { - ...operation.frozenContinuationConfig.inputConfig, + ...deriveTurnInputConfigForNewTurn(operation.frozenContinuationConfig.inputConfig), prompt: completionText(operation), chainDepth: operation.initiatorChainDepth + 1, }, diff --git a/apps/cli/src/session/acp-session-config-applier.test.ts b/apps/cli/src/session/acp-session-config-applier.test.ts index 0c2978021..ddcfd0b41 100644 --- a/apps/cli/src/session/acp-session-config-applier.test.ts +++ b/apps/cli/src/session/acp-session-config-applier.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest'; import type { ACPSessionId, SessionId } from '@lody/shared'; +import type { AcceptedWiderPermission } from '@lody/shared'; import type { AgentClient } from '@/agent/agent-client'; import type { Logger } from '@/utils/logger'; import { applyAcpSessionRunConfig } from './acp-session-config-applier'; @@ -22,13 +23,18 @@ function createLogger(): Logger { describe('applyAcpSessionRunConfig', () => { it('applies mode, model, and remaining options to an established ACP session', async () => { - const setSessionMode = vi.fn(async () => undefined); + // A real agent reports back what it accepted, so the fake does too: the + // applier reads that state rather than echoing the request. + let currentMode = 'default'; + const setSessionMode = vi.fn(async (_sessionId: string, mode: string) => { + currentMode = mode; + }); const setSessionModel = vi.fn(async () => undefined); const setSessionConfigOption = vi.fn(async () => undefined); const agentClient = { isCreated: () => true, getConfigOptions: () => [ - { id: 'permission-mode', category: 'mode', type: 'select', currentValue: 'default' }, + { id: 'permission-mode', category: 'mode', type: 'select', currentValue: currentMode }, { id: 'engine', category: 'model', type: 'select', currentValue: 'model-a' }, { id: 'effort', category: 'thought_level', type: 'select', currentValue: 'high' }, ], @@ -109,60 +115,673 @@ describe('applyAcpSessionRunConfig', () => { expect(vi.mocked(logger.debug).mock.calls.flat().join('\n')).not.toContain('private-value'); }); - it.each(['codex', 'claude'])( - 'suppresses known %s run-config mismatch warnings while retaining rejection diagnostics', - async (agentType) => { - const reject = vi.fn(async () => { - throw new Error('rejected'); + it('keeps the requested permission mode when the model switch would reset it', async () => { + // Claude rebuilds the available permission modes on a model switch and + // downgrades the current one to `default` when the new model does not + // support it. Applying the mode before the model therefore loses it — and + // loses it toward WIDER permissions than the turn asked for. + let currentMode = 'default'; + let currentModel = 'model-a'; + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'permission-mode', category: 'mode', type: 'select', currentValue: currentMode }, + { id: 'engine', category: 'model', type: 'select', currentValue: currentModel }, + ], + setSessionMode: vi.fn(async (_sessionId: string, mode: string) => { + currentMode = mode; + }), + unstable_setSessionModel: vi.fn(async (_sessionId: string, model: string) => { + currentModel = model; + currentMode = 'default'; + }), + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await applyAcpSessionRunConfig({ + session: { + sessionId: 'session-8' as SessionId, + acpSessionId: 'acp-8' as ACPSessionId, + agentClient, + }, + config: { + cliType: 'builtin', + agentType: 'claude', + modeId: 'plan', + modelId: 'model-b', + }, + logger: createLogger(), + }); + + expect(result.runtimeConfigPatch?.modelId).toBe('model-b'); + expect(result.runtimeConfigPatch?.modeId).toBe('plan'); + expect(result.warningSelections).toEqual([]); + }); + + describe('permission not applied', () => { + /** An agent whose model switch resets the permission mode to `auto`. */ + const wideningAgent = () => { + let currentMode = 'auto'; + let currentModel = 'model-a'; + return { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'permission-mode', category: 'mode', type: 'select', currentValue: currentMode }, + { id: 'engine', category: 'model', type: 'select', currentValue: currentModel }, + ], + setSessionMode: vi.fn(async (_sessionId: string, mode: string) => { + currentMode = mode; + }), + unstable_setSessionModel: vi.fn(async (_sessionId: string, model: string) => { + currentModel = model; + currentMode = 'auto'; + }), + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + }; + + const apply = async ( + config: Parameters[0]['config'], + agentClient: AgentClient + ) => + await applyAcpSessionRunConfig({ + session: { + sessionId: 'session-9' as SessionId, + acpSessionId: 'acp-9' as ACPSessionId, + agentClient, + }, + config, + logger: createLogger(), + }); + + it('reports the escalation when the agent ends up wider than requested', async () => { + // The agent refuses to stay in plan: `setSessionMode` "succeeds" but its + // own state says `auto`, which approves without asking a human. + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'permission-mode', category: 'mode', type: 'select', currentValue: 'auto' }, + ], + setSessionMode: vi.fn(async () => undefined), + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await apply( + { cliType: 'builtin', agentType: 'claude', modeId: 'plan' }, + agentClient + ); + + expect(result.permissionEscalation).toEqual({ + controlId: 'permission-mode', + requestedModeId: 'plan', + effectiveModeId: 'auto', + }); + }); + + it('does not report one when the agent honors the request', async () => { + // Same widening agent, but the ordering fix means plan is applied after + // the model that would have reset it. + const result = await apply( + { cliType: 'builtin', agentType: 'claude', modeId: 'plan', modelId: 'model-b' }, + wideningAgent() + ); + + expect(result.runtimeConfigPatch?.modeId).toBe('plan'); + expect(result.permissionEscalation).toBeUndefined(); + }); + + it('does not report one on an unconfirmed, narrower, or unranked outcome', async () => { + // Nothing to compare against: the agent publishes no mode of its own, so + // the patch carries only our own acknowledgement. + const silent = { + isCreated: () => true, + getConfigOptions: () => [], + setSessionMode: vi.fn(async () => undefined), + } as unknown as AgentClient; + expect( + (await apply({ cliType: 'builtin', agentType: 'claude', modeId: 'plan' }, silent)) + .permissionEscalation + ).toBeUndefined(); + + // Narrower than requested is a functional mismatch, not an escalation. + const narrower = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'permission-mode', category: 'mode', type: 'select', currentValue: 'plan' }, + ], + setSessionMode: vi.fn(async () => undefined), + } as unknown as AgentClient; + expect( + (await apply({ cliType: 'builtin', agentType: 'claude', modeId: 'auto' }, narrower)) + .permissionEscalation + ).toBeUndefined(); + + // A third-party mode Lody does not rank can never be judged wider. + const unranked = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'permission-mode', category: 'mode', type: 'select', currentValue: 'vendor-mode' }, + ], + setSessionMode: vi.fn(async () => undefined), + } as unknown as AgentClient; + expect( + (await apply({ cliType: 'registry', agentType: 'other', modeId: 'plan' }, unranked)) + .permissionEscalation + ).toBeUndefined(); + }); + + it('catches a widened explicit _permission selector, not just the mode', async () => { + // Grok's real permission control is a `_permission` config option, not a + // mode: `ask` requested, `always-approve` reported. + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { + id: 'permission_mode', + category: '_permission', + type: 'select', + currentValue: 'always-approve', + }, + ], + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await apply( + { + cliType: 'builtin', + agentType: 'grok', + configOptionValues: { permission_mode: 'ask' }, + }, + agentClient + ); + + expect(result.permissionEscalation).toEqual({ + controlId: 'permission_mode', + requestedModeId: 'ask', + effectiveModeId: 'always-approve', + }); + }); + + it('ranks the remaining builtin permission values', async () => { + const permissionAgent = (currentValue: string) => + ({ + isCreated: () => true, + getConfigOptions: () => [ + { id: 'permission_mode', category: '_permission', type: 'select', currentValue }, + ], + setSessionConfigOption: vi.fn(async () => undefined), + }) as unknown as AgentClient; + + // Grok `ask` → `auto`: approval moves from a human to the model. + expect( + ( + await apply( + { + cliType: 'builtin', + agentType: 'grok', + configOptionValues: { permission_mode: 'ask' }, + }, + permissionAgent('auto') + ) + ).permissionEscalation + ).toEqual({ controlId: 'permission_mode', requestedModeId: 'ask', effectiveModeId: 'auto' }); + + // DeepSeek Harness `read-only` → `workspace-write`. + expect( + ( + await apply( + { + cliType: 'builtin', + agentType: 'deepseek', + configOptionValues: { permission_mode: 'read-only' }, + }, + permissionAgent('workspace-write') + ) + ).permissionEscalation + ).toEqual({ + controlId: 'permission_mode', + requestedModeId: 'read-only', + effectiveModeId: 'workspace-write', + }); + + // The other direction is a functional mismatch, not an escalation. + expect( + ( + await apply( + { + cliType: 'builtin', + agentType: 'grok', + configOptionValues: { permission_mode: 'always-approve' }, + }, + permissionAgent('ask') + ) + ).permissionEscalation + ).toBeUndefined(); + }); + + it('applies an explicit _permission selector after the model', async () => { + // Same reset hazard as the mode: a model switch must not be able to + // overwrite the permission the turn asked for. + let currentPermission = 'always-approve'; + let currentModel = 'model-a'; + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { + id: 'permission_mode', + category: '_permission', + type: 'select', + currentValue: currentPermission, + }, + { id: 'engine', category: 'model', type: 'select', currentValue: currentModel }, + ], + unstable_setSessionModel: vi.fn(async (_sessionId: string, model: string) => { + currentModel = model; + currentPermission = 'always-approve'; + }), + setSessionConfigOption: vi.fn( + async (_sessionId: string, configId: string, value: unknown) => { + if (configId === 'permission_mode' && typeof value === 'string') { + currentPermission = value; + } + } + ), + } as unknown as AgentClient; + + const result = await apply( + { + cliType: 'builtin', + agentType: 'grok', + modelId: 'model-b', + configOptionValues: { permission_mode: 'ask' }, + }, + agentClient + ); + + expect(result.permissionEscalation).toBeUndefined(); + expect(result.runtimeConfigPatch?.configOptionValues?.['permission_mode']).toBe('ask'); + }); + + it('stands down for a _permission escalation the turn accepted', async () => { + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { + id: 'permission_mode', + category: '_permission', + type: 'select', + currentValue: 'always-approve', + }, + ], + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await apply( + { + cliType: 'builtin', + agentType: 'grok', + configOptionValues: { permission_mode: 'ask' }, + acceptWiderPermissions: [ + { + controlId: 'permission_mode', + requestedModeId: 'ask', + effectiveModeId: 'always-approve', + }, + ], + }, + agentClient + ); + + expect(result.permissionEscalation).toBeUndefined(); + expect(result.warningSelections).toEqual(['permission_mode="ask"']); + }); + + it('still stops when the agent moved further than what was accepted', async () => { + // Accepted `plan → auto` after the first stop; by the time the turn + // re-runs the agent reports `always-approve`. That is a difference the + // user was never shown, so the acceptance does not cover it. + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { + id: 'permission-mode', + category: 'mode', + type: 'select', + currentValue: 'always-approve', + }, + ], + setSessionMode: vi.fn(async () => undefined), + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await apply( + { + cliType: 'builtin', + agentType: 'claude', + modeId: 'plan', + acceptWiderPermissions: [ + { controlId: 'permission-mode', requestedModeId: 'plan', effectiveModeId: 'auto' }, + ], + }, + agentClient + ); + + expect(result.permissionEscalation).toEqual({ + controlId: 'permission-mode', + requestedModeId: 'plan', + effectiveModeId: 'always-approve', }); + }); + + it('still stops on a second permission control the user never saw', async () => { + // Grok carries both a mode selector and an explicit `_permission` one. + // Accepting the disclosed one must not wave the other one through. const agentClient = { isCreated: () => true, getConfigOptions: () => [ - { id: 'effort', category: 'thought_level' }, - { id: 'fast', category: 'fast-mode' }, - { id: 'collaboration_mode', category: 'collaboration_mode' }, - { id: 'custom-option', category: 'custom' }, + { id: 'interaction_mode', category: 'mode', type: 'select', currentValue: 'auto' }, + { + id: 'permission_mode', + category: '_permission', + type: 'select', + currentValue: 'always-approve', + }, ], - setSessionMode: reject, - unstable_setSessionModel: reject, - setSessionConfigOption: reject, + setSessionMode: vi.fn(async () => undefined), + setSessionConfigOption: vi.fn(async () => undefined), } as unknown as AgentClient; - await expect( - applyAcpSessionRunConfig({ - session: { - sessionId: 'session-3' as SessionId, - acpSessionId: 'acp-3' as ACPSessionId, - agentClient, + const result = await apply( + { + cliType: 'builtin', + agentType: 'grok', + modeId: 'plan', + configOptionValues: { permission_mode: 'ask' }, + acceptWiderPermissions: [ + { controlId: 'interaction_mode', requestedModeId: 'plan', effectiveModeId: 'auto' }, + ], + }, + agentClient + ); + + expect(result.permissionEscalation).toEqual({ + controlId: 'permission_mode', + requestedModeId: 'ask', + effectiveModeId: 'always-approve', + }); + }); + + it('keeps scanning past the accepted item to the one still undisclosed', async () => { + // The accepted control is reached FIRST in the apply order, so a skip that + // ended the scan would let the later mode escalation through. + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'interaction_mode', category: 'mode', type: 'select', currentValue: 'auto' }, + { + id: 'permission_mode', + category: '_permission', + type: 'select', + currentValue: 'always-approve', }, - config: { - cliType: 'builtin', - agentType, - modeId: 'plan', - modelId: 'model-a', - configOptionValues: { - effort: 'high', - fast: false, - collaboration_mode: 'plan', - 'custom-option': 'enabled', + ], + setSessionMode: vi.fn(async () => undefined), + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await apply( + { + cliType: 'builtin', + agentType: 'grok', + modeId: 'plan', + configOptionValues: { permission_mode: 'ask' }, + acceptWiderPermissions: [ + { + controlId: 'permission_mode', + requestedModeId: 'ask', + effectiveModeId: 'always-approve', }, + ], + }, + agentClient + ); + + expect(result.permissionEscalation).toEqual({ + controlId: 'interaction_mode', + requestedModeId: 'plan', + effectiveModeId: 'auto', + }); + }); + + it('does not let an acceptance for one control cover the same values on another', async () => { + // Same `plan → auto` values, different control. Matching on the values + // alone would silently transfer consent between permission dimensions. + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { + id: 'permission_mode', + category: '_permission', + type: 'select', + currentValue: 'auto', }, - logger: createLogger(), - }) - ).resolves.toEqual({ - rejectedSelections: [ - 'mode="plan"', - 'model="model-a"', - 'effort="high"', - 'fast=false', - 'collaboration_mode="plan"', - 'custom-option="enabled"', ], - warningSelections: ['custom-option="enabled"'], - runtimeConfigPatch: { acpSessionId: 'acp-3', configOptionValues: {} }, + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await apply( + { + cliType: 'builtin', + agentType: 'grok', + configOptionValues: { permission_mode: 'plan' }, + acceptWiderPermissions: [ + { controlId: 'interaction_mode', requestedModeId: 'plan', effectiveModeId: 'auto' }, + ], + }, + agentClient + ); + + expect(result.permissionEscalation).toEqual({ + controlId: 'permission_mode', + requestedModeId: 'plan', + effectiveModeId: 'auto', + }); + }); + + it('reaches a runnable turn after both disclosures are accepted', async () => { + // The whole loop for two controls widening at once: stop on one, accept + // it, stop on the other, accept both, run. Without accumulation the + // second acceptance would replace the first and the user would alternate + // between the two notices with no way through. + const grokAgent = () => + ({ + isCreated: () => true, + getConfigOptions: () => [ + { id: 'interaction_mode', category: 'mode', type: 'select', currentValue: 'auto' }, + { + id: 'permission_mode', + category: '_permission', + type: 'select', + currentValue: 'always-approve', + }, + ], + setSessionMode: vi.fn(async () => undefined), + setSessionConfigOption: vi.fn(async () => undefined), + }) as unknown as AgentClient; + const turn = (acceptWiderPermissions: AcceptedWiderPermission[]) => ({ + cliType: 'builtin' as const, + agentType: 'grok', + modeId: 'plan', + configOptionValues: { permission_mode: 'ask' }, + ...(acceptWiderPermissions.length > 0 ? { acceptWiderPermissions } : {}), + }); + + const first = await apply(turn([]), grokAgent()); + const firstEscalation = first.permissionEscalation; + expect(firstEscalation).toEqual({ + controlId: 'permission_mode', + requestedModeId: 'ask', + effectiveModeId: 'always-approve', + }); + + // Accepting only the first still stops, on the one never disclosed. + const second = await apply(turn([firstEscalation!]), grokAgent()); + const secondEscalation = second.permissionEscalation; + expect(secondEscalation).toEqual({ + controlId: 'interaction_mode', + requestedModeId: 'plan', + effectiveModeId: 'auto', }); - } - ); + + // Carrying BOTH — what the retry builder produces from the stopped turn — + // finally runs, and both mismatches are still reported. + const third = await apply(turn([firstEscalation!, secondEscalation!]), grokAgent()); + expect(third.permissionEscalation).toBeUndefined(); + expect(third.warningSelections).toEqual(['permission_mode="ask"', 'mode="plan"']); + }); + + it('stands down for a turn that carries the informed acceptance', async () => { + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'permission-mode', category: 'mode', type: 'select', currentValue: 'auto' }, + ], + setSessionMode: vi.fn(async () => undefined), + } as unknown as AgentClient; + + const result = await apply( + { + cliType: 'builtin', + agentType: 'claude', + modeId: 'plan', + acceptWiderPermissions: [ + { controlId: 'permission-mode', requestedModeId: 'plan', effectiveModeId: 'auto' }, + ], + }, + agentClient + ); + + expect(result.permissionEscalation).toBeUndefined(); + // Still reported: accepting the run does not make the mismatch invisible. + expect(result.warningSelections).toEqual(['mode="plan"']); + }); + }); + + it('reports a selection the agent accepted but dropped from its own state', async () => { + // The codex shape: `fast-mode` is accepted without error on a model with no + // fast speed tier, and simply does not come back in the published state, so + // the turn runs at normal speed with nothing thrown. + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'model', category: 'model', type: 'select', currentValue: 'gpt-5.2' }, + { + id: 'reasoning_effort', + category: 'thought_level', + type: 'select', + currentValue: 'high', + }, + ], + unstable_setSessionModel: vi.fn(async () => undefined), + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + await expect( + applyAcpSessionRunConfig({ + session: { + sessionId: 'session-3' as SessionId, + acpSessionId: 'acp-3' as ACPSessionId, + agentClient, + }, + config: { + cliType: 'builtin', + agentType: 'codex', + modelId: 'gpt-5.2', + configOptionValues: { reasoning_effort: 'high', 'fast-mode': true }, + }, + logger: createLogger(), + }) + ).resolves.toEqual({ + // Nothing was rejected, so diagnostics stay empty: the published state is + // the only evidence Fast is not running. + rejectedSelections: [], + warningSelections: ['fast-mode=true'], + runtimeConfigPatch: { + acpSessionId: 'acp-3', + modelId: 'gpt-5.2', + configOptionValues: { model: 'gpt-5.2', reasoning_effort: 'high' }, + }, + }); + }); + + it('stays quiet when a rejected selection was already effective', async () => { + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'fast', category: 'model_config', type: 'boolean', currentValue: false }, + { + id: 'effort', + category: 'thought_level', + type: 'select', + currentValue: 'high', + }, + ], + setSessionConfigOption: vi.fn(async () => { + throw new Error('rejected'); + }), + } as unknown as AgentClient; + + await expect( + applyAcpSessionRunConfig({ + session: { + sessionId: 'session-6' as SessionId, + acpSessionId: 'acp-6' as ACPSessionId, + agentClient, + }, + config: { + cliType: 'builtin', + agentType: 'claude', + configOptionValues: { fast: false, effort: 'low' }, + }, + logger: createLogger(), + }) + ).resolves.toEqual({ + rejectedSelections: ['fast=false', 'effort="low"'], + // `fast` already held the requested value, so the rejection changed + // nothing; `effort` did not, so it is worth saying. + warningSelections: ['effort="low"'], + runtimeConfigPatch: { + acpSessionId: 'acp-6', + configOptionValues: { fast: false, effort: 'high' }, + }, + }); + }); + + it('treats an on/off select and a boolean toggle as the same choice', async () => { + const agentClient = { + isCreated: () => true, + getConfigOptions: () => [ + { id: 'fast', category: 'model_config', type: 'select', currentValue: 'on' }, + ], + setSessionConfigOption: vi.fn(async () => undefined), + } as unknown as AgentClient; + + await expect( + applyAcpSessionRunConfig({ + session: { + sessionId: 'session-7' as SessionId, + acpSessionId: 'acp-7' as ACPSessionId, + agentClient, + }, + config: { cliType: 'builtin', agentType: 'claude', configOptionValues: { fast: true } }, + logger: createLogger(), + }) + ).resolves.toEqual({ + rejectedSelections: [], + warningSelections: [], + runtimeConfigPatch: { acpSessionId: 'acp-7', configOptionValues: { fast: 'on' } }, + }); + }); it('keeps known run-config rejection warnings for other agents', async () => { const agentClient = { diff --git a/apps/cli/src/session/acp-session-config-applier.ts b/apps/cli/src/session/acp-session-config-applier.ts index 164240551..89142485c 100644 --- a/apps/cli/src/session/acp-session-config-applier.ts +++ b/apps/cli/src/session/acp-session-config-applier.ts @@ -1,9 +1,10 @@ import { - ACP_PLAN_PERMISSION_MODE_ID, - ACP_REASONING_EFFORT_CONFIG_ID, + ACP_CONFIG_OPTION_OFF_VALUE, + ACP_CONFIG_OPTION_ON_VALUE, isAcpFastModeConfigId, + isAcpPermissionWiderThanRequested, + type AcceptedWiderPermission, isAcpPlanModeConfigOption, - isAcpThoughtLevelConfigOption, isSensitiveAcpConfigOptionId, type ACPSessionId, type AcpConfigOptionValue, @@ -56,6 +57,8 @@ export type AcpSessionRunConfig = { modeId?: string; modelId?: string; configOptionValues?: Record; + /** Differences disclosed and accepted for this turn, each matched exactly. */ + acceptWiderPermissions?: AcceptedWiderPermission[]; }; type AcpSessionRunConfigApplyResult = { @@ -65,27 +68,63 @@ type AcpSessionRunConfigApplyResult = { warningSelections: string[]; /** Agent-confirmed state after applying the requested selections. */ runtimeConfigPatch: SessionAcpRuntimeConfigPatch | null; + /** + * The agent's own reported state says this turn would run with MORE + * permission than it asked for. Present only on that contradiction — never + * from a snapshot, never when either mode is unranked, never when the agent + * reported nothing to compare. + */ + permissionEscalation?: AcceptedWiderPermission; }; -function isCodexOrClaudeRunConfig(config: AcpSessionRunConfig): boolean { - return config.agentType === 'codex' || config.agentType === 'claude'; -} - -function isKnownRunConfigOption( - configId: string, - agentConfigOptions: ReadonlyArray<{ id: string; category?: string | null }> +/** A boolean toggle and an `on`/`off` select express the same choice. */ +function configValuesMatch( + requested: AcpConfigOptionValue, + effective: AcpConfigOptionValue | undefined ): boolean { - if ( - configId === ACP_REASONING_EFFORT_CONFIG_ID || - isAcpFastModeConfigId(configId) || - isAcpPlanModeConfigOption({ id: configId }) - ) { + if (requested === effective) { return true; } - const option = agentConfigOptions.find((candidate) => candidate.id === configId); - return option - ? isAcpThoughtLevelConfigOption({ id: option.id, category: option.category ?? undefined }) - : false; + const toggle = (value: boolean): string => + value ? ACP_CONFIG_OPTION_ON_VALUE : ACP_CONFIG_OPTION_OFF_VALUE; + if (typeof requested === 'boolean' && typeof effective === 'string') { + return effective === toggle(requested); + } + if (typeof requested === 'string' && typeof effective === 'boolean') { + return requested === toggle(effective); + } + return false; +} + +/** One requested selection, judged against the agent's own answer for it. */ +type AppliedSelection = { + /** Diagnostic label, already redacted for sensitive ids. */ + label: string; + requested: AcpConfigOptionValue; + /** How to read the agent's state for this selection once everything is applied. */ + source: { kind: 'mode' } | { kind: 'model' } | { kind: 'configOption'; configId: string }; + /** The agent threw while applying it. */ + rejected: boolean; +}; + +/** + * Whether the agent's post-apply state contradicts what the turn asked for. + * + * A rejection alone does not answer this, in either direction. Codex ACCEPTS + * `fast-mode` on a model without a fast speed tier and then simply omits the + * option from the state it publishes — the turn runs at normal speed and + * nothing threw — so the published state is the only evidence Fast is not on. + * Conversely a rejected selection that is already effective changed nothing and + * is not worth a notice. Only where the agent published nothing to compare + * against does the failed call remain the sole signal. + */ +function divergesFromAgentState(args: { + requested: AcpConfigOptionValue; + effective: AcpConfigOptionValue | undefined; + known: boolean; + rejected: boolean; +}): boolean { + return args.known ? !configValuesMatch(args.requested, args.effective) : args.rejected; } export async function applyAcpSessionRunConfig(args: { @@ -116,17 +155,10 @@ export async function applyAcpSessionRunConfig(args: { } const rejectedSelections: string[] = []; - const warningSelections: string[] = []; + const appliedSelections: AppliedSelection[] = []; let confirmedLegacyModeId: string | undefined; let confirmedLegacyModelId: string | undefined; const agentConfigOptions = agentClient.getConfigOptions?.() ?? []; - const suppressKnownRunConfigWarnings = isCodexOrClaudeRunConfig(config); - const recordRejection = (selection: string, suppressWarning: boolean): void => { - rejectedSelections.push(selection); - if (!suppressWarning) { - warningSelections.push(selection); - } - }; const modeConfigId = agentConfigOptions.find((option) => option.category === 'mode')?.id ?? 'mode'; const modelConfigId = @@ -135,77 +167,120 @@ export async function applyAcpSessionRunConfig(args: { const targetModelId = config.modelId ?? (typeof configOptionModelId === 'string' ? configOptionModelId : undefined); - if (config.modeId) { + const applyMode = async (value: string, label: string): Promise => { + let rejected = false; try { - await agentClient.setSessionMode?.(acpSessionId, config.modeId); - confirmedLegacyModeId = config.modeId; + await agentClient.setSessionMode?.(acpSessionId, value); + confirmedLegacyModeId = value; } catch (error) { - recordRejection( - `mode=${JSON.stringify(config.modeId)}`, - suppressKnownRunConfigWarnings && config.modeId === ACP_PLAN_PERMISSION_MODE_ID - ); - logger.debug( - `[${sessionId}] Failed to set ACP mode ${JSON.stringify(config.modeId)}: ${String(error)}` - ); + rejected = true; + rejectedSelections.push(label); + logger.debug(`[${sessionId}] Failed to set ACP mode ${label}: ${String(error)}`); } - } - if (config.modelId) { + appliedSelections.push({ label, requested: value, source: { kind: 'mode' }, rejected }); + }; + + const applyModel = async (value: string, label: string): Promise => { + let rejected = false; + try { + await agentClient.unstable_setSessionModel?.(acpSessionId, value); + confirmedLegacyModelId = value; + } catch (error) { + rejected = true; + rejectedSelections.push(label); + logger.debug(`[${sessionId}] Failed to set ACP model ${label}: ${String(error)}`); + } + appliedSelections.push({ label, requested: value, source: { kind: 'model' }, rejected }); + }; + + const applyConfigOption = async ( + configId: string, + value: AcpConfigOptionValue + ): Promise => { + const label = `${configId}=${formatAcpConfigValueForLog(configId, value)}`; + let rejected = false; try { - await agentClient.unstable_setSessionModel?.(acpSessionId, config.modelId); - confirmedLegacyModelId = config.modelId; + await agentClient.setSessionConfigOption(acpSessionId, configId, value); } catch (error) { - recordRejection(`model=${JSON.stringify(config.modelId)}`, suppressKnownRunConfigWarnings); - logger.debug( - `[${sessionId}] Failed to set ACP model ${JSON.stringify(config.modelId)}: ${String(error)}` - ); + rejected = true; + rejectedSelections.push(label); + logger.debug(`[${sessionId}] Failed to set ACP config option ${configId}: ${String(error)}`); } + appliedSelections.push({ + label, + requested: value, + source: { kind: 'configOption', configId }, + rejected, + }); + }; + + /* Every shape a permission control arrives in. Agents publish it three ways: + the legacy `session/set_mode` selector, a `category: 'mode'` config option, + and an explicit `category: '_permission'` one (Grok's `permission_mode`). + Matching only the first two let a requested `ask` run as `always-approve` + with nothing but a warning. */ + const permissionConfigIds = new Set( + agentConfigOptions + .filter((option) => option.category === 'mode' || option.category === '_permission') + .map((option) => option.id) + ); + + /** + * Permission-bearing controls go LAST, and that ordering is load-bearing. + * + * Claude rebuilds the available permission modes on every model switch and + * downgrades the current one to `default` when the new model does not support + * it — so a mode applied before the model is silently widened by the model + * that follows it. Applying the model and the ordinary options first, then the + * permission-bearing ones, means the last word belongs to what the user asked + * for. `applyPromptConfig` runs before `prompt`, so the state read below is + * still taken before the agent can act on it. + */ + const isPermissionBearing = (configId: string): boolean => + configId === modeConfigId || + permissionConfigIds.has(configId) || + isAcpPlanModeConfigOption({ id: configId }); + const configOptionEntryFor = (configId: string): AcpConfigOptionValue | undefined => + configOptionEntries.find(([id]) => id === configId)?.[1]; + + const duplicateModelValue = configOptionEntryFor(modelConfigId); + if (config.modelId) { + await applyModel(config.modelId, `model=${JSON.stringify(config.modelId)}`); + } else if (typeof duplicateModelValue === 'string') { + await applyModel( + duplicateModelValue, + `${modelConfigId}=${formatAcpConfigValueForLog(modelConfigId, duplicateModelValue)}` + ); } for (const [configId, value] of configOptionEntries) { - if (configId === modeConfigId) { - if (!config.modeId && typeof value === 'string') { - try { - await agentClient.setSessionMode?.(acpSessionId, value); - confirmedLegacyModeId = value; - } catch (error) { - logger.debug( - `[${sessionId}] Failed to set ACP mode option ${configId}=${formatAcpConfigValueForLog( - configId, - value - )}: ${String(error)}` - ); - } - } - continue; - } - if (configId === modelConfigId) { - if (!config.modelId && typeof value === 'string') { - try { - await agentClient.unstable_setSessionModel?.(acpSessionId, value); - confirmedLegacyModelId = value; - } catch (error) { - logger.debug( - `[${sessionId}] Failed to set ACP model option ${configId}=${formatAcpConfigValueForLog( - configId, - value - )}: ${String(error)}` - ); - } - } + if (configId === modeConfigId || configId === modelConfigId || isPermissionBearing(configId)) { continue; } if (shouldSkipFableFastModeDisable({ modelId: targetModelId, configId, value })) { continue; } - try { - await agentClient.setSessionConfigOption(acpSessionId, configId, value); - } catch (error) { - recordRejection( - `${configId}=${formatAcpConfigValueForLog(configId, value)}`, - suppressKnownRunConfigWarnings && isKnownRunConfigOption(configId, agentConfigOptions) - ); - logger.debug(`[${sessionId}] Failed to set ACP config option ${configId}: ${String(error)}`); + await applyConfigOption(configId, value); + } + + for (const [configId, value] of configOptionEntries) { + if (configId === modeConfigId || !isPermissionBearing(configId)) { + continue; } + await applyConfigOption(configId, value); + } + + // An explicit `config.modeId` outranks the duplicate config-option entry, and + // is judged in its place: losing a precedence contest is not the agent + // disagreeing. + const duplicateModeValue = configOptionEntryFor(modeConfigId); + if (config.modeId) { + await applyMode(config.modeId, `mode=${JSON.stringify(config.modeId)}`); + } else if (typeof duplicateModeValue === 'string') { + await applyMode( + duplicateModeValue, + `${modeConfigId}=${formatAcpConfigValueForLog(modeConfigId, duplicateModeValue)}` + ); } logger.debug(`[${sessionId}] applyAcpSessionRunConfig completed`); @@ -213,11 +288,18 @@ export async function applyAcpSessionRunConfig(args: { acpSessionId, agentClient.getConfigOptions() ); - if (confirmedLegacyModeId) { + // A `session/set_mode` that did not throw is an acknowledgement, not proof of + // the resulting state: the agent may change the mode again while applying the + // rest of the turn (Claude downgrades it on an unsupported model switch). So + // it only FILLS a mode the agent's own state does not report — never + // overwrites one, which would report the request back as if it were the + // outcome and leave every mode divergence invisible. + if (confirmedLegacyModeId && runtimeConfigPatch.modeId === undefined) { runtimeConfigPatch.modeId = confirmedLegacyModeId; if ( !isSensitiveAcpConfigOptionId(modeConfigId) && - agentConfigOptions.some((option) => option.id === modeConfigId) + agentConfigOptions.some((option) => option.id === modeConfigId) && + runtimeConfigPatch.configOptionValues?.[modeConfigId] === undefined ) { runtimeConfigPatch.configOptionValues = { ...runtimeConfigPatch.configOptionValues, @@ -228,9 +310,113 @@ export async function applyAcpSessionRunConfig(args: { if (confirmedLegacyModelId && !runtimeConfigPatch.modelId) { runtimeConfigPatch.modelId = confirmedLegacyModelId; } + + // An agent that publishes no config options at all answered nothing here, and + // the effective table deliberately omits sensitive ids, so neither can be read + // as "the agent dropped it". + const publishesConfigOptions = agentConfigOptions.length > 0; + const effectiveConfigOptionValues = runtimeConfigPatch.configOptionValues ?? {}; + const warningSelections = appliedSelections + .filter((selection) => { + const effective = + selection.source.kind === 'mode' + ? runtimeConfigPatch.modeId + : selection.source.kind === 'model' + ? runtimeConfigPatch.modelId + : effectiveConfigOptionValues[selection.source.configId]; + const known = + selection.source.kind === 'configOption' + ? publishesConfigOptions && !isSensitiveAcpConfigOptionId(selection.source.configId) + : effective !== undefined; + return divergesFromAgentState({ + requested: selection.requested, + effective, + known, + rejected: selection.rejected, + }); + }) + .map((selection) => selection.label); + + /* Every permission-bearing selection this turn made, against the value the + agent reports for it after everything has been applied. The effective side + is the agent's own state: `runtimeConfigPatch.modeId` is only filled from a + `set_mode` acknowledgement when the agent reports no mode of its own (in + which case the two are equal and nothing fires), and the config table comes + straight from what the agent published. So this cannot be triggered by a + snapshot, by a stale cache, or by an unconfirmed request. */ + const accepted = config.acceptWiderPermissions ?? []; + const findPermissionEscalation = (): AcceptedWiderPermission | undefined => { + for (const selection of appliedSelections) { + if ( + selection.source.kind === 'model' || + (selection.source.kind === 'configOption' && + !isPermissionBearing(selection.source.configId)) + ) { + continue; + } + const controlId = selection.source.kind === 'mode' ? modeConfigId : selection.source.configId; + const effective = + selection.source.kind === 'mode' + ? runtimeConfigPatch.modeId + : effectiveConfigOptionValues[controlId]; + if ( + typeof selection.requested !== 'string' || + typeof effective !== 'string' || + !isAcpPermissionWiderThanRequested(selection.requested, effective) + ) { + continue; + } + /* Only differences the user was actually shown are skipped, each matched + exactly, and the scan CONTINUES. A bare "accepted" would also wave through a difference they + never saw: the agent may have moved further still by the time the turn + re-runs (`plan → auto` accepted, `plan → always-approve` live), and a + second permission control may have widened alongside the one in the + notice. Either way this is a new, undisclosed escalation, and it gets + its own accurate stop. */ + if ( + accepted.some( + (entry) => + entry.controlId === controlId && + entry.requestedModeId === selection.requested && + entry.effectiveModeId === effective + ) + ) { + continue; + } + return { controlId, requestedModeId: selection.requested, effectiveModeId: effective }; + } + return undefined; + }; + const permissionEscalation = findPermissionEscalation(); + if (permissionEscalation) { + logger.debug( + `[${sessionId}] Permission not applied for ${permissionEscalation.controlId}: requested ${permissionEscalation.requestedModeId}, effective ${permissionEscalation.effectiveModeId}` + ); + } + return { rejectedSelections, warningSelections, runtimeConfigPatch, + ...(permissionEscalation ? { permissionEscalation } : {}), }; } + +/** + * The agent's own state reports a wider permission than the turn requested. + * + * Thrown before `prompt`, so the turn never runs. Carries both mode ids so the + * failure notice can name them and offer the one-time informed downgrade. + */ +export class AcpPermissionNotAppliedError extends Error { + constructor( + readonly controlId: string, + readonly requestedModeId: string, + readonly effectiveModeId: string + ) { + super( + `The agent did not apply the requested permission mode "${requestedModeId}" and would run with "${effectiveModeId}", which allows more than was asked for.` + ); + this.name = 'AcpPermissionNotAppliedError'; + } +} diff --git a/apps/cli/src/session/session-dispatch-watcher.ts b/apps/cli/src/session/session-dispatch-watcher.ts index 764f5ceda..d5c711419 100644 --- a/apps/cli/src/session/session-dispatch-watcher.ts +++ b/apps/cli/src/session/session-dispatch-watcher.ts @@ -1,6 +1,7 @@ import type { RepoTransportRoomStatus, RepoWatchHandle } from 'loro-repo'; import { Effect, Fiber } from 'effect'; import { + AcceptedWiderPermissionsSchema, buildMissingEmail, buildPendingUserHistoryEntry, buildSessionTurnInputConfig, @@ -1972,6 +1973,13 @@ export class SessionDispatchWatcher { agentRoleId: entry.inputConfig?.agentRoleId, agentRoleRevision: entry.inputConfig?.agentRoleRevision, issuePRMentions: entry.inputConfig?.issuePRMentions, + // Only an explicit `true` travels, and only with the turn that carries + // it: this is one-time informed acceptance of a wider permission, so a + // rebuild that dropped it would stop the very turn the user just + // accepted, and one that defaulted it would accept for every turn. + ...(entry.inputConfig?.acceptWiderPermissions?.length + ? { acceptWiderPermissions: entry.inputConfig.acceptWiderPermissions } + : {}), resume: entry.inputConfig?.resume ?? resolveDispatchAcpSessionId(meta), }, userTurnId: entry.id, @@ -2016,6 +2024,9 @@ export class SessionDispatchWatcher { agentRoleId: entry.inputConfig?.agentRoleId, agentRoleRevision: entry.inputConfig?.agentRoleRevision, issuePRMentions: entry.inputConfig?.issuePRMentions, + ...(entry.inputConfig?.acceptWiderPermissions?.length + ? { acceptWiderPermissions: entry.inputConfig.acceptWiderPermissions } + : {}), resume: entry.inputConfig?.resume, }, worktreeSetup: launchConfig?.worktreeSetup, @@ -2115,6 +2126,16 @@ export class SessionDispatchWatcher { agentRoleId: queuedItem.acpSessionConfig?.agentRoleId, agentRoleRevision: queuedItem.acpSessionConfig?.agentRoleRevision, issuePRMentions: queuedItem.acpSessionConfig?.issuePRMentions, + // A queued turn keeps the acceptance it was queued with; promotion must + // not quietly turn it back into a turn that will be stopped. + // The queued value crosses a CRDT, so it is re-validated rather than + // trusted: a malformed acceptance must read as no acceptance. + ...(() => { + const parsed = AcceptedWiderPermissionsSchema.safeParse( + queuedItem.acpSessionConfig?.acceptWiderPermissions + ); + return parsed.success ? { acceptWiderPermissions: parsed.data } : {}; + })(), resume: resolveResumableAcpSessionId(meta), }); const pendingEntry = buildPendingUserHistoryEntry({ diff --git a/apps/cli/src/session/session-edit-and-resend-service.test.ts b/apps/cli/src/session/session-edit-and-resend-service.test.ts index bfe90b775..fa909211d 100644 --- a/apps/cli/src/session/session-edit-and-resend-service.test.ts +++ b/apps/cli/src/session/session-edit-and-resend-service.test.ts @@ -51,6 +51,9 @@ const historyFixture = (): SessionHistoryInput[] => [ configOptionValues: { collaboration_mode: 'plan', }, + acceptWiderPermissions: [ + { controlId: 'mode', requestedModeId: 'plan', effectiveModeId: 'auto' }, + ], }, }, { @@ -227,6 +230,20 @@ describe('SessionEditAndResendService', () => { ); }); + it("does not carry the original turn's permission acceptance onto the new prompt", async () => { + // The client already drops it, but a rule the daemon does not enforce is a + // rule the next client forgets: the replacement config only LACKS the field, + // and a missing key does not overwrite a present one in a spread. + const harness = createHarness({ active: true }); + + await expect(harness.service.editAndResend(spec)).resolves.toMatchObject({ success: true }); + + const replacement = harness.getHistory().at(-1); + expect(replacement?.inputConfig).not.toHaveProperty('acceptWiderPermissions'); + // The rest of the original turn's config still carries over. + expect(replacement?.inputConfig).toMatchObject({ modelId: 'model-1' }); + }); + it('leaves the active turn untouched when provider fork fails', async () => { const harness = createHarness({ active: true, diff --git a/apps/cli/src/session/session-edit-and-resend-service.ts b/apps/cli/src/session/session-edit-and-resend-service.ts index dd50fb482..28e51cbc3 100644 --- a/apps/cli/src/session/session-edit-and-resend-service.ts +++ b/apps/cli/src/session/session-edit-and-resend-service.ts @@ -1,4 +1,5 @@ import { + deriveTurnInputConfigForNewTurn, buildPendingUserHistoryEntry, getServerNow, getSessionRoomId, @@ -484,9 +485,12 @@ export class SessionEditAndResendService { replacement: SessionTurnInputConfig, preparedSessionId: ACPSessionId ): SessionTurnInputConfig { + // A spread would carry the ORIGINAL turn's one-time permission acceptance + // onto this new prompt: `replacement` merely lacks the field, and a missing + // key does not overwrite a present one. The client already drops it, but a + // rule the daemon does not enforce is a rule the next client forgets. return { - ...original.inputConfig, - ...replacement, + ...deriveTurnInputConfigForNewTurn(original.inputConfig, replacement), cliType: meta.cliType, agentType: meta.agentType, resume: preparedSessionId, diff --git a/apps/cli/src/session/session-execution-service.ts b/apps/cli/src/session/session-execution-service.ts index 4ca91a1d9..6a59291a3 100644 --- a/apps/cli/src/session/session-execution-service.ts +++ b/apps/cli/src/session/session-execution-service.ts @@ -4,6 +4,7 @@ import { type AgentConfigCliType, type AgentConfigMeta, type ChatFailedCode, + type ChatFailedMeta, type ChatFailedReason, type IssuePRMention, type LocalProjectId, @@ -88,6 +89,8 @@ import { } from '@/agent/managed-agent-runtime'; import type { FetchAcpCapabilitiesOptions } from '@/agent/acp-capabilities'; import { AcpAuthenticationRequiredError, AgentSteerNotDeliveredError } from '@/agent/agent-client'; +import type { DeclaredModelCapabilities } from '@lody/shared'; +import { AcpPermissionNotAppliedError } from '@/session/acp-session-config-applier'; import { AcpAuthenticationManager, type AcpAuthenticationProgressEvent, @@ -455,7 +458,8 @@ export type SessionExecutionServiceDeps = { sessionDoc: SessionDocument, reason: ChatFailedReason, message?: string, - code?: ChatFailedCode + code?: ChatFailedCode, + permission?: ChatFailedMeta['permission'] ) => Promise; maybeGenerateAndStoreSessionTitle: ( sessionId: SessionId, @@ -505,6 +509,8 @@ export type SessionExecutionServiceDeps = { sessionFork: boolean; acknowledgedSteer: boolean; modelReasoningEfforts?: Record; + measuredForModelId?: string; + declaredModelCapabilities?: DeclaredModelCapabilities; capabilitySourceVersion?: string; }>; /** Evict idle sessions if system memory is under pressure */ @@ -1873,6 +1879,19 @@ export class SessionExecutionService { // of a generic "failed before the agent could start". if (error instanceof AcpAuthenticationRequiredError) { await this.deps.recordChatFailure(sessionDoc, 'acp_auth_required', message); + } else if (error instanceof AcpPermissionNotAppliedError) { + // Keep the specific reason AND both mode ids: they are what let the client + // name the two permissions and offer to run this exact turn once with the + // one the agent actually has, instead of a generic pre-prompt error. + // The turn id travels with the notice: the client must not have to guess + // which prompt this stop belongs to by looking at whatever user entry + // happens to sit above it. + await this.deps.recordChatFailure(sessionDoc, 'permission_not_applied', message, undefined, { + controlId: error.controlId, + requestedModeId: error.requestedModeId, + effectiveModeId: error.effectiveModeId, + ...(runtime.userTurnId ? { userTurnId: runtime.userTurnId } : {}), + }); } else if (isGitExecutableNotFoundError(error)) { await this.deps.recordChatFailure( sessionDoc, @@ -4864,7 +4883,15 @@ export class SessionExecutionService { capabilities.sessionFork, sourceVersion, capabilities.modelReasoningEfforts, - capabilities.acknowledgedSteer + capabilities.acknowledgedSteer, + { + ...(capabilities.measuredForModelId + ? { measuredForModelId: capabilities.measuredForModelId } + : {}), + ...(capabilities.declaredModelCapabilities + ? { declaredModelCapabilities: capabilities.declaredModelCapabilities } + : {}), + } ); })().catch((error: unknown) => { this.deps.logger.debug( @@ -5174,6 +5201,8 @@ export class SessionExecutionService { sessionFork, acknowledgedSteer, modelReasoningEfforts, + measuredForModelId, + declaredModelCapabilities, capabilitySourceVersion, } = await this.deps.fetchAcpCapabilities( message.cliType, @@ -5213,7 +5242,11 @@ export class SessionExecutionService { }), modelReasoningEfforts, acknowledgedSteer, - { signal: options.signal } + { + signal: options.signal, + ...(measuredForModelId ? { measuredForModelId } : {}), + ...(declaredModelCapabilities ? { declaredModelCapabilities } : {}), + } ); return { diff --git a/apps/cli/tests/session-execution-service.test.ts b/apps/cli/tests/session-execution-service.test.ts index c7dd6932f..7eb07588f 100644 --- a/apps/cli/tests/session-execution-service.test.ts +++ b/apps/cli/tests/session-execution-service.test.ts @@ -2051,7 +2051,10 @@ describe('SessionExecutionService', () => { // Per-model reasoning efforts: absent for this agent, which publishes no // legacy `model[effort]` combination list. undefined, - true + true, + // The trailing options bag: this response names no current model and + // carries no `_meta` declaration, so there is nothing to record. + {} ) ); }); diff --git a/locales/en.json b/locales/en.json index d9d0ce135..65e32c100 100644 --- a/locales/en.json +++ b/locales/en.json @@ -1,4 +1,10 @@ { + "chat.fastCapabilitiesUnknown": "Fast mode not confirmed", + "chat.fastCapabilitiesUnsupported": "This model does not offer Fast mode", + "chat.reasoningCapabilitiesUnknown": "Reasoning levels not confirmed", + "chat.reasoningCapabilitiesUnsupported": "This model offers no selectable reasoning levels", + "chat.reasoningCapabilitiesRefreshHint": "Refresh this provider’s capabilities in Settings, or update Lody on the selected machine.", + "chat.retainedConfigValue": "Saved selection: {{value}}", "agents.acpCapabilities.refreshError": "Refresh failed", "agents.acpCapabilities.refreshModelsAndModes": "Refresh models and modes", "agents.acpCapabilities.refreshSuccess": "Capabilities refreshed: {{modelCount}} models, {{modeCount}} modes", @@ -2127,6 +2133,11 @@ "sessions.systemNotices.chatFailed.sessionNotFound": "Session not found", "sessions.systemNotices.chatFailed.sessionRestoreFailed": "Failed to restore session", "sessions.systemNotices.chatFailed.turnPrePromptFailed": "Failed before the agent could start", + "sessions.systemNotices.chatFailed.permissionNotApplied": "The agent did not apply the requested permission mode, so the turn was stopped before it ran", + "sessions.systemNotices.chatFailed.permissionNotAppliedDetail": "Stopped before running: this turn asked for \"{{requested}}\" but the agent reported \"{{effective}}\", which allows more", + "sessions.systemNotices.chatFailed.permissionRunFailed": "Failed to start the turn", + "sessions.systemNotices.chatFailed.permissionRunOnce": "Run once with \"{{effective}}\"", + "sessions.systemNotices.chatFailed.permissionRunning": "Starting…", "sessions.systemNotices.chatFailed.messageDeliveryFailed": "Message delivery failed - please resend after sync recovers", "sessions.systemNotices.chatFailed.machineAccessDenied": "Machine access denied", "sessions.systemNotices.chatFailed.memoryPressure": "The machine is low on memory - free some memory and retry", diff --git a/locales/zh_CN.json b/locales/zh_CN.json index 730278567..6c6b7faa0 100644 --- a/locales/zh_CN.json +++ b/locales/zh_CN.json @@ -1,4 +1,10 @@ { + "chat.fastCapabilitiesUnknown": "Fast 模式未确认", + "chat.fastCapabilitiesUnsupported": "此模型不支持 Fast 模式", + "chat.reasoningCapabilitiesUnknown": "推理档位未确认", + "chat.reasoningCapabilitiesUnsupported": "此模型未提供可选推理档位", + "chat.reasoningCapabilitiesRefreshHint": "请在设置中刷新此 Provider 的能力，或更新所选机器上的 Lody。", + "chat.retainedConfigValue": "保留的选择：{{value}}", "agents.acpCapabilities.refreshError": "刷新失败", "agents.acpCapabilities.refreshModelsAndModes": "刷新模型和模式", "agents.acpCapabilities.refreshSuccess": "能力已刷新：{{modelCount}} 个模型，{{modeCount}} 个模式", @@ -2127,6 +2133,11 @@ "sessions.systemNotices.chatFailed.sessionNotFound": "会话不存在", "sessions.systemNotices.chatFailed.sessionRestoreFailed": "会话恢复失败", "sessions.systemNotices.chatFailed.turnPrePromptFailed": "Agent 启动前失败", + "sessions.systemNotices.chatFailed.permissionNotApplied": "该 Agent 未应用请求的权限模式，本轮已在执行前停止", + "sessions.systemNotices.chatFailed.permissionNotAppliedDetail": "已在执行前停止：本轮请求 \"{{requested}}\"，但该 Agent 报告的是 \"{{effective}}\"，权限更宽", + "sessions.systemNotices.chatFailed.permissionRunFailed": "无法启动本轮", + "sessions.systemNotices.chatFailed.permissionRunOnce": "以 \"{{effective}}\" 运行一次", + "sessions.systemNotices.chatFailed.permissionRunning": "正在启动…", "sessions.systemNotices.chatFailed.messageDeliveryFailed": "消息送达失败 - 请在同步恢复后重发", "sessions.systemNotices.chatFailed.machineAccessDenied": "机器访问被拒绝", "sessions.systemNotices.chatFailed.memoryPressure": "机器内存不足 - 请释放内存后重试", diff --git a/packages/acp-extension-codex b/packages/acp-extension-codex index f90613645..9b4c96140 160000 --- a/packages/acp-extension-codex +++ b/packages/acp-extension-codex @@ -1 +1 @@ -Subproject commit f906136459bace175d3dd88eee70aa4846c58f2c +Subproject commit 9b4c96140c90100ea60c1f4ce3a7fdd7e6cb4b4f diff --git a/packages/components/src/AGENTS.md b/packages/components/src/AGENTS.md index 2723a9d66..d042148aa 100644 --- a/packages/components/src/AGENTS.md +++ b/packages/components/src/AGENTS.md @@ -52,11 +52,13 @@ Parent `AGENTS.md` files also apply. ## ACP selectors -- Built-in Codex reasoning selectors normalize cached options against exact model support - in `components/shared/acp-selector-options.ts`: Astra, Sol, and Terra expose Max/Ultra; - Luna exposes Max only. Keep this aligned with the ACP model catalog; a model version - threshold cannot represent per-model differences, and cached efforts may belong to - a different selected model. +- Runtime reasoning/Fast menus use `resolveAcpModelControls` in shared, projected by + `components/shared/acp-selector-options.ts` for the SELECTED model. Fresh declarations + precede legacy per-model efforts; snapshot controls speak only for their measured model. + Never extend runtime catalogs with the static Codex model table. Missing evidence is + unknown, not unsupported: show `AcpControlAvailability` on desktop/mobile/inline surfaces. + A synthetic control has no default until the agent reports one; explicit model and + per-model requests survive catalog changes and remain subject to runtime reconciliation. ## ACP authentication diff --git a/packages/components/src/components/ai-gui/index.tsx b/packages/components/src/components/ai-gui/index.tsx index 1cdf37e9c..89e0b75d2 100644 --- a/packages/components/src/components/ai-gui/index.tsx +++ b/packages/components/src/components/ai-gui/index.tsx @@ -1,3 +1,4 @@ +import type { PermissionRetryControl } from '@/lib/permission-not-applied-retry'; import { forwardRef, memo, @@ -99,6 +100,7 @@ export interface SessionChatStreamProps { /** Resends an undelivered (missing-history-acked) user turn's content as a * NEW message; the row's "Not delivered" label opens the confirmation dialog. */ onResendUndelivered?: (userTurnId: string, inputBlocks: SessionInputBlock[]) => Promise; + permissionRetry?: PermissionRetryControl; /** Bounded continuation control for the latest provider-capacity failure. */ capacityRetry?: CapacityRetryControl; forkingAssistantMessageId?: string | null; @@ -120,6 +122,7 @@ const MessageRowConnected = memo(function MessageRowConnected({ onNavigateSession, onEditLastUser, onResendUndelivered, + permissionRetry, capacityRetry, conversationFontSize, }: { @@ -131,6 +134,7 @@ const MessageRowConnected = memo(function MessageRowConnected({ /** Resends an undelivered (missing-history-acked) user turn's content as a * NEW message; the row's "Not delivered" label opens the confirmation dialog. */ onResendUndelivered?: (userTurnId: string, inputBlocks: SessionInputBlock[]) => Promise; + permissionRetry?: PermissionRetryControl; capacityRetry?: CapacityRetryControl; conversationFontSize: ConversationFontSize; }) { @@ -147,6 +151,7 @@ const MessageRowConnected = memo(function MessageRowConnected({ onNavigateSession={onNavigateSession} onEdit={onEditLastUser} onResendUndelivered={onResendUndelivered} + permissionRetry={permissionRetry} capacityRetry={capacityRetry} conversationFontSize={conversationFontSize} /> @@ -181,6 +186,7 @@ const SessionChatStreamImpl = forwardRef ); @@ -259,6 +266,7 @@ const SessionChatStreamImpl = forwardRef Promise; onResendUndelivered?: (userTurnId: string, inputBlocks: SessionInputBlock[]) => Promise; capacityRetry?: CapacityRetryControl; + permissionRetry?: PermissionRetryControl; user?: SessionChatUser; conversationFontSize?: ConversationFontSize; }) { @@ -1843,6 +1846,7 @@ export const MessageRowView = memo(function MessageRowView({ sessionId={sessionId} onNavigateSession={onNavigateSession} capacityRetry={capacityRetry} + permissionRetry={permissionRetry} /> ); } @@ -1876,11 +1880,13 @@ const SystemMessageRowView = ({ sessionId, onNavigateSession, capacityRetry, + permissionRetry, }: { message: SessionHistoryParsed; sessionId: SessionId; onNavigateSession?: (target: SessionNavigationTarget) => void; capacityRetry?: CapacityRetryControl; + permissionRetry?: PermissionRetryControl; }) => { const tasksEnabled = useAtomValue(tasksFeatureEnabledAtom); const systemItems = message.items.flatMap((item, itemIndex) => @@ -1909,6 +1915,7 @@ const SystemMessageRowView = ({ sessionId={sessionId} onNavigateSession={onNavigateSession} capacityRetry={capacityRetry} + permissionRetry={permissionRetry} /> ) : item.type === 'worktree_script' ? ( ; sessionId: SessionId; onNavigateSession?: (target: SessionNavigationTarget) => void; capacityRetry?: CapacityRetryControl; + permissionRetry?: PermissionRetryControl; }) => { const { t } = useTranslation(); switch (notice.name) { case 'chat_failed': return ( - + ); case 'agent_warning': return ; @@ -2213,10 +2227,12 @@ const ChatFailedNoticeView = ({ notice, sessionId, capacityRetry, + permissionRetry, }: { notice: Extract; sessionId: SessionId; capacityRetry?: CapacityRetryControl; + permissionRetry?: PermissionRetryControl; }) => { const { t } = useTranslation(); const sessionMeta = useAtomValue(sessionMetaAtomFamily(getSessionRoomId(sessionId))); @@ -2292,6 +2308,21 @@ const ChatFailedNoticeView = ({ 'sessions.systemNotices.chatFailed.turnPrePromptFailed', 'Failed before the agent could start' ); + case 'permission_not_applied': { + const permission = ( + notice.meta as { permission?: { requestedModeId?: string; effectiveModeId?: string } } + )?.permission; + return permission?.requestedModeId && permission.effectiveModeId + ? t( + 'sessions.systemNotices.chatFailed.permissionNotAppliedDetail', + 'Stopped before running: this turn asked for "{{requested}}" but the agent reported "{{effective}}", which allows more', + { requested: permission.requestedModeId, effective: permission.effectiveModeId } + ) + : t( + 'sessions.systemNotices.chatFailed.permissionNotApplied', + 'The agent did not apply the requested permission mode, so the turn was stopped before it ran' + ); + } case 'message_delivery_failed': return t( 'sessions.systemNotices.chatFailed.messageDeliveryFailed', @@ -2459,6 +2490,26 @@ const ChatFailedNoticeView = ({ ) : null; + /* Naming both permissions is the point: "run anyway" would hide which one the + turn is about to run with. The ids are what the agent and the composer both + use, so they are what the user can match against. */ + const permissionAction = permissionRetry ? ( + + ) : null; + return (
{/* Tapping the notice opens a modal instead of a hover tooltip: a tooltip @@ -2478,6 +2529,7 @@ const ChatFailedNoticeView = ({
{noticeBody}
)} {retryAction} + {permissionAction}
{hasDetail ? ( {selectors.map((selector) => - selector.type === 'select' ? ( + selector.availability ? ( + + ) : selector.type === 'select' ? ( + ); return ( ) : null} - {thinkingSelector && thinkingOptions.length > 0 ? ( + {thinkingSelector?.availability ? ( + + ) : thinkingSelector && thinkingOptions.length > 0 ? ( id="run-config-reasoning" @@ -663,7 +669,12 @@ function MobileRunConfigSheetRows({ /> ) : null} - {fastSelector ? ( + {fastSelector?.availability ? ( + + ) : fastSelector ? (