From c1dd2360d708ee321f5421f151928ef4b2be0d17 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 01:21:03 +0000 Subject: [PATCH 01/11] feat(opencode): add optional model param to Task tool Allow a Task subagent call to specify the model it runs on via an optional "providerID/modelID" string param. The model is parsed with Provider.parseModel and validated with Provider.getModel; an explicit model overrides both the agent's configured model and parent inheritance, forcing variant to undefined. Omitting the param preserves the existing inherit-from-agent/parent behavior. Co-authored-by: yui-soul --- packages/opencode/src/tool/task.ts | 47 +++- packages/opencode/src/tool/task.txt | 1 + .../__snapshots__/parameters.test.ts.snap | 4 + packages/opencode/test/tool/task.test.ts | 211 +++++++++++++++++- 4 files changed, 256 insertions(+), 7 deletions(-) diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index dac340184cfc..dbb652019d25 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -7,10 +7,11 @@ import { Session } from "@/session/session" import { SessionID, MessageID } from "../session/schema" import { MessageV2 } from "../session/message-v2" import { Agent } from "../agent/agent" +import { Provider } from "@/provider/provider" import { deriveSubagentSessionPermission } from "../agent/subagent-permissions" import type { SessionPrompt } from "../session/prompt" import { Config } from "@/config/config" -import { Effect, Exit, Schema, Scope } from "effect" +import { Effect, Exit, Option, Schema, Scope } from "effect" import { EffectBridge } from "@/effect/bridge" import { RuntimeFlags } from "@/effect/runtime-flags" import { Database } from "@opencode-ai/core/database/database" @@ -49,6 +50,10 @@ const BaseParameterFields = { "This should only be set if you mean to resume a previous task (you can pass a prior task_id and the task will continue the same subagent session as before instead of creating a fresh one)", }), command: Schema.optional(Schema.String).annotate({ description: "The command that triggered this task" }), + model: Schema.optional(Schema.String).annotate({ + description: + 'The model the subagent should use, in "providerID/modelID" form (e.g. "anthropic/claude-sonnet-4"). Overrides the subagent\'s default/inherited model.', + }), } const BaseParameters = Schema.Struct(BaseParameterFields) @@ -88,6 +93,10 @@ export const TaskTool = Tool.define( const scope = yield* Scope.Scope const flags = yield* RuntimeFlags.Service const database = yield* Database.Service + // Provider is resolved optionally so the tool still constructs in contexts + // that do not wire it up; it is only consulted when an explicit `model` + // param needs validation. + const providerOption = yield* Effect.serviceOption(Provider.Service) const run = Effect.fn("TaskTool.execute")(function* ( params: Schema.Schema.Type, @@ -168,10 +177,36 @@ export const TaskTool = Tool.define( if (msg.info.role !== "assistant") return yield* Effect.fail(new Error("Not an assistant message")) const variant = msg.info.variant - const model = next.model ?? { - modelID: msg.info.modelID, - providerID: msg.info.providerID, - } + // An explicit `model` param overrides BOTH the agent's own model and the + // parent assistant inheritance. Parse "providerID/modelID" and validate it + // through Provider.getModel before spawning the subagent so an unknown or + // malformed value fails fast (and never reaches ops.prompt). + const override = params.model + ? yield* Effect.gen(function* () { + const requested = params.model! + const resolver = Option.getOrUndefined(providerOption) + if (!resolver) + return yield* Effect.fail( + new Error(`Cannot resolve task model "${requested}": the provider service is unavailable.`), + ) + const parsed = Provider.parseModel(requested) + yield* resolver.getModel(parsed.providerID, parsed.modelID).pipe( + Effect.mapError( + () => + new Error( + `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, + ), + ), + ) + return parsed + }) + : undefined + + const model = override ?? + next.model ?? { + modelID: msg.info.modelID, + providerID: msg.info.providerID, + } const metadata = { parentSessionId: ctx.sessionID, sessionId: nextSession.id, @@ -196,7 +231,7 @@ export const TaskTool = Tool.define( modelID: model.modelID, providerID: model.providerID, }, - variant: next.model ? undefined : variant, + variant: override || next.model ? undefined : variant, agent: next.name, parts, }) diff --git a/packages/opencode/src/tool/task.txt b/packages/opencode/src/tool/task.txt index c5e412f409d9..77a051d2fc7e 100644 --- a/packages/opencode/src/tool/task.txt +++ b/packages/opencode/src/tool/task.txt @@ -17,3 +17,4 @@ Usage notes: 5. The agent's outputs should generally be trusted 6. Clearly tell the agent whether you expect it to write code or just to do research (search, file reads, web fetches, etc.), since it is not aware of the user's intent. Tell it how to verify its work if possible (e.g., relevant test commands). 7. If the agent description mentions that it should be used proactively, then you should try your best to use it without the user having to ask for it first. Use your judgement. +8. You may optionally pass a model in "providerID/modelID" form (e.g. "anthropic/claude-sonnet-4") to override the subagent's default model for this task. diff --git a/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap b/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap index b187b191c1f5..98cd02df0fb0 100644 --- a/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap +++ b/packages/opencode/test/tool/__snapshots__/parameters.test.ts.snap @@ -331,6 +331,10 @@ exports[`tool parameters JSON Schema (wire shape) task 1`] = ` "description": "A short (3-5 words) description of the task", "type": "string", }, + "model": { + "description": "The model the subagent should use, in "providerID/modelID" form (e.g. "anthropic/claude-sonnet-4"). Overrides the subagent's default/inherited model.", + "type": "string", + }, "prompt": { "description": "The task for the agent to perform", "type": "string", diff --git a/packages/opencode/test/tool/task.test.ts b/packages/opencode/test/tool/task.test.ts index 97bb7db065bd..8415af96559e 100644 --- a/packages/opencode/test/tool/task.test.ts +++ b/packages/opencode/test/tool/task.test.ts @@ -1,7 +1,45 @@ +/** + * @spec-handoff + * @interface TaskTool input schema — new OPTIONAL `model` parameter + * model?: string // format "providerID/modelID", e.g. "anthropic/claude-sonnet-4" + * Added to BaseParameterFields in src/tool/task.ts (line ~43-52) so it flows into + * BOTH `BaseParameters` (jsonSchema, line 54) and `Parameters` (line 56). + * + * @behavior + * 1. model OMITTED → unchanged: subagent runs on `agent.model` if set, else inherits + * the parent assistant message's { providerID, modelID } (task.ts:171-174). The + * `model` passed to ops.prompt (task.ts:195-198) equals that inherited/agent model; + * `variant` stays the parent variant when the agent has no model (task.ts:199). + * 2. model = VALID "providerID/modelID" → overrides BOTH agent.model and parent + * inheritance. Parse with Provider.parseModel(params.model) (provider.ts:1944) and + * validate with Provider.getModel(parsed.providerID, parsed.modelID) (provider.ts:1747). + * The `model` passed to ops.prompt equals the parsed { providerID, modelID }, and + * `variant` becomes undefined (treated like an explicit agent-model override). + * 3. model = unknown provider OR unknown model → Provider.getModel throws + * ModelNotFoundError (provider.ts:1757/1766); the tool FAILS with a clear, actionable + * error whose message contains the offending "providerID/modelID". ops.prompt is never + * called (validation short-circuits before spawning the subagent). + * 4. model = malformed string with no "/" → FAIL with a clear error mentioning the bad + * string. No silent fallthrough to the inherited model. + * + * @edge-cases + * - Only a provided model string triggers validation; omission preserves current behavior. + * - Override validation runs through Provider.getModel BEFORE ops.prompt — a failed + * validation must short-circuit so ops.prompt is never invoked. + * - The error message must surface the user-supplied model string for actionability. + * + * @change-sites src/tool/task.ts + * - schema: BaseParameterFields (line ~43) — add `model: Schema.optional(Schema.String)` + * - model resolution: lines 171-174 — when `params.model` is set, override with the + * parsed + validated model instead of agent/parent inheritance + * - ops.prompt call: lines 195-199 — pass the overridden model; set `variant` undefined on override + * @helpers Provider.parseModel, Provider.getModel, Provider.ModelNotFoundError + * @see src/provider/provider.ts + */ import { afterEach, describe, expect } from "bun:test" import { SessionV1 } from "@opencode-ai/core/v1/session" import { Database } from "@opencode-ai/core/database/database" -import { Deferred, Effect, Exit, Fiber, Layer } from "effect" +import { Cause, Deferred, Effect, Exit, Fiber, Layer } from "effect" import { Agent } from "../../src/agent/agent" import { BackgroundJob } from "@/background/job" import { EventV2Bridge } from "@/event-v2-bridge" @@ -22,6 +60,8 @@ import { disposeAllInstances } from "../fixture/fixture" import { testEffect } from "../lib/effect" import { ProviderV2 } from "@opencode-ai/core/provider" import { ModelV2 } from "@opencode-ai/core/model" +import { Provider } from "@/provider/provider" +import { ProviderTest } from "../fake/provider" afterEach(async () => { await disposeAllInstances() @@ -51,6 +91,25 @@ const layer = (flags: Partial = {}) => const it = testEffect(layer()) const background = testEffect(layer({ experimentalBackgroundSubagents: true })) +// The one model the Task tool's `model` override is allowed to resolve to. +// Distinct from `ref` (the inherited parent/seed model) so override vs. inheritance +// can be told apart by the captured ops.prompt input. +const overrideRef = { + providerID: ProviderV2.ID.make("anthropic"), + modelID: ModelV2.ID.make("claude-sonnet-4"), +} + +// Stub Provider.getModel: resolves only `overrideRef`, otherwise raises the same +// ModelNotFoundError the real provider raises for unknown provider/model. +const providerMock = Layer.mock(Provider.Service)({ + getModel: (providerID, modelID) => + providerID === overrideRef.providerID && modelID === overrideRef.modelID + ? Effect.succeed(ProviderTest.model({ id: modelID, providerID })) + : Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions: [] })), +}) + +const withModel = testEffect(Layer.mergeAll(layer(), providerMock)) + function defer() { let resolve!: (value: T | PromiseLike) => void const promise = new Promise((done) => { @@ -896,3 +955,153 @@ describe("tool.task", () => { }), ) }) + +describe("tool.task model override", () => { + // Behavior 1 — baseline guard (expected GREEN): omitting `model` preserves the + // current inheritance. The captured ops.prompt input must use the parent/seed + // model and keep the parent variant (the "general" agent has no model of its own). + withModel.instance("omitting model preserves inherited parent model and variant", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let seen: SessionPrompt.PromptInput | undefined + const promptOps = stubOps({ onPrompt: (input) => (seen = input) }) + + yield* def.execute( + { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + }, + { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }, + ) + + expect(seen?.model).toEqual(ref) + expect(seen?.variant).toBe("xhigh") + }), + ) + + // Behavior 2 — valid override (expected RED until implemented): a valid + // "providerID/modelID" overrides both agent.model and parent inheritance. The + // captured model must equal Provider.parseModel(params.model), and variant must + // be undefined (treated like an explicit agent-model override). + withModel.instance("valid model param overrides agent and parent inheritance", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let seen: SessionPrompt.PromptInput | undefined + const promptOps = stubOps({ onPrompt: (input) => (seen = input) }) + + // Stored in a const so the extra `model` key is accepted by width subtyping + // until task.ts adds it to the schema (RED phase). + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "anthropic/claude-sonnet-4", + } + + yield* def.execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + + expect(seen?.model).toEqual(overrideRef) + expect(seen?.variant).toBeUndefined() + }), + ) + + // Behavior 3 — unknown model (expected RED until implemented): a well-formed + // "providerID/modelID" that Provider.getModel rejects must fail the tool with a + // clear, actionable error mentioning the offending model, and ops.prompt must + // never be called. + withModel.instance("unknown model param fails with an actionable error", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "bogus/does-not-exist", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("bogus/does-not-exist") + expect(prompted).toBe(false) + }), + ) + + // Behavior 4 — malformed model (expected RED until implemented): a string with no + // "/" must fail with a clear error mentioning the bad value — no silent fallthrough + // to the inherited model. + withModel.instance("malformed model param without a slash fails clearly", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "not-a-valid-model", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("not-a-valid-model") + expect(prompted).toBe(false) + }), + ) +}) From 5008785998b93fb6dac4b09ea8c46d20654e91e7 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 01:21:07 +0000 Subject: [PATCH 02/11] feat(ui): show provider next to model in messages Display the provider alongside the model as "provider/model" in both assistant footer and user header of the shared message component (used by app web and desktop). Extracts a pure formatModelLabel helper with per-field catalog fallback to raw providerID/modelID and a no-model guard that never renders a stray slash. Co-authored-by: yui-soul --- .../src/components/message-part-model.test.ts | 75 +++++++++++++++++++ .../ui/src/components/message-part-model.ts | 12 +++ packages/ui/src/components/message-part.tsx | 5 +- 3 files changed, 90 insertions(+), 2 deletions(-) create mode 100644 packages/ui/src/components/message-part-model.test.ts create mode 100644 packages/ui/src/components/message-part-model.ts diff --git a/packages/ui/src/components/message-part-model.test.ts b/packages/ui/src/components/message-part-model.test.ts new file mode 100644 index 000000000000..da1c69b1440a --- /dev/null +++ b/packages/ui/src/components/message-part-model.test.ts @@ -0,0 +1,75 @@ +/** + * @spec-handoff + * @interface formatModelLabel(providerID: string | undefined, modelID: string | undefined, provider: Provider | undefined): string + * + * Pure helper that renders the model label shown next to assistant and user + * messages. It MUST be extracted into a new sibling module + * `./message-part-model.ts` and then consumed by BOTH inline memos in + * `message-part.tsx`: + * - Assistant footer `model` memo — message-part.tsx lines 1478-1483 + * (resolve provider via `data.store.provider?.all?.get(message.providerID)`, + * then call `formatModelLabel(message.providerID, message.modelID, match)`) + * - User header `model` memo — message-part.tsx lines 1069-1088 + * (resolve provider via `data.store.provider?.all?.get(providerID)`, + * then call `formatModelLabel(providerID, modelID, match)`) + * + * @format + * - Forward slash, NO surrounding spaces: `provider/model` (e.g. "anthropic/claude-sonnet"). + * - Catalog HIT (provider entry resolved): use the catalog display names → + * `${provider.name}/${provider.models[modelID].name}`. + * - Catalog MISS (per-field `??` fallback, mirrors today's `?? modelID`): + * providerLabel = provider?.name ?? providerID + * modelLabel = provider?.models?.[modelID]?.name ?? modelID + * So a fully-missing catalog entry yields the RAW IDs: `providerID/modelID`. + * + * @no-model-guard + * - When providerID OR modelID is absent (user message with no model), return + * "" — never render a stray "/". Preserves the existing user-memo guard at + * message-part.tsx:1072 (`if (!providerID || !modelID) return ""`). + * + * @edge-cases + * - Provider resolves but the specific model is absent in its catalog → + * `${provider.name}/${modelID}` (provider name + raw model id). + * + * @see ./message-part.tsx (lines 1478-1483 assistant memo, 1069-1088 user memo) + * @see ./message-part-text.ts (sibling pure-helper module pattern to mirror) + */ +import { describe, expect, test } from "bun:test" +import type { Provider } from "@opencode-ai/sdk/v2" +import { formatModelLabel } from "./message-part-model" + +function provider(part: Partial = {}): Provider { + return { + id: "anthropic", + name: "Anthropic", + source: "config", + env: [], + options: {}, + models: { + "claude-sonnet-4-20250514": { + name: "Claude Sonnet", + }, + } as unknown as Provider["models"], + ...part, + } +} + +describe("formatModelLabel", () => { + test("assistant catalog hit renders provider and model display names as provider/model", () => { + expect(formatModelLabel("anthropic", "claude-sonnet-4-20250514", provider())).toBe("Anthropic/Claude Sonnet") + }) + + test("assistant catalog miss falls back to raw providerID/modelID", () => { + expect(formatModelLabel("anthropic", "claude-sonnet-4-20250514", undefined)).toBe( + "anthropic/claude-sonnet-4-20250514", + ) + }) + + test("user message with model renders provider/model from the catalog", () => { + expect(formatModelLabel("anthropic", "claude-sonnet-4-20250514", provider())).toBe("Anthropic/Claude Sonnet") + }) + + test("user message without model renders empty string and never a stray slash", () => { + expect(formatModelLabel(undefined, undefined, undefined)).toBe("") + }) +}) diff --git a/packages/ui/src/components/message-part-model.ts b/packages/ui/src/components/message-part-model.ts new file mode 100644 index 000000000000..787805db7c12 --- /dev/null +++ b/packages/ui/src/components/message-part-model.ts @@ -0,0 +1,12 @@ +import type { Provider } from "@opencode-ai/sdk/v2" + +export function formatModelLabel( + providerID: string | undefined, + modelID: string | undefined, + provider: Provider | undefined, +): string { + if (!providerID || !modelID) return "" + const providerLabel = provider?.name ?? providerID + const modelLabel = provider?.models?.[modelID]?.name ?? modelID + return `${providerLabel}/${modelLabel}` +} diff --git a/packages/ui/src/components/message-part.tsx b/packages/ui/src/components/message-part.tsx index 35912e119d40..cd3ba61a3289 100644 --- a/packages/ui/src/components/message-part.tsx +++ b/packages/ui/src/components/message-part.tsx @@ -58,6 +58,7 @@ import { animate } from "motion" import { useLocation } from "@solidjs/router" import { attached, inline, kind } from "./message-file" import { readPartText } from "./message-part-text" +import { formatModelLabel } from "./message-part-model" async function writeClipboard(text: string): Promise { const body = typeof document === "undefined" ? undefined : document.body @@ -1071,7 +1072,7 @@ export function UserMessageDisplay(props: { message: UserMessage; parts: PartTyp const modelID = props.message.model?.modelID if (!providerID || !modelID) return "" const match = data.store.provider?.all?.get(providerID) - return match?.models?.[modelID]?.name ?? modelID + return formatModelLabel(providerID, modelID, match) }) const timefmt = createMemo(() => new Intl.DateTimeFormat(i18n.locale(), { timeStyle: "short" })) @@ -1479,7 +1480,7 @@ PART_MAPPING["text"] = function TextPartDisplay(props) { if (props.message.role !== "assistant") return "" const message = props.message as AssistantMessage const match = data.store.provider?.all?.get(message.providerID) - return match?.models?.[message.modelID]?.name ?? message.modelID + return formatModelLabel(message.providerID, message.modelID, match) }) const duration = createMemo(() => { From cabeeb8c0ca8a57b68f92cbc05d3309e9b73e943 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 01:21:12 +0000 Subject: [PATCH 03/11] feat(tui): show provider next to model in session and transcript Add Model.providerModel formatter returning "provider/model" with raw providerID/modelID fallback on catalog miss, leaving Model.name intact. Switch the session transcript header and the session view model label to the new formatter. Existing transcript tests updated to the new format. Co-authored-by: yui-soul --- packages/tui/src/routes/session/index.tsx | 2 +- packages/tui/src/util/model.ts | 16 +++ packages/tui/src/util/transcript.ts | 2 +- .../test/util/provider-model-label.test.ts | 120 ++++++++++++++++++ packages/tui/test/util/transcript.test.ts | 14 +- 5 files changed, 145 insertions(+), 9 deletions(-) create mode 100644 packages/tui/test/util/provider-model-label.test.ts diff --git a/packages/tui/src/routes/session/index.tsx b/packages/tui/src/routes/session/index.tsx index 7736eb75b0c3..ff5cd394217d 100644 --- a/packages/tui/src/routes/session/index.tsx +++ b/packages/tui/src/routes/session/index.tsx @@ -1470,7 +1470,7 @@ function AssistantMessage(props: { message: AssistantMessage; parts: Part[]; las const { theme } = useTheme() const sync = useSync() const messages = createMemo(() => sync.data.message[props.message.sessionID] ?? []) - const model = createMemo(() => Model.name(ctx.providers(), props.message.providerID, props.message.modelID)) + const model = createMemo(() => Model.providerModel(ctx.providers(), props.message.providerID, props.message.modelID)) const final = createMemo(() => { return props.message.finish && !["tool-calls", "unknown"].includes(props.message.finish) diff --git a/packages/tui/src/util/model.ts b/packages/tui/src/util/model.ts index b6a5c77f545d..3209b2ed2482 100644 --- a/packages/tui/src/util/model.ts +++ b/packages/tui/src/util/model.ts @@ -26,3 +26,19 @@ export function name( ) { return get(list, providerID, modelID)?.name ?? modelID } + +export function providerModel( + list: Provider[] | ReadonlyMap | undefined, + providerID: string, + modelID: string, +) { + const provider = + list instanceof Map + ? list.get(providerID) + : Array.isArray(list) + ? list.find((item) => item.id === providerID) + : undefined + const model = provider?.models[modelID] + if (provider && model) return `${provider.name}/${model.name}` + return `${providerID}/${modelID}` +} diff --git a/packages/tui/src/util/transcript.ts b/packages/tui/src/util/transcript.ts index d727c19fb8b5..a0e918f1d966 100644 --- a/packages/tui/src/util/transcript.ts +++ b/packages/tui/src/util/transcript.ts @@ -76,7 +76,7 @@ export function formatAssistantHeader( const duration = msg.time.completed && msg.time.created ? ((msg.time.completed - msg.time.created) / 1000).toFixed(1) + "s" : "" - const modelName = Model.name(providers, msg.providerID, msg.modelID) + const modelName = Model.providerModel(providers, msg.providerID, msg.modelID) return `## Assistant (${Locale.titlecase(msg.agent)} · ${modelName}${duration ? ` · ${duration}` : ""})\n\n` } diff --git a/packages/tui/test/util/provider-model-label.test.ts b/packages/tui/test/util/provider-model-label.test.ts new file mode 100644 index 000000000000..9d64c375439a --- /dev/null +++ b/packages/tui/test/util/provider-model-label.test.ts @@ -0,0 +1,120 @@ +/** + * @spec-handoff + * @interface Model.providerModel( + * list: Provider[] | ReadonlyMap | undefined, + * providerID: string, + * modelID: string, + * ): string + * NEW export added to packages/tui/src/util/model.ts. + * @behavior + * - Catalog hit (provider + model both resolve): returns "/" + * e.g. ("anthropic","claude-sonnet-4-20250514") -> "Anthropic/Claude Sonnet 4" + * - Catalog miss (provider OR model not found): falls back to "/" + * e.g. ("openai","gpt-5") -> "openai/gpt-5" + * - Format rule: single forward slash, NO surrounding spaces ("a/b", never "a / b"). + * @edge-cases + * - Provider resolves but model does not -> full raw-ID fallback "/". + * - undefined catalog -> "/". + * @invariants + * - Model.name(...) MUST remain unchanged (other callers depend on model-name-only output). + * This file does not test Model.name; existing model/transcript tests guard it. + * @call-sites-to-switch (Kou — implementation step, NOT covered here) + * - packages/tui/src/routes/session/index.tsx:1473 (model memo: Model.name -> Model.providerModel) + * rendered at index.tsx:1557. SolidJS route component is not unit-tested here; the + * formatter + transcript tests below cover the observable behavior. + * - packages/tui/src/util/transcript.ts:79-81 (formatAssistantHeader: Model.name -> Model.providerModel) + * @see packages/tui/src/util/model.ts + * @see packages/tui/src/util/transcript.ts + */ +import { describe, expect, test } from "bun:test" +import { providerModel } from "../../src/util/model" +import { formatAssistantHeader } from "../../src/util/transcript" +import type { AssistantMessage, Provider } from "@opencode-ai/sdk/v2" + +const providers: Provider[] = [ + { + id: "anthropic", + name: "Anthropic", + source: "api", + env: [], + options: {}, + models: { + "claude-sonnet-4-20250514": { + id: "claude-sonnet-4-20250514", + providerID: "anthropic", + api: { + id: "claude-sonnet-4-20250514", + url: "https://example.com/claude-sonnet-4-20250514", + npm: "@ai-sdk/anthropic", + }, + name: "Claude Sonnet 4", + capabilities: { + temperature: true, + reasoning: true, + attachment: true, + toolcall: true, + input: { text: true, audio: false, image: true, video: false, pdf: true }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: false, + }, + cost: { input: 0, output: 0, cache: { read: 0, write: 0 } }, + limit: { context: 200_000, output: 8_192 }, + status: "active", + options: {}, + headers: {}, + release_date: "2025-05-14", + }, + }, + }, +] + +describe("util.model.providerModel", () => { + test("catalog hit: returns providerName/modelName", () => { + expect(providerModel(providers, "anthropic", "claude-sonnet-4-20250514")).toBe("Anthropic/Claude Sonnet 4") + }) + + test("catalog miss (unknown provider): falls back to providerID/modelID", () => { + expect(providerModel(providers, "openai", "gpt-5")).toBe("openai/gpt-5") + }) + + test("partial miss (known provider, unknown model): falls back to providerID/modelID", () => { + expect(providerModel(providers, "anthropic", "claude-unknown")).toBe("anthropic/claude-unknown") + }) + + test("undefined catalog: falls back to providerID/modelID", () => { + expect(providerModel(undefined, "anthropic", "claude-sonnet-4-20250514")).toBe("anthropic/claude-sonnet-4-20250514") + }) + + test("format rule: single forward slash, no surrounding spaces", () => { + const result = providerModel(providers, "anthropic", "claude-sonnet-4-20250514") + expect(result).not.toContain(" / ") + expect(result.split("/")).toHaveLength(2) + }) +}) + +describe("transcript assistant header shows provider/model", () => { + const baseMsg: AssistantMessage = { + id: "msg_123", + sessionID: "ses_123", + role: "assistant", + agent: "build", + modelID: "claude-sonnet-4-20250514", + providerID: "anthropic", + mode: "", + parentID: "msg_parent", + path: { cwd: "/test", root: "/test" }, + cost: 0.001, + tokens: { input: 100, output: 50, reasoning: 0, cache: { read: 0, write: 0 } }, + time: { created: 1000000, completed: 1005400 }, + } + + test("header includes provider next to model as provider/model", () => { + const result = formatAssistantHeader(baseMsg, true, providers) + expect(result).toContain("Anthropic/Claude Sonnet 4") + }) + + test("header falls back to raw provider/model ids on catalog miss", () => { + const result = formatAssistantHeader(baseMsg, true) + expect(result).toContain("anthropic/claude-sonnet-4-20250514") + }) +}) diff --git a/packages/tui/test/util/transcript.test.ts b/packages/tui/test/util/transcript.test.ts index 02d6ef0bb958..620737c01e50 100644 --- a/packages/tui/test/util/transcript.test.ts +++ b/packages/tui/test/util/transcript.test.ts @@ -80,12 +80,12 @@ describe("transcript", () => { test("includes metadata when enabled", () => { const result = formatAssistantHeader(baseMsg, true) - expect(result).toBe("## Assistant (Build · claude-sonnet-4-20250514 · 5.4s)\n\n") + expect(result).toBe("## Assistant (Build · anthropic/claude-sonnet-4-20250514 · 5.4s)\n\n") }) - test("uses model display name when available", () => { + test("uses provider and model display name when available", () => { const result = formatAssistantHeader(baseMsg, true, providers) - expect(result).toBe("## Assistant (Build · Claude Sonnet 4 · 5.4s)\n\n") + expect(result).toBe("## Assistant (Build · Anthropic/Claude Sonnet 4 · 5.4s)\n\n") }) test("excludes metadata when disabled", () => { @@ -96,7 +96,7 @@ describe("transcript", () => { test("handles missing completed time", () => { const msg = { ...baseMsg, time: { created: 1000000 } } const result = formatAssistantHeader(msg as AssistantMessage, true) - expect(result).toBe("## Assistant (Build · claude-sonnet-4-20250514)\n\n") + expect(result).toBe("## Assistant (Build · anthropic/claude-sonnet-4-20250514)\n\n") }) test("titlecases agent name", () => { @@ -289,7 +289,7 @@ describe("transcript", () => { } const parts: Part[] = [{ id: "p1", sessionID: "ses_123", messageID: "msg_123", type: "text", text: "Hi there" }] const result = formatMessage(msg, parts, options) - expect(result).toContain("## Assistant (Build · Claude Sonnet 4 · 5.4s)") + expect(result).toContain("## Assistant (Build · Anthropic/Claude Sonnet 4 · 5.4s)") expect(result).toContain("Hi there") }) }) @@ -344,7 +344,7 @@ describe("transcript", () => { expect(result).toContain("**Session ID:** ses_abc123") expect(result).toContain("## User") expect(result).toContain("Hello") - expect(result).toContain("## Assistant (Build · Claude Sonnet 4 · 0.5s)") + expect(result).toContain("## Assistant (Build · Anthropic/Claude Sonnet 4 · 0.5s)") expect(result).toContain("Hi!") expect(result).toContain("---") }) @@ -381,7 +381,7 @@ describe("transcript", () => { assistantMetadata: true, }) - expect(result).toContain("## Assistant (Build · claude-sonnet-4-20250514 · 0.5s)") + expect(result).toContain("## Assistant (Build · anthropic/claude-sonnet-4-20250514 · 0.5s)") }) test("formats transcript without assistant metadata", () => { From 97268f6afdef83101e83c08f8ad13f3a786f7be3 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 06:53:36 +0000 Subject: [PATCH 04/11] fix(opencode): harden Task model param validation Address audit findings on the Task tool model override: - treat the param as present when defined (model: "" now errors instead of silently inheriting the parent model) - reject empty and prototype-polluting segments (__proto__, constructor, prototype) before any provider lookup, and catch defects so a crafted value never leaks a raw TypeError - validate the model before creating the child subagent session, so an invalid model fails fast without spawning an orphan session Adds tests for empty-string, prototype keys, multi-slash modelID, and override-beats-agent-own-model. Co-authored-by: yui-soul --- packages/opencode/src/tool/task.ts | 79 +++-- packages/opencode/test/tool/task.test.ts | 375 ++++++++++++++++++++--- 2 files changed, 385 insertions(+), 69 deletions(-) diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index dbb652019d25..d8173be81ddd 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -127,6 +127,60 @@ export const TaskTool = Tool.define( return yield* Effect.fail(new Error(`Unknown agent type: ${params.subagent_type} is not a valid agent type`)) } + // An explicit `model` param overrides BOTH the agent's own model and the + // parent assistant inheritance. Validate it here — BEFORE the child session + // is created and before ops.prompt — so an unknown, malformed, or unsafe + // value short-circuits without spawning an orphan child session. The gate is + // `!== undefined` (not truthiness) so an empty string is treated as PRESENT + // and validated/rejected rather than silently inheriting. + const override = + params.model !== undefined + ? yield* Effect.gen(function* () { + const requested = params.model! + const resolver = Option.getOrUndefined(providerOption) + if (!resolver) + return yield* Effect.fail( + new Error(`Cannot resolve task model "${requested}": the provider service is unavailable.`), + ) + const parsed = Provider.parseModel(requested) + // Reject empty segments and prototype-polluting keys BEFORE any provider + // lookup. An unguarded record index on these keys would otherwise resolve + // Object.prototype as a "found" model, or throw a raw TypeError that leaks + // "Cannot read properties of undefined" instead of an actionable error. + const unsafe = (segment: string) => + segment === "" || + segment === "__proto__" || + segment === "constructor" || + segment === "prototype" + if (unsafe(parsed.providerID) || unsafe(parsed.modelID)) + return yield* Effect.fail( + new Error( + `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, + ), + ) + yield* resolver.getModel(parsed.providerID, parsed.modelID).pipe( + Effect.mapError( + () => + new Error( + `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, + ), + ), + // Defensive: the local guard above should make this unreachable, but a + // raw DEFECT (e.g. a TypeError from unguarded record indexing) is NOT + // caught by mapError. Convert any defect to the same actionable error so + // no raw "Cannot read properties of undefined" can leak. + Effect.catchDefect(() => + Effect.fail( + new Error( + `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, + ), + ), + ), + ) + return parsed + }) + : undefined + const session = params.task_id ? yield* sessions.get(SessionID.make(params.task_id)).pipe(Effect.catchCause(() => Effect.succeed(undefined))) : undefined @@ -177,31 +231,6 @@ export const TaskTool = Tool.define( if (msg.info.role !== "assistant") return yield* Effect.fail(new Error("Not an assistant message")) const variant = msg.info.variant - // An explicit `model` param overrides BOTH the agent's own model and the - // parent assistant inheritance. Parse "providerID/modelID" and validate it - // through Provider.getModel before spawning the subagent so an unknown or - // malformed value fails fast (and never reaches ops.prompt). - const override = params.model - ? yield* Effect.gen(function* () { - const requested = params.model! - const resolver = Option.getOrUndefined(providerOption) - if (!resolver) - return yield* Effect.fail( - new Error(`Cannot resolve task model "${requested}": the provider service is unavailable.`), - ) - const parsed = Provider.parseModel(requested) - yield* resolver.getModel(parsed.providerID, parsed.modelID).pipe( - Effect.mapError( - () => - new Error( - `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, - ), - ), - ) - return parsed - }) - : undefined - const model = override ?? next.model ?? { modelID: msg.info.modelID, diff --git a/packages/opencode/test/tool/task.test.ts b/packages/opencode/test/tool/task.test.ts index 8415af96559e..39224222b0eb 100644 --- a/packages/opencode/test/tool/task.test.ts +++ b/packages/opencode/test/tool/task.test.ts @@ -1,40 +1,49 @@ /** * @spec-handoff - * @interface TaskTool input schema — new OPTIONAL `model` parameter + * @interface TaskTool input schema — OPTIONAL `model` parameter * model?: string // format "providerID/modelID", e.g. "anthropic/claude-sonnet-4" - * Added to BaseParameterFields in src/tool/task.ts (line ~43-52) so it flows into - * BOTH `BaseParameters` (jsonSchema, line 54) and `Parameters` (line 56). + * Lives in BaseParameterFields (src/tool/task.ts:53-56) so it flows into BOTH + * `BaseParameters` (jsonSchema) and `Parameters`. * * @behavior - * 1. model OMITTED → unchanged: subagent runs on `agent.model` if set, else inherits - * the parent assistant message's { providerID, modelID } (task.ts:171-174). The - * `model` passed to ops.prompt (task.ts:195-198) equals that inherited/agent model; - * `variant` stays the parent variant when the agent has no model (task.ts:199). + * 1. model OMITTED → subagent runs on `agent.model` if set, else inherits the parent + * assistant message's { providerID, modelID }. The `model` passed to ops.prompt + * equals that inherited/agent model; `variant` stays the parent variant when the + * agent has no model of its own. * 2. model = VALID "providerID/modelID" → overrides BOTH agent.model and parent - * inheritance. Parse with Provider.parseModel(params.model) (provider.ts:1944) and - * validate with Provider.getModel(parsed.providerID, parsed.modelID) (provider.ts:1747). + * inheritance. Parsed with Provider.parseModel and validated with Provider.getModel. * The `model` passed to ops.prompt equals the parsed { providerID, modelID }, and - * `variant` becomes undefined (treated like an explicit agent-model override). - * 3. model = unknown provider OR unknown model → Provider.getModel throws - * ModelNotFoundError (provider.ts:1757/1766); the tool FAILS with a clear, actionable - * error whose message contains the offending "providerID/modelID". ops.prompt is never - * called (validation short-circuits before spawning the subagent). + * `variant` becomes undefined (treated like an explicit agent-model override). The + * override beats the subagent's OWN configured model, not just parent inheritance. + * 3. model = unknown provider OR unknown model → Provider.getModel raises + * ModelNotFoundError; the tool FAILS with a clear, actionable error whose message + * contains the offending "providerID/modelID". ops.prompt is never called and NO + * child subagent session is created (validation short-circuits before spawning). * 4. model = malformed string with no "/" → FAIL with a clear error mentioning the bad * string. No silent fallthrough to the inherited model. + * 5. model = "" (empty string) → FAIL with a clear error. Empty is NOT treated as + * "omitted": the gate is `params.model !== undefined`, not a truthiness check, so an + * empty string never silently inherits. ops.prompt is never called. + * 6. model = prototype-polluting key, e.g. "anthropic/__proto__" or "__proto__/x" → + * FAIL with a clear, actionable error naming the offending value. Must NOT bypass + * validation (treating Object.prototype as a "found" model) and must NOT surface a + * raw "Cannot read properties of undefined" TypeError. ops.prompt is never called. + * 7. model = VALID multi-slash modelID, e.g. "openrouter/anthropic/claude-3.5" → + * providerID is the FIRST segment ("openrouter"); modelID is the remainder joined + * back with "/" ("anthropic/claude-3.5"). Resolves and is passed through to ops.prompt + * exactly as { providerID: "openrouter", modelID: "anthropic/claude-3.5" }. * * @edge-cases - * - Only a provided model string triggers validation; omission preserves current behavior. - * - Override validation runs through Provider.getModel BEFORE ops.prompt — a failed - * validation must short-circuit so ops.prompt is never invoked. - * - The error message must surface the user-supplied model string for actionability. + * - Only a present (`!== undefined`) model string triggers validation; omission alone + * preserves the inheritance behavior. An empty string is present → it validates → fails. + * - Override validation runs through Provider.getModel BEFORE the child session is created + * and BEFORE ops.prompt — a failed validation must short-circuit so neither happens. + * - The error message must surface the user-supplied model string for actionability, + * including for prototype-polluting keys (no raw TypeError leakage). * - * @change-sites src/tool/task.ts - * - schema: BaseParameterFields (line ~43) — add `model: Schema.optional(Schema.String)` - * - model resolution: lines 171-174 — when `params.model` is set, override with the - * parsed + validated model instead of agent/parent inheritance - * - ops.prompt call: lines 195-199 — pass the overridden model; set `variant` undefined on override * @helpers Provider.parseModel, Provider.getModel, Provider.ModelNotFoundError - * @see src/provider/provider.ts + * @see src/provider/provider.ts (getModel record lookup: provider.ts:1747-1768; + * parseModel split-on-"/": provider.ts:1944) */ import { afterEach, describe, expect } from "bun:test" import { SessionV1 } from "@opencode-ai/core/v1/session" @@ -91,7 +100,7 @@ const layer = (flags: Partial = {}) => const it = testEffect(layer()) const background = testEffect(layer({ experimentalBackgroundSubagents: true })) -// The one model the Task tool's `model` override is allowed to resolve to. +// A model the Task tool's `model` override is allowed to resolve to. // Distinct from `ref` (the inherited parent/seed model) so override vs. inheritance // can be told apart by the captured ops.prompt input. const overrideRef = { @@ -99,13 +108,47 @@ const overrideRef = { modelID: ModelV2.ID.make("claude-sonnet-4"), } -// Stub Provider.getModel: resolves only `overrideRef`, otherwise raises the same -// ModelNotFoundError the real provider raises for unknown provider/model. +// A second valid model whose modelID itself contains slashes. parseModel keeps the +// FIRST "/" segment as the providerID and joins the rest back as the modelID, so the +// string "openrouter/anthropic/claude-3.5" must resolve to this pair. +const multiSlashRef = { + providerID: ProviderV2.ID.make("openrouter"), + modelID: ModelV2.ID.make("anthropic/claude-3.5"), +} + +// Known-model catalog for the override-validation tests. Declared as plain object +// literals ON PURPOSE: the real Provider.getModel resolves models with record lookups +// (`s.providers[providerID]` provider.ts:1749, then `provider.models[modelID]` +// provider.ts:1760). A prototype key like "__proto__" therefore resolves through +// Object.prototype here exactly as it would against real provider state, which is what +// lets the prototype-key tests exercise the genuine lookup hazard instead of a +// pre-sanitized mock. +const knownModels: Record }> = { + anthropic: { + models: { + "claude-sonnet-4": ProviderTest.model({ id: overrideRef.modelID, providerID: overrideRef.providerID }), + }, + }, + openrouter: { + models: { + "anthropic/claude-3.5": ProviderTest.model({ id: multiSlashRef.modelID, providerID: multiSlashRef.providerID }), + }, + }, +} + +// Mirror Provider.getModel (provider.ts:1747-1768): a missing provider OR a missing +// model raises ModelNotFoundError; whatever the records resolve is treated as a found +// model — including anything reachable via the prototype chain. The record lookups are +// deliberately unguarded so prototype-polluting keys behave as they do in production. const providerMock = Layer.mock(Provider.Service)({ getModel: (providerID, modelID) => - providerID === overrideRef.providerID && modelID === overrideRef.modelID - ? Effect.succeed(ProviderTest.model({ id: modelID, providerID })) - : Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions: [] })), + Effect.gen(function* () { + const provider = knownModels[providerID] + if (!provider) return yield* Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions: [] })) + const info = provider.models[modelID] + if (!info) return yield* Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions: [] })) + return info + }), }) const withModel = testEffect(Layer.mergeAll(layer(), providerMock)) @@ -957,9 +1000,9 @@ describe("tool.task", () => { }) describe("tool.task model override", () => { - // Behavior 1 — baseline guard (expected GREEN): omitting `model` preserves the - // current inheritance. The captured ops.prompt input must use the parent/seed - // model and keep the parent variant (the "general" agent has no model of its own). + // Behavior 1 — omitting `model` preserves inheritance. The captured ops.prompt input + // uses the parent/seed model and keeps the parent variant (the "general" agent has no + // model of its own). withModel.instance("omitting model preserves inherited parent model and variant", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() @@ -991,10 +1034,9 @@ describe("tool.task model override", () => { }), ) - // Behavior 2 — valid override (expected RED until implemented): a valid - // "providerID/modelID" overrides both agent.model and parent inheritance. The - // captured model must equal Provider.parseModel(params.model), and variant must - // be undefined (treated like an explicit agent-model override). + // Behavior 2 — a valid "providerID/modelID" overrides both agent.model and parent + // inheritance. The captured model equals Provider.parseModel(params.model), and variant + // is undefined (treated like an explicit agent-model override). withModel.instance("valid model param overrides agent and parent inheritance", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() @@ -1028,10 +1070,9 @@ describe("tool.task model override", () => { }), ) - // Behavior 3 — unknown model (expected RED until implemented): a well-formed - // "providerID/modelID" that Provider.getModel rejects must fail the tool with a - // clear, actionable error mentioning the offending model, and ops.prompt must - // never be called. + // Behavior 3 — a well-formed "providerID/modelID" that Provider.getModel rejects fails + // the tool with a clear, actionable error mentioning the offending model; ops.prompt is + // never called. withModel.instance("unknown model param fails with an actionable error", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() @@ -1062,14 +1103,14 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("Invalid task model") expect(rendered).toContain("bogus/does-not-exist") expect(prompted).toBe(false) }), ) - // Behavior 4 — malformed model (expected RED until implemented): a string with no - // "/" must fail with a clear error mentioning the bad value — no silent fallthrough - // to the inherited model. + // Behavior 4 — a string with no "/" fails with a clear error mentioning the bad value — + // no silent fallthrough to the inherited model. withModel.instance("malformed model param without a slash fails clearly", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() @@ -1100,8 +1141,254 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("Invalid task model") expect(rendered).toContain("not-a-valid-model") expect(prompted).toBe(false) }), ) + + // Behavior 5 (M1) — RED until the gate becomes `params.model !== undefined`. An empty + // string is currently falsy, so the impl silently inherits and reaches ops.prompt. The + // spec is the opposite: an empty model string is PRESENT, so it must validate and fail, + // and ops.prompt must never be called. + withModel.instance("empty-string model param fails instead of silently inheriting", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + expect(prompted).toBe(false) + }), + ) + + // Behavior 6 (M3) — RED until prototype keys are rejected. The provider mock resolves + // models with the same unguarded record lookup the real Provider.getModel uses, so + // "anthropic/__proto__" currently resolves Object.prototype as a "found" model and + // BYPASSES validation, reaching ops.prompt. The spec is that a prototype-polluting key + // fails with a clear error naming the offending value, and ops.prompt is never called. + withModel.instance("prototype-key model param (anthropic/__proto__) fails clearly", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "anthropic/__proto__", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("anthropic/__proto__") + expect(prompted).toBe(false) + }), + ) + + // Behavior 6 (M3) — RED until prototype keys are rejected. With "__proto__" as the + // providerID the real record lookup resolves Object.prototype as the provider, then + // reads `provider.models` (undefined) and indexes it, surfacing a raw + // "Cannot read properties of undefined" TypeError instead of an actionable error. The + // spec is a clear error naming the offending value, with ops.prompt never called. + withModel.instance("prototype-key model param (__proto__/x) fails with an actionable error", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "__proto__/x", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("__proto__/x") + expect(rendered).not.toContain("Cannot read properties of undefined") + expect(prompted).toBe(false) + }), + ) + + // Behavior 7 (M6) — a VALID model whose modelID itself contains slashes resolves and is + // passed through to ops.prompt verbatim. parseModel keeps only the FIRST segment as the + // providerID and joins the rest as the modelID, so "openrouter/anthropic/claude-3.5" + // must reach ops.prompt as { providerID: "openrouter", modelID: "anthropic/claude-3.5" }. + withModel.instance("valid multi-slash modelID resolves and is passed through verbatim", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let seen: SessionPrompt.PromptInput | undefined + const promptOps = stubOps({ onPrompt: (input) => (seen = input) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "openrouter/anthropic/claude-3.5", + } + + yield* def.execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + + expect(seen?.model).toEqual(multiSlashRef) + expect(seen?.variant).toBeUndefined() + }), + ) + + // Behavior 2 (M6) — the explicit override beats the subagent's OWN configured model, not + // merely parent inheritance. The "scoped" subagent is configured with its own model + // (anthropic/claude-sonnet-4); calling Task with a DIFFERENT valid model + // (openrouter/anthropic/claude-3.5) must send the explicit override — not the agent's own + // model and not the inherited parent model — to ops.prompt. + withModel.instance( + "explicit model param beats the subagent's own configured model", + () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let seen: SessionPrompt.PromptInput | undefined + const promptOps = stubOps({ onPrompt: (input) => (seen = input) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "scoped", + model: "openrouter/anthropic/claude-3.5", + } + + yield* def.execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + + // The explicit override wins over the agent's own model (overrideRef) and over the + // inherited parent model (ref). + expect(seen?.model).toEqual(multiSlashRef) + expect(seen?.model).not.toEqual(overrideRef) + expect(seen?.variant).toBeUndefined() + }), + { + config: { + agent: { + scoped: { + mode: "subagent", + model: "anthropic/claude-sonnet-4", + }, + }, + }, + }, + ) + + // Behavior 3/5 (M5) — RED until model validation moves BEFORE child-session creation. + // The impl currently creates the child session (task.ts:155-171) before validating the + // override (task.ts:184-203), so an invalid model leaves an orphan child session behind. + // The spec is that an invalid model short-circuits before any child session is created. + withModel.instance("invalid model param creates no child session (validates first)", () => + Effect.gen(function* () { + const sessions = yield* Session.Service + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "bogus/does-not-exist", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + expect(prompted).toBe(false) + // Observe the side effect directly: a failed validation must not spawn a child. + const kids = yield* sessions.children(chat.id) + expect(kids).toHaveLength(0) + }), + ) }) From df87770ab40de9352bc77990bdb0e01b3846ddf1 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 06:53:47 +0000 Subject: [PATCH 05/11] fix(tui): align provider/model partial-miss to per-field fallback When the provider resolves but the model is unknown, render the provider display name with the raw model id (provider/model), matching the shared UI helper instead of falling back to raw IDs for both. Full miss still yields raw providerID/modelID. Adds direct Model.name tests for the retained helper. Co-authored-by: yui-soul --- packages/tui/src/util/model.ts | 5 ++- .../test/util/provider-model-label.test.ts | 37 +++++++++++++++---- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/packages/tui/src/util/model.ts b/packages/tui/src/util/model.ts index 3209b2ed2482..e8d3313e90d3 100644 --- a/packages/tui/src/util/model.ts +++ b/packages/tui/src/util/model.ts @@ -39,6 +39,7 @@ export function providerModel( ? list.find((item) => item.id === providerID) : undefined const model = provider?.models[modelID] - if (provider && model) return `${provider.name}/${model.name}` - return `${providerID}/${modelID}` + const providerLabel = provider ? provider.name : providerID + const modelLabel = model ? model.name : modelID + return `${providerLabel}/${modelLabel}` } diff --git a/packages/tui/test/util/provider-model-label.test.ts b/packages/tui/test/util/provider-model-label.test.ts index 9d64c375439a..0983108c43d1 100644 --- a/packages/tui/test/util/provider-model-label.test.ts +++ b/packages/tui/test/util/provider-model-label.test.ts @@ -9,15 +9,21 @@ * @behavior * - Catalog hit (provider + model both resolve): returns "/" * e.g. ("anthropic","claude-sonnet-4-20250514") -> "Anthropic/Claude Sonnet 4" - * - Catalog miss (provider OR model not found): falls back to "/" + * - PARTIAL miss (provider resolves, model does NOT): PER-FIELD fallback now + * ALIGNED to the UI helper -> "/" + * e.g. ("anthropic","claude-unknown") -> "Anthropic/claude-unknown" + * - FULL miss (provider not found, name unavailable): raw fallback "/" * e.g. ("openai","gpt-5") -> "openai/gpt-5" * - Format rule: single forward slash, NO surrounding spaces ("a/b", never "a / b"). * @edge-cases - * - Provider resolves but model does not -> full raw-ID fallback "/". + * - Provider resolves but model does not -> per-field "/" + * (provider DISPLAY NAME + raw model id). Matches UI formatModelLabel. * - undefined catalog -> "/". * @invariants - * - Model.name(...) MUST remain unchanged (other callers depend on model-name-only output). - * This file does not test Model.name; existing model/transcript tests guard it. + * - Model.name(...) is RETAINED intentionally (export kept even though tui src + * callers now use Model.providerModel). It is directly guarded by the + * "util.model.name" describe block below: catalog hit -> model display name, + * miss -> raw modelID. * @call-sites-to-switch (Kou — implementation step, NOT covered here) * - packages/tui/src/routes/session/index.tsx:1473 (model memo: Model.name -> Model.providerModel) * rendered at index.tsx:1557. SolidJS route component is not unit-tested here; the @@ -27,10 +33,13 @@ * @see packages/tui/src/util/transcript.ts */ import { describe, expect, test } from "bun:test" -import { providerModel } from "../../src/util/model" +import { name, providerModel } from "../../src/util/model" import { formatAssistantHeader } from "../../src/util/transcript" import type { AssistantMessage, Provider } from "@opencode-ai/sdk/v2" +// NOTE (deferred cleanup): this fully-typed provider fixture is duplicated in +// test/util/transcript.test.ts. If a third consumer appears, extract a shared +// test fixture; not worth extracting for two call sites today. const providers: Provider[] = [ { id: "anthropic", @@ -77,8 +86,8 @@ describe("util.model.providerModel", () => { expect(providerModel(providers, "openai", "gpt-5")).toBe("openai/gpt-5") }) - test("partial miss (known provider, unknown model): falls back to providerID/modelID", () => { - expect(providerModel(providers, "anthropic", "claude-unknown")).toBe("anthropic/claude-unknown") + test("partial miss (known provider, unknown model): per-field fallback providerName/modelID", () => { + expect(providerModel(providers, "anthropic", "claude-unknown")).toBe("Anthropic/claude-unknown") }) test("undefined catalog: falls back to providerID/modelID", () => { @@ -92,6 +101,20 @@ describe("util.model.providerModel", () => { }) }) +describe("util.model.name", () => { + test("catalog hit: returns the model display name", () => { + expect(name(providers, "anthropic", "claude-sonnet-4-20250514")).toBe("Claude Sonnet 4") + }) + + test("catalog miss (unknown model): falls back to raw modelID", () => { + expect(name(providers, "anthropic", "claude-unknown")).toBe("claude-unknown") + }) + + test("catalog miss (unknown provider): falls back to raw modelID", () => { + expect(name(providers, "openai", "gpt-5")).toBe("gpt-5") + }) +}) + describe("transcript assistant header shows provider/model", () => { const baseMsg: AssistantMessage = { id: "msg_123", From 968dcd088f457695e58c2920a216ed0d914ed19c Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 06:53:47 +0000 Subject: [PATCH 06/11] test(ui): cover provider/model partial-miss and no-model guard Add tests for the partial-miss branch (provider known, model unknown) and both branches of the no-model guard, and replace the brittle double-cast fixture with a fully-typed one. Co-authored-by: yui-soul --- .../src/components/message-part-model.test.ts | 47 ++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/packages/ui/src/components/message-part-model.test.ts b/packages/ui/src/components/message-part-model.test.ts index da1c69b1440a..b4271f9b7d35 100644 --- a/packages/ui/src/components/message-part-model.test.ts +++ b/packages/ui/src/components/message-part-model.test.ts @@ -25,14 +25,19 @@ * @no-model-guard * - When providerID OR modelID is absent (user message with no model), return * "" — never render a stray "/". Preserves the existing user-memo guard at - * message-part.tsx:1072 (`if (!providerID || !modelID) return ""`). + * message-part.tsx:1072 (`if (!providerID || !modelID) return ""`). The guard + * is an OR: EITHER field absent → "". Both OR-branches are tested so a stray + * `||`→`&&` refactor cannot slip through. * * @edge-cases - * - Provider resolves but the specific model is absent in its catalog → - * `${provider.name}/${modelID}` (provider name + raw model id). + * - PARTIAL MISS — provider resolves but the specific model is absent in its + * catalog → `${provider.name}/${modelID}` (provider DISPLAY NAME + raw model + * id). This is the per-field branch where UI historically diverged from TUI; + * the TUI `Model.providerModel` is now ALIGNED to this same per-field result. * * @see ./message-part.tsx (lines 1478-1483 assistant memo, 1069-1088 user memo) * @see ./message-part-text.ts (sibling pure-helper module pattern to mirror) + * @see packages/tui/test/util/provider-model-label.test.ts (aligned TUI per-field semantics) */ import { describe, expect, test } from "bun:test" import type { Provider } from "@opencode-ai/sdk/v2" @@ -47,9 +52,31 @@ function provider(part: Partial = {}): Provider { options: {}, models: { "claude-sonnet-4-20250514": { + id: "claude-sonnet-4-20250514", + providerID: "anthropic", + api: { + id: "claude-sonnet-4-20250514", + url: "https://example.com/claude-sonnet-4-20250514", + npm: "@ai-sdk/anthropic", + }, name: "Claude Sonnet", + capabilities: { + temperature: true, + reasoning: true, + attachment: true, + toolcall: true, + input: { text: true, audio: false, image: true, video: false, pdf: true }, + output: { text: true, audio: false, image: false, video: false, pdf: false }, + interleaved: false, + }, + cost: { input: 0, output: 0, cache: { read: 0, write: 0 } }, + limit: { context: 200_000, output: 8_192 }, + status: "active", + options: {}, + headers: {}, + release_date: "2025-05-14", }, - } as unknown as Provider["models"], + } satisfies Provider["models"], ...part, } } @@ -65,8 +92,16 @@ describe("formatModelLabel", () => { ) }) - test("user message with model renders provider/model from the catalog", () => { - expect(formatModelLabel("anthropic", "claude-sonnet-4-20250514", provider())).toBe("Anthropic/Claude Sonnet") + test("partial miss (known provider, unknown model) renders provider display name + raw model id", () => { + expect(formatModelLabel("anthropic", "unknown-model", provider())).toBe("Anthropic/unknown-model") + }) + + test("no-model guard returns empty string when modelID is absent (provider known)", () => { + expect(formatModelLabel("anthropic", undefined, provider())).toBe("") + }) + + test("no-model guard returns empty string when providerID is absent", () => { + expect(formatModelLabel(undefined, "claude-sonnet-4-20250514", provider())).toBe("") }) test("user message without model renders empty string and never a stray slash", () => { From 9e79d90e0315960fa02ca753e5186aa493a31393 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 07:29:05 +0000 Subject: [PATCH 07/11] fix(opencode): reject inherited prototype keys in Task model param Replace the fixed denylist with a prototype-membership check (segment in {}) so inherited Object.prototype member names such as toString and valueOf cannot pass as a modelID against a valid provider and spawn an orphan subagent session. Adds regression tests and strengthens the empty-string assertion. Co-authored-by: yui-soul --- packages/opencode/src/tool/task.ts | 6 +- packages/opencode/test/tool/task.test.ts | 103 +++++++++++++++++------ 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index d8173be81ddd..e667a4d414f2 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -147,11 +147,7 @@ export const TaskTool = Tool.define( // lookup. An unguarded record index on these keys would otherwise resolve // Object.prototype as a "found" model, or throw a raw TypeError that leaks // "Cannot read properties of undefined" instead of an actionable error. - const unsafe = (segment: string) => - segment === "" || - segment === "__proto__" || - segment === "constructor" || - segment === "prototype" + const unsafe = (segment: string) => segment === "" || segment in {} if (unsafe(parsed.providerID) || unsafe(parsed.modelID)) return yield* Effect.fail( new Error( diff --git a/packages/opencode/test/tool/task.test.ts b/packages/opencode/test/tool/task.test.ts index 39224222b0eb..fb11372732e5 100644 --- a/packages/opencode/test/tool/task.test.ts +++ b/packages/opencode/test/tool/task.test.ts @@ -24,10 +24,15 @@ * 5. model = "" (empty string) → FAIL with a clear error. Empty is NOT treated as * "omitted": the gate is `params.model !== undefined`, not a truthiness check, so an * empty string never silently inherits. ops.prompt is never called. - * 6. model = prototype-polluting key, e.g. "anthropic/__proto__" or "__proto__/x" → - * FAIL with a clear, actionable error naming the offending value. Must NOT bypass - * validation (treating Object.prototype as a "found" model) and must NOT surface a - * raw "Cannot read properties of undefined" TypeError. ops.prompt is never called. + * 6. model = prototype-polluting key, e.g. "anthropic/__proto__" or "__proto__/x", OR an + * inherited Object.prototype method name as the modelID, e.g. "anthropic/toString" or + * "anthropic/valueOf" (valid provider + a method reachable only via the prototype + * chain) → FAIL with a clear, actionable error naming the offending value. Must NOT + * bypass validation (treating an inherited Object.prototype member as a "found" model) + * and must NOT surface a raw "Cannot read properties of undefined" TypeError. + * ops.prompt is never called and NO child session is created. A denylist of specific + * keys is insufficient — validation must use a prototype-membership check (own-property + * lookup) so inherited members like toString/valueOf are rejected too. * 7. model = VALID multi-slash modelID, e.g. "openrouter/anthropic/claude-3.5" → * providerID is the FIRST segment ("openrouter"); modelID is the remainder joined * back with "/" ("anthropic/claude-3.5"). Resolves and is passed through to ops.prompt @@ -1045,8 +1050,7 @@ describe("tool.task model override", () => { let seen: SessionPrompt.PromptInput | undefined const promptOps = stubOps({ onPrompt: (input) => (seen = input) }) - // Stored in a const so the extra `model` key is accepted by width subtyping - // until task.ts adds it to the schema (RED phase). + // Stored in a const so the extra `model` key is accepted by width subtyping. const params = { description: "inspect bug", prompt: "look into the cache key path", @@ -1147,10 +1151,9 @@ describe("tool.task model override", () => { }), ) - // Behavior 5 (M1) — RED until the gate becomes `params.model !== undefined`. An empty - // string is currently falsy, so the impl silently inherits and reaches ops.prompt. The - // spec is the opposite: an empty model string is PRESENT, so it must validate and fail, - // and ops.prompt must never be called. + // Behavior 5 (M1) — the gate is `params.model !== undefined`, so an empty string counts + // as PRESENT: it validates and fails rather than silently inheriting, and ops.prompt is + // never called. withModel.instance("empty-string model param fails instead of silently inheriting", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() @@ -1180,15 +1183,17 @@ describe("tool.task model override", () => { .pipe(Effect.exit) expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("Invalid task model") expect(prompted).toBe(false) }), ) - // Behavior 6 (M3) — RED until prototype keys are rejected. The provider mock resolves - // models with the same unguarded record lookup the real Provider.getModel uses, so - // "anthropic/__proto__" currently resolves Object.prototype as a "found" model and - // BYPASSES validation, reaching ops.prompt. The spec is that a prototype-polluting key - // fails with a clear error naming the offending value, and ops.prompt is never called. + // Behavior 6 (M3) — prototype-polluting keys are rejected. The provider mock resolves + // models with the same unguarded record lookup the real Provider.getModel uses, so a + // naive lookup would resolve Object.prototype as a "found" model for "anthropic/__proto__". + // Validation rejects it with a clear error naming the offending value, and ops.prompt is + // never called. withModel.instance("prototype-key model param (anthropic/__proto__) fails clearly", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() @@ -1224,11 +1229,11 @@ describe("tool.task model override", () => { }), ) - // Behavior 6 (M3) — RED until prototype keys are rejected. With "__proto__" as the - // providerID the real record lookup resolves Object.prototype as the provider, then - // reads `provider.models` (undefined) and indexes it, surfacing a raw - // "Cannot read properties of undefined" TypeError instead of an actionable error. The - // spec is a clear error naming the offending value, with ops.prompt never called. + // Behavior 6 (M3) — prototype-polluting keys are rejected. With "__proto__" as the + // providerID a naive record lookup would resolve Object.prototype as the provider, then + // read `provider.models` (undefined) and index it, surfacing a raw + // "Cannot read properties of undefined" TypeError. Validation instead fails with a clear + // error naming the offending value, and ops.prompt is never called. withModel.instance("prototype-key model param (__proto__/x) fails with an actionable error", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() @@ -1351,10 +1356,9 @@ describe("tool.task model override", () => { }, ) - // Behavior 3/5 (M5) — RED until model validation moves BEFORE child-session creation. - // The impl currently creates the child session (task.ts:155-171) before validating the - // override (task.ts:184-203), so an invalid model leaves an orphan child session behind. - // The spec is that an invalid model short-circuits before any child session is created. + // Behavior 3/5 (M5) — model validation runs BEFORE child-session creation, so an invalid + // model short-circuits before any child session is created and never leaves an orphan + // child session behind. withModel.instance("invalid model param creates no child session (validates first)", () => Effect.gen(function* () { const sessions = yield* Session.Service @@ -1391,4 +1395,55 @@ describe("tool.task model override", () => { expect(kids).toHaveLength(0) }), ) + + // Behavior 6 (F1) — inherited Object.prototype method names as the modelID must be + // rejected. "anthropic" is a known provider, but "toString"/"valueOf" are NOT real models: + // an unguarded `provider.models[modelID]` lookup resolves the inherited Object.prototype + // method (a truthy function) and a denylist of only ""/__proto__/constructor/prototype + // would treat it as a "found" model, BYPASSING validation. The spec requires a + // prototype-membership check (e.g. Object.hasOwn) so these fail with the actionable + // "Invalid task model" error, ops.prompt is never called, and no child session is created. + for (const inherited of ["toString", "valueOf"]) { + withModel.instance( + `inherited-prototype-method modelID (anthropic/${inherited}) fails and creates no child session`, + () => + Effect.gen(function* () { + const sessions = yield* Session.Service + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: `anthropic/${inherited}`, + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("Invalid task model") + expect(rendered).toContain(`anthropic/${inherited}`) + expect(prompted).toBe(false) + // A bypassed validation would spawn an orphan child — assert zero directly. + const kids = yield* sessions.children(chat.id) + expect(kids).toHaveLength(0) + }), + ) + } }) From 6e9302b7435a950dd2006fd2f2d94b03acfb2a93 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 07:32:22 +0000 Subject: [PATCH 08/11] docs: document optional model param on the Task tool Note that the invoking agent can override the subagent model per call via the Task tool's optional model parameter (provider/model form), and that an invalid value fails with an Invalid task model error. Co-authored-by: yui-soul --- packages/web/src/content/docs/agents.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web/src/content/docs/agents.mdx b/packages/web/src/content/docs/agents.mdx index 53048b7927b9..ef09e0da31c6 100644 --- a/packages/web/src/content/docs/agents.mdx +++ b/packages/web/src/content/docs/agents.mdx @@ -353,7 +353,7 @@ This path is relative to where the config file is located. So this works for bot Use the `model` config to override the model for this agent. Useful for using different models optimized for different tasks. For example, a faster model for planning, a more capable model for implementation. :::tip -If you don’t specify a model, primary agents use the [model globally configured](/docs/config#models) while subagents will use the model of the primary agent that invoked the subagent. +If you don’t specify a model, primary agents use the [model globally configured](/docs/config#models) while subagents will use the model of the primary agent that invoked the subagent. The invoking agent can override this per call by passing an optional `model` parameter, in `provider/model` form (e.g. `anthropic/claude-sonnet-4`), to the Task tool; an unknown or malformed value fails with an `Invalid task model` error. ::: ```json title="opencode.json" From 92d31965104a7812e5649ac150c04ce8aca601a4 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 10:28:52 +0000 Subject: [PATCH 09/11] feat(opencode): differentiate Task model error messages Split the single generic Task model error into format vs availability: - malformed format (no slash, empty segment, prototype-chain key) keeps a "provider/model format" explanation - a well-formed but unknown provider reports "provider ... is not configured" - a known provider with an unknown model reports "model ... is not available for provider ..." Availability errors surface getModel suggestions (Did you mean: ...), discriminated by the typed ModelNotFoundError tag and Provider.list() membership rather than message text. Co-authored-by: yui-soul --- packages/opencode/src/tool/task.ts | 63 +++-- packages/opencode/test/tool/task.test.ts | 288 +++++++++++++++++++---- 2 files changed, 285 insertions(+), 66 deletions(-) diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index e667a4d414f2..92acd8e3d263 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -143,35 +143,46 @@ export const TaskTool = Tool.define( new Error(`Cannot resolve task model "${requested}": the provider service is unavailable.`), ) const parsed = Provider.parseModel(requested) - // Reject empty segments and prototype-polluting keys BEFORE any provider - // lookup. An unguarded record index on these keys would otherwise resolve - // Object.prototype as a "found" model, or throw a raw TypeError that leaks - // "Cannot read properties of undefined" instead of an actionable error. - const unsafe = (segment: string) => segment === "" || segment in {} - if (unsafe(parsed.providerID) || unsafe(parsed.modelID)) - return yield* Effect.fail( - new Error( - `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, - ), + // FORMAT guard (case A): reject empty segments and prototype-polluting keys + // BEFORE any provider lookup. An unguarded record index on these keys would + // otherwise resolve Object.prototype as a "found" model, or throw a raw + // TypeError that leaks "Cannot read properties of undefined" instead of an + // actionable error. `segment in {}` rejects inherited members (toString, + // valueOf, __proto__) too — a fixed denylist is insufficient. + const formatError = () => + new Error( + `Invalid task model "${requested}": expected "provider/model" format, e.g. "anthropic/claude-sonnet-4".`, ) + const unsafe = (segment: string) => segment === "" || segment in {} + if (unsafe(parsed.providerID) || unsafe(parsed.modelID)) return yield* Effect.fail(formatError()) yield* resolver.getModel(parsed.providerID, parsed.modelID).pipe( - Effect.mapError( - () => - new Error( - `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, - ), - ), - // Defensive: the local guard above should make this unreachable, but a - // raw DEFECT (e.g. a TypeError from unguarded record indexing) is NOT - // caught by mapError. Convert any defect to the same actionable error so - // no raw "Cannot read properties of undefined" can leak. - Effect.catchDefect(() => - Effect.fail( - new Error( - `Invalid task model "${requested}": expected "providerID/modelID" naming a known provider and model.`, - ), - ), + // UNAVAILABLE (cases B/C): the well-formed string named a provider/model + // the resolver could not find. Discriminate by list() membership of the + // parsed providerID — ABSENT → case B (provider not configured), PRESENT + // → case C (model missing for a known provider) — never by message text. + Effect.catchTag("ProviderModelNotFoundError", (error) => + Effect.gen(function* () { + const configured = yield* resolver.list() + const suffix = + error.suggestions && error.suggestions.length + ? ` Did you mean: ${error.suggestions.join(", ")}?` + : "" + if (Object.hasOwn(configured, parsed.providerID)) + return yield* Effect.fail( + new Error( + `Model unavailable: model "${parsed.modelID}" is not available for provider "${parsed.providerID}".${suffix}`, + ), + ) + return yield* Effect.fail( + new Error(`Model unavailable: provider "${parsed.providerID}" is not configured.${suffix}`), + ) + }), ), + // Defensive: the FORMAT guard above should make this unreachable, but a raw + // DEFECT (e.g. a TypeError from unguarded record indexing) is NOT caught by + // catchTag. Convert any defect to the FORMAT error so no raw "Cannot read + // properties of undefined" can leak. + Effect.catchDefect(() => Effect.fail(formatError())), ) return parsed }) diff --git a/packages/opencode/test/tool/task.test.ts b/packages/opencode/test/tool/task.test.ts index fb11372732e5..322bd39f459e 100644 --- a/packages/opencode/test/tool/task.test.ts +++ b/packages/opencode/test/tool/task.test.ts @@ -15,24 +15,54 @@ * The `model` passed to ops.prompt equals the parsed { providerID, modelID }, and * `variant` becomes undefined (treated like an explicit agent-model override). The * override beats the subagent's OWN configured model, not just parent inheritance. - * 3. model = unknown provider OR unknown model → Provider.getModel raises - * ModelNotFoundError; the tool FAILS with a clear, actionable error whose message - * contains the offending "providerID/modelID". ops.prompt is never called and NO + * --- DIFFERENTIATED failure messages (replaces the single generic "Invalid task model + * …: expected \"providerID/modelID\" naming a known provider and model." string). A + * failed override surfaces exactly ONE of three messages, chosen by the impl from the + * FAILURE SHAPE, never by matching on message substrings: + * + * (A) FORMAT — malformed input caught by the LOCAL format guard BEFORE any provider + * lookup: no-slash (parseModel yields an empty modelID), empty string, empty + * segment ("anthropic/"), or a prototype-chain key ("anthropic/__proto__", + * "__proto__/x", "anthropic/toString", "anthropic/valueOf"). Message: + * Invalid task model "": expected "provider/model" format, e.g. "anthropic/claude-sonnet-4". + * Contains "format"; must NOT read as "unavailable"; must NOT leak a raw + * "Cannot read properties of undefined" TypeError. The guard uses a + * prototype-membership check (`segment in {}`), so inherited members like + * toString/valueOf are rejected too — a fixed denylist is insufficient. + * + * (B) UNAVAILABLE provider — a WELL-FORMED string whose providerID is NOT among the + * configured providers (Provider.list() membership). getModel raises + * ModelNotFoundError; the impl confirms the providerID is ABSENT from list() and + * emits: + * Model unavailable: provider "" is not configured. + * plus, when the error carries suggestions, ` Did you mean: ?`. + * Contains "is not configured" + the provider name + suggestion names; NOT "format". + * + * (C) UNAVAILABLE model — a WELL-FORMED string whose providerID IS configured + * (PRESENT in Provider.list()) but whose modelID is missing. getModel raises + * ModelNotFoundError; the impl confirms the providerID is PRESENT in list() and + * emits: + * Model unavailable: model "" is not available for provider "". + * plus the same ` Did you mean: ?` suggestions clause. Contains + * "is not available for provider" + the model + provider names + suggestion names. + * + * Discrimination is by the ModelNotFoundError TAG ("ProviderModelNotFoundError") plus + * Provider.list() membership on the impl side — NOT by message substrings. Tests + * assert only the user-facing message TEXT (the contract the user sees). + * + * 3. model = unknown provider (well-formed) → case (B). ops.prompt is never called and NO * child subagent session is created (validation short-circuits before spawning). - * 4. model = malformed string with no "/" → FAIL with a clear error mentioning the bad - * string. No silent fallthrough to the inherited model. - * 5. model = "" (empty string) → FAIL with a clear error. Empty is NOT treated as - * "omitted": the gate is `params.model !== undefined`, not a truthiness check, so an - * empty string never silently inherits. ops.prompt is never called. - * 6. model = prototype-polluting key, e.g. "anthropic/__proto__" or "__proto__/x", OR an - * inherited Object.prototype method name as the modelID, e.g. "anthropic/toString" or - * "anthropic/valueOf" (valid provider + a method reachable only via the prototype - * chain) → FAIL with a clear, actionable error naming the offending value. Must NOT - * bypass validation (treating an inherited Object.prototype member as a "found" model) - * and must NOT surface a raw "Cannot read properties of undefined" TypeError. - * ops.prompt is never called and NO child session is created. A denylist of specific - * keys is insufficient — validation must use a prototype-membership check (own-property - * lookup) so inherited members like toString/valueOf are rejected too. + * 4. model = unknown model in a KNOWN provider (well-formed) → case (C). ops.prompt is + * never called and NO child session is created. + * 5. model = malformed string with no "/", "" (empty string), or empty segment → case (A). + * Empty is NOT treated as "omitted": the gate is `params.model !== undefined`, not a + * truthiness check, so an empty string never silently inherits. ops.prompt is never + * called. + * 6. model = prototype-polluting key ("anthropic/__proto__", "__proto__/x") OR an inherited + * Object.prototype method name as the modelID ("anthropic/toString", "anthropic/valueOf") + * → case (A). Must NOT bypass validation (treating an inherited Object.prototype member + * as a "found" model) and must NOT surface a raw "Cannot read properties of undefined" + * TypeError. ops.prompt is never called and NO child session is created. * 7. model = VALID multi-slash modelID, e.g. "openrouter/anthropic/claude-3.5" → * providerID is the FIRST segment ("openrouter"); modelID is the remainder joined * back with "/" ("anthropic/claude-3.5"). Resolves and is passed through to ops.prompt @@ -45,8 +75,14 @@ * and BEFORE ops.prompt — a failed validation must short-circuit so neither happens. * - The error message must surface the user-supplied model string for actionability, * including for prototype-polluting keys (no raw TypeError leakage). + * - Case (B) vs (C) is decided by Provider.list() membership of the parsed providerID, NOT + * by inspecting the ModelNotFoundError fields or message text. The mock harness therefore + * EXCLUDES the case-(B) providerID from list() and INCLUDES the case-(C) providerID. + * - ModelNotFoundError raised for cases (B)/(C) must carry a NON-EMPTY `suggestions` array + * (real strings) so the ` Did you mean: …?` clause is exercised — an empty array is an + * assertion vacuum. * - * @helpers Provider.parseModel, Provider.getModel, Provider.ModelNotFoundError + * @helpers Provider.parseModel, Provider.getModel, Provider.list, Provider.ModelNotFoundError * @see src/provider/provider.ts (getModel record lookup: provider.ts:1747-1768; * parseModel split-on-"/": provider.ts:1944) */ @@ -141,17 +177,51 @@ const knownModels: Record }> = }, } -// Mirror Provider.getModel (provider.ts:1747-1768): a missing provider OR a missing -// model raises ModelNotFoundError; whatever the records resolve is treated as a found -// model — including anything reachable via the prototype chain. The record lookups are -// deliberately unguarded so prototype-polluting keys behave as they do in production. +// Real, NON-EMPTY suggestion strings the mock attaches to ModelNotFoundError. The impl +// surfaces these in the " Did you mean: …?" clause; tests assert their presence by name. +// Leaving these empty/undefined would make the suggestion assertions vacuous. +const providerSuggestions = ["anthropic", "openrouter"] +const modelSuggestions = ["claude-sonnet-4", "claude-haiku-4"] + +// What resolver.list() reports as CONFIGURED. The impl discriminates UNAVAILABLE case B +// (provider ABSENT here → "provider … is not configured") from case C (provider PRESENT +// here but the model missing → "model … is not available for provider …") by list() +// MEMBERSHIP — not by message substrings. So "anthropic"/"openrouter" are present (case C +// source: e.g. "anthropic/ghost-model") and "bogus" is absent (case B source). +const configuredProviders: Record = { + [overrideRef.providerID]: ProviderTest.info( + { id: overrideRef.providerID }, + knownModels.anthropic.models[overrideRef.modelID], + ), + [multiSlashRef.providerID]: ProviderTest.info( + { id: multiSlashRef.providerID }, + knownModels.openrouter.models[multiSlashRef.modelID], + ), +} + +// Mirror Provider.getModel (provider.ts:1747-1768): a missing provider OR a missing model +// raises a REAL ModelNotFoundError (tag "ProviderModelNotFoundError") carrying a NON-EMPTY +// suggestions array; whatever the records resolve is treated as a found model — including +// anything reachable via the prototype chain. The record lookups are deliberately unguarded +// so prototype-polluting keys behave as they do in production (they never reach here anyway: +// the impl's local format guard rejects them first — case A). `list()` returns the configured +// providers so the impl can tell case B (absent) from case C (present) by membership. const providerMock = Layer.mock(Provider.Service)({ + list: () => Effect.succeed(configuredProviders), getModel: (providerID, modelID) => Effect.gen(function* () { const provider = knownModels[providerID] - if (!provider) return yield* Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions: [] })) + // Case B source: provider not in the catalog → suggest known provider names. + if (!provider) + return yield* Effect.fail( + new Provider.ModelNotFoundError({ providerID, modelID, suggestions: providerSuggestions }), + ) const info = provider.models[modelID] - if (!info) return yield* Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions: [] })) + // Case C source: provider known, model missing → suggest known model names. + if (!info) + return yield* Effect.fail( + new Provider.ModelNotFoundError({ providerID, modelID, suggestions: modelSuggestions }), + ) return info }), }) @@ -1074,10 +1144,12 @@ describe("tool.task model override", () => { }), ) - // Behavior 3 — a well-formed "providerID/modelID" that Provider.getModel rejects fails - // the tool with a clear, actionable error mentioning the offending model; ops.prompt is - // never called. - withModel.instance("unknown model param fails with an actionable error", () => + // Case B — UNAVAILABLE provider. A well-formed "providerID/modelID" whose providerID is + // NOT among the configured providers (resolver.list()). getModel raises a real + // ModelNotFoundError; the impl, seeing the providerID ABSENT from list(), surfaces the + // "provider … is not configured" message plus a " Did you mean: …?" suggestion clause. + // It is NOT a FORMAT error (the string is well-formed). + withModel.instance("unavailable provider fails with a 'not configured' message and suggestions", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() const tool = yield* TaskTool @@ -1107,15 +1179,63 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" - expect(rendered).toContain("Invalid task model") - expect(rendered).toContain("bogus/does-not-exist") + expect(rendered).toContain("is not configured") + expect(rendered).toContain("bogus") + expect(rendered).toContain("Did you mean") + for (const suggestion of providerSuggestions) expect(rendered).toContain(suggestion) + expect(rendered).not.toContain("format") + expect(prompted).toBe(false) + }), + ) + + // Case C — UNAVAILABLE model in a KNOWN provider. "anthropic" IS configured (present in + // resolver.list()), but "ghost-model" is not one of its models. getModel raises a real + // ModelNotFoundError; the impl, seeing the providerID PRESENT in list(), surfaces the + // "model … is not available for provider …" message plus a " Did you mean: …?" clause. + withModel.instance("unavailable model in a known provider fails with a model-level message and suggestions", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "anthropic/ghost-model", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("is not available for provider") + expect(rendered).toContain("ghost-model") + expect(rendered).toContain("anthropic") + expect(rendered).toContain("Did you mean") + for (const suggestion of modelSuggestions) expect(rendered).toContain(suggestion) + expect(rendered).not.toContain("format") expect(prompted).toBe(false) }), ) - // Behavior 4 — a string with no "/" fails with a clear error mentioning the bad value — - // no silent fallthrough to the inherited model. - withModel.instance("malformed model param without a slash fails clearly", () => + // Case A — FORMAT error. A string with no "/" makes parseModel yield an empty modelID, + // caught by the LOCAL format guard BEFORE any provider lookup. The message names the bad + // value and the expected "provider/model" format; it must NOT read as "unavailable". + withModel.instance("malformed model without a slash fails with a FORMAT error", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() const tool = yield* TaskTool @@ -1145,16 +1265,17 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" - expect(rendered).toContain("Invalid task model") - expect(rendered).toContain("not-a-valid-model") + expect(rendered).toContain('Invalid task model "not-a-valid-model"') + expect(rendered).toContain("format") + expect(rendered).not.toContain("unavailable") expect(prompted).toBe(false) }), ) - // Behavior 5 (M1) — the gate is `params.model !== undefined`, so an empty string counts - // as PRESENT: it validates and fails rather than silently inheriting, and ops.prompt is - // never called. - withModel.instance("empty-string model param fails instead of silently inheriting", () => + // Case A (M1) — the gate is `params.model !== undefined`, so an empty string counts as + // PRESENT: it hits the FORMAT guard (empty segments) and fails rather than silently + // inheriting. ops.prompt is never called. + withModel.instance("empty-string model fails with a FORMAT error instead of inheriting", () => Effect.gen(function* () { const { chat, assistant } = yield* seed() const tool = yield* TaskTool @@ -1184,7 +1305,48 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" - expect(rendered).toContain("Invalid task model") + expect(rendered).toContain("format") + expect(rendered).not.toContain("unavailable") + expect(prompted).toBe(false) + }), + ) + + // Case A — FORMAT error. An empty trailing segment ("anthropic/") leaves modelID empty; + // the format guard rejects it BEFORE any provider lookup, even though "anthropic" is a + // configured provider. This is a FORMAT failure, not an UNAVAILABLE one. + withModel.instance("empty-segment model (anthropic/) fails with a FORMAT error", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "anthropic/", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain('Invalid task model "anthropic/"') + expect(rendered).toContain("format") + expect(rendered).not.toContain("unavailable") expect(prompted).toBe(false) }), ) @@ -1225,6 +1387,8 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" expect(rendered).toContain("anthropic/__proto__") + expect(rendered).toContain("format") + expect(rendered).not.toContain("unavailable") expect(prompted).toBe(false) }), ) @@ -1265,6 +1429,8 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" expect(rendered).toContain("__proto__/x") + expect(rendered).toContain("format") + expect(rendered).not.toContain("unavailable") expect(rendered).not.toContain("Cannot read properties of undefined") expect(prompted).toBe(false) }), @@ -1438,6 +1604,8 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" expect(rendered).toContain("Invalid task model") + expect(rendered).toContain("format") + expect(rendered).not.toContain("unavailable") expect(rendered).toContain(`anthropic/${inherited}`) expect(prompted).toBe(false) // A bypassed validation would spawn an orphan child — assert zero directly. @@ -1446,4 +1614,44 @@ describe("tool.task model override", () => { }), ) } + + // GREEN no-regression — a VALID override still routes to ops.prompt AND creates the child + // subagent session. Differentiating the FAILURE messages (cases A/B/C) must not regress + // the happy path: the parsed { providerID, modelID } reaches ops.prompt and exactly one + // child session is spawned under the parent. + withModel.instance("valid model override routes to ops.prompt and creates the child session", () => + Effect.gen(function* () { + const sessions = yield* Session.Service + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let seen: SessionPrompt.PromptInput | undefined + const promptOps = stubOps({ onPrompt: (input) => (seen = input) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "anthropic/claude-sonnet-4", + } + + const result = yield* def.execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + + // Routed to ops.prompt with the parsed override model. + expect(seen?.model).toEqual(overrideRef) + // And a single child subagent session was created under the parent. + const kids = yield* sessions.children(chat.id) + expect(kids).toHaveLength(1) + expect(kids[0]?.id).toBe(result.metadata.sessionId) + }), + ) }) From 5137d400f2f6a48b584dc89ee609dab303a70f52 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 10:43:25 +0000 Subject: [PATCH 10/11] test(opencode): cover no-suggestions branch and join format Add cases for the empty/omitted suggestions branch (no dangling Did you mean), pin the comma-join separator and order, cover the single-suggestion no-comma path, guard the suggestion-presence loops against vacuum, and assert the empty-string format echo. Co-authored-by: yui-soul --- packages/opencode/test/tool/task.test.ts | 196 +++++++++++++++++++++-- 1 file changed, 183 insertions(+), 13 deletions(-) diff --git a/packages/opencode/test/tool/task.test.ts b/packages/opencode/test/tool/task.test.ts index 322bd39f459e..16655dd20b82 100644 --- a/packages/opencode/test/tool/task.test.ts +++ b/packages/opencode/test/tool/task.test.ts @@ -46,6 +46,15 @@ * plus the same ` Did you mean: ?` suggestions clause. Contains * "is not available for provider" + the model + provider names + suggestion names. * + * SUGGESTIONS CLAUSE (cases B and C) — the ` Did you mean: …?` tail is CONDITIONAL on + * `error.suggestions && error.suggestions.length`: + * • Populated (length ≥ 1) → ` Did you mean: ?` where the names are + * `suggestions.join(", ")` — a ", " separator that preserves the array order. A + * SINGLE-element array therefore renders with NO comma ("Did you mean: anthropic?"). + * • EMPTY array ([]) OR OMITTED (undefined) → the FALSE branch: NO clause is appended; + * the base message stands alone with no "Did you mean" text. Both falsey shapes are + * exercised so the conditional's false arm is not an assertion vacuum. + * * Discrimination is by the ModelNotFoundError TAG ("ProviderModelNotFoundError") plus * Provider.list() membership on the impl side — NOT by message substrings. Tests * assert only the user-facing message TEXT (the contract the user sees). @@ -78,9 +87,12 @@ * - Case (B) vs (C) is decided by Provider.list() membership of the parsed providerID, NOT * by inspecting the ModelNotFoundError fields or message text. The mock harness therefore * EXCLUDES the case-(B) providerID from list() and INCLUDES the case-(C) providerID. - * - ModelNotFoundError raised for cases (B)/(C) must carry a NON-EMPTY `suggestions` array - * (real strings) so the ` Did you mean: …?` clause is exercised — an empty array is an - * assertion vacuum. + * - ModelNotFoundError raised for cases (B)/(C) carries a `suggestions` array whose + * CONTENT is varied per request by the mock so BOTH arms of the conditional clause are + * covered: NON-EMPTY (multi- and single-element) exercises the rendered ` Did you mean: + * …?` tail and its ", " join, while EMPTY ([]) and OMITTED (undefined) exercise the false + * branch where no clause is appended. A uniformly non-empty fixture would leave the false + * branch an assertion vacuum. * * @helpers Provider.parseModel, Provider.getModel, Provider.list, Provider.ModelNotFoundError * @see src/provider/provider.ts (getModel record lookup: provider.ts:1747-1768; @@ -183,6 +195,22 @@ const knownModels: Record }> = const providerSuggestions = ["anthropic", "openrouter"] const modelSuggestions = ["claude-sonnet-4", "claude-haiku-4"] +// Sentinel suggestion fixtures keyed by a request identifier, so ONE mock can drive BOTH +// the populated " Did you mean: …?" clause AND its FALSE branch without per-test rewiring. +// The impl renders the clause only when `suggestions && suggestions.length`, so an empty +// array AND an omitted (undefined) field must BOTH suppress it. A single-element array +// pins the no-comma rendering ("Did you mean: anthropic?"). Keys absent here fall back to +// the default NON-EMPTY suggestions above. +// - case B (provider not configured), keyed by the parsed providerID: +const providerSuggestionsByID: Record = { + "bogus-empty": [], // false branch: empty array → no clause + "bogus-single": ["anthropic"], // single element → clause has no ", " separator +} +// - case C (model missing in a known provider), keyed by the parsed modelID: +const modelSuggestionsByID: Record = { + "ghost-undefined": undefined, // false branch: omitted suggestions → no clause +} + // What resolver.list() reports as CONFIGURED. The impl discriminates UNAVAILABLE case B // (provider ABSENT here → "provider … is not configured") from case C (provider PRESENT // here but the model missing → "model … is not available for provider …") by list() @@ -211,17 +239,25 @@ const providerMock = Layer.mock(Provider.Service)({ getModel: (providerID, modelID) => Effect.gen(function* () { const provider = knownModels[providerID] - // Case B source: provider not in the catalog → suggest known provider names. - if (!provider) - return yield* Effect.fail( - new Provider.ModelNotFoundError({ providerID, modelID, suggestions: providerSuggestions }), - ) + // Case B source: provider not in the catalog → suggest known provider names. The + // suggestions VARY by providerID so a single mock exercises the populated clause, + // the empty-array false branch, and the single-element (no-comma) rendering. + if (!provider) { + const suggestions = Object.hasOwn(providerSuggestionsByID, providerID) + ? providerSuggestionsByID[providerID] + : providerSuggestions + return yield* Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions })) + } const info = provider.models[modelID] - // Case C source: provider known, model missing → suggest known model names. - if (!info) - return yield* Effect.fail( - new Provider.ModelNotFoundError({ providerID, modelID, suggestions: modelSuggestions }), - ) + // Case C source: provider known, model missing → suggest known model names. The + // suggestions VARY by modelID so a single mock exercises the populated clause and + // the omitted-suggestions (undefined) false branch. + if (!info) { + const suggestions = Object.hasOwn(modelSuggestionsByID, modelID) + ? modelSuggestionsByID[modelID] + : modelSuggestions + return yield* Effect.fail(new Provider.ModelNotFoundError({ providerID, modelID, suggestions })) + } return info }), }) @@ -1182,6 +1218,10 @@ describe("tool.task model override", () => { expect(rendered).toContain("is not configured") expect(rendered).toContain("bogus") expect(rendered).toContain("Did you mean") + // Pin the exact join: a ", " separator preserving fixture order (not a bare list). + expect(rendered).toContain("Did you mean: anthropic, openrouter") + // Anti-vacuum guard: a fixture emptied by accident must not let the loop pass silently. + expect(providerSuggestions.length).toBeGreaterThan(0) for (const suggestion of providerSuggestions) expect(rendered).toContain(suggestion) expect(rendered).not.toContain("format") expect(prompted).toBe(false) @@ -1226,12 +1266,141 @@ describe("tool.task model override", () => { expect(rendered).toContain("ghost-model") expect(rendered).toContain("anthropic") expect(rendered).toContain("Did you mean") + // Pin the exact join: a ", " separator preserving fixture order (not a bare list). + expect(rendered).toContain("Did you mean: claude-sonnet-4, claude-haiku-4") + // Anti-vacuum guard: a fixture emptied by accident must not let the loop pass silently. + expect(modelSuggestions.length).toBeGreaterThan(0) for (const suggestion of modelSuggestions) expect(rendered).toContain(suggestion) expect(rendered).not.toContain("format") expect(prompted).toBe(false) }), ) + // Case B, FALSE suggestions branch — `error.suggestions` is an EMPTY array, so the + // `error.suggestions && error.suggestions.length ? …` clause renders nothing. The + // "provider … is not configured" message must stand alone with NO " Did you mean: …?" + // tail. This pins the untested false arm of the conditional suffix. + withModel.instance("unavailable provider with empty suggestions omits the 'Did you mean' clause", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "bogus-empty/does-not-exist", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("is not configured") + expect(rendered).toContain("bogus-empty") + expect(rendered).not.toContain("Did you mean") + expect(rendered).not.toContain("format") + expect(prompted).toBe(false) + }), + ) + + // Case C, FALSE suggestions branch — `error.suggestions` is OMITTED (undefined), so the + // `error.suggestions && …` clause short-circuits and renders nothing. The "model … is not + // available for provider …" message must stand alone with NO " Did you mean: …?" tail. + withModel.instance("unavailable model with omitted suggestions omits the 'Did you mean' clause", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "anthropic/ghost-undefined", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("is not available for provider") + expect(rendered).toContain("ghost-undefined") + expect(rendered).toContain("anthropic") + expect(rendered).not.toContain("Did you mean") + expect(rendered).not.toContain("format") + expect(prompted).toBe(false) + }), + ) + + // Suggestions JOIN format, single element — a ONE-element suggestions array renders with + // NO ", " separator: "Did you mean: anthropic?" exactly. This complements the multi-element + // B/C tests (which pin the ", " separator) by pinning the degenerate single-suggestion case. + withModel.instance("single suggestion renders without a comma separator", () => + Effect.gen(function* () { + const { chat, assistant } = yield* seed() + const tool = yield* TaskTool + const def = yield* tool.init() + let prompted = false + const promptOps = stubOps({ onPrompt: () => (prompted = true) }) + + const params = { + description: "inspect bug", + prompt: "look into the cache key path", + subagent_type: "general", + model: "bogus-single/does-not-exist", + } + + const exit = yield* def + .execute(params, { + sessionID: chat.id, + messageID: assistant.id, + agent: "build", + abort: new AbortController().signal, + extra: { promptOps }, + messages: [], + metadata: () => Effect.void, + ask: () => Effect.void, + }) + .pipe(Effect.exit) + + expect(Exit.isFailure(exit)).toBe(true) + const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain("is not configured") + // Exactly one suggestion → the clause ends right after the name, with no ", ". + expect(rendered).toContain("Did you mean: anthropic?") + expect(rendered).not.toContain("Did you mean: anthropic,") + expect(prompted).toBe(false) + }), + ) + // Case A — FORMAT error. A string with no "/" makes parseModel yield an empty modelID, // caught by the LOCAL format guard BEFORE any provider lookup. The message names the bad // value and the expected "provider/model" format; it must NOT read as "unavailable". @@ -1305,6 +1474,7 @@ describe("tool.task model override", () => { expect(Exit.isFailure(exit)).toBe(true) const rendered = Exit.isFailure(exit) ? Cause.pretty(exit.cause) : "" + expect(rendered).toContain('Invalid task model ""') expect(rendered).toContain("format") expect(rendered).not.toContain("unavailable") expect(prompted).toBe(false) From acd6c43205112ad215bdf8dfb7703553e344f775 Mon Sep 17 00:00:00 2001 From: Rodolfo Castelo Date: Wed, 10 Jun 2026 10:46:09 +0000 Subject: [PATCH 11/11] docs: clarify differentiated Task model errors Distinguish the malformed-format Invalid task model error from the well-formed-but-unavailable Model unavailable error (with suggestions). Co-authored-by: yui-soul --- packages/web/src/content/docs/agents.mdx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/web/src/content/docs/agents.mdx b/packages/web/src/content/docs/agents.mdx index ef09e0da31c6..64f27871d709 100644 --- a/packages/web/src/content/docs/agents.mdx +++ b/packages/web/src/content/docs/agents.mdx @@ -353,7 +353,7 @@ This path is relative to where the config file is located. So this works for bot Use the `model` config to override the model for this agent. Useful for using different models optimized for different tasks. For example, a faster model for planning, a more capable model for implementation. :::tip -If you don’t specify a model, primary agents use the [model globally configured](/docs/config#models) while subagents will use the model of the primary agent that invoked the subagent. The invoking agent can override this per call by passing an optional `model` parameter, in `provider/model` form (e.g. `anthropic/claude-sonnet-4`), to the Task tool; an unknown or malformed value fails with an `Invalid task model` error. +If you don’t specify a model, primary agents use the [model globally configured](/docs/config#models) while subagents will use the model of the primary agent that invoked the subagent. The invoking agent can override this per call by passing an optional `model` parameter, in `provider/model` form (e.g. `anthropic/claude-sonnet-4`), to the Task tool; a malformed value (e.g. missing the slash) fails with an `Invalid task model` error, while a well-formed but unavailable model fails with a `Model unavailable` error (suggesting a close match when one exists). ::: ```json title="opencode.json"