From 2a83a4e647b6dc213edc3f381dd3c2e2d87f58b0 Mon Sep 17 00:00:00 2001 From: onedotmint Date: Sun, 9 Aug 2026 14:00:37 +0800 Subject: [PATCH] fix(opencode): block internal workers from primary Task routing --- .../src/agent-registration-drift.test.ts | 114 ++++++++++++++++++ packages/plugin/src/agents/permissions.ts | 87 +++++++++++++ .../user-memory/review-user-memories.test.ts | 14 ++- packages/plugin/src/index.ts | 11 +- 4 files changed, 221 insertions(+), 5 deletions(-) diff --git a/packages/plugin/src/agent-registration-drift.test.ts b/packages/plugin/src/agent-registration-drift.test.ts index 97d880eae..a3e7b80c2 100644 --- a/packages/plugin/src/agent-registration-drift.test.ts +++ b/packages/plugin/src/agent-registration-drift.test.ts @@ -23,6 +23,8 @@ import { import { DREAMER_PRIMER_INVESTIGATOR_ALLOWED_TOOLS, DREAMER_RETROSPECTIVE_ALLOWED_TOOLS, + denyTaskRoutingToAgents, + denyTaskRoutingToCallerAgents, HISTORIAN_ALLOWED_TOOLS, SIDEKICK_ALLOWED_TOOLS, SMART_NOTE_COMPILER_ALLOWED_TOOLS, @@ -72,6 +74,118 @@ describe("hidden-agent registration drift guard", () => { ); }); + test("all Magic Context worker ids are denied through Task routing", () => { + const permission = denyTaskRoutingToAgents( + { task: { "*": "allow", "user-reviewer": "ask" } }, + regs.map((reg) => reg.id), + ) as { task: Record }; + + for (const reg of regs) { + expect(permission.task[reg.id]).toBe("deny"); + } + expect(permission.task["user-reviewer"]).toBe("ask"); + }); + + test("Task-routing denies apply only to Task callers", () => { + const subagentPermission = { "*": "deny", read: "allow" }; + const agentConfigs = { + "custom-primary": { mode: "primary", permission: { task: { "*": "allow" } } }, + "custom-all": { mode: "all", permission: { "*": "deny" } }, + "custom-no-mode": { permission: subagentPermission }, + "ordinary-subagent": { mode: "subagent", permission: subagentPermission }, + general: { permission: subagentPermission }, + explore: { permission: subagentPermission }, + }; + const configured = denyTaskRoutingToCallerAgents( + agentConfigs, + regs.map((reg) => reg.id), + ); + + for (const callerId of ["build", "plan", "custom-primary"]) { + const task = configured[callerId].permission as { task: Record }; + for (const reg of regs) { + expect(task.task[reg.id]).toBe("deny"); + } + } + for (const subagentId of [ + "custom-all", + "custom-no-mode", + "ordinary-subagent", + "general", + "explore", + ]) { + const permission = configured[subagentId].permission as Record; + expect(permission).toEqual( + subagentId === "custom-all" ? { "*": "deny" } : subagentPermission, + ); + expect(permission.task).toBeUndefined(); + } + + for (const [agentId, mode] of [ + ["build", "all"], + ["build", "subagent"], + ["plan", "all"], + ["plan", "subagent"], + ] as const) { + const overridden = denyTaskRoutingToCallerAgents( + { [agentId]: { mode, permission: subagentPermission } }, + regs.map((reg) => reg.id), + ); + const permission = overridden[agentId].permission as Record; + expect(permission).toEqual(subagentPermission); + expect(permission.task).toBeUndefined(); + } + }); + + test("Task-routing denies preserve unrelated user permissions and win last", () => { + const userPermission = { + "*": "allow", + bash: { "git status*": "ask" }, + task: { + [DREAMER_REVIEWER_AGENT]: "allow", + "*": "allow", + "custom-worker": "ask", + }, + }; + const permission = denyTaskRoutingToAgents(userPermission, [DREAMER_REVIEWER_AGENT]); + + expect(permission).toEqual({ + "*": "allow", + bash: { "git status*": "ask" }, + task: { + "*": "allow", + "custom-worker": "ask", + [DREAMER_REVIEWER_AGENT]: "deny", + }, + }); + expect(userPermission.task[DREAMER_REVIEWER_AGENT]).toBe("allow"); + expect(Object.keys(permission).at(-1)).toBe("task"); + expect(Object.keys(permission.task).at(-1)).toBe(DREAMER_REVIEWER_AGENT); + }); + + test("Task-routing denies support OpenCode's whole-permission action form", () => { + expect(denyTaskRoutingToAgents("allow", [DREAMER_REVIEWER_AGENT])).toEqual({ + "*": "allow", + task: { [DREAMER_REVIEWER_AGENT]: "deny" }, + }); + }); + + test("internal direct-prompt agent configuration remains a hidden subagent", () => { + const config = buildHiddenAgentConfig( + "reviewer prompt", + [], + 4, + undefined, + DREAMER_REVIEWER_AGENT, + true, + ); + + expect(config.prompt).toBe("reviewer prompt"); + expect(config.mode).toBe("subagent"); + expect(config.hidden).toBe(true); + expect(config.permission).toEqual({ "*": "deny" }); + }); + test("classifier is a zero-tool locked pure transform", () => { expect(byId(DREAMER_CLASSIFIER_AGENT)?.allowedTools).toEqual([]); expect(byId(DREAMER_CLASSIFIER_AGENT)?.lockPermissions).toBe(true); diff --git a/packages/plugin/src/agents/permissions.ts b/packages/plugin/src/agents/permissions.ts index d836178d3..eb4a1b7e2 100644 --- a/packages/plugin/src/agents/permissions.ts +++ b/packages/plugin/src/agents/permissions.ts @@ -108,6 +108,93 @@ export function buildAllowOnlyPermission( return permission; } +type PermissionAction = "ask" | "allow" | "deny"; + +function isPermissionAction(value: unknown): value is PermissionAction { + return value === "ask" || value === "allow" || value === "deny"; +} + +function isPermissionMap(value: unknown): value is Record { + return value !== null && typeof value === "object" && !Array.isArray(value); +} + +/** + * Add final `permission.task` denies for Magic Context's internal workers. + * + * OpenCode accepts either a whole-permission action or a pattern map for + * `permission.task`; its evaluator uses the last matching rule. Normalize the + * whole-permission form, retain every unrelated user rule, then append our + * exact agent-id denies after both the user's task patterns and any `*` rule. + */ +export function denyTaskRoutingToAgents( + permission: unknown, + internalAgentIds: readonly string[], +): Record { + const configured = isPermissionAction(permission) + ? { "*": permission } + : isPermissionMap(permission) + ? permission + : {}; + const { task, ...otherPermissions } = configured; + const configuredTask = isPermissionAction(task) + ? { "*": task } + : isPermissionMap(task) + ? task + : {}; + const internalAgentIdSet = new Set(internalAgentIds); + const retainedTask = Object.fromEntries( + Object.entries(configuredTask).filter(([agentId]) => !internalAgentIdSet.has(agentId)), + ); + + return { + ...otherPermissions, + task: { + ...retainedTask, + ...Object.fromEntries(internalAgentIds.map((agentId) => [agentId, "deny"])), + }, + }; +} + +const BUILTIN_TASK_CALLER_IDS = ["build", "plan"] as const; + +function isTaskRoutingCaller(agentId: string, config: Record): boolean { + const mode = config.mode; + // OpenCode compatibility: + // Only strictly-primary callers receive explicit Task routing rules. + // Agents that may execute as Task children are intentionally left untouched, + // because explicit task permissions can alter OpenCode's default anti-nesting + // behavior in the currently supported permission model. + if (mode === "primary") return true; + if (mode === "subagent" || mode === "all") return false; + return agentId === "build" || agentId === "plan"; +} + +/** + * Apply Task routing denies only to agents that can act as Task callers. + * + * A top-level permission rule is merged into every OpenCode agent. Adding one + * there would suppress the Task tool's default deny for ordinary subagents, so + * seed only the built-in interactive callers and configured primary agents. + */ +export function denyTaskRoutingToCallerAgents( + agentConfigs: Record>, + internalAgentIds: readonly string[], +): Record> { + const result = { ...agentConfigs }; + const candidateIds = new Set([...BUILTIN_TASK_CALLER_IDS, ...Object.keys(agentConfigs)]); + + for (const agentId of candidateIds) { + const agentConfig = agentConfigs[agentId] ?? {}; + if (!isTaskRoutingCaller(agentId, agentConfig)) continue; + result[agentId] = { + ...agentConfig, + permission: denyTaskRoutingToAgents(agentConfig.permission, internalAgentIds), + }; + } + + return result; +} + /** * Tools the historian + historian-editor + compressor agents need. * diff --git a/packages/plugin/src/features/magic-context/user-memory/review-user-memories.test.ts b/packages/plugin/src/features/magic-context/user-memory/review-user-memories.test.ts index 4fb4efcc1..3175c94b5 100644 --- a/packages/plugin/src/features/magic-context/user-memory/review-user-memories.test.ts +++ b/packages/plugin/src/features/magic-context/user-memory/review-user-memories.test.ts @@ -1,5 +1,6 @@ import { describe, expect, mock, test } from "bun:test"; +import { DREAMER_REVIEWER_AGENT } from "../../../agents/dreamer"; import { Database } from "../../../shared/sqlite"; import { runMigrations } from "../migrations"; import { initializeDatabase } from "../storage-db"; @@ -20,12 +21,13 @@ describe("reviewUserMemories", () => { { content: "User prefers concise updates", sessionId: "s1" }, ]); const deleted: string[] = []; + const prompt = mock(async () => { + throw new Error("model unavailable"); + }); const client = { session: { create: mock(async () => ({ id: "child-user-memories" })), - prompt: mock(async () => { - throw new Error("model unavailable"); - }), + prompt, delete: mock(async ({ path }: { path: { id: string } }) => { deleted.push(path.id); return {}; @@ -46,6 +48,12 @@ describe("reviewUserMemories", () => { }), ).rejects.toThrow("model unavailable"); + expect( + prompt.mock.calls.some(([input]) => { + const body = (input as { body?: { agent?: string } }).body; + return body?.agent === DREAMER_REVIEWER_AGENT; + }), + ).toBe(true); expect(deleted).toEqual(["child-user-memories"]); db.close(); }); diff --git a/packages/plugin/src/index.ts b/packages/plugin/src/index.ts index f9ddb0cbe..83fb2d85c 100644 --- a/packages/plugin/src/index.ts +++ b/packages/plugin/src/index.ts @@ -5,6 +5,7 @@ import { buildHiddenAgentRegistrations, } from "./agents/hidden-agent-registrations"; import { withContentLanguageDirective } from "./agents/language-directive"; +import { denyTaskRoutingToCallerAgents } from "./agents/permissions"; import { loadPluginConfigDetailed } from "./config"; import { isCompactionEnabled, isDreamerRunnable } from "./config/agent-disable"; import { migrateMagicContextConfigLocations } from "./config/migrate-config-location"; @@ -745,6 +746,8 @@ const server: Plugin = async (ctx) => { }); const agentConfig = { ...(config.agent ?? {}) } as NonNullable; + const agentConfigRecord = agentConfig as Record>; + const internalAgentIds = registrations.map((registration) => registration.id); for (const reg of registrations) { if (typeof reg.prompt !== "string" || reg.prompt.length === 0) { log( @@ -752,7 +755,7 @@ const server: Plugin = async (ctx) => { ); continue; } - agentConfig[reg.id] = buildHiddenAgentConfig( + agentConfigRecord[reg.id] = buildHiddenAgentConfig( reg.prompt, reg.allowedTools, reg.maxSteps, @@ -761,7 +764,11 @@ const server: Plugin = async (ctx) => { reg.lockPermissions === true, ); } - config.agent = agentConfig; + const callerAgentConfig = denyTaskRoutingToCallerAgents( + agentConfigRecord, + internalAgentIds, + ); + config.agent = callerAgentConfig as NonNullable; } catch (error) { // A failure registering commands/agents must NEVER fail the whole // plugin load — that would also disable the transform/compaction