From 34c7f106d904fe0edce7fc7f6a22c874cb8ca934 Mon Sep 17 00:00:00 2001
From: metaphorics <152830360+metaphorics@users.noreply.github.com>
Date: Sun, 14 Jun 2026 12:36:31 +0900
Subject: [PATCH] fix(coding-agent): restrict auto-learn tools to top-level
 sessions

The `manage_skill`/`learn` force-include and `isToolAllowed` gates only
checked `autolearn.enabled`, not session depth. A subagent created with an
explicit `tools:[...]` whitelist (which runs `approvalMode: "yolo"`) would
silently gain write-capable tools that can mutate `~/.omp/agent/managed-skills`.
The auto-learn controller only runs for top-level sessions, so gate both the
force-include and `isToolAllowed` for `manage_skill`/`learn` to `taskDepth === 0`.

Also tidies the gating test file: drop a module-level `Bun.env` mutation that
was never restored (the test runner skips the Python preflight already) and a
no-op `afterEach` homedir spy in a block that never mocks homedir. Adds a
subagent-exclusion test.

Addresses review threads on PR #2542 (threads 2, 3, 8, 16).
---
 packages/coding-agent/src/tools/index.ts      | 11 +++++---
 .../test/autolearn-tools-gating.test.ts       | 25 ++++++++++++++-----
 2 files changed, 26 insertions(+), 10 deletions(-)

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<Record<SettingPath, unknown>> = {},
 	extra: Partial<ToolSession> = {},
@@ -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,