Skip to content

fix(core): cross-instance plugin tool schemas, null tool inputs, and TUI default-model display - #43535

Draft
kitlangton wants to merge 3 commits into
v2from
fix/session-tool-bugs
Draft

fix(core): cross-instance plugin tool schemas, null tool inputs, and TUI default-model display#43535
kitlangton wants to merge 3 commits into
v2from
fix/session-tool-bugs

Conversation

@kitlangton

Copy link
Copy Markdown
Contributor

What

Fixes three bugs hit while driving opencode from another agent session:

  1. Plugin tools with Effect schemas misvalidated every call (session.create rejected valid input with a bogus Expected a value with a length of at least 1 at ["title"]).
  2. Plugin tools with branded-ID inputs (session.notify) hard-failed with a bare Tool execution failed.
  3. The TUI claimed "No provider selected · Connect a provider" for sessions stored without a model, even though the run was actively using the server's default model.

Bugs 1 and 2 turned out to be the same root cause: cross-instance Effect schemas. Plugins load effect from their own node_modules while the server bundles its own copy. A live Effect schema cannot be interpreted across instances, even at the identical version: the parser compiles leaf parsers through the foreign AST's methods, then compares results against its own private sentinels (sameExit, the args symbol), so the value handed to filter checks is an internal sentinel instead of the input. Checks false-fail on valid values, and branded types throw Sync adapter can only throw schema errors, which surfaces as the generic Tool execution failed.

Before / After

Bug 1+2, before: every session.create call failed with Invalid tool input: Expected a value with a length of at least 1 at ["title"] even for { title: "probe" }; session.notify with a real session ID died as a defect and the caller saw only Tool execution failed.

After: with an updated @opencode-ai/plugin, schemas are converted at the plugin boundary and validate correctly (including proper error paths). With an older plugin package, the server detects the foreign schema, logs a warning, and skips validation instead of misvalidating, so all eight session tools work again with a server update alone.

Null smell, before: the advertised Code Mode signature says agent?: string | null (Effect renders optional as X | null because JSON cannot express undefined), but passing agent: null failed with Expected string | undefined at ["agent"].

After: decode retries with null-valued properties removed when the first attempt fails, so null means "omitted" exactly as advertised. Schemas that genuinely accept null (Schema.NullOr) succeed on the first attempt and are unaffected; if the retry cannot help, the original error is reported.

Bug 3, before: opening a session created via POST /api/session with only {title, directory} (stored model: null) showed "Build · No provider selected Connect a provider" and submitting opened the Connect-an-integration modal, while the run itself happily used the default model.

After: the status line falls back to GET /api/model/default (gated on the model still being in the catalog), so it shows the effective model. "No provider selected" appears only when there genuinely is no usable default.

How

  • packages/plugin/src/effect/tool-schema.ts (new): converts tool input/output Effect schemas into detached Standard Schema wrappers (toStandardSchemaV1 + toStandardJSONSchemaV1; outputs via Schema.flip so validate runs in the encode direction). The wrapper's closures are bound to the instance that created the schema, so the host runs them as-is. Detachment matters: the augmented original still satisfies Schema.isSchema and would route back into cross-instance interpretation.
  • packages/plugin/src/effect/plugin.ts: Plugin.define wraps the context so draft.add converts schemas while authoring-instance code is still on the stack. Promise plugins flow through the same path via fromPromise.
  • packages/core/src/tool/runtime.ts:
    • Detects still-foreign live schemas (Schema.isSchema passes but schema.ast instanceof SchemaAST.Base fails, since AST classes are plain per-instance classes) and skips validation with a warning instead of misvalidating. This keeps older plugin packages working with a server update alone.
    • Standard Schema validation errors now include the issue path (... at ["title"]).
    • decodeInput retries with null properties stripped (object properties only; array elements are positional and untouched).
  • packages/tui/src/context/local.tsx: session model selection falls back to a model.default resource; withDefaultModelFallback is exported for tests.
sequenceDiagram
    participant P as Plugin (own effect copy)
    participant D as Plugin.define (authoring instance)
    participant R as Core tool runtime (host instance)
    P->>D: draft.add({ input: Effect schema })
    D->>D: convert to detached Standard Schema wrapper
    D->>R: register tool
    Note over R: validate via wrapper closures<br/>(run in authoring instance)
    R->>P: execute(decoded input)
Loading

Scope

  • Fixing tool schemas covers the reported breakage. Other plugin surfaces that might accept Effect schemas across the same boundary (forms, commands) are not audited here.
  • The deeper alternative, sharing the server's effect instance with plugins via module aliasing, is a larger architectural change and deliberately not attempted.
  • session.notify's post-decode behavior (event subscription across instances) was not re-verified live; the reported failure was at input decode, which is fixed and tested.

Testing

  • packages/core: full suite, 1892 pass / 0 fail. New test/tool-runtime-foreign-schema.test.ts copies the real effect package to a temp dir and imports it as a genuinely foreign instance, reproducing both failure modes; new test/tool-input-null.test.ts covers the null-as-omitted retry.
  • packages/plugin: full suite. New test/instance-safe-tool.test.ts covers the define conversion (wrapper detachment, decode/encode direction, non-Effect schemas untouched).
  • packages/tui: full suite, 736 pass / 0 fail (fixture now serves /api/model/default). New unit tests for withDefaultModelFallback.
  • End-to-end for bug 3: created a session with model: null on the live server and opened it in the dev TUI before and after the fix (screenshots below).
  • bun typecheck in packages/core, packages/plugin, packages/tui.

Demo

Session stored with model: null, before (claims no provider; the plugin-failed toast is unrelated local config):

before

After (shows the server default, Gemini 3.7 Flash):

after

…ance

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.
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant