refactor(core): migrate ConfigPermission.Info to Effect Schema canonical
Follow-up to #23716. Moves ConfigPermission.Info from zod-first (with a preprocess hack) to Effect Schema canonical using Schema.StructWithRest + Schema.decodeTo, and deletes the now-unused ZodPreprocess plumbing. Core change: rule precedence in `Permission.fromConfig` now sorts top-level keys so wildcard permissions (e.g. `*`, `mcp_*`) come before specific ones (e.g. `bash`, `edit`). Combined with `findLast` in evaluate(), this gives the intuitive semantic 'specific tool rules override the `*` fallback' regardless of the user's JSON key order. This silently fixes the previously-broken case `{bash: "allow", "*": "deny"}` (which under the old semantics denied bash because `*` came last). Once rule precedence no longer depends on JSON insertion order, the `__originalKeys` + ZodPreprocess hack can go — StructWithRest's natural canonicalisation is fine because fromConfig sorts anyway. - src/config/permission.ts: rewrite. InputObject is StructWithRest with known permission keys (read/edit/bash/... as Rule, todowrite/webfetch/... as Action-only for type narrowing) + Record rest. Schema.decodeTo normalises the Action shorthand into { "*": action }. .zod is derived — walker already carries the decodeTo transform. - src/config/config.ts, src/config/agent.ts: reference ConfigPermission.Info directly instead of via Schema.Any + ZodOverride. The Effect decoder now applies the permission transform at load time. - src/permission/index.ts: fromConfig sorts wildcards-before-specifics at top level. Sub-pattern order inside a tool key is preserved (documented `*` first, specifics after). - src/util/effect-zod.ts: delete ZodPreprocess symbol, its walkUncached branch, and the TODO comment. Zero remaining consumers. - test/permission/next.test.ts: 6 new tests pinning the new semantics — order-independent precedence, wildcard-as-fallback, sub-pattern order preservation, canonical documented-example regression guard. - test/config/config.test.ts: updated the "preserves key order" test to reflect the new canonical output shape (declaration-order known fields, then input-order rest keys). Behavioural guarantees live in the new permission tests. - test/util/effect-zod.test.ts: delete the ZodPreprocess describe block (~115 lines of tests for the now-removed feature). SDK diff vs dev: - Removed `__originalKeys?: Array<string>` (internal leak). - Catchall cleaned up (no unrelated `Array<string>`). - Known-field types preserved (autocomplete + narrowing). - Only shape change: PermissionConfig union order swap (commutative). Safety audit: no config, test, or doc in the repo (including all 16 translations) exercises the pattern where specifics come before wildcards at the top level. The only configs whose behaviour changes are ones that were silently broken.
This commit is contained in:
Kit Langton
@@ -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_*",
|
||||
|
||||
@@ -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" }])
|
||||
|
||||
@@ -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 })
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user