From 58a27e75554d6e6e7d9883bdb0473ff03a28dfda Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 19 Aug 2026 18:30:02 -0400 Subject: [PATCH 1/3] fix(plugin): keep tool schema validation in the authoring effect instance Plugins load their own copy of effect, so their live Effect schemas cannot be interpreted by the host instance: parser sentinels and AST class identity are per-instance, making checks false-fail on valid input (bogus minLength errors) and branded IDs die as defects surfaced as bare 'Tool execution failed'. Plugin.define now converts tool input/output schemas to detached Standard Schema wrappers at the draft.add boundary, so validation and JSON Schema generation run as closures bound to the instance that created the schema. The core tool runtime detects still-foreign live schemas from older plugin packages and skips validation with a warning instead of misvalidating, and standard-schema validation errors now include the issue path. --- packages/core/src/tool/runtime.ts | 37 ++++- .../test/tool-runtime-foreign-schema.test.ts | 138 ++++++++++++++++++ packages/plugin/src/effect/plugin.ts | 22 ++- packages/plugin/src/effect/tool-schema.ts | 38 +++++ .../plugin/test/instance-safe-tool.test.ts | 71 +++++++++ 5 files changed, 300 insertions(+), 6 deletions(-) create mode 100644 packages/core/test/tool-runtime-foreign-schema.test.ts create mode 100644 packages/plugin/src/effect/tool-schema.ts create mode 100644 packages/plugin/test/instance-safe-tool.test.ts diff --git a/packages/core/src/tool/runtime.ts b/packages/core/src/tool/runtime.ts index f75dc543a421..d3d3a0cc4659 100644 --- a/packages/core/src/tool/runtime.ts +++ b/packages/core/src/tool/runtime.ts @@ -1,7 +1,7 @@ import type { ToolDefinition } from "@opencode-ai/ai" import { Tool } from "@opencode-ai/schema/tool" import type { StandardJSONSchemaV1, StandardSchemaV1 } from "@standard-schema/spec" -import { Effect, JsonSchema, Schema } from "effect" +import { Effect, JsonSchema, Schema, SchemaAST } from "effect" export const definition = (tool: Tool.Info): ToolDefinition => ({ name: effectiveName(tool), @@ -32,22 +32,26 @@ export const execute = (tool: Tool.Info, input: unknown, context: Tool }) const decodeInput = (schema: Tool.ValueSchema, value: unknown) => { - if (Schema.isSchema(schema)) + if (Schema.isSchema(schema)) { + if (isForeignSchema(schema)) return foreignSchemaPassthrough(value) return Schema.decodeUnknownEffect(schema)(value).pipe( Effect.mapError((error) => new Tool.Error({ message: `Invalid tool input: ${error.message}` })), ) + } if (isStandardSchema(schema)) return validateStandard(schema, value, "Invalid tool input") return Effect.succeed(value) } const encodeOutput = (schema: Tool.ValueSchema, value: unknown) => { - if (Schema.isSchema(schema)) + if (Schema.isSchema(schema)) { + if (isForeignSchema(schema)) return foreignSchemaPassthrough(value) return Schema.encodeEffect(schema)(value).pipe( Effect.mapError( (error) => new Tool.Error({ message: `Tool returned an invalid value for its output schema: ${error.message}` }), ), ) + } if (isStandardSchema(schema)) return validateStandard(schema, value, "Tool returned an invalid value for its output schema") return Schema.decodeUnknownEffect(Schema.Json)(value).pipe( @@ -57,6 +61,23 @@ const encodeOutput = (schema: Tool.ValueSchema, value: unknown) => { ) } +// A schema created by a different copy of `effect` (for example one loaded from a +// plugin's own node_modules) still satisfies `Schema.isSchema` because the type +// identifier is a shared string, but it cannot be interpreted by this instance: +// schema parsing relies on per-instance sentinels and class identity, so checks +// false-fail on valid values and branded types die as defects. AST classes are plain +// classes, so an instanceof test against this instance's AST base distinguishes the +// two reliably. +const isForeignSchema = (schema: Schema.Top) => !(schema.ast instanceof SchemaAST.Base) + +// Current @opencode-ai/plugin versions convert plugin schemas to Standard Schema +// wrappers before registration, keeping validation in the authoring instance. For +// plugins built against older versions, skip validation rather than misvalidate. +const foreignSchemaPassthrough = (value: unknown) => + Effect.logWarning( + "Tool schema was created by a different `effect` module instance; skipping validation. Update the plugin's @opencode-ai/plugin dependency to restore validation.", + ).pipe(Effect.as(value)) + const isStandardSchema = ( schema: Tool.ValueSchema, ): schema is StandardSchemaV1 & StandardJSONSchemaV1 => @@ -78,11 +99,19 @@ const validateStandard = ( : pending if (result.issues) return yield* new Tool.Error({ - message: `${prefix}: ${result.issues.map((issue) => issue.message).join(", ")}`, + message: `${prefix}: ${result.issues.map(standardIssueText).join(", ")}`, }) return result.value }) +const standardIssueText = (issue: StandardSchemaV1.Issue) => { + if (issue.path === undefined || issue.path.length === 0) return issue.message + const segments = issue.path.map((segment) => + typeof segment === "object" && segment !== null && "key" in segment ? segment.key : segment, + ) + return `${issue.message} at ${JSON.stringify(segments)}` +} + const standardFailure = (prefix: string, error: unknown) => new Tool.Error({ message: `${prefix}: ${error instanceof Error ? error.message : String(error)}` }) diff --git a/packages/core/test/tool-runtime-foreign-schema.test.ts b/packages/core/test/tool-runtime-foreign-schema.test.ts new file mode 100644 index 000000000000..e72af07f07b6 --- /dev/null +++ b/packages/core/test/tool-runtime-foreign-schema.test.ts @@ -0,0 +1,138 @@ +import { beforeAll, expect, test } from "bun:test" +import { cp, mkdir, mkdtemp, readFile, symlink } from "node:fs/promises" +import { tmpdir } from "node:os" +import path from "node:path" +import { fileURLToPath, pathToFileURL } from "node:url" +import { Tool } from "@opencode-ai/core/tool" +import { definition, execute } from "@opencode-ai/core/tool/runtime" +import { Agent } from "@opencode-ai/schema/agent" +import { Session } from "@opencode-ai/schema/session" +import { SessionMessage } from "@opencode-ai/schema/session-message" +import type { Info } from "@opencode-ai/schema/tool" +import { Effect, Schema } from "effect" + +const context = { + sessionID: Session.ID.make("ses_foreign"), + agent: Agent.ID.make("build"), + messageID: SessionMessage.ID.make("msg_foreign"), + id: Tool.CallID.make("call_foreign"), + progress: () => Effect.void, +} + +// Plugins load `effect` from their own node_modules, so their schemas come from a +// different module instance than the host's. Simulate that by copying the effect +// package to a temporary directory and importing the copy: same version, distinct +// instance, exactly like a plugin installed in the config directory. +let foreign: typeof Schema + +beforeAll(async () => { + const source = path.dirname(fileURLToPath(import.meta.resolve("effect/package.json"))) + const base = await mkdtemp(path.join(tmpdir(), "opencode-foreign-effect-")) + const target = path.join(base, "node_modules", "effect") + await cp(source, target, { recursive: true }) + const dependencies = JSON.parse(await readFile(path.join(source, "package.json"), "utf8")).dependencies ?? {} + for (const name of Object.keys(dependencies)) { + const real = path.dirname(Bun.resolveSync(`${name}/package.json`, source)) + const link = path.join(base, "node_modules", name) + await mkdir(path.dirname(link), { recursive: true }) + await symlink(real, link, "dir") + } + const mod = (await import(pathToFileURL(path.join(target, "dist", "index.js")).href)) as { Schema: typeof Schema } + foreign = mod.Schema + expect(foreign).not.toBe(Schema) +}) + +test("foreign live schemas skip validation instead of misvalidating checks", async () => { + // Regression: a minLength check from a foreign instance used to fail on valid + // values ('Expected a value with a length of at least 1 at ["title"]') because the + // host parser hands the foreign filter an internal sentinel instead of the value. + const input = foreign.Struct({ + title: foreign.optional(foreign.String.check(foreign.isMinLength(1))), + prompt: foreign.optional(foreign.String), + }) + expect(Schema.isSchema(input)).toBe(true) + let received: unknown + const tool: Info = { + name: "create", + description: "Create", + input, + execute: (value) => { + received = value + return Effect.succeed({ content: "ok" }) + }, + } + const result = await Effect.runPromise(execute(tool, { title: "probe", prompt: "Say ready." }, context)) + expect(result.content).toEqual([{ type: "text", text: "ok" }]) + expect(received).toEqual({ title: "probe", prompt: "Say ready." }) +}) + +test("foreign branded schemas no longer die as defects", async () => { + // Regression: decoding a foreign branded ID (like Session.ID) threw "Sync adapter + // can only throw schema errors", surfacing as a bare "Tool execution failed". + const input = foreign.Struct({ + sessionID: foreign.String.check(foreign.isStartsWith("ses")).pipe(foreign.brand("SessionID")), + }) + const tool: Info = { + name: "notify", + description: "Notify", + input, + execute: (value) => Effect.succeed({ content: JSON.stringify(value) }), + } + const result = await Effect.runPromise(execute(tool, { sessionID: "ses_123" }, context)) + expect(result.content).toEqual([{ type: "text", text: '{"sessionID":"ses_123"}' }]) +}) + +test("foreign output schemas pass the produced value through", async () => { + const tool: Info = { + name: "get", + description: "Get", + input: foreign.Struct({}), + output: foreign.Struct({ sessionID: foreign.String }), + execute: () => Effect.succeed({ output: { sessionID: "ses_123" } }), + } + const result = await Effect.runPromise(execute(tool, {}, context)) + expect(result.output).toEqual({ sessionID: "ses_123" }) +}) + +// Mirrors the conversion current @opencode-ai/plugin versions perform in the +// authoring instance before registration (see packages/plugin/src/effect/tool-schema.ts). +const convert = (schema: unknown, direction: "input" | "output") => { + const anyForeign = foreign as any + const oriented = direction === "input" ? schema : anyForeign.flip(schema) + const augmented = anyForeign.toStandardJSONSchemaV1(anyForeign.toStandardSchemaV1(oriented)) + return { "~standard": augmented["~standard"] } as Info["input"] +} + +test("converted standard wrappers validate in the authoring instance", async () => { + const input = convert( + foreign.Struct({ + title: foreign.optional(foreign.String.check(foreign.isMinLength(1))), + }), + "input", + ) + expect(Schema.isSchema(input)).toBe(false) + let received: unknown + const tool: Info = { + name: "create", + description: "Create", + input, + output: convert(foreign.Struct({ sessionID: foreign.String }), "output"), + execute: (value) => { + received = value + return Effect.succeed({ output: { sessionID: "ses_123" }, content: "created" }) + }, + } + + const success = await Effect.runPromise(execute(tool, { title: "probe" }, context)) + expect(received).toEqual({ title: "probe" }) + expect(success.output).toEqual({ sessionID: "ses_123" }) + + const failure = await Effect.runPromiseExit(execute(tool, { title: "" }, context)) + expect(failure.toString()).toContain("Invalid tool input") + expect(failure.toString()).toContain("a value with a length of at least 1") + expect(failure.toString()).toContain('at ["title"]') + + const derived = definition(tool) + expect(derived.inputSchema).toMatchObject({ type: "object" }) + expect((derived.inputSchema as { properties?: Record }).properties).toHaveProperty("title") +}) diff --git a/packages/plugin/src/effect/plugin.ts b/packages/plugin/src/effect/plugin.ts index bd4270346555..19703c7f5e12 100644 --- a/packages/plugin/src/effect/plugin.ts +++ b/packages/plugin/src/effect/plugin.ts @@ -1,5 +1,6 @@ import type { PluginApi } from "@opencode-ai/client/effect/api" import type { Effect, Scope } from "effect" +import { instanceSafeTool } from "./tool-schema.js" import type { PluginOptions } from "../options.js" import type { App } from "../app.js" import type { AgentDomain } from "./agent.js" @@ -41,6 +42,23 @@ export interface Plugin { readonly effect: (context: Context) => Effect.Effect } -export function define(plugin: Plugin) { - return plugin +export function define(plugin: Plugin): Plugin { + return { + ...plugin, + effect: (context) => plugin.effect(instanceSafeContext(context)), + } +} + +// Tool schemas cross from the plugin's module world into the host at `draft.add`; +// convert them while authoring-instance code is still on the stack so the host never +// interprets a foreign Effect schema. See `instanceSafeTool`. +function instanceSafeContext(context: Context): Context { + return { + ...context, + tool: { + ...context.tool, + transform: (callback) => + context.tool.transform((draft) => callback({ add: (tool) => draft.add(instanceSafeTool(tool)) })), + }, + } } diff --git a/packages/plugin/src/effect/tool-schema.ts b/packages/plugin/src/effect/tool-schema.ts new file mode 100644 index 000000000000..6d235d7b6c37 --- /dev/null +++ b/packages/plugin/src/effect/tool-schema.ts @@ -0,0 +1,38 @@ +import { Schema } from "effect" +import type { Tool } from "@opencode-ai/schema/tool" + +/** + * Converts a tool's Effect schemas into detached Standard Schema wrappers so they + * survive the crossing from the plugin's module world into the host. + * + * Plugins often load their own copy of `effect` (for example from the config + * directory's node_modules) while the host bundles a different instance. A live + * Effect schema cannot be interpreted across that boundary: schema parsing relies on + * per-instance sentinels and class identity, so the host misvalidates checks and + * turns branded-type failures into defects. A Standard Schema wrapper instead carries + * validation and JSON Schema generation as closures bound to the instance that + * created the schema, which the host invokes as-is. + */ +export function instanceSafeTool(tool: Tool.Info): Tool.Info { + const input = instanceSafeValueSchema(tool.input, "input") + const output = tool.output === undefined ? undefined : instanceSafeValueSchema(tool.output, "output") + if (input === tool.input && output === tool.output) return tool + return { ...tool, input, ...(output === undefined ? {} : { output }) } +} + +function instanceSafeValueSchema(schema: Tool.ValueSchema, direction: "input" | "output"): Tool.ValueSchema { + if (!Schema.isSchema(schema)) return schema + // Inputs are decoded (Encoded -> Type) but outputs are encoded (Type -> Encoded), + // so outputs use the flipped schema: its standard `validate` runs in the encode + // direction and its `jsonSchema.output` still describes the encoded shape. + const oriented = direction === "input" ? (schema as Schema.Top) : Schema.flip(schema as Schema.Top) + // Both converters augment the schema object in place and return it; the host must + // receive a plain wrapper instead, because the augmented object still satisfies + // `Schema.isSchema` and would route back into cross-instance interpretation. + const augmented = Schema.toStandardJSONSchemaV1( + Schema.toStandardSchemaV1(oriented as never) as never, + ) as unknown as StandardWrapper + return { "~standard": augmented["~standard"] } as Tool.ValueSchema +} + +type StandardWrapper = { readonly "~standard": Record } diff --git a/packages/plugin/test/instance-safe-tool.test.ts b/packages/plugin/test/instance-safe-tool.test.ts new file mode 100644 index 000000000000..e19d561a5284 --- /dev/null +++ b/packages/plugin/test/instance-safe-tool.test.ts @@ -0,0 +1,71 @@ +import { expect, test } from "bun:test" +import type { StandardJSONSchemaV1, StandardSchemaV1 } from "@standard-schema/spec" +import { Effect, Schema } from "effect" +import { Plugin } from "../src/effect/index.js" +import type { Tool } from "@opencode-ai/schema/tool" + +// `define` must hand the host detached Standard Schema wrappers instead of live +// Effect schemas: hosts may run a different `effect` instance, which cannot +// interpret foreign schemas (checks false-fail and branded types die as defects). +const collectTool = async (tool: Tool.Info) => { + const added: Array> = [] + const context = { + tool: { + transform: (callback: (draft: { add: (tool: Tool.Info) => void }) => void) => { + callback({ add: (item) => added.push(item) }) + return Effect.succeed({ dispose: Effect.void }) + }, + }, + } as unknown as Plugin.Context + const plugin = Plugin.define({ + id: "test.instance-safe", + effect: (ctx) => ctx.tool.transform((draft) => draft.add(tool)).pipe(Effect.asVoid), + }) + await Effect.runPromise(Effect.scoped(plugin.effect(context))) + expect(added).toHaveLength(1) + return added[0] +} + +type StandardValue = StandardSchemaV1 & StandardJSONSchemaV1 + +test("define converts Effect schemas to detached standard wrappers", async () => { + const execute = (input: { title?: string }) => Effect.succeed({ output: { id: `ses_${input.title}` } }) + const registered = await collectTool({ + name: "create", + description: "Create", + input: Schema.Struct({ title: Schema.optional(Schema.String.check(Schema.isMinLength(1))) }), + output: Schema.Struct({ id: Schema.String }), + execute, + }) + + expect(registered.execute).toBe(execute) + expect(Schema.isSchema(registered.input)).toBe(false) + expect(Schema.isSchema(registered.output)).toBe(false) + + const input = registered.input as StandardValue + expect(await input["~standard"].validate({ title: "probe" })).toEqual({ value: { title: "probe" } }) + const invalid = await input["~standard"].validate({ title: "" }) + expect(invalid.issues?.[0]?.message).toContain("a value with a length of at least 1") + expect(input["~standard"].jsonSchema.input({ target: "draft-2020-12" })).toMatchObject({ type: "object" }) + + // Outputs validate in the encode direction (Type -> Encoded) and describe the + // encoded shape. + const output = registered.output as StandardValue + expect(await output["~standard"].validate({ id: "ses_x" })).toEqual({ value: { id: "ses_x" } }) + expect(output["~standard"].jsonSchema.output({ target: "draft-2020-12" })).toMatchObject({ + type: "object", + required: ["id"], + }) +}) + +test("define leaves non-Effect schemas untouched", async () => { + const input = { type: "object" as const } + const registered = await collectTool({ + name: "raw", + description: "Raw", + input, + execute: () => Effect.succeed({ content: "ok" }), + }) + expect(registered.input).toBe(input) + expect(registered.output).toBeUndefined() +}) From 98310eb310b33ca4093d2424c8d2ebe3233cc9aa Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 19 Aug 2026 18:31:04 -0400 Subject: [PATCH 2/3] fix(core): accept null as omitted for optional tool inputs The JSON Schema advertised for tools renders optional fields as `X | null` because JSON cannot express undefined, so callers (models, Code Mode agents) legitimately pass null meaning "omit" and were rejected with 'Expected string | undefined'. Decode now retries with null-valued object properties removed when the first attempt fails: schemas that genuinely accept null succeed on the first attempt, array elements stay positional, and the original error is reported when the retry cannot help. --- packages/core/src/tool/runtime.ts | 33 ++++++- packages/core/test/tool-input-null.test.ts | 110 +++++++++++++++++++++ 2 files changed, 142 insertions(+), 1 deletion(-) create mode 100644 packages/core/test/tool-input-null.test.ts diff --git a/packages/core/src/tool/runtime.ts b/packages/core/src/tool/runtime.ts index d3d3a0cc4659..b048f180691e 100644 --- a/packages/core/src/tool/runtime.ts +++ b/packages/core/src/tool/runtime.ts @@ -31,7 +31,38 @@ export const execute = (tool: Tool.Info, input: unknown, context: Tool } }) -const decodeInput = (schema: Tool.ValueSchema, value: unknown) => { +const decodeInput = (schema: Tool.ValueSchema, value: unknown) => + attemptDecodeInput(schema, value).pipe( + Effect.catchTag("Tool.Error", (error) => { + // JSON Schema derived from Effect schemas advertises `X | null` for optional + // fields because JSON cannot express undefined, so callers legitimately send + // null to mean "omitted". Retry with null properties removed: schemas that + // genuinely accept null succeed on the first attempt, and the original error + // is reported when the retry cannot help. + const stripped = withoutNullProperties(value) + if (stripped === value) return error + return attemptDecodeInput(schema, stripped).pipe(Effect.catchTag("Tool.Error", () => error)) + }), + ) + +// Removes null-valued object properties recursively. Array elements are positional +// and stay untouched. Returns the input reference when nothing changed. +const withoutNullProperties = (value: unknown): unknown => { + if (Array.isArray(value)) { + const items = value.map(withoutNullProperties) + return items.some((item, index) => item !== value[index]) ? items : value + } + if (typeof value !== "object" || value === null) return value + const entries = Object.entries(value).flatMap(([key, item]) => + item === null ? [] : [[key, withoutNullProperties(item)] as const], + ) + const changed = + entries.length !== Object.keys(value).length || + entries.some(([key, item]) => (value as Record)[key] !== item) + return changed ? Object.fromEntries(entries) : value +} + +const attemptDecodeInput = (schema: Tool.ValueSchema, value: unknown) => { if (Schema.isSchema(schema)) { if (isForeignSchema(schema)) return foreignSchemaPassthrough(value) return Schema.decodeUnknownEffect(schema)(value).pipe( diff --git a/packages/core/test/tool-input-null.test.ts b/packages/core/test/tool-input-null.test.ts new file mode 100644 index 000000000000..c5d9c41f50ac --- /dev/null +++ b/packages/core/test/tool-input-null.test.ts @@ -0,0 +1,110 @@ +import { expect, test } from "bun:test" +import { Tool } from "@opencode-ai/core/tool" +import { execute } from "@opencode-ai/core/tool/runtime" +import { Agent } from "@opencode-ai/schema/agent" +import { Session } from "@opencode-ai/schema/session" +import { SessionMessage } from "@opencode-ai/schema/session-message" +import type { Info } from "@opencode-ai/schema/tool" +import { Effect, Schema } from "effect" + +const context = { + sessionID: Session.ID.make("ses_null"), + agent: Agent.ID.make("build"), + messageID: SessionMessage.ID.make("msg_null"), + id: Tool.CallID.make("call_null"), + progress: () => Effect.void, +} + +// The JSON Schema advertised for these tools renders optional fields as `X | null` +// (JSON cannot express undefined), so callers legitimately send null to mean +// "omitted". The runtime must accept that without weakening schemas that +// genuinely distinguish null. +const collect = (input: Info["input"]) => { + let received: unknown + const tool: Info = { + name: "probe", + description: "Probe", + input, + execute: (value) => { + received = value + return Effect.succeed({ content: "ok" }) + }, + } + return { + tool, + run: (value: unknown) => Effect.runPromise(execute(tool, value, context)).then(() => received), + fail: (value: unknown) => Effect.runPromiseExit(execute(tool, value, context)).then((exit) => exit.toString()), + } +} + +test("null optional properties decode as omitted", async () => { + const probe = collect( + Schema.Struct({ + title: Schema.String, + agent: Schema.optional(Schema.String), + }), + ) + expect(await probe.run({ title: "probe", agent: null })).toEqual({ title: "probe" }) +}) + +test("nested null optional properties decode as omitted", async () => { + const probe = collect( + Schema.Struct({ + worktree: Schema.optional( + Schema.Struct({ + branch: Schema.String, + base: Schema.optional(Schema.String), + }), + ), + }), + ) + expect(await probe.run({ worktree: { branch: "main", base: null } })).toEqual({ worktree: { branch: "main" } }) +}) + +test("schemas that accept null keep it", async () => { + const probe = collect(Schema.Struct({ next: Schema.NullOr(Schema.String) })) + expect(await probe.run({ next: null })).toEqual({ next: null }) +}) + +test("null array elements survive the retry", async () => { + const probe = collect( + Schema.Struct({ + tags: Schema.Array(Schema.NullOr(Schema.String)), + agent: Schema.optional(Schema.String), + }), + ) + expect(await probe.run({ tags: ["a", null], agent: null })).toEqual({ tags: ["a", null] }) +}) + +test("unfixable nulls report the original error", async () => { + const probe = collect(Schema.Struct({ title: Schema.String })) + const message = await probe.fail({ title: null }) + expect(message).toContain("Invalid tool input") + expect(message).toContain("Expected string") +}) + +test("standard schema inputs get the same retry", async () => { + const attempts: Array = [] + const input = { + "~standard": { + version: 1, + vendor: "test", + validate: (value: unknown) => { + attempts.push(value) + const record = value as Record + if ("agent" in record && record.agent === null) return { issues: [{ message: "Expected string | undefined" }] } + return { value } + }, + jsonSchema: { + input: () => ({ type: "object" }), + output: () => ({ type: "object" }), + }, + }, + } as unknown as Info["input"] + const probe = collect(input) + expect(await probe.run({ title: "probe", agent: null })).toEqual({ title: "probe" }) + expect(attempts).toEqual([ + { title: "probe", agent: null }, + { title: "probe" }, + ]) +}) From 128d07421d677f99b485a26d5e94b940e2d6653d Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Wed, 19 Aug 2026 18:32:34 -0400 Subject: [PATCH 3/3] fix(tui): show the default model for sessions stored without one Sessions created through the raw API with no model (agent: null, model: null) run fine on the server's default model, but the prompt status line rendered only the stored session model and claimed 'No provider selected - Connect a provider' even with providers connected. The session selection now falls back to GET /api/model/default, gated on the model still being available, so 'No provider selected' appears only when there genuinely is no usable provider. --- packages/tui/src/context/local.tsx | 42 +++++++++++++++++++++-- packages/tui/test/context/local.test.ts | 44 ++++++++++++++++++++++++- packages/tui/test/fixture/tui-client.ts | 5 +++ 3 files changed, 88 insertions(+), 3 deletions(-) diff --git a/packages/tui/src/context/local.tsx b/packages/tui/src/context/local.tsx index 7e6da553f47e..1f23071ba8c1 100644 --- a/packages/tui/src/context/local.tsx +++ b/packages/tui/src/context/local.tsx @@ -1,7 +1,7 @@ import { createStore } from "solid-js/store" import { dedupeWith } from "effect/Array" import { createSimpleContext } from "./helper" -import { batch, createMemo, onCleanup } from "solid-js" +import { batch, createMemo, createResource, onCleanup } from "solid-js" import { useEvent } from "./event" import path from "path" import { useTuiPaths } from "./runtime" @@ -32,6 +32,22 @@ export function parseModel(model: string) { } } +/** + * A session stored without a model runs on the server's default model, so the + * status line shows that effective model instead of claiming no provider is + * selected. "No provider selected" remains only when no usable default exists. + */ +export function withDefaultModelFallback(options: { + selection: (ModelPreferenceModel & { variant?: string }) | undefined + defaultModel: ModelPreferenceModel | undefined + isValid: (model: ModelPreferenceModel) => boolean + variantPreference: (model: ModelPreferenceModel) => string | undefined +}) { + if (options.selection) return options.selection + if (!options.defaultModel || !options.isValid(options.defaultModel)) return undefined + return { ...options.defaultModel, variant: normalizeModelVariant(options.variantPreference(options.defaultModel)) } +} + export function recentModels(model: ModelPreferenceModel, recent: ModelPreferenceModel[]) { const seen = new Set() return [model, ...recent] @@ -217,8 +233,30 @@ export const { use: useLocal, provider: LocalProvider } = createSimpleContext({ ) }) + const [serverDefaultModel] = createResource( + () => { + const ref = location.ref ?? data.location.default() + // Refetch when the catalog changes, such as a provider connecting. + return JSON.stringify([ref.directory, ref.workspaceID, models()?.length ?? -1]) + }, + async () => { + const ref = location.ref ?? data.location.default() + const response = await client.api.model + .default({ location: { directory: ref.directory, workspace: ref.workspaceID } }) + .catch(() => undefined) + if (!response?.data) return undefined + return { providerID: response.data.providerID, modelID: response.data.id } + }, + ) + const currentSelection = createMemo(() => { - if (route.data.type === "session") return sessionSelection(route.data.sessionID) + if (route.data.type === "session") + return withDefaultModelFallback({ + selection: sessionSelection(route.data.sessionID), + defaultModel: serverDefaultModel(), + isValid: isModelValid, + variantPreference: (model) => preferences.variant[modelPreferenceKey(model)], + }) const model = newSessionModel() if (!model) return return { ...model, variant: normalizeModelVariant(preferences.variant[modelPreferenceKey(model)]) } diff --git a/packages/tui/test/context/local.test.ts b/packages/tui/test/context/local.test.ts index e2f1e45f75a9..eb61d818acd8 100644 --- a/packages/tui/test/context/local.test.ts +++ b/packages/tui/test/context/local.test.ts @@ -1,5 +1,5 @@ import { expect, test } from "bun:test" -import { parseModel, recentModels } from "../../src/context/local" +import { parseModel, recentModels, withDefaultModelFallback } from "../../src/context/local" test("parses model IDs containing slashes", () => { expect(parseModel("provider/family/model")).toEqual({ @@ -20,3 +20,45 @@ test("moves a model to the front, deduplicates, and limits recents", () => { ...recent.slice(6, 10), ]) }) + +test("session selection wins over the default model", () => { + const selection = { providerID: "openai", modelID: "gpt", variant: "high" } + expect( + withDefaultModelFallback({ + selection, + defaultModel: { providerID: "opencode", modelID: "fable" }, + isValid: () => true, + variantPreference: () => undefined, + }), + ).toBe(selection) +}) + +test("sessions without a stored model fall back to the server default", () => { + expect( + withDefaultModelFallback({ + selection: undefined, + defaultModel: { providerID: "opencode", modelID: "fable" }, + isValid: () => true, + variantPreference: (model) => (model.modelID === "fable" ? "max" : undefined), + }), + ).toEqual({ providerID: "opencode", modelID: "fable", variant: "max" }) +}) + +test("no provider is reported only without a usable default", () => { + expect( + withDefaultModelFallback({ + selection: undefined, + defaultModel: undefined, + isValid: () => true, + variantPreference: () => undefined, + }), + ).toBeUndefined() + expect( + withDefaultModelFallback({ + selection: undefined, + defaultModel: { providerID: "gone", modelID: "model" }, + isValid: () => false, + variantPreference: () => undefined, + }), + ).toBeUndefined() +}) diff --git a/packages/tui/test/fixture/tui-client.ts b/packages/tui/test/fixture/tui-client.ts index 5fcb6f4bbd14..fd53c5beba31 100644 --- a/packages/tui/test/fixture/tui-client.ts +++ b/packages/tui/test/fixture/tui-client.ts @@ -152,6 +152,11 @@ export function createFetch(override?: FetchHandler, events?: ReturnType