fix(server): project /config response to JSON-safe at the HTTP boundary#26553
Closed
kitlangton wants to merge 3 commits intodevfrom
Closed
fix(server): project /config response to JSON-safe at the HTTP boundary#26553kitlangton wants to merge 3 commits intodevfrom
kitlangton wants to merge 3 commits intodevfrom
Conversation
Mirrors #26550 (Provider.toPublicInfo): internal runtime objects may legitimately carry function/symbol/bigint/undefined values, but the HTTP API contract is JSON-safe data matching the typed schema. Plugin `config:` hooks are documented to mutate cfg in place (registering agents/providers); that contract is preserved. The projection happens in ConfigHttpApi.get only, so internal consumers keep raw cfg and HTTP clients see a clean response. Active reproducer asserts the actual product invariant — GET /config returns a JSON-safe body even when a plugin attached a function to cfg. Three trigger-output reproducers stay skipped: per-hook design needed because the documented mutation contract makes a blanket clone unsafe and `tool.parameters` is a live Effect Schema with function-valued own keys that recursive in-place scrub would corrupt.
691a3cd to
6833070
Compare
Reverts the redundant-cleanup hypothesis. The reproducer was upgraded to exercise the actual gemini-auth bug shape — function and bigint values under provider.options (a Schema.Any-typed declared field, not an excess property) — and that fails /config encode without the projection. Effect's onExcessProperty handles excess keys for free, but Schema.Any pass-through plus encode validation doesn't, so the toJsonSafe clone at the HTTP boundary is load-bearing for this case.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Companion to #26550 (
Provider.toPublicInfo) at the/configHTTP boundary. Internal opencode state — including the live config object — may legitimately carry function/symbol/bigint/undefined values from pluginconfig:hooks. The product invariant is that the public HTTP API is JSON-safe data matching its typed schema, not that the internal in-memory cfg is.This PR adds
toJsonSafeand applies it inConfigHttpApi.getonly. Plugin mutation contract forconfig:is preserved (cfg passes through unchanged); the projection happens at the wire boundary, mirroring howProvider.toPublicInfoalready projects providers in/providerand/config/providers.Why this shape (and not in-place scrub or clone-before-hook)
agent-plugin.ts, etc.) register agents/providers by mutating cfg. Cloning silently discards those edits.Reproducers
packages/opencode/test/plugin/mutation-repro.test.ts:tool.parameters. Per-hook design needed — separate follow-up.Also drops an orphan
returns structured validation errorstest fromhttpapi-sync.test.tsthat no longer matches the route shape after #26550.Test
Followups (not in this PR)