diff --git a/packages/coding-agent/src/tools/index.ts b/packages/coding-agent/src/tools/index.ts index 9ec7fe955..6b5256433 100644 --- a/packages/coding-agent/src/tools/index.ts +++ b/packages/coding-agent/src/tools/index.ts @@ -527,9 +527,11 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P } // Auto-learn tools are gated by `autolearn.enabled` but, like the memory // tools above, must also be force-included into an explicit requestedTools - // list so a restricted session whose controller/guidance is active still - // exposes the tools the nudge points at. - if (session.settings.get("autolearn.enabled")) { + // list so a restricted top-level session whose controller/guidance is + // active still exposes the tools the nudge points at. Gated to top-level + // (taskDepth 0): the controller only runs there, so a subagent's explicit + // tool whitelist must never be silently widened with write-capable tools. + if (session.settings.get("autolearn.enabled") && (session.taskDepth ?? 0) === 0) { if (!requestedTools.includes("manage_skill")) requestedTools.push("manage_skill"); if ( ["hindsight", "mnemopi", "local"].includes(session.settings.get("memory.backend") ?? "") && @@ -571,10 +573,11 @@ export async function createTools(session: ToolSession, toolNames?: string[]): P if (name === "retain" || name === "recall" || name === "reflect") { return ["hindsight", "mnemopi"].includes(session.settings.get("memory.backend") ?? ""); } - if (name === "manage_skill") return session.settings.get("autolearn.enabled"); + if (name === "manage_skill") return session.settings.get("autolearn.enabled") && (session.taskDepth ?? 0) === 0; if (name === "learn") { return ( session.settings.get("autolearn.enabled") && + (session.taskDepth ?? 0) === 0 && ["hindsight", "mnemopi", "local"].includes(session.settings.get("memory.backend") ?? "") ); } diff --git a/packages/coding-agent/test/autolearn-tools-gating.test.ts b/packages/coding-agent/test/autolearn-tools-gating.test.ts index bb59f968c..ab98f7288 100644 --- a/packages/coding-agent/test/autolearn-tools-gating.test.ts +++ b/packages/coding-agent/test/autolearn-tools-gating.test.ts @@ -9,8 +9,6 @@ import { createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools"; import { LearnTool } from "@oh-my-pi/pi-coding-agent/tools/learn"; import { ManageSkillTool } from "@oh-my-pi/pi-coding-agent/tools/manage-skill"; -Bun.env.PI_PYTHON_SKIP_CHECK = "1"; - function makeSession( settingsOverrides: Partial> = {}, extra: Partial = {}, @@ -27,10 +25,6 @@ function makeSession( } describe("autolearn tool gating", () => { - afterEach(() => { - spyOn(os, "homedir").mockRestore(); - }); - it("offers neither tool by default (autolearn disabled)", async () => { const names = (await createTools(makeSession())).map(t => t.name); expect(names).not.toContain("learn"); @@ -72,6 +66,25 @@ describe("autolearn tool gating", () => { expect(noBackend).not.toContain("learn"); }); + it("excludes the tools from a subagent even with an explicit list", async () => { + // taskDepth > 0: the controller never runs here, so a subagent's explicit + // whitelist must not be silently widened with write-capable tools. + const sub = ( + await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "mnemopi" }, { taskDepth: 1 }), [ + "read", + ]) + ).map(t => t.name); + expect(sub).not.toContain("manage_skill"); + expect(sub).not.toContain("learn"); + + // Nor via discovery (no explicit list) at depth. + const subDiscovered = ( + await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "mnemopi" }, { taskDepth: 1 })) + ).map(t => t.name); + expect(subDiscovered).not.toContain("manage_skill"); + expect(subDiscovered).not.toContain("learn"); + }); + it("offers learn with the file-based local backend", async () => { const names = (await createTools(makeSession({ "autolearn.enabled": true, "memory.backend": "local" }))).map( t => t.name,