diff options
| -rw-r--r-- | packages/opencode/src/session/prompt.ts | 40 | ||||
| -rw-r--r-- | packages/opencode/src/tool/task.ts | 38 | ||||
| -rw-r--r-- | packages/opencode/test/permission-task.test.ts | 156 |
3 files changed, 36 insertions, 198 deletions
diff --git a/packages/opencode/src/session/prompt.ts b/packages/opencode/src/session/prompt.ts index 09155c86e..f89161227 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" @@ -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<string, boolean> processor: SessionProcessor.Info - userInvokedAgents: string[] + bypassAgentCheck: boolean }) { using _ = log.time("resolveTools") const tools: Record<string, AITool> = {} @@ -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) @@ -800,28 +798,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 a30a5a675..53b501ba9 100644 --- a/packages/opencode/src/tool/task.ts +++ b/packages/opencode/src/tool/task.ts @@ -12,35 +12,37 @@ 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<typeof parameters>, 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], diff --git a/packages/opencode/test/permission-task.test.ts b/packages/opencode/test/permission-task.test.ts index 21a039d12..3d592a3d9 100644 --- a/packages/opencode/test/permission-task.test.ts +++ b/packages/opencode/test/permission-task.test.ts @@ -1,147 +1,9 @@ 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" -describe("filterSubagents - permission.task filtering", () => { - const createRuleset = (rules: Record<string, "allow" | "deny" | "ask">): 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<string, "allow" | "deny" | "ask">): PermissionNext.Ruleset => Object.entries(rules).map(([pattern, action]) => ({ @@ -277,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, @@ -300,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") }, }) }) @@ -323,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") }, }) }) |