refactor(core): migrate ConfigPermission.Info to Effect Schema canonical (#23740)

This commit is contained in:
Kit Langton
2026-04-21 17:33:13 -04:00
committed by GitHub
parent 2ae64f426b
commit b0f565b74a
9 changed files with 133 additions and 209 deletions

View File

@@ -22,12 +22,6 @@ const Color = Schema.Union([
Schema.Literals(["primary", "secondary", "accent", "success", "warning", "error", "info"]),
])
// ConfigPermission.Info is a zod schema (its `.preprocess(...).transform(...)`
// shape lives outside the Effect Schema type system), so the walker reaches it
// via ZodOverride rather than a pure Schema reference. This preserves the
// `$ref: PermissionConfig` emitted in openapi.json.
const PermissionRef = Schema.Any.annotate({ [ZodOverride]: ConfigPermission.Info })
const AgentSchema = Schema.StructWithRest(
Schema.Struct({
model: Schema.optional(ConfigModelID),
@@ -54,7 +48,7 @@ const AgentSchema = Schema.StructWithRest(
description: "Maximum number of agentic iterations before forcing text-only response",
}),
maxSteps: Schema.optional(PositiveInt).annotate({ description: "@deprecated Use 'steps' field instead." }),
permission: Schema.optional(PermissionRef),
permission: Schema.optional(ConfigPermission.Info),
}),
[Schema.Record(Schema.String, Schema.Any)],
)

View File

