Skip to content

refactor: simplify task tool subagent filtering #7165

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rekram1-node merged 4 commits into from
Jan 7, 2026
+36 −198
Merged

refactor: simplify task tool subagent filtering #7165

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a89a4a7
refactor: make task tool filter subagents internally
malhashemi Jan 7, 2026
99765f4
refactor: remove redundant filterSubagents tests
malhashemi Jan 7, 2026
8846d0e
refactor: simplify userInvokedAgents to bypassAgentCheck boolean
malhashemi Jan 7, 2026
274c272
Merge branch 'dev' into refactor/task-tool-self-contained-filtering
rekram1-node Jan 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
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
  • Loading branch information
@malhashemi
malhashemi committed Jan 7, 2026
commit 99765f4804947499915b9610e8709e989b04d5cc
160 changes: 8 additions & 152 deletions packages/opencode/test/permission-task.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<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]) => ({
Expand Down Expand Up @@ -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,
Expand All @@ -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")
},
})
})
Expand All @@ -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")
},
})
})
Expand Down