From 18f3b31f1c42c46524f76faea0a181b09484cb40 Mon Sep 17 00:00:00 2001 From: Kit Langton Date: Sat, 9 May 2026 20:37:44 -0400 Subject: [PATCH] fix(config): drop malformed top-level fields instead of crashing the load Reported by Ben Matthews on Discord against v1.14.45: a `skills:` field shaped as an array (rather than the schema's `{paths?, urls?}` object) caused the entire config load to fail with ConfigInvalidError, the server returned 500, and the desktop app couldn't start. Per Kit: > for all of these things that we load from the user's computer, they > should be kind of tolerant. We can have warnings that we log > somewhere, but Jesus. It shouldn't break opencode. This makes `ConfigParse.effectSchema` field-tolerant: - Unknown top-level keys are stripped (with a warning log) instead of throwing `unrecognized_keys`. - Top-level fields whose value fails decoding are dropped (with a warning log naming the field + the issue summary), the rest of the config decodes normally. - Root-level errors (e.g. data is not an object at all) and post-strip failures still throw the original `InvalidError` so we don't silently swallow truly broken configs. Three new tests cover the reported shape (skills as array), unknown keys, and multi-bad-field cases. Two existing tests that asserted the strict-throw behavior were updated to the new tolerant contract. Pre-fix: opencode unstartable for users with malformed configs. Post-fix: opencode starts, the bad field is ignored, the user sees a warning in the log naming what was dropped. --- packages/opencode/src/config/parse.ts | 85 +++++++++++++------ packages/opencode/test/config/config.test.ts | 86 +++++++++++++++++--- 2 files changed, 136 insertions(+), 35 deletions(-) diff --git a/packages/opencode/src/config/parse.ts b/packages/opencode/src/config/parse.ts index f964ed4e15..2d05e25eb3 100644 --- a/packages/opencode/src/config/parse.ts +++ b/packages/opencode/src/config/parse.ts @@ -2,10 +2,13 @@ export * as ConfigParse from "./parse" import { type ParseError as JsoncParseError, parse as parseJsoncImpl, printParseErrorCode } from "jsonc-parser" import { Cause, Exit, Schema as EffectSchema, SchemaIssue } from "effect" +import * as Log from "@opencode-ai/core/util/log" import z from "zod" import type { DeepMutable } from "@opencode-ai/core/schema" import { InvalidError, JsonError } from "./error" +const log = Log.create({ service: "config.parse" }) + type ZodSchema = z.ZodType export function jsonc(text: string, filepath: string): unknown { @@ -50,34 +53,70 @@ export function effectSchema>( data: unknown, source: string, ): DeepMutable { - const extra = topLevelExtraKeys(schema, data) - if (extra.length) { - throw new InvalidError({ - path: source, - issues: [ - { - code: "unrecognized_keys", - keys: extra, - path: [], - message: `Unrecognized key${extra.length === 1 ? "" : "s"}: ${extra.join(", ")}`, - } as z.core.$ZodIssue, - ], - }) - } + // The user's config lives on disk and may legitimately be stale, hand-edited, + // or carry leftover keys from older versions. Crashing the whole load on a + // single bad field would make opencode unstartable for those users (see Ben + // Matthews / Discord, v1.14.45). Strip the malformed top-level fields and + // keep going — log every drop so users can see what was ignored and fix it. + const cleaned = stripUnknownTopLevelKeys(schema, data, source) + return decodeWithFieldTolerance(schema, cleaned, source) +} +function stripUnknownTopLevelKeys(schema: EffectSchema.Top, data: unknown, source: string): unknown { + if (typeof data !== "object" || data === null || Array.isArray(data)) return data + const extra = topLevelExtraKeys(schema, data) + if (extra.length === 0) return data + log.warn("ignoring unrecognized config keys", { source, keys: extra }) + const obj = data as Record + return Object.fromEntries(Object.entries(obj).filter(([key]) => !extra.includes(key))) +} + +function decodeWithFieldTolerance>( + schema: S, + data: unknown, + source: string, +): DeepMutable { + // Try a clean decode first. If it succeeds we're done — common path. const decoded = EffectSchema.decodeUnknownExit(schema)(data, { errors: "all", propertyOrder: "original" }) if (Exit.isSuccess(decoded)) return decoded.value as DeepMutable const error = Cause.squash(decoded.cause) + const issues = EffectSchema.isSchemaError(error) + ? (SchemaIssue.makeFormatterStandardSchemaV1()(error.issue).issues as z.core.$ZodIssue[]) + : ([{ code: "custom", message: String(error), path: [] }] as z.core.$ZodIssue[]) - throw new InvalidError( - { - path: source, - issues: EffectSchema.isSchemaError(error) - ? (SchemaIssue.makeFormatterStandardSchemaV1()(error.issue).issues as z.core.$ZodIssue[]) - : ([{ code: "custom", message: String(error), path: [] }] as z.core.$ZodIssue[]), - }, - { cause: error }, - ) + // Identify malformed top-level fields. Anything with a non-empty path is a + // field-scoped issue we can drop and retry. Issues with an empty path are + // root-level (e.g. data is not an object at all) and can't be field-recovered. + const badFields = collectTopLevelFieldNames(issues) + if (badFields.size === 0 || typeof data !== "object" || data === null || Array.isArray(data)) { + throw new InvalidError({ path: source, issues }, { cause: error }) + } + + log.warn("ignoring invalid config fields", { + source, + fields: [...badFields], + summary: issues + .filter((issue) => issue.path && issue.path.length > 0) + .map((issue) => `${issue.path?.join(".")}: ${issue.message}`) + .slice(0, 8), + }) + + const obj = data as Record + const cleaned = Object.fromEntries(Object.entries(obj).filter(([key]) => !badFields.has(key))) + // Retry without the bad fields. If THIS fails, we're past field-tolerance — + // fall back to the original strict error so the user sees the real cause. + const retry = EffectSchema.decodeUnknownExit(schema)(cleaned, { errors: "all", propertyOrder: "original" }) + if (Exit.isSuccess(retry)) return retry.value as DeepMutable + throw new InvalidError({ path: source, issues }, { cause: error }) +} + +function collectTopLevelFieldNames(issues: z.core.$ZodIssue[]): Set { + const names = new Set() + for (const issue of issues) { + const head = issue.path?.[0] + if (typeof head === "string") names.add(head) + } + return names } function topLevelExtraKeys(schema: EffectSchema.Top, data: unknown) { diff --git a/packages/opencode/test/config/config.test.ts b/packages/opencode/test/config/config.test.ts index bbe585237b..b39922a78a 100644 --- a/packages/opencode/test/config/config.test.ts +++ b/packages/opencode/test/config/config.test.ts @@ -558,20 +558,22 @@ test("handles file inclusion with replacement tokens", async () => { }) }) -test("validates config schema and throws on invalid fields", async () => { +test("config loader is tolerant: drops unknown fields, keeps the rest", async () => { await using tmp = await tmpdir({ init: async (dir) => { await writeConfig(dir, { $schema: "https://opencode.ai/config.json", - invalid_field: "should cause error", + username: "kept", + invalid_field: "should be dropped, not crash the app", }) }, }) await provideTestInstance({ directory: tmp.path, fn: async () => { - // Strict schema should throw an error for invalid fields - await expect(load()).rejects.toThrow() + const config = await load() + expect(config.username).toBe("kept") + expect((config as Record).invalid_field).toBeUndefined() }, }) }) @@ -1681,7 +1683,70 @@ test("permission config preserves user key order", async () => { }) }) -test("Effect config parser preserves permission order while rejecting unknown top-level keys", () => { +// Discord bug report (Ben Matthews, v1.14.45): a malformed `skills:` field +// (array instead of object) made the WHOLE config fail to load, the server +// returned 500, and the desktop app couldn't start. Per Kit: +// "for all of these things that we load from the user's computer, they +// should be kind of tolerant. ... It shouldn't break opencode." +// The contract: drop the malformed top-level field, log a warning, keep +// the rest of the config so the app starts. +test("config parser is tolerant: drops malformed top-level fields, keeps the rest", () => { + const config = ConfigParse.effectSchema( + Config.Info, + { + $schema: "https://opencode.ai/config.json", + username: "ben", + // Wrong shape — schema expects { paths?, urls? }, user has an array + // (looks like the LOADED skills list got pasted into the config). + skills: [ + { name: "scss-layout-accessibility", path: ".opencode/skills/scss-layout-accessibility.md" }, + { name: "testing", path: ".opencode/skills/testing.md" }, + ], + }, + "test", + ) + + // Pre-fix this throws ConfigInvalidError and the user can't start opencode. + // Post-fix the bad field is dropped and the rest of the config loads. + expect(config.username).toBe("ben") + expect(config.skills).toBeUndefined() +}) + +test("config parser is tolerant: drops unrecognized top-level keys instead of throwing", () => { + const config = ConfigParse.effectSchema( + Config.Info, + { + $schema: "https://opencode.ai/config.json", + username: "ben", + // Typo or stale key — pre-fix this threw `unrecognized_keys`. + autoshrare: true, + }, + "test", + ) + + expect(config.username).toBe("ben") + expect((config as Record).autoshrare).toBeUndefined() +}) + +test("config parser is tolerant: drops multiple bad fields in one pass", () => { + const config = ConfigParse.effectSchema( + Config.Info, + { + $schema: "https://opencode.ai/config.json", + username: "ben", + skills: ["wrong shape"], + autoshare: 42, // wrong type — schema wants string literal | undefined + not_a_real_key: "ignore me", + }, + "test", + ) + + expect(config.username).toBe("ben") + expect(config.skills).toBeUndefined() + expect(config.autoshare).toBeUndefined() +}) + +test("Effect config parser preserves permission order while dropping unknown top-level keys", () => { const config = ConfigParse.effectSchema( Config.Info, { @@ -1695,13 +1760,10 @@ test("Effect config parser preserves permission order while rejecting unknown to ) expect(Object.keys(config.permission!)).toEqual(["bash", "*", "edit"]) - try { - ConfigParse.effectSchema(Config.Info, { invalid_field: true }, "test") - throw new Error("expected config parse to fail") - } catch (err) { - const error = err as { data?: { issues?: Array<{ code?: string; keys?: string[]; path?: string[] }> } } - expect(error.data?.issues?.[0]).toMatchObject({ code: "unrecognized_keys", keys: ["invalid_field"], path: [] }) - } + // Tolerant parser: unknown keys are stripped (with a warning log) instead + // of failing the entire config load. + const stripped = ConfigParse.effectSchema(Config.Info, { invalid_field: true }, "test") + expect((stripped as Record).invalid_field).toBeUndefined() }) // MCP config merging tests