diff --git a/packages/opencode/src/util/json-safe.ts b/packages/opencode/src/util/json-safe.ts index 1ebb4b6d8f..0d40becb7b 100644 --- a/packages/opencode/src/util/json-safe.ts +++ b/packages/opencode/src/util/json-safe.ts @@ -1,9 +1,13 @@ export function toJsonSafe(value: T): T { - return JSON.parse( - JSON.stringify(value, (_, v) => { - if (typeof v === "function" || typeof v === "symbol" || v === undefined) return undefined - if (typeof v === "bigint") return v.toString() - return v - }), - ) + const ancestors: object[] = [] + const json = JSON.stringify(value, function (this: unknown, _key, v) { + if (typeof v === "function" || typeof v === "symbol" || v === undefined) return undefined + if (typeof v === "bigint") return v.toString() + if (v === null || typeof v !== "object") return v + while (ancestors.length > 0 && ancestors.at(-1) !== this) ancestors.pop() + if (ancestors.includes(v)) return undefined + ancestors.push(v) + return v + }) + return json === undefined ? (undefined as T) : JSON.parse(json) } diff --git a/packages/opencode/test/plugin/mutation-repro.test.ts b/packages/opencode/test/plugin/mutation-repro.test.ts index fa3ebbbb4d..d5f312fa35 100644 --- a/packages/opencode/test/plugin/mutation-repro.test.ts +++ b/packages/opencode/test/plugin/mutation-repro.test.ts @@ -42,7 +42,10 @@ afterAll(() => { process.env.OPENCODE_DISABLE_DEFAULT_PLUGINS = disableDefault }) -function withProject(source: string, self: Effect.Effect | ((dir: string) => Effect.Effect)) { +function withProject( + source: string, + self: Effect.Effect | ((dir: string) => Effect.Effect), +) { return provideTmpdirInstance((dir) => Effect.gen(function* () { const file = path.join(dir, "plugin.ts") @@ -70,11 +73,11 @@ function withProject(source: string, self: Effect.Effect | ((d ) } -/** True if `value` contains no function values anywhere in its (mutable, - * enumerable) tree — i.e. survives a round-trip through JSON.stringify - * without silently losing fields. */ +/** True if `value` contains only values that can cross JSON boundaries without loss. */ function isJsonSafe(value: unknown, seen = new WeakSet()): boolean { - if (typeof value === "function") return false + if (typeof value === "function" || typeof value === "symbol" || typeof value === "bigint" || value === undefined) { + return false + } if (value === null || typeof value !== "object") return true if (seen.has(value as object)) return true seen.add(value as object) @@ -113,9 +116,9 @@ describe("plugin hook mutation reproducers (analog of #26546)", () => { parameters: { type: "object", properties: {} }, } yield* plugin.trigger("tool.definition", { toolID: "test_tool" }, output) - // Post-fix contract: opencode must hand the plugin a defensive - // copy and re-scrub the result, so function-valued mutations - // never leak back to the tool definition pipeline. + // Future contract: function-valued mutations must not leak back + // to the tool definition pipeline without breaking output + // mutation semantics. expect(isJsonSafe(output)).toBe(true) }), ), @@ -147,7 +150,7 @@ describe("plugin hook mutation reproducers (analog of #26546)", () => { const plugin = yield* Plugin.Service const output = { messages: [] as any[] } yield* plugin.trigger("experimental.chat.messages.transform", {}, output) - // Post-fix contract: messages handed back to the prompt / + // Future contract: messages handed back to the prompt / // compaction pipeline must be JSON-safe. The same hook fires from // both session/prompt.ts:1566 and session/compaction.ts:407. expect(isJsonSafe(output)).toBe(true) @@ -185,7 +188,7 @@ describe("plugin hook mutation reproducers (analog of #26546)", () => { }, output, ) - // Post-fix contract: the system prompt array opencode keeps + // Future contract: the system prompt array opencode keeps // forwarding to the LLM provider must contain only JSON-safe // primitives. expect(isJsonSafe(output)).toBe(true) @@ -196,19 +199,26 @@ describe("plugin hook mutation reproducers (analog of #26546)", () => { // ------------------------------------------------------------------------- // Finding 4: plugin.config() — plugin/index.ts:235 // The plugin's `config` hook receives the LIVE Config.Info object and can - // mutate it. Downstream `/config/get` and any JSON serialization drop the - // function-valued fields, but the in-memory state stays corrupted. + // mutate it. Internal cfg may keep runtime values; HTTP `/config` must + // project those values out before response encoding. // ------------------------------------------------------------------------- it.live("plugin.config(): GET /config response stays JSON-safe after plugin mutation", () => withProject( [ "export default async () => ({", " config: (cfg) => {", - " // Misbehaving plugin attaches a function-valued field to the", - " // shared config object. Internal state is allowed to carry it,", - " // but the HTTP API (and any other JSON boundary) must project", - " // it out so callers see the typed schema.", - " ;(cfg as any).__pluginFn = () => 'mutated'", + " // Misbehaving plugin attaches runtime-only values under a", + " // schema-allowed field. Internal state is allowed to carry", + " // them, but the HTTP API must project them to JSON-safe data.", + " ;(cfg as any).provider = {", + " ...((cfg as any).provider ?? {}),", + " plugin_runtime: {", + " options: {", + " fetch: async (input, init) => fetch(input, init),", + " pluginBigInt: BigInt(1),", + " },", + " },", + " }", " },", "})", "", @@ -217,15 +227,22 @@ describe("plugin hook mutation reproducers (analog of #26546)", () => { Effect.gen(function* () { const plugin = yield* Plugin.Service yield* plugin.init() + const config = yield* Config.Service + const cfg = yield* config.get() + expect(typeof cfg.provider?.plugin_runtime?.options?.fetch).toBe("function") const headers = { "x-opencode-directory": dir } - const response = yield* Effect.promise(() => Promise.resolve(Server.Default().app.request("/config", { headers }))) + const response = yield* Effect.promise(() => + Promise.resolve(Server.Default().app.request("/config", { headers })), + ) expect(response.status).toBe(200) const body = (yield* Effect.promise(() => response.json())) as Record // Post-fix contract: HTTP /config is the boundary. Whatever the // live in-memory cfg looks like, the wire response must be // JSON-safe and match the typed schema. expect(isJsonSafe(body)).toBe(true) - expect("__pluginFn" in body).toBe(false) + const provider = body.provider as Record }> | undefined + expect(provider?.plugin_runtime?.options?.fetch).toBeUndefined() + expect(provider?.plugin_runtime?.options?.pluginBigInt).toBe("1") }), ), )