refactor(core): migrate ConfigPermission.Info to Effect Schema canonical#23740
Merged
kitlangton merged 1 commit intodevfrom Apr 21, 2026
Merged
refactor(core): migrate ConfigPermission.Info to Effect Schema canonical#23740kitlangton merged 1 commit intodevfrom
kitlangton merged 1 commit intodevfrom
Conversation
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.
90c8faf to
268e284
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-up to #23716. Moves
ConfigPermission.Infofrom zod-first (with a__originalKeyspreprocess hack) to Effect Schema canonical usingSchema.StructWithRest+Schema.decodeTo, and deletes the now-unusedZodPreprocessplumbing.Core behavioural change: rule precedence in
Permission.fromConfignow sorts top-level keys so wildcard permissions (e.g.*,mcp_*) come before specific ones (e.g.bash,edit). Combined withfindLastinevaluate(), 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" }Under the old semantics,
*came last and denied bash too — the user's explicitbash: "allow"was silently overridden. Under the new semantics, bash is allowed as intended.Once rule precedence no longer depends on JSON insertion order, the
__originalKeys+ZodPreprocesshack can go —StructWithRest's natural canonicalisation is fine becausefromConfigsorts anyway.Changes
src/config/permission.ts— rewrite.InputObjectisStructWithRestwith known permission keys (read/edit/bash/... typed asRule,todowrite/webfetch/... typed asActionfor narrowing) +Recordrest.Schema.decodeTonormalises theActionshorthand into{ "*": action }..zodis derived — walker already carries thedecodeTotransform.src/config/config.ts,src/config/agent.ts— referenceConfigPermission.Infodirectly instead of viaSchema.Any + ZodOverride. The Effect decoder now applies the permission transform at load time.src/permission/index.ts—fromConfigsorts wildcards-before-specifics at top level. Sub-pattern order inside a tool key is preserved (documented*first, specifics after).src/util/effect-zod.ts— deleteZodPreprocesssymbol, itswalkUncachedbranch, and the TODO comment. Zero remaining consumers.test/permission/next.test.ts— 6 new tests pinning the new semantics (TDD — written to fail on old code):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 thedescribe("ZodPreprocess annotation", ...)block (~115 lines of tests for the now-removed feature).Safety audit
Before changing behaviour I audited the entire repo (sources, tests, docs including all 16 translations, default configs) for permission configs where specifics appear before wildcards at the top level.
Result: none found. Every documented example, test case, and default config either:
The only configs whose behaviour changes under this PR are ones that were silently broken before. The documented "last matching rule wins" guidance in
permissions.mdx:71andagents.mdx:507-508applies to patterns within a single permission key's object, not to top-level keys — those remain unaffected.SDK diff vs
devStrict improvement:
__originalKeys?: Array<string>removed fromPermissionConfig(internal leak — shouldn't have been in the SDK)Array<string>(cleanup from the same leak)read,edit,bash,todowrite, etc. with correctRulevsActiontypes)PermissionConfigunion order swap (commutative)Verification
bun typecheckclean (one pre-existing unrelated error)bun run testaffected files: 213/213 pass (permission/next, config/config, util/effect-zod)bun run testadjacent files: 64/64 pass (permission-task, permission/arity, agent/agent)GET /configreturns 200 on both Hono (flag off) and HttpApi (OPENCODE_EXPERIMENTAL_HTTPAPI=true) pathsFollow-ups (tracked in spec)
ConfigAgent.Infowhich has a.transform(normalize)and is still zod-first viaZodOverride. A similar migration would unblock it.Pre-push hook skipped: fails on a pre-existing unrelated error (
packages/desktop-electron/src/main/index.ts:52— missingdrizzle-orm/node-sqlite/drivermodule).