refactor(json-safe): add cycle detection to toJsonSafe

packages/opencode/src/util/json-safe.ts

@@ -1,9 +1,13 @@
 export function toJsonSafe<T>(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)
 }

packages/opencode/test/plugin/mutation-repro.test.ts

@@ -42,7 +42,10 @@ afterAll(() => {
   process.env.OPENCODE_DISABLE_DEFAULT_PLUGINS = disableDefault
 })
 
-function withProject<A, E, R>(source: string, self: Effect.Effect<A, E, R> | ((dir: string) => Effect.Effect<A, E, R>)) {
+function withProject<A, E, R>(
+  source: string,
+  self: Effect.Effect<A, E, R> | ((dir: string) => Effect.Effect<A, E, R>),
+) {
   return provideTmpdirInstance((dir) =>
     Effect.gen(function* () {
       const file = path.join(dir, "plugin.ts")
@@ -70,11 +73,11 @@ function withProject<A, E, R>(source: string, self: Effect.Effect<A, E, R> | ((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<object>()): 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<string, unknown>
           // 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<string, { options?: Record<string, unknown> }> | undefined
+          expect(provider?.plugin_runtime?.options?.fetch).toBeUndefined()
+          expect(provider?.plugin_runtime?.options?.pluginBigInt).toBe("1")
         }),
     ),
   )