@@ -86,7 +86,6 @@ export type Layout = ConfigLayout.Layout
// 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 PermissionRef = Schema.Any.annotate({ [ZodOverride]: ConfigPermission.Info })
const LogLevelRef = Schema.Any.annotate({ [ZodOverride]: Log.Level })
const PositiveInt = Schema.Number.check(Schema.isInt()).check(Schema.isGreaterThan(0))
@@ -198,7 +197,7 @@ export const Info = Schema.Struct({
description: "Additional instruction files or patterns to include",
}),
layout: Schema.optional(ConfigLayout.Layout).annotate({ description: "@deprecated Always uses stretch layout." }),
permission: Schema.optional(PermissionRef),
permission: Schema.optional(ConfigPermission.Info),
tools: Schema.optional(Schema.Record(Schema.String, Schema.Boolean)),
enterprise: Schema.optional(
Schema.Struct({

View File

@@ -1,6 +1,6 @@
export * as ConfigPermission from "./permission"
import { Schema } from "effect"
import { zod, ZodPreprocess } from "@/util/effect-zod"
import { Schema, SchemaGetter } from "effect"
import { zod } from "@/util/effect-zod"
import { withStatics } from "@/util/schema"
export const Action = Schema.Literals(["ask", "allow", "deny"])
@@ -18,21 +18,19 @@ export const Rule = Schema.Union([Action, Object])
.pipe(withStatics((s) => ({ zod: zod(s) })))
export type Rule = Schema.Schema.Type<typeof Rule>
// Captures the user's original property insertion order before Schema.Struct
// canonicalises the object. See the `ZodPreprocess` comment in
// `util/effect-zod.ts` for the full rationale — in short: rule precedence is
// encoded in JSON key order (`evaluate.ts` uses `findLast`, so later keys win)
// and `Schema.StructWithRest` would otherwise drop that order.
const permissionPreprocess = (val: unknown) => {
if (typeof val === "object" && val !== null && !Array.isArray(val)) {
return { __originalKeys: globalThis.Object.keys(val), ...val }
}
return val
}
const ObjectShape = Schema.StructWithRest(
// Known permission keys get explicit types — most are full Rule (either a
// single Action or a per-pattern object), but a handful of tools take no
// sub-target patterns and are Action-only. Unknown keys fall through the
// Record rest signature as Rule.
//
// StructWithRest canonicalises key order on decode (known first, then rest),
// which used to require the `__originalKeys` preprocess hack because
// `Permission.fromConfig` depended on the user's insertion order. That
// dependency is gone — `fromConfig` now sorts top-level keys so wildcard
// permissions come before specifics, making the final precedence
// order-independent.
const InputObject = Schema.StructWithRest(
Schema.Struct({
__originalKeys: Schema.optional(Schema.mutable(Schema.Array(Schema.String))),
read: Schema.optional(Rule),
edit: Schema.optional(Rule),
glob: Schema.optional(Rule),
@@ -53,24 +51,29 @@ const ObjectShape = Schema.StructWithRest(
[Schema.Record(Schema.String, Rule)],
)
const InnerSchema = Schema.Union([ObjectShape, Action]).annotate({
[ZodPreprocess]: permissionPreprocess,
})
// Input the user writes in config: either a single Action (shorthand for "*")
// or an object of per-target rules.
const InputSchema = Schema.Union([Action, InputObject])
// Post-parse: drop the __originalKeys metadata and rebuild the rule map in the
// user's original insertion order. A plain string input (the Action branch of
// the union) becomes `{ "*": action }`.
const transform = (x: unknown): Record<string, Rule> => {
if (typeof x === "string") return { "*": x as Action }
const obj = x as { __originalKeys?: string[] } & Record<string, unknown>
const { __originalKeys, ...rest } = obj
if (!__originalKeys) return rest as Record<string, Rule>
const result: Record<string, Rule> = {}
for (const key of __originalKeys) {
if (key in rest) result[key] = rest[key] as Rule
}
return result
}
// Normalise the Action shorthand into `{ "*": action }`. Object inputs pass
// through untouched.
const normalizeInput = (input: Schema.Schema.Type<typeof InputSchema>): Schema.Schema.Type<typeof InputObject> =>
typeof input === "string" ? { "*": input } : input
export const Info = zod(InnerSchema).transform(transform).meta({ ref: "PermissionConfig" })
export type Info = Record<string, Rule>
export const Info = InputSchema.pipe(
Schema.decodeTo(InputObject, {
decode: SchemaGetter.transform(normalizeInput),
// Not perfectly invertible (we lose whether the user originally typed an
// Action shorthand), but the object form is always a valid representation
// of the same rules.
encode: SchemaGetter.passthrough({ strict: false }),
}),
)
.annotate({ identifier: "PermissionConfig" })
.pipe(
// Walker already emits the decodeTo transform into the derived zod (see
// `encoded()` in effect-zod.ts), so just expose that directly.
withStatics((s) => ({ zod: zod(s) })),
)
type _Info = Schema.Schema.Type<typeof InputObject>
export type Info = { -readonly [K in keyof _Info]: _Info[K] }

View File

@@ -290,8 +290,18 @@ function expand(pattern: string): string {
}
export function fromConfig(permission: ConfigPermission.Info) {
// Sort top-level keys so wildcard permissions (`*`, `mcp_*`) come before
// specific ones. Combined with `findLast` in evaluate(), this gives the
// intuitive semantic "specific tool rules override the `*` fallback"
// regardless of the user's JSON key order. Sub-pattern order inside a
// single permission key is preserved — only top-level keys are sorted.
const entries = Object.entries(permission).sort(([a], [b]) => {
const aWild = a.includes("*")
const bWild = b.includes("*")
return aWild === bWild ? 0 : aWild ? -1 : 1
})
const ruleset: Ruleset = []
for (const [key, value] of Object.entries(permission)) {
for (const [key, value] of entries) {
if (typeof value === "string") {
ruleset.push({ permission: key, action: value, pattern: "*" })
continue

View File

@@ -8,43 +8,6 @@ import z from "zod"
*/
export const ZodOverride: unique symbol = Symbol.for("effect-zod/override")
/**
* Annotation key for a pre-parse transform that runs on the raw input before
* the derived Zod schema validates it. The walker emits
* `z.preprocess(fn, inner)` when this annotation is present.
*
* Models zod's `z.preprocess(fn, schema)` pattern — useful when the schema
* needs to inspect the user's raw input (e.g. to capture insertion order)
* before `Schema.Struct` canonicalises the object.
*
* TODO: This exists to paper over a missing Effect Schema feature. The
* parser canonicalises open struct output (known fields first in
* declaration order, then catchall fields) before any user-defined
* transform sees the value, and there is no pre-parse hook — so the
* user's original property insertion order is gone by the time
* `Schema.decodeTo` or `middlewareDecoding` runs.
*
* That canonicalisation is a reasonable default, but `config/permission.ts`
* encodes rule precedence in the user's JSON key order (`evaluate.ts`
* uses `findLast`, so later entries win), which the canonicalisation
* silently destroys.
*
* The cleanest upstream fix would be either:
*
* 1. A `preserveInputOrder` option on `Schema.Struct` /
* `Schema.StructWithRest` that keeps the input's insertion order in
* the parsed object (opt-in; canonical order stays default).
* 2. A generic pre-parse hook (`Schema.preprocess(schema, fn)` or a
* transformation whose decode receives the raw `unknown`).
*
* Either of those would let us delete `ZodPreprocess` and the
* `__originalKeys` hack. Alternatively, the permission model could move
* to specificity-based precedence (exact keys beat wildcards) or an
* explicit ordered array of rules, which removes the ordering
* dependency at the data-model level.
*/
export const ZodPreprocess: unique symbol = Symbol.for("effect-zod/preprocess")
// AST nodes are immutable and frequently shared across schemas (e.g. a single
// Schema.Class embedded in multiple parents). Memoizing by node identity
// avoids rebuilding equivalent Zod subtrees and keeps derived children stable
@@ -85,11 +48,9 @@ function walkUncached(ast: SchemaAST.AST): z.ZodTypeAny {
const hasTransform = hasEncoding && !(SchemaAST.isOptional(ast) && extractDefault(ast) !== undefined)
const base = hasTransform ? encoded(ast) : body(ast)
const checked = ast.checks?.length ? applyChecks(base, ast.checks, ast) : base
const preprocess = (ast.annotations as { [ZodPreprocess]?: (val: unknown) => unknown } | undefined)?.[ZodPreprocess]
const out = preprocess ? z.preprocess(preprocess, checked) : checked
const desc = SchemaAST.resolveDescription(ast)
const ref = SchemaAST.resolveIdentifier(ast)
const described = desc ? out.describe(desc) : out
const described = desc ? checked.describe(desc) : checked
return ref ? described.meta({ ref }) : described
}

View File

@@ -1495,7 +1495,16 @@ test("merges legacy tools with existing permission config", async () => {
})
})
test("permission config preserves key order", async () => {
test("permission config canonicalises known keys first, preserves rest-key insertion order", async () => {
// ConfigPermission.Info is a StructWithRest schema — the decoder reorders
// keys into declaration-order for known permission names (edit, read,
// todowrite, external_directory are declared in `config/permission.ts`),
// followed by rest keys in the user's insertion order.
//
// Rule precedence is NOT affected by this reordering: `Permission.fromConfig`
// sorts wildcards before specifics before iterating. See the
// "fromConfig - specific key beats wildcard regardless of JSON key order"
// test in test/permission/next.test.ts for the behavioural guarantee.
await using tmp = await tmpdir({
init: async (dir) => {
await Filesystem.write(
@@ -1523,12 +1532,15 @@ test("permission config preserves key order", async () => {
fn: async () => {
const config = await load()
expect(Object.keys(config.permission!)).toEqual([
"*",
"edit",
"write",
"external_directory",
// known fields that the user provided, in declaration order from
// config/permission.ts (read, edit, ..., external_directory, todowrite)
"read",
"edit",
"external_directory",
"todowrite",
// rest keys (not in the known list), in user's insertion order
"*",
"write",
"thoughts_*",
"reasoning_model_*",
"tools_*",

View File

@@ -128,6 +128,67 @@ test("fromConfig - does not expand tilde in middle of path", () => {
expect(result).toEqual([{ permission: "external_directory", pattern: "/some/~/path", action: "allow" }])
})
// Top-level wildcard-vs-specific precedence semantics.
//
// fromConfig sorts top-level keys so wildcard permissions (containing "*")
// come before specific permissions. Combined with `findLast` in evaluate(),
// this gives the intuitive semantic "specific tool rules override the `*`
// fallback", regardless of the order the user wrote the keys in their JSON.
//
// Sub-pattern order inside a single permission key (e.g. `bash: { "*": "allow", "rm": "deny" }`)
// still depends on insertion order — only top-level keys are sorted.
test("fromConfig - specific key beats wildcard regardless of JSON key order", () => {
const wildcardFirst = Permission.fromConfig({ "*": "deny", bash: "allow" })
const specificFirst = Permission.fromConfig({ bash: "allow", "*": "deny" })
// Both orderings produce the same ruleset
expect(wildcardFirst).toEqual(specificFirst)
// And both evaluate bash → allow (bash rule wins over * fallback)
expect(Permission.evaluate("bash", "ls", wildcardFirst).action).toBe("allow")
expect(Permission.evaluate("bash", "ls", specificFirst).action).toBe("allow")
})
test("fromConfig - wildcard acts as fallback for permissions with no specific rule", () => {
const ruleset = Permission.fromConfig({ bash: "allow", "*": "ask" })
expect(Permission.evaluate("edit", "foo.ts", ruleset).action).toBe("ask")
expect(Permission.evaluate("bash", "ls", ruleset).action).toBe("allow")
})
test("fromConfig - top-level ordering: wildcards first, specifics after", () => {
const ruleset = Permission.fromConfig({
bash: "allow",
"*": "ask",
edit: "deny",
"mcp_*": "allow",
})
// wildcards (* and mcp_*) come before specifics (bash, edit)
const permissions = ruleset.map((r) => r.permission)
expect(permissions.slice(0, 2).sort()).toEqual(["*", "mcp_*"])
expect(permissions.slice(2)).toEqual(["bash", "edit"])
})
test("fromConfig - sub-pattern insertion order inside a tool key is preserved (only top-level sorts)", () => {
// Sub-patterns within a single tool key use the documented "`*` first,
// specific patterns after" convention (findLast picks specifics). The
// top-level sort must not touch sub-pattern ordering.
const ruleset = Permission.fromConfig({ bash: { "*": "deny", "git *": "allow" } })
expect(ruleset.map((r) => r.pattern)).toEqual(["*", "git *"])
// * fallback for unknown commands
expect(Permission.evaluate("bash", "rm foo", ruleset).action).toBe("deny")
// specific pattern wins for git commands (it's last, findLast picks it)
expect(Permission.evaluate("bash", "git status", ruleset).action).toBe("allow")
})
test("fromConfig - canonical documented example unchanged", () => {
// Regression guard for the example in docs/permissions.mdx
const ruleset = Permission.fromConfig({ "*": "ask", bash: "allow", edit: "deny" })
expect(Permission.evaluate("bash", "ls", ruleset).action).toBe("allow")
expect(Permission.evaluate("edit", "foo.ts", ruleset).action).toBe("deny")
expect(Permission.evaluate("read", "foo.ts", ruleset).action).toBe("ask")
})
test("fromConfig - expands exact tilde to home directory", () => {
const result = Permission.fromConfig({ external_directory: { "~": "allow" } })
expect(result).toEqual([{ permission: "external_directory", pattern: os.homedir(), action: "allow" }])

View File

@@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"
import { Effect, Schema, SchemaGetter } from "effect"
import z from "zod"
import { zod, ZodOverride, ZodPreprocess } from "../../src/util/effect-zod"
import { zod, ZodOverride } from "../../src/util/effect-zod"
function json(schema: z.ZodTypeAny) {
const { $schema: _, ...rest } = z.toJSONSchema(schema)
@@ -751,119 +751,4 @@ describe("util.effect-zod", () => {
expect(schema.parse({ foo: "hi" })).toEqual({ foo: "hi" })
})
})
describe("ZodPreprocess annotation", () => {
test("preprocess runs on raw input before the inner schema parses", () => {
// Models the permission.ts __originalKeys pattern: capture the original
// insertion order of a user-provided object BEFORE Schema parsing
// canonicalises the keys.
const preprocess = (val: unknown) => {
if (typeof val === "object" && val !== null && !Array.isArray(val)) {
return { __keys: Object.keys(val), ...(val as Record<string, unknown>) }
}
return val
}
const Inner = Schema.Struct({
__keys: Schema.optional(Schema.mutable(Schema.Array(Schema.String))),
a: Schema.optional(Schema.String),
b: Schema.optional(Schema.String),
}).annotate({ [ZodPreprocess]: preprocess })
const schema = zod(Inner)
const parsed = schema.parse({ b: "1", a: "2" }) as {
__keys?: string[]
a?: string
b?: string
}
expect(parsed.__keys).toEqual(["b", "a"])
expect(parsed.a).toBe("2")
expect(parsed.b).toBe("1")
})
test("preprocess does not transform already-shaped input", () => {
// When the user passes an object that already has __keys, preprocess
// returns it unchanged because spreading preserves any existing key.
const preprocess = (val: unknown) => {
if (typeof val === "object" && val !== null && !("__keys" in val)) {
return { __keys: Object.keys(val), ...(val as Record<string, unknown>) }
}
return val
}
const Inner = Schema.Struct({
__keys: Schema.optional(Schema.mutable(Schema.Array(Schema.String))),
a: Schema.optional(Schema.String),
}).annotate({ [ZodPreprocess]: preprocess })
const schema = zod(Inner)
const parsed = schema.parse({ __keys: ["existing"], a: "hi" }) as {
__keys?: string[]
a?: string
}
expect(parsed.__keys).toEqual(["existing"])
})
test("preprocess composes with a union (either object or string)", () => {
// Mirrors permission.ts exactly: input can be either an object (with
// preprocess injecting metadata) or a plain string action.
const Action = Schema.Literals(["ask", "allow", "deny"])
const Obj = Schema.Struct({
__keys: Schema.optional(Schema.mutable(Schema.Array(Schema.String))),
read: Schema.optional(Action),
write: Schema.optional(Action),
})
const preprocess = (val: unknown) => {
if (typeof val === "object" && val !== null && !Array.isArray(val)) {
return { __keys: Object.keys(val), ...(val as Record<string, unknown>) }
}
return val
}
const Inner = Schema.Union([Obj, Action]).annotate({ [ZodPreprocess]: preprocess })
const schema = zod(Inner)
// String branch — passes through preprocess unchanged
expect(schema.parse("allow")).toBe("allow")
// Object branch — __keys injected, preserves order
const parsed = schema.parse({ write: "allow", read: "deny" }) as {
__keys?: string[]
read?: string
write?: string
}
expect(parsed.__keys).toEqual(["write", "read"])
expect(parsed.write).toBe("allow")
expect(parsed.read).toBe("deny")
})
test("JSON Schema output comes from the inner schema — preprocess is runtime-only", () => {
const Inner = Schema.Struct({
a: Schema.optional(Schema.String),
b: Schema.optional(Schema.Number),
}).annotate({ [ZodPreprocess]: (v: unknown) => v })
const shape = json(zod(Inner)) as any
expect(shape.type).toBe("object")
expect(shape.properties.a.type).toBe("string")
expect(shape.properties.b.type).toBe("number")
})
test("identifier + description propagate through the preprocess wrapper", () => {
const Inner = Schema.Struct({
x: Schema.optional(Schema.String),
}).annotate({
identifier: "WithPreproc",
description: "A schema with preprocess",
[ZodPreprocess]: (v: unknown) => v,
})
const schema = zod(Inner)
expect(schema.meta()?.ref).toBe("WithPreproc")
expect(schema.meta()?.description).toBe("A schema with preprocess")
})
test("preprocess inside a struct field applies only to that field", () => {
const Inner = Schema.String.annotate({
[ZodPreprocess]: (v: unknown) => (typeof v === "number" ? String(v) : v),
})
const schema = zod(Schema.Struct({ name: Inner, raw: Schema.Number }))
expect(schema.parse({ name: 42, raw: 7 })).toEqual({ name: "42", raw: 7 })
})
})
})

View File

@@ -1205,8 +1205,8 @@ export type PermissionObjectConfig = {
export type PermissionRuleConfig = PermissionActionConfig | PermissionObjectConfig
export type PermissionConfig =
| PermissionActionConfig
| {
__originalKeys?: Array<string>
read?: PermissionRuleConfig
edit?: PermissionRuleConfig
glob?: PermissionRuleConfig
@@ -1223,9 +1223,8 @@ export type PermissionConfig =
lsp?: PermissionRuleConfig
doom_loop?: PermissionActionConfig
skill?: PermissionRuleConfig
[key: string]: PermissionRuleConfig | Array<string> | PermissionActionConfig | undefined
[key: string]: PermissionRuleConfig | PermissionActionConfig | undefined
}
| PermissionActionConfig
export type AgentConfig = {
model?: string