diff --git a/docs/webui.md b/docs/webui.md index eaffa518..30867c91 100644 --- a/docs/webui.md +++ b/docs/webui.md @@ -202,14 +202,14 @@ Current declarations (both transcribed from the audited matrix and re-verified a | sessionCrud | full | full | | streamingSend | full | full | | interrupt | full | full | -| toolSkillInvocation | full | full | +| toolSkillInvocation | partial — missing `setMode` (no session-mode write; M3-B9) | partial — missing `setMode` | | turnDiff | full | none (implementation-absent on the adapter) | | turnRewindRedo | full | partial — missing `reapplyTurnDiff` | | plugins | full | partial — missing `previewGithubPlugin`, `importGithubPlugin`, `listEnabledPlugins` | | mcp | full | full | | subagents | partial — missing `getDelegationSnapshot`, `stopDelegation` (they live on the adapter's access-context, not the CliService surface) | full | | usageStats | full | full | -| authCredentials | full | full | +| authCredentials | partial — missing `setConfigOption` (the GENERIC config write; M3-B9) | partial — missing `setConfigOption` | | updateCheck | none (interface-absent) | none (implementation-absent) | | fileReadWrite | partial — missing `file-write` | partial — missing `file-write` | | gitOperations | partial — missing `git-diff`, `git-commit`, `git-branch` | partial — missing `git-diff`, `git-commit`, `git-branch` | @@ -237,6 +237,27 @@ Default provider is `local-runtime-v2` (the only registered host provider until 501, not 400/404/500: the request was well-formed; the *engine provider* lacks the feature. This mirrors the existing `unsupported` → 501 mapping in `routes/protocol.js`. The frontend treats `engine_capability_not_supported` as expected degradation (hide the entry point per the level table), never as an error toast. +### Behaviour change: the two mode-write endpoints (M3-B9) + +`POST /api/protocol/set-mode` (#67) and `POST /api/protocol/set-config-option` (#68) sit behind a **hard** capability gate, and they are the first endpoints in the migration whose answers change for some deployments. The change has exactly one trigger — *the connected engine provider declares the capability absent* — and it is worth being precise about, because everything outside it is unchanged byte for byte. + +| Request | Before | After | +| --- | --- | --- | +| #67, provider declares `toolSkillInvocation.setMode` | forwarded to the engine; whatever it answered | `501 {ok:false, code:"engine_capability_not_supported", capability:"toolSkillInvocation", provider, missing:["setMode"], reason, error}` | +| #68 with any config id other than `model` / `permissionMode`, provider declares `authCredentials.setConfigOption` absent | forwarded to the engine; whatever it answered | `501 {… capability:"authCredentials", missing:["setConfigOption"] …}` | +| #68 with `model` or `permissionMode` | forwarded to the engine | **unchanged** — the bridge below | +| any of the above, the engine itself answers `unsupported` | `501 {ok:false, code:"unsupported", fallback:"send_plan_as_prompt"}` | **unchanged, including the `fallback` field** | +| any of the above, the provider does **not** declare the capability absent (including every request on the default `acp` transport) | unchanged | **unchanged** | + +Two consequences of that table are deliberate rather than incidental: + +- **The capability 501 carries no `fallback`.** The hint is the degraded action for a feature that exists and whose call failed. Where the engine has no mode write at all there is nothing to degrade to, and advertising `send_plan_as_prompt` from a "this is not available" response would offer a workaround for a missing feature. The engine's own `unsupported` refusal keeps its hint. +- **On the default `acp` transport nothing changes at all.** No provider is registered for `acp` until migration step M4, so the gate reports `unregistered-transport` and every response is the pre-M3 one. The refusals above are reachable on the `runtime` transport, where `local-runtime-v2` is the registered provider. + +**The bridge.** A provider can refuse the *generic* config-option write and still have the two dedicated writers webui's own controls depend on. #68's gate therefore asks for a sub-item derived from the request: `model` asks for `selectModel` and `permissionMode` asks for `setPermissionMode`, both of which pass a provider that denies `setConfigOption`; every other config id asks for `setConfigOption` and gets the 501. The exemption is exactly two named ids — never a prefix, never a default — and it does not survive a `none`: a provider with no `authCredentials` at all has no dedicated writer either. + +**What the user sees.** The permission-mode selector and the model selector are hidden, not disabled and not accompanied by an error message (`webapp/lib/engine-capabilities.ts`, wired in `webapp/components/composer.tsx`). A toast would report a failure for something the user was never able to do, offer nothing to act on, and reappear on every click. The rule is fail-open: the controls are shown until the declaration positively says the engine cannot do it, so a failed or slow `/api/engine-capabilities` request never removes a working control. + ### Migration state and constraints - **M1 done in this batch**: host construction (`createCatalogueHost`) moved verbatim into `server/engine/providers/local-runtime-v2.js`; `runtime-host.js` re-exports it, so every existing importer is untouched. No existing route's behaviour changed; `GET /api/engine-capabilities` is a new, additive endpoint. diff --git a/docs/webui.zh-CN.md b/docs/webui.zh-CN.md index c504fdb1..6964dcd7 100644 --- a/docs/webui.zh-CN.md +++ b/docs/webui.zh-CN.md @@ -202,14 +202,14 @@ webui 服务端新增了一个内部引擎层 `packages/webui/server/engine/`， | sessionCrud | full | full | | streamingSend | full | full | | interrupt | full | full | -| toolSkillInvocation | full | full | +| toolSkillInvocation | partial——缺 `setMode`（无会话模式写入面，M3-B9） | partial——缺 `setMode` | | turnDiff | full | none（adapter 实现无） | | turnRewindRedo | full | partial——缺 `reapplyTurnDiff` | | plugins | full | partial——缺 `previewGithubPlugin`、`importGithubPlugin`、`listEnabledPlugins` | | mcp | full | full | | subagents | partial——缺 `getDelegationSnapshot`、`stopDelegation`（在 adapter 上下文，不在 CliService 面） | full | | usageStats | full | full | -| authCredentials | full | full | +| authCredentials | partial——缺 `setConfigOption`（**通用**配置项写入面，M3-B9） | partial——缺 `setConfigOption` | | updateCheck | none（接口无） | none（实现无） | | fileReadWrite | partial——缺 `file-write` | partial——缺 `file-write` | | gitOperations | partial——缺 `git-diff`、`git-commit`、`git-branch` | partial——缺 `git-diff`、`git-commit`、`git-branch` | @@ -237,6 +237,27 @@ GET /api/engine-capabilities[?provider=] 用 501 而非 400/404/500：请求本身没写错，是**引擎面缺这个功能**——与 `routes/protocol.js` 既有的 `unsupported` → 501 同款。前端把 `engine_capability_not_supported` 当作**预期降级**（按上表三档隐藏入口），不弹错误提示。 +### 行为变更：两个 mode 写端点（M3-B9） + +`POST /api/protocol/set-mode`（#67）与 `POST /api/protocol/set-config-option`（#68）挂在**硬**能力门后，是迁移过程中第一批**会在部分部署上改变应答**的端点。变更只有一个触发条件——*当前引擎 provider 声明该能力不存在*。这条线必须画清楚，因为线外的一切逐字节不变。 + +| 请求 | 变更前 | 变更后 | +| --- | --- | --- | +| #67，provider 声明 `toolSkillInvocation.setMode` 缺失 | 请求照发给引擎，引擎答什么就是什么 | `501 {ok:false, code:"engine_capability_not_supported", capability:"toolSkillInvocation", provider, missing:["setMode"], reason, error}` | +| #68 用 `model` / `permissionMode` 以外的任何 config id，且 provider 声明 `authCredentials.setConfigOption` 缺失 | 请求照发给引擎 | `501 {… capability:"authCredentials", missing:["setConfigOption"] …}` | +| #68 用 `model` 或 `permissionMode` | 请求照发给引擎 | **不变**——见下面的桥接 | +| 以上任一，而**引擎自己**答 `unsupported` | `501 {ok:false, code:"unsupported", fallback:"send_plan_as_prompt"}` | **逐字节不变，`fallback` 字段也保留** | +| 以上任一，而 provider 并未声明该能力缺失（**包括默认 `acp` 传输下的全部请求**） | 不变 | **不变** | + +表里两处是刻意为之，不是顺带： + +- **能力 501 不带 `fallback`。** 这个提示是「功能存在、但这次调用失败」的降级动作。引擎压根没有模式写入面时，没有任何东西可以降级过去；从一个「此功能不可用」的应答里推销 `send_plan_as_prompt`，等于给一个缺失的功能兜售替代方案。引擎自身的 `unsupported` 拒绝保留它的提示。 +- **默认 `acp` 传输下什么都不变。** M4 把 ACP 包成 provider 之前，没有 provider 认领 `acp`，门报 `unregistered-transport`，每个应答都是 M3 之前的那个。上面的拒绝只在 `runtime` 传输上可达——那里注册的 provider 是 `local-runtime-v2`。 + +**桥接。** provider 可以拒绝**通用**配置项写入，同时仍保有 webui 自己的两个控件依赖的专用写入面。因此 #68 的门按请求推导子项：`model` 问 `selectModel`、`permissionMode` 问 `setPermissionMode`，两者都能通过一个拒绝 `setConfigOption` 的 provider；其余任何 config id 问 `setConfigOption`，拿到 501。豁免严格只有两个具名 id——绝不是前缀，绝不是默认分支——而且它撑不过 `none`：完全没有 `authCredentials` 的 provider 同样没有专用写入面。 + +**用户看到什么。** 权限模式选择器与模型选择器被**隐藏**，不是禁用，也不配任何错误提示（`webapp/lib/engine-capabilities.ts`，接线在 `webapp/components/composer.tsx`）。toast 会为一件用户从来就做不到的事报一次失败、无从处理、而且每点一次就再报一次。这条规则是 fail-open 的：控件会一直显示，直到声明明确说引擎做不到——因此一次失败或超时的 `/api/engine-capabilities` 请求绝不会拿掉一个本来能用的控件。 + ### 迁移状态与边界 - **本批只做迁移第一步 M1**：host 构造（`createCatalogueHost`）原样移入 `engine/providers/local-runtime-v2.js`，`runtime-host.js` 转发导出，既有引用方零改动；没有任何现有路由行为变化，`GET /api/engine-capabilities` 是纯新增端点。 diff --git a/packages/webui/server/engine/index.js b/packages/webui/server/engine/index.js index a0182e36..09d05487 100644 --- a/packages/webui/server/engine/index.js +++ b/packages/webui/server/engine/index.js @@ -37,8 +37,10 @@ // (engine/host.js), so the plugins and turn-diff routes no longer name // lib/acp-client.js. M3 batches B1 (#9 #10 #72 #74 #75), B2 (#8 #11), // B3 (#15 #16 #17 #19), B4 (#20 #57 #73), B5 (#7 #4 #6), B6 (#3), -// B7 (#13 #69 #70 #71), B8a (#12's pure layer + gate) and B8b (#12's -// runner + route branch) done. The rest of M3, then M4, will route +// B7 (#13 #69 #70 #71), B8a (#12's pure layer + gate), B8b (#12's +// runner + route branch) and B9 (#67 #68 — the first family whose gate +// changes what a client sees, gated HARD on purpose; see the +// mode-writes.js block below) done. The rest of M3, then M4, will route // their consumers through this facade one endpoint family at a time. import { ENGINE_CAPABILITY_KEYS } from "./capabilities.js"; @@ -360,6 +362,37 @@ export { loadFailureWireCode, resolveSessionLoadProvider, } from "./session-load.js"; +// The SESSION MODE WRITE family (step M3, batch B9): #67 set-mode, #68 +// set-config-option. Same cycle, same TDZ rule, same reasoning: +// mode-writes.js's `MODE_WRITE_ENDPOINTS` and +// `MODE_WRITE_BRIDGED_CONFIG_IDS` are both literals and every binding it +// needs is read inside a function body; a new top-level `const X = +// SOMETHING_FROM_INDEX` there breaks this re-export exactly as it would +// anywhere else. Its only static imports are `engine/capabilities.js` +// and `engine/index.js`; the RPC wrapper and the config are reached +// through `await import()` inside the data-plane functions. +// +// Both endpoints gate HARD, and this is the one M3 family where the hard +// gate is the batch's REASON rather than a consequence of having no +// fallback: it is the first family that deliberately changes what a +// client sees, and the entire change is "a provider that declares the +// capability absent answers the gate's 501 instead of having the write +// forwarded". `MODE_WRITE_BRIDGED_CONFIG_IDS` is the other half of +// that sentence — the two config ids webui's own controls depend on +// (`model`, `permissionMode`) are exempt from the generic-write +// refusal, and the frontend reads the same two names to decide which +// controls to hide. +export { + MODE_WRITE_BRIDGED_CONFIG_IDS, + MODE_WRITE_ENDPOINTS, + assertModeWriteCapability, + resolveModeWriteProvider, + resolveModeWriteSubItem, + setConfigOptionFailureStatus, + setEngineSessionConfigOption, + setEngineSessionMode, + setModeFailureStatus, +} from "./mode-writes.js"; /** * Registered providers. `transport` records which wire form the provider diff --git a/packages/webui/server/engine/mode-writes.js b/packages/webui/server/engine/mode-writes.js new file mode 100644 index 00000000..073049e1 --- /dev/null +++ b/packages/webui/server/engine/mode-writes.js @@ -0,0 +1,500 @@ +// webui/server/engine/mode-writes.js +// +// Migration step M3, batch B9: the SESSION MODE WRITE family — +// +// #67 POST /api/protocol/set-mode — put the session in a mode +// #68 POST /api/protocol/set-config-option — write one config option +// +// This is the FIRST M3 batch that deliberately changes what a client +// sees. Every batch before it kept the wire byte for byte; this one +// does not, and the change is the batch's whole reason for existing. +// The boundary is drawn once, here, and it is a single sentence: +// +// ONLY THE "THE PROVIDER HAS NO SUCH SURFACE" ANSWER CHANGES. +// +// A provider that DECLARES the capability keeps every status, every +// field and every ordering it had before this batch — including the +// pre-existing 501 for an engine that answers `code: "unsupported"`, +// including the `fallback: "send_plan_as_prompt"` hint on that 501, and +// including the two status tables' deliberate disagreement (#67 answers +// 502 for an unmapped code, #68 answers 500). A provider that +// DECLARES the capability absent used to have its request forwarded to +// the engine anyway; it now answers the engine gate's 501 with the +// shared structured body. That is the whole cut. +// +// Why a HARD gate, when B6 and B7 soft-gated their families. Both of +// those had a truthful degradation to fall back on, and hard-gating +// them would have removed a working endpoint over an enrichment. These +// two have none, and the reason is structural rather than a judgement +// call: each endpoint's ENTIRE product is the engine write. #67 has no +// webui-side meaning — a mode that webui recorded locally and the +// engine never entered is a mode the user is not in. #68 is the same +// for a config value. A provider with no write surface therefore cannot +// produce a truthful answer to any of {the write did not happen, the +// control in the panel now shows the new value, the state push told +// every other tab}. Answering 200 there is #110's fake success in its +// purest form, so the gate throws and `app.js#invokeHandler` maps it. +// +// Where each endpoint's declaration comes from, and why the two are +// not the same shape: +// +// - #67 names `toolSkillInvocation` · `setMode`. The 14-key matrix +// has no "mode write" row, and the plan (§3a, row 67) puts session +// mode control under the tool/skill capability. v2 has no `setMode` +// anywhere on the cliService surface — the snapshot audit +// (`test/lib/engine/capability-snapshot.test.js`) is what proves +// that, mechanically, and it keeps proving it: the moment a host +// grows a `setMode` method the declaration's `missing` entry goes +// red and has to be re-audited. v2 can READ plan state +// (`getPlanModeCapabilities` / `getLatestPlanReview`) and enters +// plan through the questionnaire mechanism; it has no write. +// +// - #68 names `authCredentials` · `setConfigOption` — the GENERIC +// config option, per the plan (§3a, row 68). The two config ids +// webui's own controls depend on, `model` and `permissionMode`, are +// NOT the generic write, and the plan requires them to survive it +// ("通用 configId 真 501；两个常用 id 桥接"). So the gate's +// sub-item is a function of the request: the two bridged ids ask +// for their own sub-item and pass a provider that denies the +// generic one, and every other config id asks for `setConfigOption` +// and gets the 501. `MODE_WRITE_BRIDGED_CONFIG_IDS` is that table, +// exported because the frontend needs the same names to decide +// which controls to hide (see `webapp/lib/engine-capabilities.ts`, +// and the tripwire test that pins the two tables to each other). +// +// What the two 501s on these routes now are, and why they must not be +// confused. B7 recorded the same collision for #70 and this batch adds +// two more instances of it, so it is worth stating flatly: +// +// - THE ENGINE-GATE 501 (new). Body: +// `{ok:false, code:"engine_capability_not_supported", capability, +// provider, missing?, reason?, error}` from +// `errors.js#engineCapabilityHttpResponse`, written by the router's +// central mapping. It has NO `fallback` field, and it is not +// reachable from the route's own code path at all — the route never +// catches it. +// - THE ROUTE'S 501 (pre-existing). Body: +// `{ok:false, error, code:"unsupported", fallback:"send_plan_as_prompt"}` +// (plus the 501 shape #68 already had, without `fallback`). This +// one is the ENGINE refusing a call it does accept, and it is +// preserved byte for byte. +// +// The gate's 501 losing `fallback` is deliberate and is the one place +// where this batch's behaviour change is visible to a client that +// special-cases the field: design §4.2 says the UI hides the entry +// point rather than falling back to a degraded action, and a capability +// that is not there has no degraded action to fall back TO — offering +// `send_plan_as_prompt` from a 501 that says "there is no way to enter +// plan mode here" would be advertising a workaround for a missing +// feature. The engine's own refusal keeps its hint because there the +// feature exists and only this call did not work. KNOWN DEBT 1. +// +// What this file deliberately does NOT do: +// +// - It does not own the client-state writes. `cs.planMode` (#67) and +// `cs.permissions` (#68) are webui's own view of the client, and +// the state push is a transport concern; both stay in the route, +// which is also what keeps them running only on success. +// - It does not own the 400s. A missing `sessionId` / `mode` / `key` +// is caller confusion, not an engine limitation, and the matrix +// says an unknown provider id answers 404 for the same reason. +// - It does not build a host. There is no host on this path. +// - It does not migrate `/api/set-model` and `/api/permissions`, +// which are B10's two endpoints. They call the same +// `lib/mcode-rpc.js#setConfigOption` from a different route and are +// untouched here — see KNOWN DEBT 2, which is about exactly that. +// +// Boot-path weight. `app.js` imports the routes, the routes import this +// file, so this file is on the boot path. It statically imports +// `engine/capabilities.js` and `engine/index.js` (both pure +// declaration modules) and nothing else; `lib/mcode-rpc.js` and +// `lib/config.js` are reached through `await import()` inside the +// data-plane functions. + +import { assertEngineCapability } from "./capabilities.js"; +import { DEFAULT_ENGINE_PROVIDER_ID, getEngineProvider } from "./index.js"; + +/** + * Transport → registered engine provider id. Absent means "no provider + * claims this transport yet" (M4), NOT "the capability is + * unavailable" — the two answer differently on purpose, mirroring + * `session-reads.js`, `session-tree-reads.js`, `usage-reads.js`, + * `account-reads.js`, `session-writes.js`, `session-switch.js`, + * `interrupt.js` and `session-load.js` rather than merging with any of + * them: eight families with separate contracts, and a shared table + * would force this one to inherit another's policy. + * + * Built per call rather than frozen at module scope: `engine/index.js` + * re-exports this module, so a module-level table would read + * `DEFAULT_ENGINE_PROVIDER_ID` while that binding is still in its + * temporal dead zone on a cold `import("./engine/index.js")`. Every + * consumer of the table is a function anyway. + * + * @returns {Readonly>} + */ +function providerByTransport() { + return Object.freeze({ runtime: DEFAULT_ENGINE_PROVIDER_ID }); +} + +// --------------------------------------------------------------------------- +// The declaration, and the bridge table +// --------------------------------------------------------------------------- + +/** + * The declaration this family's engine-facing half needs. + * + * @type {Readonly>} + */ +export const MODE_WRITE_ENDPOINTS = Object.freeze({ + "POST /api/protocol/set-mode": Object.freeze({ + capability: "toolSkillInvocation", + subItem: "setMode", + enforcement: "hard", + }), + "POST /api/protocol/set-config-option": Object.freeze({ + capability: "authCredentials", + subItem: "setConfigOption", + enforcement: "hard", + }), +}); + +/** + * The two config ids that survive a provider denying the GENERIC + * config-option write, and the sub-item each one asks for instead. + * + * The plan (§3a, row 68) is explicit that the generic `configId` has + * nowhere to be delivered under a provider with no generic write, while + * these two have dedicated equivalents — "两个常用 id 桥接到 + * `selectModel`/`setPermissionMode`". Naming the sub-items rather than + * quietly widening the gate is what keeps the 501 honest: a provider + * that declares `authCredentials` partial with `missing: + * ["setConfigOption"]` says "I have the dedicated model and permission + * writers but not a generic one", and the gate reads exactly that. + * + * Exported because the frontend asks the same question about the same + * two controls, and two hand-maintained copies of a set of engine + * sub-item names is a drift waiting to happen. The tripwire test in + * `webapp/test/engine-capabilities-degradation.test.ts` reads this + * table out of the server source and fails if the two ever disagree. + * + * @type {Readonly>} + */ +export const MODE_WRITE_BRIDGED_CONFIG_IDS = Object.freeze({ + model: "selectModel", + permissionMode: "setPermissionMode", +}); + +/** + * Which sub-item an endpoint's gate asks for, given the request. + * + * #67 has one answer. #68 has two, and the split is the whole of the + * bridge: a bridged config id asks for its dedicated sub-item, and + * everything else asks for the generic one. An `undefined` or + * non-bridged config id is the generic case, which is the safe + * direction — a name nobody recognised must not quietly inherit the + * exemption reserved for the two ids this batch audited. + * + * @param {string} endpoint A key of MODE_WRITE_ENDPOINTS. + * @param {string} [configId] #68 only. + * @returns {string} + */ +export function resolveModeWriteSubItem(endpoint, configId) { + const need = MODE_WRITE_ENDPOINTS[endpoint]; + if (need === undefined) { + const err = new Error( + `resolveModeWriteSubItem: "${endpoint}" is not part of the mode-write family ` + + `(known: ${Object.keys(MODE_WRITE_ENDPOINTS).join(", ")})`, + ); + err.code = "unknown_mode_write_endpoint"; + throw err; + } + if (endpoint !== "POST /api/protocol/set-config-option") return need.subItem; + const bridged = MODE_WRITE_BRIDGED_CONFIG_IDS[configId]; + return typeof bridged === "string" ? bridged : need.subItem; +} + +/** + * Resolve the provider that answers the mode-write family on + * `transport`, or `null` when none is registered yet. + * + * @param {string} transport One of the `MCODE_WEBUI_TRANSPORT` values. + * @returns {{id: string, transport: string, capabilities: object}|null} + */ +export function resolveModeWriteProvider(transport) { + const providerId = providerByTransport()[transport]; + if (!providerId) return null; + return getEngineProvider(providerId); +} + +/** + * HARD gate for both endpoints. Throws + * `EngineCapabilityNotSupportedError` for a declared `none`, and for a + * `partial` naming the sub-item the request actually needs, which the + * router maps to 501 with `engineCapabilityHttpResponse`'s payload. + * + * Both endpoints are hard by the argument in the module header: there + * is no webui-side meaning left to answer with once the engine write + * is gone, so a truthful 200 does not exist. + * + * `configId` is #68's bridge input and is ignored for #67. Passing one + * for #67 must not change the answer, and the suite pins that, because + * the alternative is a gate whose verdict depends on a field the + * endpoint does not have. + * + * @param {string} endpoint A key of MODE_WRITE_ENDPOINTS. + * @param {string} transport The active transport. + * @param {string} [configId] #68 only. + * @returns {{endpoint: string, gate: string, provider: string|null, capability: string, subItem: string, enforcement: "hard"}} + */ +export function assertModeWriteCapability(endpoint, transport, configId) { + const need = MODE_WRITE_ENDPOINTS[endpoint]; + if (need === undefined) { + // Caller confusion, not an engine limitation. A plain Error, so a + // typo in webui's own key can never be reported to a user as an + // engine limitation. + const err = new Error( + `assertModeWriteCapability: "${endpoint}" is not part of the mode-write family ` + + `(known: ${Object.keys(MODE_WRITE_ENDPOINTS).join(", ")})`, + ); + err.code = "unknown_mode_write_endpoint"; + throw err; + } + const subItem = resolveModeWriteSubItem(endpoint, configId); + const base = { + endpoint, + provider: null, + capability: need.capability, + subItem, + enforcement: need.enforcement, + }; + const provider = resolveModeWriteProvider(transport); + if (!provider) return { ...base, gate: "unregistered-transport" }; + // Throws for `none`, and for `partial` whose `missing` names THIS + // sub-item. A bridged config id therefore passes a provider that + // denies the generic write, and 501s under a provider that denies + // the dedicated one — which is the same distinction in the other + // direction and the reason the two tables are not merged. + assertEngineCapability(provider.capabilities, need.capability, provider.id, subItem); + return { ...base, gate: "checked", provider: provider.id }; +} + +// --------------------------------------------------------------------------- +// Pure derivations. Exported and tested on their INPUTS. +// --------------------------------------------------------------------------- + +/** + * #67's `code` → HTTP status, preserved byte for byte. + * + * The default row is 502, not the 500 every sibling family uses, and + * that asymmetry is pre-existing: `handleSetMode` has always answered + * 502 for a code it cannot classify while `handleSetConfigOption` + * answers 500. Unifying them would change one endpoint's wire to match + * the other, which is a decision about the two endpoints' contracts + * rather than a migration step, and B7 recorded the same asymmetry + * across #67/#70 for the same reason. Pinned as a value, including the + * rows no fixture reaches. + * + * @param {string|undefined} code The RPC wrapper's `code`. + * @returns {number} + */ +export function setModeFailureStatus(code) { + if (code === "unsupported") return 501; + if (code === "no_client") return 503; + if (code && /not.found|invalid/i.test(code)) return 404; + if (code && /conflict|policy/i.test(code)) return 409; + return 502; +} + +/** + * #68's `code` → HTTP status. #67's table with the last row at 500. + * + * @param {string|undefined} code The RPC wrapper's `code`. + * @returns {number} + */ +export function setConfigOptionFailureStatus(code) { + if (code === "unsupported") return 501; + if (code === "no_client") return 503; + if (code && /not.found|invalid/i.test(code)) return 404; + if (code && /conflict|policy/i.test(code)) return 409; + return 500; +} + +// --------------------------------------------------------------------------- +// Data plane +// --------------------------------------------------------------------------- + +/** + * #67 — put the session into `mode`. + * + * The order below IS the endpoint's contract: + * + * 1. HARD CAPABILITY CHECK. Throws for a provider that declares the + * mode write absent; the router answers 501. Under the default + * `acp` transport no provider is registered and the check reports + * `unregistered-transport`, which is the pre-B9 behaviour. + * 2. ENGINE WRITE. A client throw is caught and folded into + * `client_throw` rather than escaping as a 500, so the endpoint + * keeps its "never throw" rule. This is pre-existing and + * unchanged: a transport that blows up is a 502 here, not a + * crash. + * 3. STATUS + BODY. `setModeFailureStatus` on the engine's refusal; + * `{ok:true, mode, data}` on success, with `mode` echoed from the + * request exactly as the route always echoed it. + * + * `fallback: "send_plan_as_prompt"` rides on the engine's own + * `unsupported` refusal and on nothing else — see the module header for + * why the capability 501 does not carry it. + * + * @param {object} options + * @param {string} options.sessionId Already validated non-empty. + * @param {string} options.mode Already validated non-empty. + * @param {string} [options.transport] + * @returns {Promise<{payload: object, statusHint: number, gate: object, transport: string}>} + */ +export async function setEngineSessionMode(options = {}) { + const endpoint = options.endpoint || "POST /api/protocol/set-mode"; + const [rpc, config] = await Promise.all([ + import("../lib/mcode-rpc.js"), + import("../lib/config.js"), + ]); + const transport = options.transport || config.MCODE_WEBUI_TRANSPORT; + const gate = assertModeWriteCapability(endpoint, transport); + const { sessionId, mode } = options; + let r; + try { + r = await rpc.setMode(sessionId, mode); + } catch (e) { + // mcode acp 客户端炸了 (例如 session 未知导致底层 jsonrpc 抛) + // 避免 500 — 包成 fail 让前端能看 + console.warn(`[protocol.set-mode] caught throw: ${e.message || e}`); + r = { ok: false, error: e.message || String(e), code: "client_throw" }; + } + if (!r.ok) { + return { + payload: { + ok: false, + error: r.error, + code: r.code, + fallback: "send_plan_as_prompt", + }, + statusHint: setModeFailureStatus(r.code), + gate, + transport, + }; + } + return { + payload: { ok: true, mode, data: r.data }, + statusHint: 200, + gate, + transport, + }; +} + +/** + * #68 — write one config option. + * + * Same four steps as #67 with one difference that is not a + * simplification: there is NO `try/catch` around the engine call. #67 + * has always had one and its `client_throw` code is on the wire; #68 + * has never had one, and giving it one now would turn a crash into a + * 500 on an endpoint whose failure modes are currently only the ones + * the wrapper returns. The two endpoints' error handling is not + * symmetric today and this batch keeps it that way. + * + * @param {object} options + * @param {string} options.sessionId Already validated non-empty. + * @param {string} options.key Already validated non-empty; the + * config id, which also selects the gate's sub-item. + * @param {string} options.value + * @param {string} [options.cid] + * @param {string} [options.transport] + * @returns {Promise<{payload: object, statusHint: number, gate: object, transport: string}>} + */ +export async function setEngineSessionConfigOption(options = {}) { + const endpoint = options.endpoint || "POST /api/protocol/set-config-option"; + const [rpc, config] = await Promise.all([ + import("../lib/mcode-rpc.js"), + import("../lib/config.js"), + ]); + const transport = options.transport || config.MCODE_WEBUI_TRANSPORT; + const { sessionId, key, value } = options; + const gate = assertModeWriteCapability(endpoint, transport, key); + const r = await rpc.setConfigOption(sessionId, key, value, options.cid); + if (!r.ok) { + return { + payload: { ok: false, error: r.error, code: r.code }, + statusHint: setConfigOptionFailureStatus(r.code), + gate, + transport, + }; + } + return { + payload: { ok: true, key, value, data: r.data }, + statusHint: 200, + gate, + transport, + }; +} + +// --------------------------------------------------------------------------- +// KNOWN DEBT +// --------------------------------------------------------------------------- +// +// Recorded here rather than fixed, because each item is a decision that +// belongs to a human or to a later batch: +// +// 1. THE CAPABILITY 501 HAS NO `fallback`, AND TWO DIFFERENT 501s NOW +// REACH EACH OF THESE ROUTES. The engine-gate 501 carries +// `engineCapabilityHttpResponse`'s body and no `fallback`; the +// pre-existing route 501 for `code === "unsupported"` carries +// `fallback: "send_plan_as_prompt"` on #67 and no `fallback` on +// #68. A client that branches on the status alone will see both. +// The router's central mapping is what keeps them from being +// confused for each other, and the shape difference is the +// intended one (see the module header). What is NOT settled is +// whether a client should be told to prefer one: today nothing in +// the shipped webapp calls either endpoint, so the question has +// never had a consumer, and answering it would mean picking a +// deprecation order for a field that predates this batch. +// +// 2. `MODEL` AND `PERMISSION_MODE` BRIDGE TO SUB-ITEMS NO AUDITED +// HOST ACTUALLY HAS YET. `selectModel` and `setPermissionMode` are +// what the plan (§3a, row 68) says the v2 surface offers, and +// keeping them out of a `missing` list is what lets the two ids +// through the gate. But the snapshot audit +// (`capability-snapshot.test.js`) only proves that a `missing` +// entry is ABSENT — it has no way to prove a non-missing +// sub-item EXISTS, because `REQUIRED_METHODS` is a hand-kept +// list, and neither name is on it. So the bridge is currently an +// audit-free exemption: honest as a forward contract for M4, +// unverified as a claim about today's host. B10 is where +// `/api/set-model` and `/api/permissions` land, and it is the +// batch that should add both names to `REQUIRED_METHODS` if and +// only if the host really has them — at which point this debt +// closes itself, and if it does not, the gate has been letting +// two ids through against a declaration that cannot support them. +// Until then the safest thing is that the exemption is narrow: +// two named ids, never a prefix, never a default. +// +// 3. `/api/set-model` AND `/api/permissions` CALL THE SAME RPC +// WRAPPER AND ARE NOT GATED. `routes/model.js` reaches +// `lib/mcode-rpc.js#setConfigOption` directly for `model`, +// `thinkingEffort` and `permissionMode`. Those are B10's two +// endpoints (#58 and #59) and they are untouched here on purpose. +// The consequence to carry forward is specific: `thinkingEffort` +// is a GENERIC config id, so the moment B10 puts +// `/api/set-model` behind this family's gate the thinking-effort +// control will start answering 501 for the same reason #68 does +// for an unrecognised config id. B10 has to decide whether to +// bridge it as a third id or to accept the 501 with a frontend +// degradation; this batch does not decide it for it. +// +// 4. `setMode` IS THE ONLY SUB-ITEM #67 ASKS FOR, AND THE MATRIX +// HAS NO ROW FOR IT. `toolSkillInvocation` is the plan's home for +// session mode control (§3a, row 67) and it is a real +// declaration with a real audit, but it is a home by +// approximation: the capability is named for tools and skills, +// and this batch is asking it to also carry "the session entered +// plan mode". If M4's provider work ever grows a mode row, #67 +// moves to it and nothing else in this file changes except the +// one string in `MODE_WRITE_ENDPOINTS`. diff --git a/packages/webui/server/engine/providers/local-runtime-v2.capabilities.js b/packages/webui/server/engine/providers/local-runtime-v2.capabilities.js index 5fdf9bdf..b8a15443 100644 --- a/packages/webui/server/engine/providers/local-runtime-v2.capabilities.js +++ b/packages/webui/server/engine/providers/local-runtime-v2.capabilities.js @@ -37,7 +37,20 @@ export const LOCAL_RUNTIME_V2_CAPABILITIES = Object.freeze({ // abortSession. interrupt: { level: "full" }, // listSkills / listRuntimeSkills + pending-permission interaction. - toolSkillInvocation: { level: "full" }, + // M3-B9: the session-mode WRITE is missing, and is listed as such. + // v2 has no `setMode` anywhere on the cliService surface — it can + // read plan state (getPlanModeCapabilities / getLatestPlanReview) and + // enters plan through the questionnaire mechanism, but nothing sets + // it (plan §3a row 67). The snapshot audit + // (test/lib/engine/capability-snapshot.test.js) proves the absence + // mechanically: it fails the moment a method named `setMode` + // appears, so this entry cannot rot into an unearned claim. + toolSkillInvocation: { + level: "partial", + missing: ["setMode"], + reason: + "no session-mode write on the v2 surface: plan state is read-only here and plan entry goes through the questionnaire mechanism (design §1.3 v2; plan §3a row 67)", + }, // service/session-system/diffs + application/session/diff-application // (getTurnDiff / revertTurnDiff / reapplyTurnDiff) — already consumed // by webui's /api/turn-diff routes. @@ -68,7 +81,21 @@ export const LOCAL_RUNTIME_V2_CAPABILITIES = Object.freeze({ usageStats: { level: "full" }, // getAccountStatus + Codex OAuth flow + MiniMax key + full user model // provider CRUD/test/discover, same source as service/model-system. - authCredentials: { level: "full" }, + // M3-B9: the GENERIC config-option write is missing, and is listed as + // such. v2 has no general `setConfigOption`; the plan (§3a, row 68) + // records dedicated equivalents only ("只有 selectModel/ + // setPermissionMode 专用"), which is why the `model` and + // `permissionMode` config ids bridge AROUND this entry in + // `engine/mode-writes.js` and every other config id is refused. The + // snapshot audit proves the absence mechanically. KNOWN DEBT 2 in + // that module records that the two bridged names are themselves not + // yet on an audited host. + authCredentials: { + level: "partial", + missing: ["setConfigOption"], + reason: + "no generic config-option write on the v2 surface; only the dedicated model and permission-mode writers exist (design §1.3 v2; plan §3a row 68)", + }, // grep of the whole package finds no update-check surface; update // checking exists only in the TUI app layer and the CLI command. updateCheck: { diff --git a/packages/webui/server/engine/providers/tui-runtime-adapter.js b/packages/webui/server/engine/providers/tui-runtime-adapter.js index 522f7729..6f899154 100644 --- a/packages/webui/server/engine/providers/tui-runtime-adapter.js +++ b/packages/webui/server/engine/providers/tui-runtime-adapter.js @@ -31,7 +31,16 @@ export const TUI_RUNTIME_ADAPTER_CAPABILITIES = Object.freeze({ interrupt: { level: "full" }, // listSkills + permission interaction (listPendingPermissions / // replyPermission); tool execution events flow over sendMessage. - toolSkillInvocation: { level: "full" }, + // M3-B9: the session-mode write is missing here for the same reason + // it is missing on the cliService (plan §3a row 67) — the adapter + // opens no `setMode`, and the snapshot audit proves it. See + // `engine/mode-writes.js` for the gate that reads this. + toolSkillInvocation: { + level: "partial", + missing: ["setMode"], + reason: + "no session-mode write on the adapter surface: plan state is read-only and plan entry goes through the questionnaire mechanism (design §1.3 tui; plan §3a row 67)", + }, // No getTurnDiff anywhere on the adapter's 91-method surface — the // capability itself lives in local-runtime v1/v2 and webui's turn-diff // routes bypass the adapter for exactly this reason (§1.3 tui). @@ -66,7 +75,16 @@ export const TUI_RUNTIME_ADAPTER_CAPABILITIES = Object.freeze({ // getSessionUsage / getSessionUsageSummary / watchSessionUsageCommits. usageStats: { level: "full" }, // getAccountStatus + OAuth + API key + user model provider CRUD. - authCredentials: { level: "full" }, + // M3-B9: the generic config-option write is missing here for the same + // reason it is missing on the cliService (plan §3a row 68); the + // snapshot audit proves the absence, and `engine/mode-writes.js` + // bridges the two dedicated config ids around it. + authCredentials: { + level: "partial", + missing: ["setConfigOption"], + reason: + "no generic config-option write on the adapter surface; only the dedicated model and permission-mode writers exist (design §1.3 tui; plan §3a row 68)", + }, // No update method on the adapter surface at all; update checking lives // in the TUI application layer (packages/tui/src/update/) and the CLI // `mcode update` command. diff --git a/packages/webui/server/routes/protocol.js b/packages/webui/server/routes/protocol.js index 1931a39c..212bf81b 100644 --- a/packages/webui/server/routes/protocol.js +++ b/packages/webui/server/routes/protocol.js @@ -12,16 +12,12 @@ // // 设计原则: 永远不 throw, 永远返 {ok, data?, error?, code?}; 状态变化后 pushStateFor(cid). -import { - setMode, - setConfigOption, - mcodePermissionToWebui, -} from "../lib/mcode-rpc.js"; -// M3-B1 (engine facade): only #72 (`list-sessions`) is gated in that -// batch. The other five handlers here still call mcode-rpc directly — -// they belong to B7/B9 (cancel, load, activate, set-mode, -// set-config-option), each of which lands its own facade call with its -// own regression evidence. +import { mcodePermissionToWebui } from "../lib/mcode-rpc.js"; +// M3-B1 (engine facade): #72 (`list-sessions`) reads through +// `engine/session-reads.js`. Every handler in this file is now behind +// the facade — B1 here, B7 for the interrupt/load pair below, B9 for +// the mode-write pair — so this module's own imports of the RPC wrapper +// are down to the one pure conversion the client-state sync needs. import { readEngineSessionList } from "../engine/session-reads.js"; // M3-B7 (engine facade): #69 (`cancel`), #70 (`load-session`) and #71 // (`activate-session`) now ask the facade. Two modules, because the @@ -38,6 +34,17 @@ import { readEngineSessionList } from "../engine/session-reads.js"; // module's header says is still open. import { sendEngineSessionCancel } from "../engine/interrupt.js"; import { loadEngineSession, activateEngineSession } from "../engine/session-load.js"; +// M3-B9 (engine facade): #67 (`set-mode`) and #68 (`set-config-option`) +// now ask `engine/mode-writes.js`, which holds both HARD gates, both +// status tables, both response bodies and the `model` / +// `permissionMode` bridge. This is the first batch where the gate +// changes what a client sees, and the two handlers below are where the +// boundary is kept: the engine-gate 501 is written by the router and +// must never be caught here, and every OTHER outcome — including the +// pre-existing `code === "unsupported"` 501 and the 502/500 asymmetry +// between the two endpoints' unmapped codes — is byte for byte what it +// was. +import { setEngineSessionMode, setEngineSessionConfigOption } from "../engine/mode-writes.js"; // M3-B4 (engine facade): #73 (`capabilities`) now reads the engine's // declared capability surface through the facade instead of reaching // into `lib/mcode-rpc.js` and `lib/acp-client.js` from inside the @@ -56,40 +63,34 @@ function respond(res, code, payload) { // ============================================================ // POST /api/protocol/set-mode { sessionId, mode } +// +// M3-B9: the hard `toolSkillInvocation` · `setMode` gate, the engine +// write, the `code` → status table and the success body live in +// `engine/mode-writes.js#setEngineSessionMode`. +// +// The gate throws for a provider that declares the mode write absent, +// and the router's existing central mapping answers it 501 — this +// route does not catch it, and must not: that 501 is "the engine cannot +// do this" and folding it into a status table here would turn it into a +// 502. What this handler keeps is the route's: the two 400s, the +// `cs.planMode` sync and the state push. +// +// The response this route can still write on a failure is the +// PRE-EXISTING one, from `code === "unsupported"` — the engine +// accepting the call and refusing it. Its body keeps +// `fallback: "send_plan_as_prompt"`, because there the feature exists +// and only this call did not work; the gate's 501 carries no `fallback` +// at all, because there is no degraded action to fall back TO. The two +// must not be confused for each other — KNOWN DEBT 1 in the engine +// module. // ============================================================ export async function handleSetMode(req, res, ctx) { const { sessionId, mode } = await readJson(req); if (!sessionId) return respond(res, 400, { ok: false, error: "sessionId required" }); if (!mode) return respond(res, 400, { ok: false, error: "mode required" }); - let r; - try { - r = await setMode(sessionId, mode); - } catch (e) { - // mcode acp 客户端炸了 (例如 session 未知导致底层 jsonrpc 抛) - // 避免 500 — 包成 fail 让前端能看 - console.warn(`[protocol.set-mode] caught throw: ${e.message || e}`); - r = { ok: false, error: e.message || String(e), code: "client_throw" }; - } - if (!r.ok) { - // `unsupported` maps to 501 so the frontend can fall back to the slash form. - const httpCode = - r.code === "unsupported" - ? 501 - : r.code === "no_client" - ? 503 - : r.code && /not.found|invalid/i.test(r.code) - ? 404 - : r.code && /conflict|policy/i.test(r.code) - ? 409 - : 502; - return respond(res, httpCode, { - ok: false, - error: r.error, - code: r.code, - fallback: "send_plan_as_prompt", - }); - } + const r = await setEngineSessionMode({ sessionId, mode }); + if (r.statusHint !== 200) return respond(res, r.statusHint, r.payload); // 同步本地 state.planMode 标志 (前端某些 UI 还读这个) // set-mode 端点只接 plan_mode 类的 mode 名, 不接 permission 类的 'off'/'default' // (off 是 permission mode, 不是 plan mode 退出值 — 误用会让 mcode 看着像"退出 plan" @@ -100,38 +101,46 @@ export async function handleSetMode(req, res, ctx) { // 其他 mode (goal_mode / 自定义) 不动 planMode } if (ctx && ctx.cid) pushStateFor(ctx.cid); - return respond(res, 200, { ok: true, mode, data: r.data }); + return respond(res, 200, r.payload); } // ============================================================ // POST /api/protocol/set-config-option { sessionId, key, value } // 通用配置选项。permissionMode / model / 等都走这里 +// +// M3-B9: the hard `authCredentials` gate, the engine write, the status +// table and the success body live in +// `engine/mode-writes.js#setEngineSessionConfigOption`. The gate's +// sub-item is the request's own `key`, because that is what the bridge +// turns on: `model` and `permissionMode` ask for their dedicated +// sub-items and pass a provider that denies the generic config-option +// write, and every other config id asks for `setConfigOption` and gets +// the gate's 501. This handler is not involved in that decision and +// must not grow its own copy of it. +// +// As with #67: the gate's 501 is the router's and is not caught here, +// and the pre-existing `code === "unsupported"` 501 is preserved byte +// for byte. The two endpoints' unmapped-code rows stay 502 and 500 +// respectively — pre-existing, asymmetric, and pinned as values. // ============================================================ export async function handleSetConfigOption(req, res, ctx) { const { sessionId, key, value } = await readJson(req); if (!sessionId) return respond(res, 400, { ok: false, error: "sessionId required" }); if (!key) return respond(res, 400, { ok: false, error: "key required" }); - const r = await setConfigOption(sessionId, key, value, ctx && ctx.cid); - if (!r.ok) { - const httpCode = - r.code === "unsupported" - ? 501 - : r.code === "no_client" - ? 503 - : r.code && /not.found|invalid/i.test(r.code) - ? 404 - : r.code && /conflict|policy/i.test(r.code) - ? 409 - : 500; - return respond(res, httpCode, { ok: false, error: r.error, code: r.code }); - } + const r = await setEngineSessionConfigOption({ + sessionId, + key, + value, + cid: ctx && ctx.cid, + }); + if (r.statusHint !== 200) return respond(res, r.statusHint, r.payload); // 权限 mode 同步到 webui cs.permissions (供前端 icon/label 显示) if (key === "permissionMode" && ctx && ctx.cs) { ctx.cs.permissions = mcodePermissionToWebui(value); } if (ctx && ctx.cid) pushStateFor(ctx.cid); - return respond(res, 200, { ok: true, key, value, data: r.data }); + return respond(res, 200, r.payload); } // ============================================================ diff --git a/packages/webui/test/lib/engine/capabilities.test.js b/packages/webui/test/lib/engine/capabilities.test.js index 026d78ba..c2f4c987 100644 --- a/packages/webui/test/lib/engine/capabilities.test.js +++ b/packages/webui/test/lib/engine/capabilities.test.js @@ -116,14 +116,14 @@ describe("LOCAL_RUNTIME_V2_CAPABILITIES", () => { sessionCrud: "full", streamingSend: "full", interrupt: "full", - toolSkillInvocation: "full", + toolSkillInvocation: "partial", turnDiff: "full", turnRewindRedo: "full", plugins: "full", mcp: "full", subagents: "partial", usageStats: "full", - authCredentials: "full", + authCredentials: "partial", updateCheck: "none", fileReadWrite: "partial", gitOperations: "partial", @@ -148,6 +148,16 @@ describe("LOCAL_RUNTIME_V2_CAPABILITIES", () => { "stopDelegation", ]); }); + + // M3-B9: the two B9 partials name exactly the sub-item each is for, + // and the generic config-option entry must NOT grow to swallow the + // bridged ones — `model` and `permissionMode` pass a provider that + // denies `setConfigOption`, so listing them here would silently + // disable the bridge this batch exists to keep working. + test("the two B9 partials enumerate exactly their own sub-item", () => { + assert.deepEqual(LOCAL_RUNTIME_V2_CAPABILITIES.toolSkillInvocation.missing, ["setMode"]); + assert.deepEqual(LOCAL_RUNTIME_V2_CAPABILITIES.authCredentials.missing, ["setConfigOption"]); + }); }); // --------------------------------------------------------------------------- @@ -164,14 +174,14 @@ describe("TUI_RUNTIME_ADAPTER_CAPABILITIES", () => { sessionCrud: "full", streamingSend: "full", interrupt: "full", - toolSkillInvocation: "full", + toolSkillInvocation: "partial", turnDiff: "none", turnRewindRedo: "partial", plugins: "partial", mcp: "full", subagents: "full", usageStats: "full", - authCredentials: "full", + authCredentials: "partial", updateCheck: "none", fileReadWrite: "partial", gitOperations: "partial", @@ -312,21 +322,36 @@ describe("engineCapabilityHttpResponse", () => { // --------------------------------------------------------------------------- describe("summarizeUnavailableCapabilities", () => { - test("local-runtime-v2: updateCheck alone is none; three keys are partial", () => { + // M3-B9 added two more `partial` keys to this declaration — the + // session-mode write and the generic config-option write, both absent + // from the audited v2 surface (see the declaration's own comments and + // `engine/mode-writes.js`). The roll-up is the frontend's input, so + // the list is pinned as a value rather than a count. + test("local-runtime-v2: updateCheck alone is none; five keys are partial", () => { const summary = summarizeUnavailableCapabilities(LOCAL_RUNTIME_V2_CAPABILITIES); assert.deepEqual(summary.none, ["updateCheck"]); assert.deepEqual( summary.partial.map((p) => p.key).sort(), - ["fileReadWrite", "gitOperations", "subagents"], + ["authCredentials", "fileReadWrite", "gitOperations", "subagents", "toolSkillInvocation"], ); }); - test("tui-runtime-adapter: turnDiff and updateCheck are none; four keys are partial", () => { + // Same M3-B9 amendment as the v2 column above, and for the same two + // reasons: neither the adapter nor the cliService opens a session-mode + // write or a generic config-option write. + test("tui-runtime-adapter: turnDiff and updateCheck are none; six keys are partial", () => { const summary = summarizeUnavailableCapabilities(TUI_RUNTIME_ADAPTER_CAPABILITIES); assert.deepEqual(summary.none, ["turnDiff", "updateCheck"]); assert.deepEqual( summary.partial.map((p) => p.key).sort(), - ["fileReadWrite", "gitOperations", "plugins", "turnRewindRedo"], + [ + "authCredentials", + "fileReadWrite", + "gitOperations", + "plugins", + "toolSkillInvocation", + "turnRewindRedo", + ], ); }); }); diff --git a/packages/webui/test/lib/engine/capability-snapshot.test.js b/packages/webui/test/lib/engine/capability-snapshot.test.js index a2e66a53..f8412d3b 100644 --- a/packages/webui/test/lib/engine/capability-snapshot.test.js +++ b/packages/webui/test/lib/engine/capability-snapshot.test.js @@ -95,22 +95,25 @@ function resolveMember(host, dottedPath) { * - `absent`: method-NAMED sub-items the partial declarations list in * `missing` — methods of this capability's domain that genuinely do * not exist on this surface (reapplyTurnDiff on the adapter, - * getDelegationSnapshot on the bare CliService). They are part of - * the snapshot so "missing must really be absent" is checked, and a - * partial that stops listing one goes red (under-declaration). + * getDelegationSnapshot on the bare CliService, and — since M3-B9 — + * setMode and setConfigOption on BOTH surfaces, which is what makes + * the mode-write family's hard gate an audited fact rather than a + * claim). They are part of the snapshot so "missing must really be + * absent" is checked, and a partial that stops listing one goes red + * (under-declaration). */ const REQUIRED_METHODS = { "tui-runtime-adapter": { sessionCrud: { on: "adapter", methods: ["createSession", "listSessions", "getSession", "renameSession", "archiveSession", "deleteSession", "forkSession"] }, streamingSend: { on: "adapter", methods: ["sendMessage", "watchSessionTurn", "watchEvents"] }, interrupt: { on: "adapter", methods: ["abortSession", "steer"] }, - toolSkillInvocation: { on: "adapter", methods: ["listSkills", "listPendingPermissions", "replyPermission"] }, + toolSkillInvocation: { on: "adapter", methods: ["listSkills", "listPendingPermissions", "replyPermission"], absent: ["setMode"] }, turnRewindRedo: { on: "adapter", methods: ["rewindSession", "getSessionRewindPreview"], absent: ["reapplyTurnDiff"] }, plugins: { on: "adapter", methods: ["listInstalledPlugins", "listMarketplacePlugins", "mutatePlugin", "refreshPlugins"], absent: ["previewGithubPlugin", "importGithubPlugin", "listEnabledPlugins"] }, mcp: { on: "adapter", methods: ["configureSessionMcpServers", "clearSessionMcpServers", "inspectProjectMcp", "listMcpServers"] }, subagents: { on: "adapter", methods: ["getDelegationSnapshot", "stopDelegation", "listBackgroundTasks"] }, usageStats: { on: "adapter", methods: ["getSessionUsage", "getSessionUsageSummary", "watchSessionUsageCommits"] }, - authCredentials: { on: "adapter", methods: ["getAccountStatus", "getCodexOAuthStatus", "startCodexOAuthLogin", "cancelCodexOAuthLogin", "getMiniMaxApiKeyStatus", "upsertMiniMaxApiKey", "listUserModelProviders", "createUserModelProvider", "updateUserModelProvider", "deleteUserModelProvider", "testUserModelProvider", "discoverUserModelsCandidate"] }, + authCredentials: { on: "adapter", methods: ["getAccountStatus", "getCodexOAuthStatus", "startCodexOAuthLogin", "cancelCodexOAuthLogin", "getMiniMaxApiKeyStatus", "upsertMiniMaxApiKey", "listUserModelProviders", "createUserModelProvider", "updateUserModelProvider", "deleteUserModelProvider", "testUserModelProvider", "discoverUserModelsCandidate"], absent: ["setConfigOption"] }, fileReadWrite: { on: "adapter", methods: ["listWorkspaceFileTree", "searchWorkspaceFiles"] }, gitOperations: { on: "adapter", methods: ["getWorkspaceGitMetadata"] }, }, @@ -118,14 +121,14 @@ const REQUIRED_METHODS = { sessionCrud: { on: "cliService", methods: ["createSession", "updateSession", "archiveSession", "deleteSession", "forkSession", "getSessionForkOptions"] }, streamingSend: { on: "cliService", methods: ["sendMessage", "resumeSession", "steerSession", "watchEvents"] }, interrupt: { on: "cliService", methods: ["abortSession"] }, - toolSkillInvocation: { on: "cliService", methods: ["listSkills", "listRuntimeSkills", "listPendingPermissions", "replyPermission"] }, + toolSkillInvocation: { on: "cliService", methods: ["listSkills", "listRuntimeSkills", "listPendingPermissions", "replyPermission"], absent: ["setMode"] }, turnDiff: { on: "applications.session.diff", methods: ["getSessionDiff", "getTurnDiff", "revertTurnDiff", "reapplyTurnDiff"] }, turnRewindRedo: { on: "cliService", methods: ["getSessionRewindPreview", "rewindSession", "editSessionMessage"] }, plugins: { on: "cliService", methods: ["refreshPlugins", "listMarketplacePlugins", "listInstalledPlugins", "listEnabledPlugins", "installPlugin", "enablePlugin", "disablePlugin", "uninstallPlugin", "previewGithubPlugin", "importGithubPlugin"] }, mcp: { on: "cliService", methods: ["configureSessionMcpServers", "inspectProjectMcp", "clearSessionMcpServers", "listMcpServers"] }, subagents: { on: "cliService", methods: ["listBackgroundTasks"], absent: ["getDelegationSnapshot", "stopDelegation"] }, usageStats: { on: "cliService", methods: ["getSessionUsage", "getSessionUsageSummary", "watchSessionUsageCommits"] }, - authCredentials: { on: "cliService", methods: ["getAccountStatus", "getCodexOAuthStatus", "startCodexOAuthLogin", "cancelCodexOAuthLogin", "getMiniMaxApiKeyStatus", "upsertMiniMaxApiKey", "listUserModelProviders", "createUserModelProvider", "updateUserModelProvider", "deleteUserModelProvider", "testUserModel", "discoverUserModelsCandidate"] }, + authCredentials: { on: "cliService", methods: ["getAccountStatus", "getCodexOAuthStatus", "startCodexOAuthLogin", "cancelCodexOAuthLogin", "getMiniMaxApiKeyStatus", "upsertMiniMaxApiKey", "listUserModelProviders", "createUserModelProvider", "updateUserModelProvider", "deleteUserModelProvider", "testUserModel", "discoverUserModelsCandidate"], absent: ["setConfigOption"] }, fileReadWrite: { on: "cliService", methods: ["listWorkspaceFileTree", "searchWorkspaceFiles"] }, gitOperations: { on: "cliService", methods: ["getWorkspaceGitMetadata", "getWorkspaceReviewLink"] }, }, diff --git a/packages/webui/test/lib/engine/mode-writes.test.js b/packages/webui/test/lib/engine/mode-writes.test.js new file mode 100644 index 00000000..b42a99b6 --- /dev/null +++ b/packages/webui/test/lib/engine/mode-writes.test.js @@ -0,0 +1,762 @@ +// webui/test/lib/engine/mode-writes.test.js +// +// M3-B9 — the SESSION MODE WRITE family (#67 set-mode, #68 +// set-config-option). +// +// This is the first M3 family whose gate changes what a client sees, so +// this file is organised around the batch's own boundary rather than +// around the code: every case names which side of it it is on. +// +// THE OLD STATE — a provider that DECLARES the capability. Status, +// body and ordering are asserted as values, including the rows no +// fixture reaches (the 502/500 asymmetry, the `fallback` hint, the +// `client_throw` fold). If any of these move, this batch broke its +// promise. +// +// THE NEW STATE — a provider that DOES NOT. The gate throws +// EngineCapabilityNotSupportedError, `app.js` maps it to 501, and +// the body is the shared one from `errors.js`. The bridge is the +// other half of the new state: `model` and `permissionMode` must +// still pass a provider that denies the generic write, or "the two +// common ids bridge" would be a claim with nothing behind it. +// +// The provider fixtures are SYNTHETIC on purpose. Both registered +// providers declare `toolSkillInvocation` and `authCredentials` as +// `partial` with exactly the sub-items B9 needs (the real declarations, +// audited by capability-snapshot.test.js), so a real-registry test can +// reach the refusals — but not the "capability exists" half of #68, and +// not a `none`. Mocking `engine/index.js` for the whole namespace is +// what makes the `none` case reachable at all. + +import { test, describe, beforeEach } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; + +import { setupMocks, absPath, registerRpcMock } from "../../helpers/_setup.js"; +// Type discrimination goes through the exported predicate, never +// `err.name`: `name` is a writable instance property, so one stray +// upstream assignment would turn a 501 back into a soft failure — a +// failure mode that reads as a passing test. +const { isEngineCapabilityNotSupportedError, engineCapabilityHttpResponse } = await import( + "../../../server/engine/errors.js" +); + +const RUNTIME = "runtime"; +const ACP = "acp"; + +const SET_MODE = "POST /api/protocol/set-mode"; +const SET_CONFIG_OPTION = "POST /api/protocol/set-config-option"; + +/** Every name `engine/mode-writes.js` exports. The namespace, not a subset. */ +const FACADE_EXPORTS = [ + "MODE_WRITE_BRIDGED_CONFIG_IDS", + "MODE_WRITE_ENDPOINTS", + "assertModeWriteCapability", + "resolveModeWriteProvider", + "resolveModeWriteSubItem", + "setConfigOptionFailureStatus", + "setEngineSessionConfigOption", + "setEngineSessionMode", + "setModeFailureStatus", +]; + +let bust = 0; + +/** The real declaration, read from the source so the sweep cannot drift. */ +function exportedNamesOf(relative) { + const fileUrl = absPath(relative); + const src = readFileSync(fileURLToPath(fileUrl), "utf8"); + const names = new Set(); + for (const m of src.matchAll(/^export\s+(?:async\s+)?function\s+([A-Za-z_$][\w$]*)/gm)) names.add(m[1]); + for (const m of src.matchAll(/^export\s+(?:const|let|var|class)\s+([A-Za-z_$][\w$]*)/gm)) names.add(m[1]); + for (const m of src.matchAll(/^export\s*\{([^}]*)\}/gm)) { + for (const part of m[1].split(",")) { + const name = part.trim().split(/\s+as\s+/).pop().trim(); + if (name) names.add(name); + } + } + return { fileUrl, names: [...names].sort() }; +} + +// --------------------------------------------------------------------------- +// Provider fixtures. Each is a PARTIAL declaration: the audit in +// `test/lib/engine/capability-snapshot.test.js` is what keeps the real +// ones honest, and a fixture only has to be good enough to reach a +// branch. +// --------------------------------------------------------------------------- + +/** Declares both B9 capabilities in full — the "old state" provider. */ +const FULL_BOTH = { + toolSkillInvocation: { level: "full" }, + authCredentials: { level: "full" }, +}; + +/** + * The real v2 shape, and the one the bridge exists for: the generic + * config-option write is denied, the two dedicated ones are not listed + * so they pass. + */ +const NO_GENERIC_CONFIG_WRITE = { + toolSkillInvocation: { level: "partial", missing: ["setMode"], reason: "test: no mode write" }, + authCredentials: { level: "partial", missing: ["setConfigOption"], reason: "test: no generic config write" }, +}; + +/** Neither capability at all. */ +const NEITHER = { + toolSkillInvocation: { level: "none", reason: "test: interface-absent" }, + authCredentials: { level: "none", reason: "test: interface-absent" }, +}; + +/** + * Boot the facade against a synthetic provider. + * + * `engine/index.js` is mocked for its WHOLE namespace (every name not + * explicitly provided throws), because a whole-namespace mock is what + * catches a new top-level read of this module in mode-writes.js — the + * temporal-dead-zone rule its header states. `mode-writes.js` is then + * imported FRESH so it picks the mock up as its live binding. + */ +async function bootFacadeWithProvider(t, capabilities) { + await setupMocks(t, {}); + const { names } = exportedNamesOf("engine/index.js"); + const namedExports = {}; + for (const name of names) { + namedExports[name] = () => { + throw new Error(`B9 test called engine/index.js#${name}, which this case did not stub`); + }; + } + Object.assign(namedExports, { + DEFAULT_ENGINE_PROVIDER_ID: "local-runtime-v2", + getEngineProvider: (id = "local-runtime-v2") => ({ + id, + transport: "runtime", + capabilities, + }), + }); + t.mock.module(absPath("engine/index.js"), { namedExports }); + return import(`${absPath("engine/mode-writes.js")}?provider=${bust++}`); +} + +/** Boot against the REAL registry — no mock of engine/index.js at all. */ +async function bootFacade(t) { + await setupMocks(t, {}); + return import(`${absPath("engine/mode-writes.js")}?provider=${bust++}`); +} + +/** Run `fn`, returning the thrown value or `null`. */ +async function caughtBy(fn) { + try { + await fn(); + } catch (e) { + return e; + } + return null; +} + +beforeEach(() => { + registerRpcMock({ + setMode: async () => ({ ok: true, data: { modeId: "plan" } }), + setConfigOption: async () => ({ ok: true, data: {} }), + }); +}); + +// --------------------------------------------------------------------------- +// The export surface +// --------------------------------------------------------------------------- + +describe("mode-writes facade — export surface", () => { + test("exports exactly the names the facade re-exports, no more and no fewer", async () => { + const module = await import(absPath("engine/mode-writes.js")); + const actual = Object.keys(module) + .filter((k) => k !== "default") + .sort(); + assert.deepEqual(actual, FACADE_EXPORTS); + }); + + test("the name list is derived from the SOURCE, so a new export cannot slip past the sweep", async () => { + const { names } = exportedNamesOf("engine/mode-writes.js"); + assert.deepEqual(names, FACADE_EXPORTS); + }); + + test("engine/index.js re-exports every one of them", async () => { + const src = readFileSync(fileURLToPath(absPath("engine/index.js")), "utf8"); + const from = 'from "./mode-writes.js";'; + assert.equal(src.split(from).length - 1, 1, "mode-writes.js must be re-exported exactly once"); + // The `export {` that belongs to THIS from-clause is the last one + // before it; an earlier match would be a different family's block. + const start = src.lastIndexOf("export {", src.indexOf(from)); + assert.ok(start > 0, "the re-export block was not found"); + const exported = src + .slice(src.indexOf("{", start) + 1, src.indexOf("}", start)) + .split(",") + .map((s) => s.trim()) + .filter(Boolean) + .sort(); + assert.deepEqual(exported, FACADE_EXPORTS); + }); +}); + +// --------------------------------------------------------------------------- +// The declarations +// --------------------------------------------------------------------------- + +describe("MODE_WRITE_ENDPOINTS", () => { + test("both endpoints gate HARD, on the two capabilities the plan names", async () => { + const { MODE_WRITE_ENDPOINTS } = await bootFacade(t0()); + assert.deepEqual(MODE_WRITE_ENDPOINTS, { + [SET_MODE]: { capability: "toolSkillInvocation", subItem: "setMode", enforcement: "hard" }, + [SET_CONFIG_OPTION]: { capability: "authCredentials", subItem: "setConfigOption", enforcement: "hard" }, + }); + for (const entry of Object.values(MODE_WRITE_ENDPOINTS)) { + assert.equal(Object.isFrozen(entry), true, "each entry must be frozen"); + } + }); + + test("the real registry declares BOTH capabilities as partial, naming exactly the B9 sub-items", async () => { + // This is the case the whole batch rests on: the refusals are + // reachable through the real registry, not only through a mock. The + // `missing` lists are pinned as values because a second entry in + // either one would silently widen or narrow the bridge. + const { getEngineProvider } = await import(absPath("engine/index.js")); + for (const id of ["local-runtime-v2", "tui-runtime-adapter"]) { + const { capabilities } = getEngineProvider(id); + assert.deepEqual(capabilities.toolSkillInvocation.missing, ["setMode"], id); + assert.deepEqual(capabilities.authCredentials.missing, ["setConfigOption"], id); + } + }); +}); + +describe("resolveModeWriteSubItem — the bridge", () => { + test("#67 asks for `setMode` whatever else it is told", async () => { + const facade = await bootFacade(t0()); + for (const configId of [undefined, "model", "permissionMode", "anything"]) { + assert.equal(facade.resolveModeWriteSubItem(SET_MODE, configId), "setMode", String(configId)); + } + }); + + test("#68 asks for the dedicated sub-item for the two bridged ids", async () => { + const facade = await bootFacade(t0()); + assert.equal(facade.resolveModeWriteSubItem(SET_CONFIG_OPTION, "model"), "selectModel"); + assert.equal(facade.resolveModeWriteSubItem(SET_CONFIG_OPTION, "permissionMode"), "setPermissionMode"); + }); + + test("#68 asks for the GENERIC sub-item for every other id, including nonsense", async () => { + // The safe direction: a config id nobody audited must NOT inherit + // the exemption reserved for the two that were. + const facade = await bootFacade(t0()); + for (const key of ["thinkingEffort", "model_", "Model", "", undefined, null, 0, "constructor", "__proto__"]) { + assert.equal( + facade.resolveModeWriteSubItem(SET_CONFIG_OPTION, key), + "setConfigOption", + `configId ${JSON.stringify(key)}`, + ); + } + }); + + test("an inherited property is not a bridge — `toString` and `__proto__` are not config ids", async () => { + const facade = await bootFacade(t0()); + // A plain object literal inherits `Object.prototype`, so + // `MODE_WRITE_BRIDGED_CONFIG_IDS["toString"]` IS a function. The + // `typeof === "string"` guard in `resolveModeWriteSubItem` is the + // only thing standing between that and a sub-item name of + // "[Function: toString]", so the guard is pinned from both sides. + assert.equal(typeof facade.MODE_WRITE_BRIDGED_CONFIG_IDS.toString, "function"); + for (const key of ["toString", "__proto__", "constructor", "hasOwnProperty"]) { + assert.equal( + facade.resolveModeWriteSubItem(SET_CONFIG_OPTION, key), + "setConfigOption", + key, + ); + } + }); + + test("an unknown endpoint key is a plain Error, never a capability error", async () => { + const facade = await bootFacade(t0()); + const caught = await caughtBy(() => facade.resolveModeWriteSubItem("POST /api/nope")); + assert.ok(caught); + assert.equal(isEngineCapabilityNotSupportedError(caught), false); + assert.equal(caught.code, "unknown_mode_write_endpoint"); + }); +}); + +// --------------------------------------------------------------------------- +// The hard gate +// --------------------------------------------------------------------------- + +describe("assertModeWriteCapability — HARD", () => { + test("reports `unregistered-transport` on acp and never throws: the pre-B9 behaviour", async (t) => { + const facade = await bootFacade(t); + // THE MOST IMPORTANT SINGLE CASE IN THIS FILE. `acp` is the default + // transport and no provider claims it until M4, so every shipped + // user is on this branch and every response they can receive is the + // one they received before this batch. + for (const endpoint of [SET_MODE, SET_CONFIG_OPTION]) { + const d = facade.assertModeWriteCapability(endpoint, ACP); + assert.equal(d.gate, "unregistered-transport", endpoint); + assert.equal(d.provider, null, endpoint); + assert.equal(d.enforcement, "hard", endpoint); + } + }); + + test("a full declaration passes and reports `checked`", async (t) => { + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const d = facade.assertModeWriteCapability(SET_MODE, RUNTIME); + assert.equal(d.gate, "checked"); + assert.equal(d.provider, "local-runtime-v2"); + assert.equal(d.subItem, "setMode"); + }); + + test("a `none` declaration throws the STRUCTURED error for both endpoints", async (t) => { + const facade = await bootFacadeWithProvider(t, NEITHER); + for (const [endpoint, capability] of [ + [SET_MODE, "toolSkillInvocation"], + [SET_CONFIG_OPTION, "authCredentials"], + ]) { + const caught = await caughtBy(() => facade.assertModeWriteCapability(endpoint, RUNTIME)); + assert.ok(isEngineCapabilityNotSupportedError(caught), endpoint); + assert.equal(caught.capability, capability, endpoint); + assert.equal(caught.provider, "local-runtime-v2", endpoint); + } + }); + + test("a `partial` throws ONLY for the sub-item it actually denies", async (t) => { + const facade = await bootFacadeWithProvider(t, NO_GENERIC_CONFIG_WRITE); + // #67's sub-item is denied. + const denied = await caughtBy(() => facade.assertModeWriteCapability(SET_MODE, RUNTIME)); + assert.ok(isEngineCapabilityNotSupportedError(denied)); + assert.equal(denied.capability, "toolSkillInvocation"); + // The two bridged ids are not, so they pass. + for (const configId of ["model", "permissionMode"]) { + const d = facade.assertModeWriteCapability(SET_CONFIG_OPTION, RUNTIME, configId); + assert.equal(d.gate, "checked", configId); + } + // A generic id is. + const generic = await caughtBy(() => + facade.assertModeWriteCapability(SET_CONFIG_OPTION, RUNTIME, "thinkingEffort"), + ); + assert.ok(isEngineCapabilityNotSupportedError(generic)); + assert.equal(generic.capability, "authCredentials"); + assert.deepEqual(generic.missing, ["setConfigOption"]); + }); + + test("#68's verdict depends on the config id, and on nothing else", async (t) => { + // The gate is the one place where a request field decides a status. + // Pinning both directions on the SAME provider is what stops a + // future refactor from making the bridge depend on the session, the + // transport, the value, or the order of two calls. + const facade = await bootFacadeWithProvider(t, NO_GENERIC_CONFIG_WRITE); + assert.equal(facade.assertModeWriteCapability(SET_CONFIG_OPTION, RUNTIME, "model").gate, "checked"); + assert.equal( + facade.assertModeWriteCapability(SET_CONFIG_OPTION, RUNTIME, "permissionMode").gate, + "checked", + ); + const caught = await caughtBy(() => facade.assertModeWriteCapability(SET_CONFIG_OPTION, RUNTIME, "other")); + assert.ok(isEngineCapabilityNotSupportedError(caught)); + // A second call with the same bridged id still passes: no caching, + // no order dependence, no state. + assert.equal(facade.assertModeWriteCapability(SET_CONFIG_OPTION, RUNTIME, "model").gate, "checked"); + }); + + test("an unknown endpoint key is a plain Error with a machine-readable code", async (t) => { + const facade = await bootFacadeWithProvider(t, NEITHER); + const caught = await caughtBy(() => facade.assertModeWriteCapability("POST /api/nope", RUNTIME)); + assert.ok(caught); + assert.equal(isEngineCapabilityNotSupportedError(caught), false); + assert.equal(caught.code, "unknown_mode_write_endpoint"); + }); +}); + +describe("resolveModeWriteProvider", () => { + test("null on a transport no provider claims, an object on one that does", async (t) => { + const facade = await bootFacade(t); + assert.equal(facade.resolveModeWriteProvider(ACP), null); + const p = facade.resolveModeWriteProvider(RUNTIME); + assert.equal(p.id, "local-runtime-v2"); + assert.equal(p.transport, "runtime"); + }); + + test("null for a transport that does not exist at all", async (t) => { + const facade = await bootFacade(t); + assert.equal(facade.resolveModeWriteProvider("exec"), null); + assert.equal(facade.resolveModeWriteProvider(undefined), null); + }); +}); + +// --------------------------------------------------------------------------- +// The status tables — THE OLD STATE, pinned as values +// --------------------------------------------------------------------------- + +describe("the two failure-status tables keep their pre-B9 rows", () => { + // Every row, including the ones no fixture reaches. The last row is + // the asymmetry this file exists partly to protect: #67 has always + // answered 502 for a code it cannot classify and #68 has always + // answered 500. Unifying them would change one endpoint's wire to + // match the other's, which is not a migration step. + const CASES = [ + ["unsupported", 501, 501], + ["no_client", 503, 503], + ["resource_not_found", 404, 404], + ["session not found", 404, 404], + ["invalidParams", 404, 404], + ["policy_violation", 409, 409], + ["conflict", 409, 409], + ["client_throw", 502, 500], + ["some_unmapped_code", 502, 500], + [undefined, 502, 500], + ["", 502, 500], + ]; + + test("setModeFailureStatus / setConfigOptionFailureStatus over every row", async (t) => { + const facade = await bootFacade(t); + for (const [code, modeStatus, configStatus] of CASES) { + assert.equal(facade.setModeFailureStatus(code), modeStatus, `set-mode ${code}`); + assert.equal( + facade.setConfigOptionFailureStatus(code), + configStatus, + `set-config-option ${code}`, + ); + } + }); + + test("the two tables differ ONLY on the default row", async (t) => { + const facade = await bootFacade(t); + for (const [code] of CASES) { + const same = facade.setModeFailureStatus(code) === facade.setConfigOptionFailureStatus(code); + const isDefaultRow = code !== "unsupported" && code !== "no_client" && !(code && /not.found|invalid/i.test(code)) && !(code && /conflict|policy/i.test(code)); + assert.equal(same, !isDefaultRow, `code ${code}`); + } + }); +}); + +// --------------------------------------------------------------------------- +// Data plane — the old state +// --------------------------------------------------------------------------- + +describe("setEngineSessionMode — capability PRESENT, byte-for-byte unchanged", () => { + test("a success echoes the request's mode and the engine's data", async (t) => { + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const r = await facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: RUNTIME }); + assert.equal(r.statusHint, 200); + assert.deepEqual(r.payload, { ok: true, mode: "plan", data: { modeId: "plan" } }); + assert.equal(r.gate.gate, "checked"); + }); + + test("the success body is the same whatever mode was asked for", async (t) => { + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + for (const mode of ["plan", "plan_mode", "default", "normal", "goal_mode"]) { + const r = await facade.setEngineSessionMode({ sessionId: "mvs_a", mode, transport: RUNTIME }); + assert.deepEqual(r.payload, { ok: true, mode, data: { modeId: "plan" } }, mode); + } + }); + + test("an engine refusal keeps `fallback: send_plan_as_prompt` on the 501", async (t) => { + registerRpcMock({ setMode: async () => ({ ok: false, code: "unsupported", error: "no" }) }); + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const r = await facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: RUNTIME }); + assert.equal(r.statusHint, 501); + assert.deepEqual(r.payload, { + ok: false, + error: "no", + code: "unsupported", + fallback: "send_plan_as_prompt", + }); + }); + + test("a client throw is folded into `client_throw`, not escaped", async (t) => { + registerRpcMock({ + setMode: async () => { + throw new Error("socket gone"); + }, + }); + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const r = await facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: RUNTIME }); + assert.equal(r.statusHint, 502); + assert.equal(r.payload.code, "client_throw"); + assert.equal(r.payload.error, "socket gone"); + assert.equal(r.payload.fallback, "send_plan_as_prompt"); + }); + + test("a non-Error throw still produces a string `error`", async (t) => { + registerRpcMock({ + setMode: async () => { + throw "plain string"; + }, + }); + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const r = await facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: RUNTIME }); + assert.equal(r.payload.error, "plain string"); + assert.equal(r.payload.code, "client_throw"); + }); +}); + +describe("setEngineSessionConfigOption — capability PRESENT, byte-for-byte unchanged", () => { + test("a success echoes key and value and the engine's data", async (t) => { + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const r = await facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key: "permissionMode", + value: "auto", + cid: "cid-1", + transport: RUNTIME, + }); + assert.equal(r.statusHint, 200); + assert.deepEqual(r.payload, { ok: true, key: "permissionMode", value: "auto", data: {} }); + }); + + test("an engine refusal carries NO `fallback` — #68 never had one", async (t) => { + registerRpcMock({ setConfigOption: async () => ({ ok: false, code: "unsupported", error: "no" }) }); + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const r = await facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key: "permissionMode", + value: "auto", + transport: RUNTIME, + }); + assert.equal(r.statusHint, 501); + assert.deepEqual(r.payload, { ok: false, error: "no", code: "unsupported" }); + assert.equal("fallback" in r.payload, false); + }); + + test("a client throw is NOT caught: #68 never had a try/catch and still has none", async (t) => { + // Deliberate asymmetry with #67, pinned so a future "let's make them + // consistent" change has to be a decision rather than a tidy-up. + registerRpcMock({ + setConfigOption: async () => { + throw new Error("socket gone"); + }, + }); + const facade = await bootFacadeWithProvider(t, FULL_BOTH); + const caught = await caughtBy(() => + facade.setEngineSessionConfigOption({ sessionId: "mvs_a", key: "x", value: "y", transport: RUNTIME }), + ); + assert.ok(caught, "a throwing transport must propagate out of #68"); + assert.equal(caught.message, "socket gone"); + }); +}); + +// --------------------------------------------------------------------------- +// Data plane — the NEW state +// --------------------------------------------------------------------------- + +describe("setEngineSessionMode — capability ABSENT", () => { + test("the gate throws; the router's shared mapping is what answers 501", async (t) => { + const facade = await bootFacadeWithProvider(t, NO_GENERIC_CONFIG_WRITE); + const caught = await caughtBy(() => + facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: RUNTIME }), + ); + assert.ok(isEngineCapabilityNotSupportedError(caught)); + // The body the client will see, computed from the very function + // app.js calls. Asserting it here means the route never has to know + // the shape — and if the shape moves, this moves with it. + const { status, payload } = engineCapabilityHttpResponse(caught); + assert.equal(status, 501); + assert.equal(payload.code, "engine_capability_not_supported"); + assert.equal(payload.capability, "toolSkillInvocation"); + assert.equal(payload.provider, "local-runtime-v2"); + assert.deepEqual(payload.missing, ["setMode"]); + assert.equal("fallback" in payload, false, "the capability 501 must not advertise a degraded action"); + }); + + test("the engine is never called when the gate refuses", async (t) => { + let called = 0; + registerRpcMock({ + setMode: async () => { + called += 1; + return { ok: true, data: {} }; + }, + }); + const facade = await bootFacadeWithProvider(t, NO_GENERIC_CONFIG_WRITE); + await caughtBy(() => facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: RUNTIME })); + assert.equal(called, 0, "a refused write must not reach the engine — a late success would be the fake-success failure"); + }); + + test("the real registry refuses #67 on the runtime transport today", async (t) => { + // No mock: this is the behaviour change this batch actually ships, + // reachable through the real provider declarations. + const facade = await bootFacade(t); + const caught = await caughtBy(() => + facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: RUNTIME }), + ); + assert.ok(isEngineCapabilityNotSupportedError(caught), "v2 declares no session-mode write"); + assert.equal(caught.provider, "local-runtime-v2"); + }); + + test("and does NOT refuse it on the default acp transport", async (t) => { + const facade = await bootFacade(t); + const r = await facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan", transport: ACP }); + assert.equal(r.statusHint, 200, "no provider claims acp, so acp keeps the pre-B9 behaviour"); + assert.equal(r.gate.gate, "unregistered-transport"); + }); +}); + +describe("setEngineSessionConfigOption — capability ABSENT, and the bridge", () => { + test("a generic config id is refused, structured, with no `fallback`", async (t) => { + const facade = await bootFacadeWithProvider(t, NO_GENERIC_CONFIG_WRITE); + const caught = await caughtBy(() => + facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key: "thinkingEffort", + value: "high", + transport: RUNTIME, + }), + ); + assert.ok(isEngineCapabilityNotSupportedError(caught)); + const { status, payload } = engineCapabilityHttpResponse(caught); + assert.equal(status, 501); + assert.equal(payload.capability, "authCredentials"); + assert.deepEqual(payload.missing, ["setConfigOption"]); + }); + + test("BOTH bridged ids still reach the engine", async (t) => { + const seen = []; + registerRpcMock({ + setConfigOption: async (sessionId, key, value, cid) => { + seen.push({ sessionId, key, value, cid }); + return { ok: true, data: { applied: true } }; + }, + }); + const facade = await bootFacadeWithProvider(t, NO_GENERIC_CONFIG_WRITE); + for (const [key, value] of [ + ["model", "gpt-x"], + ["permissionMode", "auto"], + ]) { + const r = await facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key, + value, + cid: "cid-1", + transport: RUNTIME, + }); + assert.equal(r.statusHint, 200, key); + assert.equal(r.gate.subItem, key === "model" ? "selectModel" : "setPermissionMode"); + } + assert.deepEqual(seen, [ + { sessionId: "mvs_a", key: "model", value: "gpt-x", cid: "cid-1" }, + { sessionId: "mvs_a", key: "permissionMode", value: "auto", cid: "cid-1" }, + ]); + }); + + test("the cid still reaches the RPC wrapper for a bridged id", async (t) => { + // A regression here would be silent: the write would land on the + // singleton's subprocess instead of the one holding the session, and + // the engine would answer "session not found" for a session that + // exists. + let seenCid; + registerRpcMock({ + setConfigOption: async (_sid, _key, _value, cid) => { + seenCid = cid; + return { ok: true, data: {} }; + }, + }); + const facade = await bootFacadeWithProvider(t, NO_GENERIC_CONFIG_WRITE); + await facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key: "permissionMode", + value: "auto", + cid: "cid-xyz", + transport: RUNTIME, + }); + assert.equal(seenCid, "cid-xyz"); + }); + + test("a `none` capability refuses the bridged ids too — the bridge is not a way around `none`", async (t) => { + // The bridge is an exemption from the GENERIC sub-item only. A + // provider with no `authCredentials` at all has no dedicated writer + // either, and a bridge that survived `none` would be a hole in the + // hard gate. + const facade = await bootFacadeWithProvider(t, NEITHER); + for (const key of ["model", "permissionMode", "anything"]) { + const caught = await caughtBy(() => + facade.setEngineSessionConfigOption({ sessionId: "mvs_a", key, value: "v", transport: RUNTIME }), + ); + assert.ok(isEngineCapabilityNotSupportedError(caught), key); + } + }); + + test("the real registry refuses a generic id and keeps both bridged ids on the runtime transport", async (t) => { + // The shipped behaviour change, stated as the two halves of it. + const facade = await bootFacade(t); + const generic = await caughtBy(() => + facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key: "thinkingEffort", + value: "high", + transport: RUNTIME, + }), + ); + assert.ok(isEngineCapabilityNotSupportedError(generic)); + for (const key of ["model", "permissionMode"]) { + const r = await facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key, + value: "v", + transport: RUNTIME, + }); + assert.equal(r.statusHint, 200, key); + } + }); + + test("and refuses NOTHING on the default acp transport", async (t) => { + const facade = await bootFacade(t); + for (const key of ["model", "permissionMode", "thinkingEffort"]) { + const r = await facade.setEngineSessionConfigOption({ + sessionId: "mvs_a", + key, + value: "v", + transport: ACP, + }); + assert.equal(r.statusHint, 200, key); + assert.equal(r.gate.gate, "unregistered-transport", key); + } + }); +}); + +// --------------------------------------------------------------------------- +// Transport resolution comes from the config, not from a parameter that +// a caller can forget. +// --------------------------------------------------------------------------- + +describe("the transport defaults to MCODE_WEBUI_TRANSPORT", () => { + // This suite runs under BOTH gate invocations + // (`pnpm test:webui` and `MCODE_WEBUI_TRANSPORT=acp pnpm test:webui`), + // and `lib/config.js` reads the env at module-init time. So the + // assertion is "whatever the env says, that is what the call used" — + // reading the env and comparing to a hard-coded "acp" would make this + // test red under the runtime gate for a reason that has nothing to do + // with the code. + const ENV_TRANSPORT = process.env.MCODE_WEBUI_TRANSPORT || "acp"; + + test("no `transport` option means the config's value, not a hard-coded one", async (t) => { + const facade = await bootFacade(t); + // The two transports diverge in SHAPE, not just in status: `acp` has + // no registered provider and the call returns a result, while `runtime` + // registers a provider that denies the mode write and the gate throws. + // Asserting one shape for both would be the test lying about half the + // matrix, so each is asserted on its own side. + const result = { transport: null, gate: null, statusHint: null }; + const thrown = await caughtBy(async () => { + const r = await facade.setEngineSessionMode({ sessionId: "mvs_a", mode: "plan" }); + result.transport = r.transport; + result.gate = r.gate.gate; + result.statusHint = r.statusHint; + }); + if (ENV_TRANSPORT === "runtime") { + assert.ok(isEngineCapabilityNotSupportedError(thrown), "runtime denies the mode write"); + assert.equal(thrown.provider, "local-runtime-v2"); + return; + } + assert.equal(thrown, null); + assert.equal(result.transport, ENV_TRANSPORT); + assert.equal(result.gate, "unregistered-transport"); + assert.equal(result.statusHint, 200); + }); +}); + +// A no-op test context for the pure derivations, which need no mocks. +function t0() { + return { + mock: { module: () => {} }, + afterEach: () => {}, + }; +} diff --git a/packages/webui/test/routes/protocol.check.mjs b/packages/webui/test/routes/protocol.check.mjs index 1ad3cd9c..9fe2ad00 100644 --- a/packages/webui/test/routes/protocol.check.mjs +++ b/packages/webui/test/routes/protocol.check.mjs @@ -21,6 +21,12 @@ import { test, describe, before, beforeEach } from "node:test"; import assert from "node:assert/strict"; import { Readable } from "node:stream"; import { setupMocks, absPath } from "../helpers/_setup.js"; +// M3-B9: type discrimination goes through the exported predicate, never +// `err.name` — `name` is writable, so one stray assignment would turn the +// gate's structured 501 into an unrelated failure mode. +const { isEngineCapabilityNotSupportedError } = await import( + "../helpers/_setup.js" +).then(() => import(absPath("engine/errors.js"))); let protoRoute; before(async (t) => { @@ -75,13 +81,33 @@ describe("handleSetMode — /api/protocol/set-mode", () => { assert.equal(res._status, 400); }); + // M3-B9: the two cases below are now TRANSPORT-DEPENDENT, and the + // difference is the batch's shipped behaviour rather than a flake. + // `acp` has no registered engine provider, so the hard gate reports + // `unregistered-transport` and the route answers exactly as it always + // has. `runtime` registers `local-runtime-v2`, whose declaration is + // audited to carry no `setMode`, so the gate throws and the ROUTER + // answers 501 — the handler under test never writes a status at all, + // which is why the runtime case below asserts the throw. + const ENV_TRANSPORT = process.env.MCODE_WEBUI_TRANSPORT || "acp"; + const GATED = ENV_TRANSPORT === "runtime"; + test("200 once the engine accepts the mode", async () => { const res = fakeRes(); - await protoRoute.handleSetMode( + const call = protoRoute.handleSetMode( fakeReq({ sessionId: "mvs_aaa", mode: "plan" }), res, { cs: fakeCs(), cid: "cid-1" }, ); + if (GATED) { + // The route must NOT catch the capability error — folding it into + // a status table here would turn "the engine cannot do this" into + // a 502. It propagates to app.js, which owns the 501 mapping. + await assert.rejects(call, (e) => isEngineCapabilityNotSupportedError(e)); + assert.equal(res._status, null, "the handler must not write a status for the gate's 501"); + return; + } + await call; assert.equal(res._status, 200); const body = JSON.parse(res._body); assert.equal(body.ok, true); @@ -91,10 +117,25 @@ describe("handleSetMode — /api/protocol/set-mode", () => { test("501 with the slash-command fallback hint when the engine refuses", async () => { // The wrapper no longer produces 'unsupported' itself, but the route still - // maps that code to 501 + the degraded-path hint. + // maps that code to 501 + the degraded-path hint. The hint survives the + // engine's own refusal and is deliberately NOT on the gate's 501 — a + // capability that does not exist has no degraded action to fall back to. const { registerRpcMock } = await import("../helpers/_setup.js"); registerRpcMock({ setMode: async () => ({ ok: false, code: "unsupported", error: "no" }) }); try { + if (GATED) { + // Under a provider that declares no mode write the gate refuses + // first and the engine is never asked, so the hint is unreachable + // here. Asserting the refusal is the honest version of this case. + await assert.rejects( + protoRoute.handleSetMode(fakeReq({ sessionId: "mvs_aaa", mode: "plan" }), fakeRes(), { + cs: fakeCs(), + cid: "cid-1", + }), + (e) => isEngineCapabilityNotSupportedError(e), + ); + return; + } const res = fakeRes(); await protoRoute.handleSetMode( fakeReq({ sessionId: "mvs_aaa", mode: "plan" }), diff --git a/packages/webui/test/server/mode-write-501.test.js b/packages/webui/test/server/mode-write-501.test.js new file mode 100644 index 00000000..4f3cf519 --- /dev/null +++ b/packages/webui/test/server/mode-write-501.test.js @@ -0,0 +1,269 @@ +// webui/test/server/mode-write-501.test.js +// +// M3-B9 — the USER-VISIBLE half of the batch, asserted over HTTP. +// +// The engine-layer suite (test/lib/engine/mode-writes.test.js) proves +// the gate throws and what body the router will build from it. This file +// proves the thing a client actually receives, on the real Hono app and +// the real provider registry: +// +// runtime transport — the behaviour change +// POST /api/protocol/set-mode → 501 structured +// POST /api/protocol/set-config-option (generic) → 501 structured +// POST /api/protocol/set-config-option (bridged) → 200, unchanged +// +// acp transport — no behaviour change at all +// both endpoints, every config id → 200, unchanged +// +// Both halves run from ONE file under both gate invocations +// (`pnpm test:webui` and `MCODE_WEBUI_TRANSPORT=acp pnpm test:webui`), +// because the transport is a MODULE-INIT-TIME read in `lib/config.js`: +// a second file per transport would need its own process, and the point +// of this batch is that the two transports now differ. +// +// The RPC wrapper is mocked so no `mcode acp` subprocess is spawned, and +// its call log is the second assertion in every "unchanged" case: a 200 +// is only the old behaviour if the engine was still reached. + +import { test, describe, before, beforeEach, after } from "node:test"; +import assert from "node:assert/strict"; +import { Readable } from "node:stream"; +import { absPath } from "../helpers/_setup.js"; +import { mkTmpDir, rmTmpDir } from "../helpers/tmp.js"; + +// Pinned BEFORE anything reads the config. `lib/config.js` evaluates the +// env at module-init time, so a later assignment is a no-op. +const TRANSPORT = process.env.MCODE_WEBUI_TRANSPORT || "acp"; +const tmpBase = mkTmpDir("mcode-webui-b9-mode-write-"); +process.env.MINIMAX_DATA_DIR = tmpBase; +process.env.MCODE_WEBUI_DATA_DIR = tmpBase; +process.env.MCODE_WEBUI_SETTINGS_PATH = `${tmpBase}/settings.json`; +process.env.MCODE_WEBUI_EVENTS_PATH = `${tmpBase}/events.jsonl`; +process.env.MCODE_WEBUI_SESSIONS_DB = `${tmpBase}/sessions.db`; +process.env.MCODE_WEBUI_UPLOAD_DIR = `${tmpBase}/uploads`; + +const RUNTIME = TRANSPORT === "runtime"; + +/** Every RPC the two endpoints make, plus the log they append to. */ +let rpcCalls; +let createHonoApp; + +/** + * A Node request stand-in the route handlers can actually read. + * + * Not optional detail: these are LEGACY-shaped handlers, so they consume + * `c.env.incoming` as a stream through `lib/read-json.js` and never look + * at the Hono request. A plain `{method, url}` object gets past the gate + * chain and then fails `readJson` with "req is not async iterable" — + * which surfaces as a 500 and reads like a server bug rather than a + * broken fixture. + */ +function incoming(url, body) { + const req = Readable.from([Buffer.from(JSON.stringify(body ?? {}), "utf8")]); + req.method = "POST"; + req.url = url; + req.headers = { "content-type": "application/json" }; + req.socket = { remoteAddress: "127.0.0.1" }; + return req; +} + +async function post(path, body) { + const app = createHonoApp(); + const res = await app.request( + path, + { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(body ?? {}) }, + { incoming: incoming(path, body) }, + ); + const text = await res.text(); + // A non-JSON body here would be a fixture failure, not a contract + // failure, and must not be reported as one. + return { status: res.status, body: text ? JSON.parse(text) : null }; +} + +before(async (t) => { + // The mock is the REAL namespace with two functions replaced, not a + // hand-written one: `lib/mcode-rpc.js` is imported by half the server + // (usage.js, model.js, session-reads.js, protocol.js) and a partial + // mock namespace turns each of those into a SyntaxError at import time + // — a failure that reads as "app.js cannot boot" rather than as "the + // test's mock was incomplete". The real module is safe to import here: + // it reaches `acp-client.js` through a lazy import and spawns no + // subprocess until a call is made. + const realRpc = await import(absPath("lib/mcode-rpc.js")); + t.mock.module(absPath("lib/mcode-rpc.js"), { + namedExports: { + ...realRpc, + setMode: async (sessionId, modeId) => { + rpcCalls.push({ fn: "setMode", sessionId, modeId }); + return { ok: true, data: { modeId } }; + }, + setConfigOption: async (sessionId, configId, value, cid) => { + rpcCalls.push({ fn: "setConfigOption", sessionId, configId, value, cid }); + return { ok: true, data: { applied: true } }; + }, + }, + }); + const appModule = await import(absPath("app.js")); + createHonoApp = appModule.createHonoApp; +}); + +beforeEach(() => { + rpcCalls = []; +}); + +after(() => { + rmTmpDir(tmpBase); +}); + +const SID = "mvs_b9_b9_b9_b9_b9_b9_b9_b9_b9"; + +describe(`M3-B9 · the mode-write endpoints on the ${TRANSPORT} transport`, () => { + // ------------------------------------------------------------------------- + // The OLD STATE. On acp nothing changes, and on runtime the two bridged + // config ids do not change. Pinned as whole bodies, because "the control + // still works" is a claim about the response a browser parses. + // ------------------------------------------------------------------------- + test("set-mode answers the pre-B9 200 body — on acp only", async () => { + // #67 is the endpoint this batch actually cuts, so "unchanged" is + // only true where no provider is registered. On the runtime + // transport the same request is the 501 below; asserting 200 there + // would assert the regression this batch exists to make. + const r = await post("/api/protocol/set-mode", { sessionId: SID, mode: "plan" }); + if (RUNTIME) { + assert.equal(r.status, 501); + return; + } + assert.equal(r.status, 200); + assert.deepEqual(r.body, { ok: true, mode: "plan", data: { modeId: "plan" } }); + assert.deepEqual(rpcCalls, [{ fn: "setMode", sessionId: SID, modeId: "plan" }]); + }); + + test("set-mode still answers 400 for a missing sessionId or mode, before anything else", async () => { + // The 400s are the route's and run before the gate: a caller mistake + // must never be reported as an engine limitation. + assert.equal((await post("/api/protocol/set-mode", { mode: "plan" })).status, 400); + assert.equal((await post("/api/protocol/set-mode", { sessionId: SID })).status, 400); + assert.deepEqual(rpcCalls, [], "a 400 must not reach the engine"); + }); + + test("set-config-option answers the pre-B9 200 body for `permissionMode`", async () => { + const r = await post("/api/protocol/set-config-option", { + sessionId: SID, + key: "permissionMode", + value: "auto", + }); + assert.equal(r.status, 200); + assert.deepEqual(r.body, { + ok: true, + key: "permissionMode", + value: "auto", + data: { applied: true }, + }); + if (RUNTIME) { + assert.equal(rpcCalls.length, 1); + assert.equal(rpcCalls[0].configId, "permissionMode"); + } + }); + + test("set-config-option answers the pre-B9 200 body for `model`", async () => { + const r = await post("/api/protocol/set-config-option", { + sessionId: SID, + key: "model", + value: "gpt-x", + }); + assert.equal(r.status, 200); + assert.equal(r.body.ok, true); + if (RUNTIME) assert.equal(rpcCalls[0].configId, "model"); + }); + + test("set-config-option still answers 400 for a missing sessionId or key", async () => { + assert.equal( + (await post("/api/protocol/set-config-option", { key: "model", value: "x" })).status, + 400, + ); + assert.equal( + (await post("/api/protocol/set-config-option", { sessionId: SID, value: "x" })).status, + 400, + ); + assert.deepEqual(rpcCalls, []); + }); + + // ------------------------------------------------------------------------- + // The NEW STATE. Only reachable where a provider declares the capability + // absent, which today means the runtime transport. + // ------------------------------------------------------------------------- + test(RUNTIME ? "set-mode answers 501 with the structured capability body" : "set-mode is untouched on acp", async () => { + const r = await post("/api/protocol/set-mode", { sessionId: SID, mode: "plan" }); + if (!RUNTIME) { + assert.equal(r.status, 200, "acp has no registered provider, so acp must not change"); + assert.deepEqual(rpcCalls.length, 1, "and the engine is still reached"); + return; + } + assert.equal(r.status, 501); + assert.equal(r.body.ok, false); + assert.equal(r.body.code, "engine_capability_not_supported"); + assert.equal(r.body.capability, "toolSkillInvocation"); + assert.equal(r.body.provider, "local-runtime-v2"); + assert.deepEqual(r.body.missing, ["setMode"]); + // The pre-existing degraded-action hint is NOT on this body: there is + // no way to enter plan mode here to fall back FROM. + assert.equal("fallback" in r.body, false); + assert.deepEqual(rpcCalls, [], "a refused write must never reach the engine"); + }); + + test( + RUNTIME + ? "a generic config id answers 501 with the structured capability body" + : "a generic config id is untouched on acp", + async () => { + const r = await post("/api/protocol/set-config-option", { + sessionId: SID, + key: "thinkingEffort", + value: "high", + }); + if (!RUNTIME) { + assert.equal(r.status, 200); + assert.equal(r.body.key, "thinkingEffort"); + assert.equal(rpcCalls.length, 1); + return; + } + assert.equal(r.status, 501); + assert.equal(r.body.code, "engine_capability_not_supported"); + assert.equal(r.body.capability, "authCredentials"); + assert.deepEqual(r.body.missing, ["setConfigOption"]); + assert.equal("fallback" in r.body, false); + assert.deepEqual(rpcCalls, [], "a refused write must never reach the engine"); + }, + ); + + // ------------------------------------------------------------------------- + // The boundary itself: same route, same body, two config ids, two + // outcomes. Without this the two cases above could each be passing for + // the wrong reason (a broken route, a broken mock). + // ------------------------------------------------------------------------- + test("one request, two config ids, two answers — the bridge is the difference", async () => { + const bridged = await post("/api/protocol/set-config-option", { + sessionId: SID, + key: "permissionMode", + value: "auto", + }); + const generic = await post("/api/protocol/set-config-option", { + sessionId: SID, + key: "thinkingEffort", + value: "high", + }); + assert.equal(bridged.body.key, "permissionMode"); + assert.equal(generic.body.key === "permissionMode", false); + if (RUNTIME) { + assert.equal(bridged.status, 200); + assert.equal(generic.status, 501); + // Exactly one engine call: the bridged one. + assert.equal(rpcCalls.length, 1); + assert.equal(rpcCalls[0].configId, "permissionMode"); + } else { + assert.equal(bridged.status, 200); + assert.equal(generic.status, 200); + assert.equal(rpcCalls.length, 2); + } + }); +}); diff --git a/packages/webui/webapp/components/composer.tsx b/packages/webui/webapp/components/composer.tsx index 623ad3d8..24c5bc5a 100644 --- a/packages/webui/webapp/components/composer.tsx +++ b/packages/webui/webapp/components/composer.tsx @@ -15,6 +15,8 @@ import { createPortal } from "react-dom"; import * as api from "@/lib/api"; import { clientId } from "@/lib/cid"; +import { bridgedControlAvailability, readEngineCapabilities } from "@/lib/engine-capabilities"; +import type { ControlAvailability, EngineCapabilities } from "@/lib/engine-capabilities"; import { effortControlShape, effortOptionsWithDefault, @@ -254,6 +256,15 @@ export function Composer({ // pick another mode and see nothing change (reported as "完全不能做出选择" // together with the occluded popup). Resolve either form. const permission = resolvePermissionMode(state?.permissions); + // M3-B9: the two engine-backed controls below are hidden outright when + // the connected provider declares the matching write absent. Not + // disabled, not a toast — the engine has never been able to perform the + // write, so a visible control would be advertising an action that + // cannot happen. See `lib/engine-capabilities.ts` for the fail-open + // rule and `webapp/test/engine-capabilities-degradation.test.ts` for + // the coverage of both halves. + const permissionControl = useEngineControlAvailability("permissionMode"); + const modelControl = useEngineControlAvailability("model"); const hasConversation = decodeTranscript(state?.chat ?? []).length > 0; /** Nothing to send yet — the send button is rendered but inert. */ const empty = value.trim().length === 0 && attachments.length === 0; @@ -801,19 +812,29 @@ export function Composer({ - void api.setPermissions(id)} - /> + {/* M3-B9: hidden outright when the provider declares no + permission-mode write — see the declaration comment on + `permissionControl` above. */} + {permissionControl.available ? ( + void api.setPermissions(id)} + /> + ) : null}
{/* Context-window readout, immediately left of the model selector. */} - + /> + ) : null} {/* Thinking-effort picker (ticket 04). Only rendered when the active model carries a `thinkingLevels` list; the picker is gated so models without reasoning controls @@ -1056,6 +1078,41 @@ const SelectPanel = forwardRef< ); }); +/** + * M3-B9 — the two engine-backed controls' availability, from the + * server's capability declaration. + * + * Starts as `null` and stays `null` until the probe answers or fails, + * which is what makes the degradation fail-open: a control is shown until + * something positively says the engine cannot do it. The probe is a + * single request shared by both controls (see `readEngineCapabilities`'s + * module-level cache), and it is never re-run — a provider's declaration + * does not change while the page is open. + * + * `null` and `{available:true}` are deliberately the same rendering + * decision. There is no intermediate "disabled while loading" state: a + * control that appears a moment later is worse than one that was always + * there, because the user can click it in between. + */ +function useEngineControlAvailability( + configId: "model" | "permissionMode", +): ControlAvailability { + const [declaration, setDeclaration] = useState(null); + useEffect(() => { + let live = true; + void readEngineCapabilities().then((caps) => { + if (live) setDeclaration(caps); + }); + return () => { + live = false; + }; + }, []); + return useMemo( + () => bridgedControlAvailability(declaration, configId), + [declaration, configId], + ); +} + /** * One row of a `SelectPanel`. * diff --git a/packages/webui/webapp/lib/engine-capabilities.ts b/packages/webui/webapp/lib/engine-capabilities.ts new file mode 100644 index 00000000..86da78f8 --- /dev/null +++ b/packages/webui/webapp/lib/engine-capabilities.ts @@ -0,0 +1,159 @@ +// webapp/lib/engine-capabilities.ts +// +// M3-B9 — the frontend half of the mode-write capability gate. +// +// The server answers a 501 when the connected engine provider declares no +// session-mode write or no generic config-option write (see +// `server/engine/mode-writes.js`). Design §4.2 says what the UI does with +// that: the entry point is HIDDEN, not answered with an error toast. A +// toast is the wrong shape for a capability that was never there — it +// reports a failure for something the user was never able to do, it +// cannot be acted on, and it reappears on every click. +// +// So the rule lives here, as data, and the controls read it. Nothing in +// the composer is allowed to interpret a 501 itself: two places each +// deciding "what does not-supported mean" is how a second one ends up +// growing a toast. +// +// The rule is deliberately FAIL-OPEN. A control is shown unless the +// declaration positively says the capability is absent: +// +// - the probe failed, timed out, or has not finished → SHOW. We do not +// know, and hiding a working control because a diagnostic request was +// slow is a worse failure than showing one that may not work. +// - the capability is `full` → SHOW. +// - the capability is `partial` → SHOW unless THIS control's sub-item +// is the one listed missing. +// - the capability is `none` → HIDE. +// +// Two controls are the reason this file exists: the permission-mode +// selector and the model selector. Both are declared by +// `MODE_WRITE_BRIDGED_CONFIG_IDS` on the server, the two config ids the +// mode-write gate exempts from the generic-write refusal, so a provider +// that refuses generic config options still serves both. That table is +// mirrored here — one small literal — and +// `webapp/test/engine-capabilities-degradation.test.ts` reads the server +// module's source and fails if the two ever disagree. A mirror without +// that tripwire would be exactly the kind of drift this repository has +// been bitten by before. + +/** One declared capability, as the server serialises it. */ +export interface EngineCapabilityEntry { + level: "full" | "partial" | "none"; + missing?: string[]; + reason?: string; +} + +/** The 14-key declaration, or `null` when it could not be read. */ +export type EngineCapabilities = Record | null; + +/** + * The two config ids the mode-write gate bridges, and the engine + * sub-item each asks for instead of the generic one. + * + * Mirrors `MODE_WRITE_BRIDGED_CONFIG_IDS` in + * `server/engine/mode-writes.js`. Read it there, not here, when the two + * disagree — and make them agree rather than picking one. + */ +export const BRIDGED_CONFIG_SUB_ITEMS = Object.freeze({ + model: "selectModel", + permissionMode: "setPermissionMode", +} as const); + +/** The two config ids with a dedicated engine write behind them. */ +export type BridgedConfigId = keyof typeof BRIDGED_CONFIG_SUB_ITEMS; + +/** What a control should do, and why — `reason` is for logs, not for the user. */ +export interface ControlAvailability { + available: boolean; + /** The declaration's own `reason` when the control is hidden. */ + reason: string | null; +} + +/** + * Is one engine control usable, given the declaration? + * + * Pure, and the only place the rule exists. Every input shape is + * answered, because the shapes arrive from the network and from a + * half-initialised component: a `null` declaration, a missing key, a + * `partial` with no `missing` array, an unknown `level`. + * + * @param declaration The 14-key declaration, or `null` if unread. + * @param capability One of the server's capability keys. + * @param subItem The engine sub-item this control needs. + */ +export function controlAvailability( + declaration: EngineCapabilities, + capability: string, + subItem: string, +): ControlAvailability { + // Unread declaration: show. See the module header — failing closed + // here would hide working controls because a diagnostic request was + // slow, which is a self-inflicted outage. + if (!declaration) return { available: true, reason: null }; + const entry = declaration[capability]; + // A declaration missing a key is malformed — the server's own + // validator requires all 14 — so it is not evidence of absence. + if (!entry) return { available: true, reason: null }; + if (entry.level === "full") return { available: true, reason: null }; + if (entry.level === "partial") { + const missing = Array.isArray(entry.missing) ? entry.missing : []; + if (!missing.includes(subItem)) return { available: true, reason: null }; + return { available: false, reason: entry.reason ?? null }; + } + if (entry.level === "none") return { available: false, reason: entry.reason ?? null }; + // An unrecognised level is not "absent". The server validates the three + // levels; anything else means a version skew, and version skew is not a + // reason to remove a control. + return { available: true, reason: null }; +} + +/** `controlAvailability` for one of the two bridged controls. */ +export function bridgedControlAvailability( + declaration: EngineCapabilities, + configId: BridgedConfigId, +): ControlAvailability { + return controlAvailability(declaration, "authCredentials", BRIDGED_CONFIG_SUB_ITEMS[configId]); +} + +/** + * Read the declaration once per page and share it. + * + * Module-level cache with an in-flight promise, because the two controls + * mount together and a per-component fetch would double the request on + * every composer mount. The cache is deliberately NOT invalidated: a + * provider's declaration does not change while the page is open, and a + * poller here would be a new failure surface for no benefit. + */ +let cached: Promise | null = null; + +/** Drop the cache. Test-only; production never has a reason to. */ +export function resetEngineCapabilitiesCache(): void { + cached = null; +} + +/** + * Fetch `GET /api/engine-capabilities` and return its declaration. + * + * Resolves to `null` for every failure — network, non-200, unparseable, + * wrong shape — because "we do not know" and "the engine cannot do it" + * must not look alike to a control. The caller never has to catch. + */ +export function readEngineCapabilities(): Promise { + if (cached) return cached; + cached = (async () => { + try { + const res = await fetch("/api/engine-capabilities", { + headers: { accept: "application/json" }, + }); + if (!res.ok) return null; + const body = (await res.json()) as { capabilities?: unknown }; + const caps = body?.capabilities; + if (!caps || typeof caps !== "object") return null; + return caps as Record; + } catch { + return null; + } + })(); + return cached; +} diff --git a/packages/webui/webapp/test/engine-capabilities-degradation.test.ts b/packages/webui/webapp/test/engine-capabilities-degradation.test.ts new file mode 100644 index 00000000..01197dd3 --- /dev/null +++ b/packages/webui/webapp/test/engine-capabilities-degradation.test.ts @@ -0,0 +1,340 @@ +// webapp/test/engine-capabilities-degradation.test.ts +// +// M3-B9 — the frontend half of the mode-write capability gate. +// +// The server answers 501 when the connected provider declares no +// permission-mode / model write (see `server/engine/mode-writes.js`). +// Design §4.2 says what happens next: the entry point is HIDDEN. Not an +// error toast, not a disabled control, not a message in the transcript. +// +// The distinction is the whole point of this file, and it is the one a +// reviewer cannot check by reading the call site: a toast and a hidden +// control are both "the control reacted to a 501", and only one of them +// is right. A toast reports a failure for something the user was never +// able to do, offers nothing to act on, and reappears on every click. +// +// Three things are pinned, and they are three different failure modes: +// +// 1. THE RULE, over every declaration shape the network can produce — +// including the ones that must NOT hide anything. The rule is +// fail-open, and the cases below are what make that true rather than +// accidental. +// 2. THE BRIDGE MIRROR. The frontend names two engine sub-items the +// server also names. Two hand-maintained copies of a set of engine +// identifiers drift; the tripwire reads the server module's SOURCE +// and fails when the two disagree. +// 3. THE WIRING. `components/composer.tsx` is a client component with +// no render harness in this suite, so its half is a static-source +// tripwire — the form this repository allows when no harness exists +// for the component. It is a weaker proof than (1) and says so; the +// product logic it would otherwise duplicate lives in `lib/` and is +// tested there against the real function, not a copy. + +import { test, describe, beforeEach, afterEach } from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { fileURLToPath } from "node:url"; +import { dirname, resolve } from "node:path"; + +import { + BRIDGED_CONFIG_SUB_ITEMS, + bridgedControlAvailability, + controlAvailability, + readEngineCapabilities, + resetEngineCapabilitiesCache, +} from "../lib/engine-capabilities"; +import type { EngineCapabilities } from "../lib/engine-capabilities"; + +const here = dirname(fileURLToPath(import.meta.url)); +const composerSource = readFileSync(resolve(here, "../components/composer.tsx"), "utf8"); +const serverModeWritesSource = readFileSync( + resolve(here, "../../server/engine/mode-writes.js"), + "utf8", +); + +/** The v2 declaration's two B9 keys, as the server sends them today. */ +const V2_MODE_KEYS = { + toolSkillInvocation: { + level: "partial" as const, + missing: ["setMode"], + reason: "no session-mode write on the v2 surface", + }, + authCredentials: { + level: "partial" as const, + missing: ["setConfigOption"], + reason: "no generic config-option write on the v2 surface", + }, +}; + +describe("controlAvailability — the fail-open rule", () => { + test("an unread declaration shows the control: `null` is not `none`", () => { + // The whole degradation rests on this. A probe that timed out must + // not remove a working control — that is a self-inflicted outage + // dressed up as a feature flag. + assert.deepEqual(controlAvailability(null, "authCredentials", "selectModel"), { + available: true, + reason: null, + }); + }); + + test("a declaration missing the key shows the control", () => { + // The server validates all 14 keys, so an absent one is a malformed + // declaration or a version skew — neither is evidence of absence. + const caps = { authCredentials: { level: "full" } } as unknown as EngineCapabilities; + assert.equal(controlAvailability(caps, "authCredentials", "selectModel").available, true); + }); + + test("`full` shows the control", () => { + const caps = { authCredentials: { level: "full" } } as EngineCapabilities; + assert.equal(controlAvailability(caps, "authCredentials", "selectModel").available, true); + assert.equal(controlAvailability(caps, "authCredentials", "anything").available, true); + }); + + test("`none` hides the control and carries the declaration's reason", () => { + const caps = { + authCredentials: { level: "none", reason: "interface-absent: no such method" }, + } as EngineCapabilities; + assert.deepEqual(controlAvailability(caps, "authCredentials", "selectModel"), { + available: false, + reason: "interface-absent: no such method", + }); + }); + + test("`partial` hides ONLY the sub-item it lists, and shows every other one", () => { + const caps = { authCredentials: V2_MODE_KEYS.authCredentials } as EngineCapabilities; + // The generic write is gone… + assert.equal(controlAvailability(caps, "authCredentials", "setConfigOption").available, false); + // …and the two dedicated writers are not what it denied. + assert.equal(controlAvailability(caps, "authCredentials", "selectModel").available, true); + assert.equal(controlAvailability(caps, "authCredentials", "setPermissionMode").available, true); + }); + + test("a `partial` with no `missing` array shows the control", () => { + // Shape robustness: the array is typed optional, so a declaration + // that omits it must not be read as "everything is missing". + const caps = { authCredentials: { level: "partial" } } as unknown as EngineCapabilities; + assert.equal(controlAvailability(caps, "authCredentials", "setConfigOption").available, true); + }); + + test("an unrecognised level shows the control — version skew is not absence", () => { + const caps = { + authCredentials: { level: "experimental", missing: ["selectModel"] }, + } as unknown as EngineCapabilities; + assert.equal(controlAvailability(caps, "authCredentials", "selectModel").available, true); + }); + + test("a `none` with no reason hides but does not invent one", () => { + const caps = { authCredentials: { level: "none" } } as unknown as EngineCapabilities; + assert.deepEqual(controlAvailability(caps, "authCredentials", "selectModel"), { + available: false, + reason: null, + }); + }); +}); + +describe("bridgedControlAvailability — the two controls the composer renders", () => { + test("both are available on the v2 declaration this batch ships", () => { + const caps = { authCredentials: V2_MODE_KEYS.authCredentials } as EngineCapabilities; + for (const id of ["model", "permissionMode"] as const) { + assert.equal(bridgedControlAvailability(caps, id).available, true, id); + } + }); + + test("both are hidden when the capability is `none`", () => { + const caps = { + authCredentials: { level: "none", reason: "test: interface-absent" }, + } as EngineCapabilities; + for (const id of ["model", "permissionMode"] as const) { + assert.equal(bridgedControlAvailability(caps, id).available, false, id); + } + }); + + test("a provider that denies the DEDICATED writer hides that control and keeps the other", () => { + // The bridge is per sub-item, so a provider can have one without the + // other — and the composer must not hide both because one is gone. + const caps = { + authCredentials: { level: "partial", missing: ["selectModel"], reason: "test: no model writer" }, + } as EngineCapabilities; + assert.equal(bridgedControlAvailability(caps, "model").available, false); + assert.equal(bridgedControlAvailability(caps, "permissionMode").available, true); + }); + + test("an unknown config id is not a bridge — it must not inherit the exemption", () => { + // Mirrors the server's own guard: a name nobody audited falls back + // to the generic sub-item rather than the exemption. + const caps = { authCredentials: V2_MODE_KEYS.authCredentials } as EngineCapabilities; + const subItem = (BRIDGED_CONFIG_SUB_ITEMS as Record)[ + "thinkingEffort" + ]; + assert.equal(subItem, undefined); + assert.equal( + controlAvailability(caps, "authCredentials", "setConfigOption").available, + false, + "the generic write is what a non-bridged id asks for", + ); + }); +}); + +describe("the frontend and the server name the same two engine sub-items", () => { + // The mirror is one small literal, and this is what keeps it honest. + // A rename on either side without the other is exactly the drift the + // server module's own header warns about. + test("BRIDGED_CONFIG_SUB_ITEMS matches MODE_WRITE_BRIDGED_CONFIG_IDS in the server source", () => { + const block = serverModeWritesSource.match( + /MODE_WRITE_BRIDGED_CONFIG_IDS\s*=\s*Object\.freeze\(\{([\s\S]*?)\}\)/, + ); + assert.ok(block, "the server bridge table was not found — did it move or get renamed?"); + // `assert.ok` does not narrow under this tsconfig, so the capture is + // read through a fallback: a missing table parses to zero pairs, which + // the assertion below reports in a readable sentence. + const pairs = [...(block?.[1] ?? "").matchAll(/([A-Za-z0-9_]+):\s*"([^"]+)"/g)].map( + (m) => [m[1], m[2]] as const, + ); + assert.ok(pairs.length > 0, "the server bridge table parsed to nothing"); + assert.deepEqual( + Object.fromEntries(pairs), + Object.fromEntries(Object.entries(BRIDGED_CONFIG_SUB_ITEMS)), + ); + }); + + test("the server gate really does ask for the bridged sub-item, not the generic one", () => { + // Without this, the mirror above could be faithful to a server table + // nothing reads — a table that has drifted from the gate while both + // copies still agree with each other. + assert.match( + serverModeWritesSource, + /const bridged = MODE_WRITE_BRIDGED_CONFIG_IDS\[configId\];/, + "the gate must resolve its sub-item through the bridge table", + ); + assert.match( + serverModeWritesSource, + /typeof bridged === "string" \? bridged : need\.subItem/, + "an unrecognised config id must fall back to the generic sub-item", + ); + }); +}); + +describe("the composer actually gates on it", () => { + // Static-source tripwire: composer.tsx is a client component and this + // suite has no render harness for it. Weak by construction, and stated + // as such — what it catches is the realistic regression, which is a + // later edit that drops the gate while leaving the lib alone. + test("both controls are wrapped in their availability check", () => { + assert.match( + composerSource, + /\{permissionControl\.available \? \(\s* { + assert.match(composerSource, /useEngineControlAvailability\("permissionMode"\)/); + assert.match(composerSource, /useEngineControlAvailability\("model"\)/); + assert.match( + composerSource, + /bridgedControlAvailability\(declaration, configId\)/, + "the hook must delegate to the shared rule", + ); + // A 501 is the SERVER's answer. The composer must not be growing a + // second interpretation of it — that is how a toast gets in. + assert.doesNotMatch( + composerSource, + /engine_capability_not_supported/, + "the composer must not branch on the 501 body; the declaration decides", + ); + }); + + test("the degradation is a hide, not a toast and not a disabled control", () => { + // Extract the gated JSX BLOCK, not a window of characters after it. + // A 4000-character sweep matches the next unrelated `disabled` prop + // in the file and fails for a reason that has nothing to do with the + // gate — which trains a reader to ignore this assertion. + for (const [marker, control] of [ + ["{permissionControl.available ? (", "PermissionSelect"], + ["{modelControl.available ? (", "ModelSelect"], + ] as const) { + const start = composerSource.indexOf(marker); + assert.ok(start > 0, `${control}: the availability gate is gone`); + const end = composerSource.indexOf(") : null}", start); + assert.ok(end > start, `${control}: the gate no longer ends in \`: null\``); + const block = composerSource.slice(start, end); + assert.ok(block.includes(`<${control}`), `${control}: the gate does not wrap the control`); + assert.doesNotMatch(block, /\bdisabled\b/, `${control}: hidden, not disabled`); + assert.doesNotMatch(block, /fallback|toast/i, `${control}: hidden, with no degraded rendering`); + } + }); + + test("there are exactly two gates, and both hide", () => { + const gates = [...composerSource.matchAll(/(?:permission|model)Control\.available \? \(/g)]; + assert.equal(gates.length, 2, "expected exactly two availability gates"); + }); +}); + +describe("readEngineCapabilities — never throws, and reads once", () => { + const realFetch = globalThis.fetch; + + beforeEach(() => { + resetEngineCapabilitiesCache(); + }); + + afterEach(() => { + globalThis.fetch = realFetch; + resetEngineCapabilitiesCache(); + }); + + const okResponse = (body: unknown) => + new Response(JSON.stringify(body), { + status: 200, + headers: { "Content-Type": "application/json" }, + }); + + test("returns the declaration on a good response", async () => { + globalThis.fetch = (async () => okResponse({ ok: true, capabilities: V2_MODE_KEYS })) as typeof fetch; + const caps = await readEngineCapabilities(); + assert.deepEqual(caps, V2_MODE_KEYS); + }); + + test("a non-200 resolves to null rather than rejecting", async () => { + globalThis.fetch = (async () => new Response("nope", { status: 500 })) as typeof fetch; + assert.equal(await readEngineCapabilities(), null); + }); + + test("a network failure resolves to null rather than rejecting", async () => { + globalThis.fetch = (async () => { + throw new Error("offline"); + }) as typeof fetch; + assert.equal(await readEngineCapabilities(), null); + }); + + test("a 200 with the wrong shape resolves to null", async () => { + for (const body of [{}, { capabilities: null }, { capabilities: "full" }, { capabilities: 7 }]) { + globalThis.fetch = (async () => okResponse(body)) as typeof fetch; + resetEngineCapabilitiesCache(); + assert.equal(await readEngineCapabilities(), null, JSON.stringify(body)); + } + }); + + test("unparseable JSON resolves to null", async () => { + globalThis.fetch = (async () => + new Response("502", { status: 200 })) as typeof fetch; + assert.equal(await readEngineCapabilities(), null); + }); + + test("two controls cost one request — the declaration is shared", async () => { + let calls = 0; + globalThis.fetch = (async () => { + calls += 1; + return okResponse({ capabilities: V2_MODE_KEYS }); + }) as typeof fetch; + const [a, b] = await Promise.all([readEngineCapabilities(), readEngineCapabilities()]); + const c = await readEngineCapabilities(); + assert.equal(calls, 1, "the composer mounts two controls; it must not make two requests"); + assert.equal(a, b); + assert.equal(b, c, "the cache must hand back the same declaration, not a fresh fetch"); + }); +}); diff --git a/release/public-source.json b/release/public-source.json index 83ffe902..7421b7eb 100644 --- a/release/public-source.json +++ b/release/public-source.json @@ -3453,6 +3453,7 @@ "packages/webui/server/engine/host.js", "packages/webui/server/engine/index.js", "packages/webui/server/engine/interrupt.js", + "packages/webui/server/engine/mode-writes.js", "packages/webui/server/engine/model-reads.js", "packages/webui/server/engine/providers/local-runtime-v2.capabilities.js", "packages/webui/server/engine/providers/local-runtime-v2.js", @@ -3607,6 +3608,7 @@ "packages/webui/test/lib/engine/capability-snapshot.test.js", "packages/webui/test/lib/engine/host-facade.test.js", "packages/webui/test/lib/engine/interrupt.test.js", + "packages/webui/test/lib/engine/mode-writes.test.js", "packages/webui/test/lib/engine/model-reads.test.js", "packages/webui/test/lib/engine/session-export.test.js", "packages/webui/test/lib/engine/session-load.test.js", @@ -3710,6 +3712,7 @@ "packages/webui/test/server/fs-parent-reachability.test.js", "packages/webui/test/server/gates-lan-reject.test.js", "packages/webui/test/server/graceful-shutdown.test.js", + "packages/webui/test/server/mode-write-501.test.js", "packages/webui/test/server/read-json-cap.test.js", "packages/webui/test/server/router-auth-gate.check.mjs", "packages/webui/test/server/router-cors.test.js", @@ -3798,6 +3801,7 @@ "packages/webui/webapp/lib/credential-file.ts", "packages/webui/webapp/lib/edited-files.ts", "packages/webui/webapp/lib/effort-control.ts", + "packages/webui/webapp/lib/engine-capabilities.ts", "packages/webui/webapp/lib/file-open-reason.ts", "packages/webui/webapp/lib/file-preview.ts", "packages/webui/webapp/lib/files-tree.ts", @@ -3943,6 +3947,7 @@ "packages/webui/webapp/test/conversation-usage-banner.test.ts", "packages/webui/webapp/test/credential-file.test.ts", "packages/webui/webapp/test/edited-files-card.test.ts", + "packages/webui/webapp/test/engine-capabilities-degradation.test.ts", "packages/webui/webapp/test/file-open-reason.test.ts", "packages/webui/webapp/test/file-preview.test.ts", "packages/webui/webapp/test/files-tree.test.ts", diff --git a/scripts/test-tmp-leak.check.mjs b/scripts/test-tmp-leak.check.mjs index 5ebd12ac..6fa9d67b 100644 --- a/scripts/test-tmp-leak.check.mjs +++ b/scripts/test-tmp-leak.check.mjs @@ -235,6 +235,7 @@ const KNOWN_PREFIXES = [ "mcode-resolver-mac-", "mcode-resolver-valid-", "mcode-resolver-win-", + "mcode-webui-b9-mode-write-", "mcode-webui-bind-", "mcode-webui-c08-", "mcode-webui-d02-chain-",