Skip to content

Commit 1351f05

Browse files
committed
Stop asking the operator to approve manage_tasks
manage_tasks's handler has no side effect of its own — it parses its arguments and returns a fixed string. The task list it appears to control is actually mutated earlier, by the director's decide() loop at the tool_call event, before this tool ever executes. By the time an approval prompt for it would reach the operator, there is nothing left for a denial to undo, the same reasoning that already exempts read-only tools like lsp from approval.
1 parent a42f832 commit 1351f05

3 files changed

Lines changed: 28 additions & 6 deletions

File tree

src/permission/classify.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,16 @@ import type { RootsProvider } from "./worktree-roots.js";
1414
// Read-only tools never need approval as long as they don't touch a restricted
1515
// path; they cannot change the workspace. `lsp` is included here even though
1616
// it is activated dynamically mid-session (see director.ts onActivateTools) —
17-
// hover/definition/reference lookups are as inert as a grep. Every other posix
18-
// tool is consequential and defaults to the "ask" tier. Catastrophic commands
19-
// are denied earlier by the authorization plugin, so they never reach here.
20-
const READ_ONLY_TOOLS = new Set(["read_file", "search_files", "grep", "list_dir", "lsp"]);
17+
// hover/definition/reference lookups are as inert as a grep. `manage_tasks` is
18+
// included for a related but distinct reason: its handler (src/agent/tools.ts)
19+
// has no side effect of its own — the task list is mutated earlier, by the
20+
// director's decide() loop at the tool_call event, before this tool ever
21+
// executes (see applyManageTasksToolCall in src/agent/director.ts). By the
22+
// time an operator would see an approval prompt for it, there is nothing left
23+
// for a denial to prevent. Every other posix tool is consequential and
24+
// defaults to the "ask" tier. Catastrophic commands are denied earlier by the
25+
// authorization plugin, so they never reach here.
26+
const READ_ONLY_TOOLS = new Set(["read_file", "search_files", "grep", "list_dir", "lsp", "manage_tasks"]);
2127

2228
// Tools that take a single path-like argument the gate should check against
2329
// restriction (outside the workspace boundary, or writes under the session state root).

src/permission/gate.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,6 @@ const AUTO_ALLOWED_TOOLS = new Set([
175175
"write_file",
176176
"edit_file",
177177
"delete_file",
178-
"manage_tasks",
179178
"manage_goal",
180179
"present",
181180
"tool_search",

src/permission/permission.test.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -985,13 +985,30 @@ describe("createPermissionGate", () => {
985985
const editVerdict = await gate.evaluate({ id: "c", name: "edit_file", arguments: { path: "src/a.ts" } });
986986
expect(editVerdict.allowed).toBe(true);
987987
// Benign built-ins a hands-off run should not stop for.
988-
for (const name of ["manage_tasks", "present", "tool_search", "use_skill", "search_agents", "task"]) {
988+
for (const name of ["present", "tool_search", "use_skill", "search_agents", "task"]) {
989989
const verdict = await gate.evaluate({ id: "c", name, arguments: {} });
990990
expect(verdict.allowed).toBe(true);
991991
}
992992
expect(asked).toBe(0);
993993
});
994994

995+
// manage_tasks's handler has no side effect — the task list is mutated
996+
// earlier by the director, before this tool ever executes — so denying it
997+
// cannot undo anything. It auto-allows unconditionally, not just in auto
998+
// mode, unlike the tools above.
999+
test("manage_tasks auto-allows outside auto mode too", async () => {
1000+
let asked = 0;
1001+
const gate = createPermissionGate({
1002+
approvals: [],
1003+
requestApproval: async () => { asked++; return { allow: false }; },
1004+
interactive: true,
1005+
skipPermissions: false,
1006+
});
1007+
const verdict = await gate.evaluate({ id: "c", name: "manage_tasks", arguments: {} });
1008+
expect(verdict.allowed).toBe(true);
1009+
expect(asked).toBe(0);
1010+
});
1011+
9951012
test("auto mode routes MCP tools to the operator prompt rather than blanket-allow", async () => {
9961013
let asked = 0;
9971014
const gate = createPermissionGate({

0 commit comments

Comments
 (0)