Преглед на файлове

fix(config): preserve permission order with Effect decode (#24308)

Kit Langton преди 4 месеца
родител
ревизия
a9740b9133

+ 17 - 22
packages/opencode/src/config/agent.ts

@@ -1,17 +1,16 @@
 export * as ConfigAgent from "./agent"
 
-import { Schema } from "effect"
-import z from "zod"
+import { Exit, Schema, SchemaGetter } from "effect"
 import { Bus } from "@/bus"
 import { zod } from "@/util/effect-zod"
-import { PositiveInt } from "@/util/schema"
+import { PositiveInt, withStatics } from "@/util/schema"
 import { Log } from "../util"
 import { NamedError } from "@opencode-ai/core/util/error"
 import { Glob } from "@opencode-ai/core/util/glob"
 import { configEntryNameFromPath } from "./entry-name"
-import { InvalidError } from "./error"
 import * as ConfigMarkdown from "./markdown"
 import { ConfigModelID } from "./model-id"
+import { ConfigParse } from "./parse"
 import { ConfigPermission } from "./permission"
 
 const log = Log.create({ service: "config" })
@@ -77,7 +76,7 @@ const KNOWN_KEYS = new Set([
 //  - Translate the deprecated `tools: { name: boolean }` map into the new
 //    `permission` shape (write-adjacent tools collapse into `permission.edit`).
 //  - Coalesce `steps ?? maxSteps` so downstream can ignore the deprecated alias.
-const normalize = (agent: z.infer<typeof Info>) => {
+const normalize = (agent: Schema.Schema.Type<typeof AgentSchema>): Schema.Schema.Type<typeof AgentSchema> => {
   const options: Record<string, unknown> = { ...agent.options }
   for (const [key, value] of Object.entries(agent)) {
     if (!KNOWN_KEYS.has(key)) options[key] = value
@@ -98,14 +97,15 @@ const normalize = (agent: z.infer<typeof Info>) => {
   return { ...agent, options, permission, ...(steps !== undefined ? { steps } : {}) }
 }
 
-export const Info = zod(AgentSchema).transform(normalize).meta({ ref: "AgentConfig" }) as unknown as z.ZodType<
-  Omit<z.infer<ReturnType<typeof zod<typeof AgentSchema>>>, "options" | "permission" | "steps"> & {
-    options?: Record<string, unknown>
-    permission?: ConfigPermission.Info
-    steps?: number
-  }
->
-export type Info = z.infer<typeof Info>
+export const Info = AgentSchema.pipe(
+  Schema.decodeTo(AgentSchema, {
+    decode: SchemaGetter.transform(normalize),
+    encode: SchemaGetter.passthrough({ strict: false }),
+  }),
+)
+  .annotate({ identifier: "AgentConfig" })
+  .pipe(withStatics((s) => ({ zod: zod(s) })))
+export type Info = Schema.Schema.Type<typeof Info>
 
 export async function load(dir: string) {
   const result: Record<string, Info> = {}
@@ -134,12 +134,7 @@ export async function load(dir: string) {
       ...md.data,
       prompt: md.content.trim(),
     }
-    const parsed = Info.safeParse(config)
-    if (parsed.success) {
-      result[config.name] = parsed.data
-      continue
-    }
-    throw new InvalidError({ path: item, issues: parsed.error.issues }, { cause: parsed.error })
+    result[config.name] = ConfigParse.effectSchema(Info, config, item)
   }
   return result
 }
@@ -168,10 +163,10 @@ export async function loadMode(dir: string) {
       ...md.data,
       prompt: md.content.trim(),
     }
-    const parsed = Info.safeParse(config)
-    if (parsed.success) {
+    const parsed = Schema.decodeUnknownExit(Info)(config, { errors: "all", propertyOrder: "original" })
+    if (Exit.isSuccess(parsed)) {
       result[config.name] = {
-        ...parsed.data,
+        ...parsed.value,
         mode: "primary" as const,
       }
     }

+ 20 - 22
packages/opencode/src/config/config.ts

@@ -24,7 +24,7 @@ import { InstanceState } from "@/effect"
 import { Context, Duration, Effect, Exit, Fiber, Layer, Option, Schema } from "effect"
 import { EffectFlock } from "@opencode-ai/core/util/effect-flock"
 import { InstanceRef } from "@/effect/instance-ref"
-import { zod, ZodOverride } from "@/util/effect-zod"
+import { zod } from "@/util/effect-zod"
 import { NonNegativeInt, PositiveInt, withStatics, type DeepMutable } from "@/util/schema"
 import { ConfigAgent } from "./agent"
 import { ConfigCommand } from "./command"
@@ -81,12 +81,10 @@ export const Server = ConfigServer.Server.zod
 export const Layout = ConfigLayout.Layout.zod
 export type Layout = ConfigLayout.Layout
 
-// Schemas that still live at the zod layer (have .transform / .preprocess /
-// .meta not expressible in current Effect Schema) get referenced via a
-// ZodOverride-annotated Schema.Any.  Walker sees the annotation and emits the
-// exact zod directly, preserving component $refs.
-const AgentRef = Schema.Any.annotate({ [ZodOverride]: ConfigAgent.Info })
-const LogLevelRef = Schema.Any.annotate({ [ZodOverride]: Log.Level })
+const LogLevelRef = Schema.Literals(["DEBUG", "INFO", "WARN", "ERROR"]).annotate({
+  identifier: "LogLevel",
+  description: "Log level",
+})
 
 // The Effect Schema is the canonical source of truth. The `.zod` compatibility
 // surface is derived so existing Hono validators keep working without a parallel
@@ -152,27 +150,27 @@ export const Info = Schema.Struct({
   mode: Schema.optional(
     Schema.StructWithRest(
       Schema.Struct({
-        build: Schema.optional(AgentRef),
-        plan: Schema.optional(AgentRef),
+        build: Schema.optional(ConfigAgent.Info),
+        plan: Schema.optional(ConfigAgent.Info),
       }),
-      [Schema.Record(Schema.String, AgentRef)],
+      [Schema.Record(Schema.String, ConfigAgent.Info)],
     ),
   ).annotate({ description: "@deprecated Use `agent` field instead." }),
   agent: Schema.optional(
     Schema.StructWithRest(
       Schema.Struct({
         // primary
-        plan: Schema.optional(AgentRef),
-        build: Schema.optional(AgentRef),
+        plan: Schema.optional(ConfigAgent.Info),
+        build: Schema.optional(ConfigAgent.Info),
         // subagent
-        general: Schema.optional(AgentRef),
-        explore: Schema.optional(AgentRef),
+        general: Schema.optional(ConfigAgent.Info),
+        explore: Schema.optional(ConfigAgent.Info),
         // specialized
-        title: Schema.optional(AgentRef),
-        summary: Schema.optional(AgentRef),
-        compaction: Schema.optional(AgentRef),
+        title: Schema.optional(ConfigAgent.Info),
+        summary: Schema.optional(ConfigAgent.Info),
+        compaction: Schema.optional(ConfigAgent.Info),
       }),
-      [Schema.Record(Schema.String, AgentRef)],
+      [Schema.Record(Schema.String, ConfigAgent.Info)],
     ),
   ).annotate({ description: "Agent configuration, see https://opencode.ai/docs/agents" }),
   provider: Schema.optional(Schema.Record(Schema.String, ConfigProvider.Info)).annotate({
@@ -184,7 +182,7 @@ export const Info = Schema.Struct({
       Schema.Union([
         ConfigMCP.Info,
         // Matches the legacy `{ enabled: false }` form used to disable a server.
-        Schema.Any.annotate({ [ZodOverride]: z.object({ enabled: z.boolean() }).strict() }),
+        Schema.Struct({ enabled: Schema.Boolean }),
       ]),
     ),
   ).annotate({ description: "MCP (Model Context Protocol) server configurations" }),
@@ -362,7 +360,7 @@ export const layer = Layer.effect(
         ),
       )
       const parsed = ConfigParse.jsonc(expanded, source)
-      const data = ConfigParse.schema(Info.zod, normalizeLoadedConfig(parsed, source), source)
+      const data = ConfigParse.effectSchema(Info, normalizeLoadedConfig(parsed, source), source)
       if (!("path" in options)) return data
 
       yield* Effect.promise(() => resolveLoadedPlugins(data, options.path))
@@ -754,13 +752,13 @@ export const layer = Layer.effect(
 
       let next: Info
       if (!file.endsWith(".jsonc")) {
-        const existing = ConfigParse.schema(Info.zod, ConfigParse.jsonc(before, file), file)
+        const existing = ConfigParse.effectSchema(Info, ConfigParse.jsonc(before, file), file)
         const merged = mergeDeep(writable(existing), writable(config))
         yield* fs.writeFileString(file, JSON.stringify(merged, null, 2)).pipe(Effect.orDie)
         next = merged
       } else {
         const updated = patchJsonc(before, writable(config))
-        next = ConfigParse.schema(Info.zod, ConfigParse.jsonc(updated, file), file)
+        next = ConfigParse.effectSchema(Info, ConfigParse.jsonc(updated, file), file)
         yield* fs.writeFileString(file, updated).pipe(Effect.orDie)
       }
 

+ 46 - 2
packages/opencode/src/config/parse.ts

@@ -1,10 +1,12 @@
 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 z from "zod"
+import type { DeepMutable } from "@/util/schema"
 import { InvalidError, JsonError } from "./error"
 
-type Schema<T> = z.ZodType<T>
+type ZodSchema<T> = z.ZodType<T>
 
 export function jsonc(text: string, filepath: string): unknown {
   const errors: JsoncParseError[] = []
@@ -33,7 +35,7 @@ export function jsonc(text: string, filepath: string): unknown {
   return data
 }
 
-export function schema<T>(schema: Schema<T>, data: unknown, source: string): T {
+export function schema<T>(schema: ZodSchema<T>, data: unknown, source: string): T {
   const parsed = schema.safeParse(data)
   if (parsed.success) return parsed.data
 
@@ -42,3 +44,45 @@ export function schema<T>(schema: Schema<T>, data: unknown, source: string): T {
     issues: parsed.error.issues,
   })
 }
+
+export function effectSchema<S extends EffectSchema.Decoder<unknown, never>>(
+  schema: S,
+  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,
+      ],
+    })
+  }
+
+  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)
+
+  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 },
+  )
+}
+
+function topLevelExtraKeys(schema: EffectSchema.Top, data: unknown) {
+  if (typeof data !== "object" || data === null || Array.isArray(data)) return []
+  if (schema.ast._tag !== "Objects" || schema.ast.indexSignatures.length > 0) return []
+  const known = new Set(schema.ast.propertySignatures.map((item) => String(item.name)))
+  return Object.keys(data).filter((key) => !known.has(key))
+}

+ 3 - 34
packages/opencode/src/config/permission.ts

@@ -1,7 +1,6 @@
 export * as ConfigPermission from "./permission"
 import { Schema, SchemaGetter } from "effect"
-import z from "zod"
-import { ZodOverride, zod } from "@/util/effect-zod"
+import { zod } from "@/util/effect-zod"
 import { withStatics } from "@/util/schema"
 
 export const Action = Schema.Literals(["ask", "allow", "deny"])
@@ -20,8 +19,8 @@ export const Rule = Schema.Union([Action, Object])
 export type Rule = Schema.Schema.Type<typeof Rule>
 
 // Known permission keys get explicit types in the Effect schema for generated
-// docs/types. Runtime config parsing uses `InfoZod` below so user key order is
-// preserved for permission precedence.
+// docs/types. Runtime config parsing uses Effect's `propertyOrder: "original"`
+// parse option so user key order is preserved for permission precedence.
 const InputObject = Schema.StructWithRest(
   Schema.Struct({
     read: Schema.optional(Rule),
@@ -53,35 +52,6 @@ const InputSchema = Schema.Union([Action, InputObject])
 const normalizeInput = (input: Schema.Schema.Type<typeof InputSchema>): Schema.Schema.Type<typeof InputObject> =>
   typeof input === "string" ? { "*": input } : input
 
-const InfoZod = z
-  .union([
-    zod(Action),
-    z.intersection(
-      z.record(z.string(), zod(Rule)),
-      z
-        .object({
-          read: zod(Rule).optional(),
-          edit: zod(Rule).optional(),
-          glob: zod(Rule).optional(),
-          grep: zod(Rule).optional(),
-          list: zod(Rule).optional(),
-          bash: zod(Rule).optional(),
-          task: zod(Rule).optional(),
-          external_directory: zod(Rule).optional(),
-          todowrite: zod(Action).optional(),
-          question: zod(Action).optional(),
-          webfetch: zod(Action).optional(),
-          websearch: zod(Action).optional(),
-          codesearch: zod(Action).optional(),
-          lsp: zod(Rule).optional(),
-          doom_loop: zod(Action).optional(),
-          skill: zod(Rule).optional(),
-        })
-        .catchall(zod(Rule)),
-    ),
-  ])
-  .transform(normalizeInput)
-
 export const Info = InputSchema.pipe(
   Schema.decodeTo(InputObject, {
     decode: SchemaGetter.transform(normalizeInput),
@@ -92,7 +62,6 @@ export const Info = InputSchema.pipe(
   }),
 )
   .annotate({ identifier: "PermissionConfig" })
-  .annotate({ [ZodOverride]: InfoZod })
   .pipe(
     // Walker already emits the decodeTo transform into the derived zod (see
     // `encoded()` in effect-zod.ts), so just expose that directly.

+ 60 - 10
packages/opencode/test/config/config.test.ts

@@ -645,6 +645,33 @@ Test agent prompt`,
   })
 })
 
+test("agent markdown permission config preserves user key order", async () => {
+  await using tmp = await tmpdir({
+    init: async (dir) => {
+      const agentDir = path.join(dir, ".opencode", "agent")
+      await fs.mkdir(agentDir, { recursive: true })
+
+      await Filesystem.write(
+        path.join(agentDir, "ordered.md"),
+        `---
+permission:
+  bash: allow
+  "*": deny
+  edit: ask
+---
+Ordered permissions`,
+      )
+    },
+  })
+  await Instance.provide({
+    directory: tmp.path,
+    fn: async () => {
+      const config = await load()
+      expect(Object.keys(config.agent?.ordered?.permission ?? {})).toEqual(["bash", "*", "edit"])
+    },
+  })
+})
+
 test("loads agents from .opencode/agents (plural)", async () => {
   await using tmp = await tmpdir({
     init: async (dir) => {
@@ -1540,6 +1567,29 @@ test("permission config preserves user key order", async () => {
   })
 })
 
+test("Effect config parser preserves permission order while rejecting unknown top-level keys", () => {
+  const config = ConfigParse.effectSchema(
+    Config.Info,
+    {
+      permission: {
+        bash: "allow",
+        "*": "deny",
+        edit: "ask",
+      },
+    },
+    "test",
+  )
+
+  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: [] })
+  }
+})
+
 // MCP config merging tests
 
 test("project config can override MCP server enabled status", async () => {
@@ -2222,8 +2272,8 @@ describe("OPENCODE_CONFIG_CONTENT token substitution", () => {
 // parseManagedPlist unit tests — pure function, no OS interaction
 
 test("parseManagedPlist strips MDM metadata keys", async () => {
-  const config = ConfigParse.schema(
-    Config.Info.zod,
+  const config = ConfigParse.effectSchema(
+    Config.Info,
     ConfigParse.jsonc(
       await ConfigManaged.parseManagedPlist(
         JSON.stringify({
@@ -2250,8 +2300,8 @@ test("parseManagedPlist strips MDM metadata keys", async () => {
 })
 
 test("parseManagedPlist parses server settings", async () => {
-  const config = ConfigParse.schema(
-    Config.Info.zod,
+  const config = ConfigParse.effectSchema(
+    Config.Info,
     ConfigParse.jsonc(
       await ConfigManaged.parseManagedPlist(
         JSON.stringify({
@@ -2270,8 +2320,8 @@ test("parseManagedPlist parses server settings", async () => {
 })
 
 test("parseManagedPlist parses permission rules", async () => {
-  const config = ConfigParse.schema(
-    Config.Info.zod,
+  const config = ConfigParse.effectSchema(
+    Config.Info,
     ConfigParse.jsonc(
       await ConfigManaged.parseManagedPlist(
         JSON.stringify({
@@ -2300,8 +2350,8 @@ test("parseManagedPlist parses permission rules", async () => {
 })
 
 test("parseManagedPlist parses enabled_providers", async () => {
-  const config = ConfigParse.schema(
-    Config.Info.zod,
+  const config = ConfigParse.effectSchema(
+    Config.Info,
     ConfigParse.jsonc(
       await ConfigManaged.parseManagedPlist(
         JSON.stringify({
@@ -2317,8 +2367,8 @@ test("parseManagedPlist parses enabled_providers", async () => {
 })
 
 test("parseManagedPlist handles empty config", async () => {
-  const config = ConfigParse.schema(
-    Config.Info.zod,
+  const config = ConfigParse.effectSchema(
+    Config.Info,
     ConfigParse.jsonc(
       await ConfigManaged.parseManagedPlist(JSON.stringify({ $schema: "https://opencode.ai/config.json" })),
       "test:mobileconfig",