mirror of
https://github.com/anomalyco/opencode.git
synced 2026-05-13 15:44:56 +00:00
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.
This commit is contained in:
@@ -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<T> = z.ZodType<T>
|
||||
|
||||
export function jsonc(text: string, filepath: string): unknown {
|
||||
@@ -50,34 +53,70 @@ export function effectSchema<S extends EffectSchema.Decoder<unknown, never>>(
|
||||
data: unknown,
|
||||
source: string,
|
||||
): DeepMutable<S["Type"]> {
|
||||
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<string, unknown>
|
||||
return Object.fromEntries(Object.entries(obj).filter(([key]) => !extra.includes(key)))
|
||||
}
|
||||
|
||||
function decodeWithFieldTolerance<S extends EffectSchema.Decoder<unknown, never>>(
|
||||
schema: S,
|
||||
data: unknown,
|
||||
source: string,
|
||||
): DeepMutable<S["Type"]> {
|
||||
// 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<S["Type"]>
|
||||
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<string, unknown>
|
||||
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<S["Type"]>
|
||||
throw new InvalidError({ path: source, issues }, { cause: error })
|
||||
}
|
||||
|
||||
function collectTopLevelFieldNames(issues: z.core.$ZodIssue[]): Set<string> {
|
||||
const names = new Set<string>()
|
||||
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) {
|
||||
|
||||
@@ -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<string, unknown>).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<string, unknown>).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<string, unknown>).invalid_field).toBeUndefined()
|
||||
})
|
||||
|
||||
// MCP config merging tests
|
||||
|
||||
Reference in New Issue
Block a user