From a89a4a7c4c943d8d3571180f3f2cd9ad4b28bece Mon Sep 17 00:00:00 2001 From: "M. Adel Alhashemi" Date: Wed, 7 Jan 2026 07:37:49 +0300 Subject: [PATCH 1/3] refactor: make task tool filter subagents internally Move subagent filtering logic from prompt.ts into task.ts, following the same pattern used by skill.ts. This makes the task tool self-contained and removes coupling between files. Changes: - task.ts: Filter agents using ctx?.agent permissions during description generation - prompt.ts: Remove regeneration block and imports (TASK_DESCRIPTION, filterSubagents) - test: Define filterSubagents helper locally instead of importing No functional changes - just cleaner architecture. --- packages/opencode/src/session/prompt.ts | 24 +------------- packages/opencode/src/tool/task.ts | 33 ++++++++++--------- .../opencode/test/permission-task.test.ts | 6 +++- 3 files changed, 24 insertions(+), 39 deletions(-) diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 09155c86e7d..2dc3b259198 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -37,7 +37,7 @@ import { SessionSummary } from "./summary" import { NamedError } from "@opencode-ai/util/error" import { fn } from "@/util/fn" import { SessionProcessor } from "./processor" -import { TaskTool, filterSubagents, TASK_DESCRIPTION } from "@/tool/task" +import { TaskTool } from "@/tool/task" import { Tool } from "@/tool/tool" import { PermissionNext } from "@/permission/next" import { SessionStatus } from "./status" @@ -800,28 +800,6 @@ export namespace SessionPrompt { tools[key] = item } - // Regenerate task tool description with filtered subagents - if (tools.task) { - const all = await Agent.list().then((x) => x.filter((a) => a.mode !== "primary")) - const filtered = filterSubagents(all, input.agent.permission) - - // If no subagents are permitted, remove the task tool entirely - if (filtered.length === 0) { - delete tools.task - } else { - const description = TASK_DESCRIPTION.replace( - "{agents}", - filtered - .map((a) => `- ${a.name}: ${a.description ?? "This subagent should only be called manually by the user."}`) - .join("\n"), - ) - tools.task = { - ...tools.task, - description, - } - } - } - return tools } diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index a30a5a67502..de6195ce70f 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -12,30 +12,33 @@ import { defer } from "@/util/defer" import { Config } from "../config/config" import { PermissionNext } from "@/permission/next" -export { DESCRIPTION as TASK_DESCRIPTION } - -export function filterSubagents(agents: Agent.Info[], ruleset: PermissionNext.Ruleset) { - return agents.filter((a) => PermissionNext.evaluate("task", a.name, ruleset).action !== "deny") -} +const parameters = z.object({ + description: z.string().describe("A short (3-5 words) description of the task"), + prompt: z.string().describe("The task for the agent to perform"), + subagent_type: z.string().describe("The type of specialized agent to use for this task"), + session_id: z.string().describe("Existing Task session to continue").optional(), + command: z.string().describe("The command that triggered this task").optional(), +}) -export const TaskTool = Tool.define("task", async () => { +export const TaskTool = Tool.define("task", async (ctx) => { const agents = await Agent.list().then((x) => x.filter((a) => a.mode !== "primary")) + + // Filter agents by permissions if agent provided + const caller = ctx?.agent + const accessibleAgents = caller + ? agents.filter((a) => PermissionNext.evaluate("task", a.name, caller.permission).action !== "deny") + : agents + const description = DESCRIPTION.replace( "{agents}", - agents + accessibleAgents .map((a) => `- ${a.name}: ${a.description ?? "This subagent should only be called manually by the user."}`) .join("\n"), ) return { description, - parameters: z.object({ - description: z.string().describe("A short (3-5 words) description of the task"), - prompt: z.string().describe("The task for the agent to perform"), - subagent_type: z.string().describe("The type of specialized agent to use for this task"), - session_id: z.string().describe("Existing Task session to continue").optional(), - command: z.string().describe("The command that triggered this task").optional(), - }), - async execute(params, ctx) { + parameters, + async execute(params: z.infer, ctx) { const config = await Config.get() const userInvokedAgents = (ctx.extra?.userInvokedAgents ?? []) as string[] diff --git a/packages/opencode/test/permission-task.test.ts b/packages/opencode/test/permission-task.test.ts index 21a039d12a6..3cf2b0482e8 100644 --- a/packages/opencode/test/permission-task.test.ts +++ b/packages/opencode/test/permission-task.test.ts @@ -1,11 +1,15 @@ import { describe, test, expect } from "bun:test" import type { Agent } from "../src/agent/agent" -import { filterSubagents } from "../src/tool/task" import { PermissionNext } from "../src/permission/next" import { Config } from "../src/config/config" import { Instance } from "../src/project/instance" import { tmpdir } from "./fixture/fixture" +// Helper function for tests - mirrors the filtering logic in task.ts +function filterSubagents(agents: Agent.Info[], ruleset: PermissionNext.Ruleset) { + return agents.filter((a: Agent.Info) => PermissionNext.evaluate("task", a.name, ruleset).action !== "deny") +} + describe("filterSubagents - permission.task filtering", () => { const createRuleset = (rules: Record): PermissionNext.Ruleset => Object.entries(rules).map(([pattern, action]) => ({ From 99765f4804947499915b9610e8709e989b04d5cc Mon Sep 17 00:00:00 2001 From: "M. Adel Alhashemi" Date: Wed, 7 Jan 2026 12:24:04 +0300 Subject: [PATCH 2/3] refactor: remove redundant filterSubagents tests The filterSubagents tests were testing a local helper function that mirrored the production code. The PermissionNext.evaluate tests already cover the same logic, making these tests redundant. - Remove filterSubagents helper and its test block (~130 lines) - Update integration tests to use PermissionNext.evaluate directly - All test coverage preserved through existing evaluate tests --- .../opencode/test/permission-task.test.ts | 160 +----------------- 1 file changed, 8 insertions(+), 152 deletions(-) diff --git a/packages/opencode/test/permission-task.test.ts b/packages/opencode/test/permission-task.test.ts index 3cf2b0482e8..3d592a3d981 100644 --- a/packages/opencode/test/permission-task.test.ts +++ b/packages/opencode/test/permission-task.test.ts @@ -1,151 +1,9 @@ import { describe, test, expect } from "bun:test" -import type { Agent } from "../src/agent/agent" import { PermissionNext } from "../src/permission/next" import { Config } from "../src/config/config" import { Instance } from "../src/project/instance" import { tmpdir } from "./fixture/fixture" -// Helper function for tests - mirrors the filtering logic in task.ts -function filterSubagents(agents: Agent.Info[], ruleset: PermissionNext.Ruleset) { - return agents.filter((a: Agent.Info) => PermissionNext.evaluate("task", a.name, ruleset).action !== "deny") -} - -describe("filterSubagents - permission.task filtering", () => { - const createRuleset = (rules: Record): PermissionNext.Ruleset => - Object.entries(rules).map(([pattern, action]) => ({ - permission: "task", - pattern, - action, - })) - - const mockAgents = [ - { name: "general", mode: "subagent", permission: [], options: {} }, - { name: "code-reviewer", mode: "subagent", permission: [], options: {} }, - { name: "orchestrator-fast", mode: "subagent", permission: [], options: {} }, - { name: "orchestrator-slow", mode: "subagent", permission: [], options: {} }, - ] as Agent.Info[] - - test("returns all agents when permissions config is empty", () => { - const result = filterSubagents(mockAgents, []) - expect(result).toHaveLength(4) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer", "orchestrator-fast", "orchestrator-slow"]) - }) - - test("excludes agents with explicit deny", () => { - const ruleset = createRuleset({ "code-reviewer": "deny" }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["general", "orchestrator-fast", "orchestrator-slow"]) - }) - - test("includes agents with explicit allow", () => { - const ruleset = createRuleset({ - "code-reviewer": "allow", - general: "deny", - }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["code-reviewer", "orchestrator-fast", "orchestrator-slow"]) - }) - - test("includes agents with ask permission (user approval is runtime behavior)", () => { - const ruleset = createRuleset({ - "code-reviewer": "ask", - general: "deny", - }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["code-reviewer", "orchestrator-fast", "orchestrator-slow"]) - }) - - test("includes agents with undefined permission (default allow)", () => { - const ruleset = createRuleset({ - general: "deny", - }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["code-reviewer", "orchestrator-fast", "orchestrator-slow"]) - }) - - test("supports wildcard patterns with deny", () => { - const ruleset = createRuleset({ "orchestrator-*": "deny" }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(2) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer"]) - }) - - test("supports wildcard patterns with allow", () => { - const ruleset = createRuleset({ - "*": "allow", - "orchestrator-fast": "deny", - }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer", "orchestrator-slow"]) - }) - - test("supports wildcard patterns with ask", () => { - const ruleset = createRuleset({ - "orchestrator-*": "ask", - }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(4) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer", "orchestrator-fast", "orchestrator-slow"]) - }) - - test("longer pattern takes precedence over shorter pattern", () => { - const ruleset = createRuleset({ - "orchestrator-*": "deny", - "orchestrator-fast": "allow", - }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer", "orchestrator-fast"]) - }) - - test("edge case: all agents denied", () => { - const ruleset = createRuleset({ "*": "deny" }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(0) - expect(result).toEqual([]) - }) - - test("edge case: mixed patterns with multiple wildcards", () => { - const ruleset = createRuleset({ - "*": "ask", - "orchestrator-*": "deny", - "orchestrator-fast": "allow", - }) - const result = filterSubagents(mockAgents, ruleset) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer", "orchestrator-fast"]) - }) - - test("hidden: true does not affect filtering (hidden only affects autocomplete)", () => { - const agents = [ - { name: "general", mode: "subagent", hidden: true, permission: [], options: {} }, - { name: "code-reviewer", mode: "subagent", hidden: false, permission: [], options: {} }, - { name: "orchestrator", mode: "subagent", permission: [], options: {} }, - ] as Agent.Info[] - - const result = filterSubagents(agents, []) - expect(result).toHaveLength(3) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer", "orchestrator"]) - }) - - test("hidden: true agents can be filtered by permission.task deny", () => { - const agents = [ - { name: "general", mode: "subagent", hidden: true, permission: [], options: {} }, - { name: "orchestrator-coder", mode: "subagent", hidden: true, permission: [], options: {} }, - ] as Agent.Info[] - - const ruleset = createRuleset({ general: "deny" }) - const result = filterSubagents(agents, ruleset) - expect(result).toHaveLength(1) - expect(result.map((a) => a.name)).toEqual(["orchestrator-coder"]) - }) -}) - describe("PermissionNext.evaluate for permission.task", () => { const createRuleset = (rules: Record): PermissionNext.Ruleset => Object.entries(rules).map(([pattern, action]) => ({ @@ -281,12 +139,6 @@ describe("PermissionNext.disabled for task tool", () => { // Integration tests that load permissions from real config files describe("permission.task with real config files", () => { - const mockAgents = [ - { name: "general", mode: "subagent", permission: [], options: {} }, - { name: "code-reviewer", mode: "subagent", permission: [], options: {} }, - { name: "orchestrator-fast", mode: "subagent", permission: [], options: {} }, - ] as Agent.Info[] - test("loads task permissions from opencode.json config", async () => { await using tmp = await tmpdir({ git: true, @@ -304,8 +156,10 @@ describe("permission.task with real config files", () => { fn: async () => { const config = await Config.get() const ruleset = PermissionNext.fromConfig(config.permission ?? {}) - const result = filterSubagents(mockAgents, ruleset) - expect(result.map((a) => a.name)).toEqual(["general", "orchestrator-fast"]) + // general and orchestrator-fast should be allowed, code-reviewer denied + expect(PermissionNext.evaluate("task", "general", ruleset).action).toBe("allow") + expect(PermissionNext.evaluate("task", "orchestrator-fast", ruleset).action).toBe("allow") + expect(PermissionNext.evaluate("task", "code-reviewer", ruleset).action).toBe("deny") }, }) }) @@ -327,8 +181,10 @@ describe("permission.task with real config files", () => { fn: async () => { const config = await Config.get() const ruleset = PermissionNext.fromConfig(config.permission ?? {}) - const result = filterSubagents(mockAgents, ruleset) - expect(result.map((a) => a.name)).toEqual(["general", "code-reviewer"]) + // general and code-reviewer should be ask, orchestrator-* denied + expect(PermissionNext.evaluate("task", "general", ruleset).action).toBe("ask") + expect(PermissionNext.evaluate("task", "code-reviewer", ruleset).action).toBe("ask") + expect(PermissionNext.evaluate("task", "orchestrator-fast", ruleset).action).toBe("deny") }, }) }) From 8846d0e206d328ff8a05026df504d1eb411b5828 Mon Sep 17 00:00:00 2001 From: "M. Adel Alhashemi" Date: Wed, 7 Jan 2026 12:24:11 +0300 Subject: [PATCH 3/3] refactor: simplify userInvokedAgents to bypassAgentCheck boolean Replace the userInvokedAgents array with a simple boolean that only checks the current turn's user message. This prevents the bypass from persisting across the entire session. - Check only last user message instead of all messages - Simplify task.ts permission check to just use bypassAgentCheck - Command subtasks continue to use bypassAgentCheck: true --- packages/opencode/src/session/prompt.ts | 16 +++++++--------- packages/opencode/src/tool/task.ts | 5 ++--- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 2dc3b259198..f891612272c 100644 --- a/packages/opencode/src/session/prompt.ts +++ b/packages/opencode/src/session/prompt.ts @@ -383,7 +383,7 @@ export namespace SessionPrompt { sessionID: sessionID, abort, callID: part.callID, - extra: { userInvokedAgents: [task.agent] }, + extra: { bypassAgentCheck: true }, async metadata(input) { await Session.updatePart({ ...part, @@ -545,11 +545,9 @@ export namespace SessionPrompt { abort, }) - // Track agents explicitly invoked by user via @ autocomplete - const userInvokedAgents = msgs - .filter((m) => m.info.role === "user") - .flatMap((m) => m.parts.filter((p) => p.type === "agent") as MessageV2.AgentPart[]) - .map((p) => p.name) + // Check if user explicitly invoked an agent via @ in this turn + const lastUserMsg = msgs.findLast((m) => m.info.role === "user") + const bypassAgentCheck = lastUserMsg?.parts.some((p) => p.type === "agent") ?? false const tools = await resolveTools({ agent, @@ -557,7 +555,7 @@ export namespace SessionPrompt { model, tools: lastUser.tools, processor, - userInvokedAgents, + bypassAgentCheck, }) if (step === 1) { @@ -646,7 +644,7 @@ export namespace SessionPrompt { session: Session.Info tools?: Record processor: SessionProcessor.Info - userInvokedAgents: string[] + bypassAgentCheck: boolean }) { using _ = log.time("resolveTools") const tools: Record = {} @@ -656,7 +654,7 @@ export namespace SessionPrompt { abort: options.abortSignal!, messageID: input.processor.message.id, callID: options.toolCallId, - extra: { model: input.model, userInvokedAgents: input.userInvokedAgents }, + extra: { model: input.model, bypassAgentCheck: input.bypassAgentCheck }, agent: input.agent.name, metadata: async (val: { title?: string; metadata?: any }) => { const match = input.processor.partFromToolCall(options.toolCallId) diff --git a/packages/opencode/src/tool/task.ts b/packages/opencode/src/tool/task.ts index de6195ce70f..53b501ba91a 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -41,9 +41,8 @@ export const TaskTool = Tool.define("task", async (ctx) => { async execute(params: z.infer, ctx) { const config = await Config.get() - const userInvokedAgents = (ctx.extra?.userInvokedAgents ?? []) as string[] - // Skip permission check when invoked from a command subtask (user already approved by invoking the command) - if (!ctx.extra?.bypassAgentCheck && !userInvokedAgents.includes(params.subagent_type)) { + // Skip permission check when user explicitly invoked via @ or command subtask + if (!ctx.extra?.bypassAgentCheck) { await ctx.ask({ permission: "task", patterns: [params.subagent_type],