diff --git a/packages/core/src/config/normalize.ts b/packages/core/src/config/normalize.ts index c6c58a65b9f..f858b71c817 100644 --- a/packages/core/src/config/normalize.ts +++ b/packages/core/src/config/normalize.ts @@ -130,7 +130,8 @@ export function normalize(input: unknown): Result { const legacyAgents = mapValues( decodeMap(input.agent, ConfigAgentV1.Info, ["agent"], diagnostics, decodeValue), - (value) => canonical(ConfigAgent.Info, ConfigMigrateV1.migrateAgent(value)), + (value, name) => + canonical(ConfigAgent.Info, ConfigMigrateV1.migrateAgent(value, legacyPermission(input.agent, name))), ) const legacySmallModel = own(input, "small_model") ? decodeValue(Schema.String, input.small_model, ["small_model"], diagnostics) @@ -149,8 +150,13 @@ export function normalize(input: unknown): Result { model: migratedSmallModel, ...legacyAgents.title, } - const modeAgents = mapValues(decodeMap(input.mode, ConfigAgentV1.Info, ["mode"], diagnostics, decodeValue), (value) => - canonical(ConfigAgent.Info, ConfigMigrateV1.migrateAgent({ ...value, mode: "primary" })), + const modeAgents = mapValues( + decodeMap(input.mode, ConfigAgentV1.Info, ["mode"], diagnostics, decodeValue), + (value, name) => + canonical( + ConfigAgent.Info, + ConfigMigrateV1.migrateAgent({ ...value, mode: "primary" }, legacyPermission(input.mode, name)), + ), ) const migratedAgents = mergeMaps(legacyAgents, modeAgents, ["agents"], diagnostics) const nativeAgents = decodeMap(input.agents, ConfigAgent.Info, ["agents"], diagnostics, decodeEncoded) @@ -742,10 +748,10 @@ function mergeMaps( return result } -function mapValues(input: Readonly>, map: (value: A) => unknown) { +function mapValues(input: Readonly>, map: (value: A, key: string) => unknown) { return Object.fromEntries( Object.entries(input).flatMap(([key, value]) => { - const mapped = map(value) + const mapped = map(value, key) return mapped === undefined ? [] : [[key, mapped]] }), ) @@ -795,6 +801,10 @@ function isPlainRecord(value: unknown): value is Record { return prototype === Object.prototype || prototype === null } +function legacyPermission(agents: unknown, name: string) { + return isRecord(agents) && isRecord(agents[name]) ? agents[name].permission : undefined +} + function own(value: Record, key: string) { return Object.hasOwn(value, key) } diff --git a/packages/core/src/config/plugin/agent.ts b/packages/core/src/config/plugin/agent.ts index 1d3949d015e..8bcfc726cdc 100644 --- a/packages/core/src/config/plugin/agent.ts +++ b/packages/core/src/config/plugin/agent.ts @@ -195,9 +195,8 @@ function decode(file: { directory: string; filepath: string; primary: boolean }, : markdown.data const agent = legacy ? Option.getOrUndefined( - Option.map( - decodeLegacyAgent({ name, ...markdown.data, prompt: body }, { errors: "all" }), - ConfigMigrateV1.migrateAgent, + Option.map(decodeLegacyAgent({ name, ...markdown.data, prompt: body }, { errors: "all" }), (agent) => + ConfigMigrateV1.migrateAgent(agent, markdown.data.permission), ), ) : Option.getOrUndefined(decodeAgent({ ...data, system: body }, { errors: "all" })) diff --git a/packages/core/src/v1/config/migrate.ts b/packages/core/src/v1/config/migrate.ts index 28488aa5134..a30dbfff7d7 100644 --- a/packages/core/src/v1/config/migrate.ts +++ b/packages/core/src/v1/config/migrate.ts @@ -113,6 +113,12 @@ function permissions(info?: ConfigPermissionV1.Info, tools?: Readonly (Object.hasOwn(info, key) ? [[key, info[key]]] : []))) +} + // Map v1 permission/tool keys onto their renamed v2 tool actions so migrated rules keep matching. export function normalizeAction(action: string) { if (action === "write" || action === "patch") return "edit" @@ -138,7 +144,7 @@ function agents(info: typeof ConfigV1.Info.Type) { } } -export function migrateAgent(info: ConfigAgentV1.Info) { +export function migrateAgent(info: ConfigAgentV1.Info, permission?: unknown) { const body = { ...info.options, ...(info.temperature === undefined ? {} : { temperature: info.temperature }), @@ -156,7 +162,7 @@ export function migrateAgent(info: ConfigAgentV1.Info) { color: info.color === undefined ? undefined : info.color.startsWith("#") ? info.color : "#aaaaaa", steps: info.steps, disabled: info.disable, - permissions: permissions(info.permission), + permissions: permissions(inInputOrder(info.permission, permission)), }), ), ) diff --git a/packages/core/test/config/agent.test.ts b/packages/core/test/config/agent.test.ts index 6456e6a20be..8f2f9d83f54 100644 --- a/packages/core/test/config/agent.test.ts +++ b/packages/core/test/config/agent.test.ts @@ -119,6 +119,15 @@ permissions: } } + it.live("keeps Markdown legacy permission precedence in source order", () => + Effect.gen(function* () { + const agent = yield* loadMarkdownAgent('permission:\n "*": allow\n bash: ask\n edit: deny') + expect(Permission.evaluate("shell", "ls", agent.permissions).effect).toBe("ask") + expect(Permission.evaluate("edit", "example.txt", agent.permissions).effect).toBe("deny") + expect(Permission.evaluate("webfetch", "*", agent.permissions).effect).toBe("allow") + }), + ) + for (const variant of [undefined, "high"]) { it.live(`loads Markdown legacy temperature ${variant ? "with" : "without"} a separate variant`, () => Effect.gen(function* () { diff --git a/packages/core/test/config/normalization.test.ts b/packages/core/test/config/normalization.test.ts index c7ff66bbef1..e5665fad5a2 100644 --- a/packages/core/test/config/normalization.test.ts +++ b/packages/core/test/config/normalization.test.ts @@ -240,6 +240,30 @@ describe("ConfigNormalize", () => { ]) }) + test.each(["agent", "mode"])("preserves legacy %s permission source order", (key) => { + const permission = { "*": "allow", bash: "ask", custom: "deny", edit: "deny" } + expect(normalized({ [key]: { build: { permission } } }).encoded.agents).toMatchObject({ + build: { + permissions: [ + { action: "*", resource: "*", effect: "allow" }, + { action: "shell", resource: "*", effect: "ask" }, + { action: "custom", resource: "*", effect: "deny" }, + { action: "edit", resource: "*", effect: "deny" }, + ], + }, + }) + expect( + normalized({ [key]: { review: { permission: { "*": "deny", read: "allow" } } } }).encoded.agents, + ).toMatchObject({ + review: { + permissions: [ + { action: "*", resource: "*", effect: "deny" }, + { action: "read", resource: "*", effect: "allow" }, + ], + }, + }) + }) + test("redacts permission resource keys from invalid diagnostics", () => { const result = normalized({ permission: { bash: { "curl -H Authorization:Bearer TOPSECRET *": "bogus" } },