diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c9159d46e..5709b68d0 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed `task`-spawned subagents repeating filesystem scans the parent had already completed. `ExecutorOptions` and the `createAgentSession()` call inside `runSubprocess()` did not forward `rules`, `preloadedExtensions`, or the discovered `.omp/tools/` (a.k.a. `LoadedCustomTool[]`) results, so each subagent re-ran `loadCapability()`, `loadSessionExtensions()`, and `discoverAndLoadCustomTools()`. The toolsession now caches each (`session.rules`, `session.extensionsResult`, `session.loadedCustomTools`), `runSubprocess()` threads them through, and `createAgentSession()` accepts a new `preloadedCustomTools` option. `preloadedExtensions` is now shallow-cloned inside the SDK so inline-extension augmentation (autoresearch + custom-tools wrapper) can never bleed back into the caller's array ([#2190](https://github.com/can1357/oh-my-pi/issues/2190)). +- Fixed `task`-spawned subagents repeating filesystem scans the parent had already completed. `ExecutorOptions` and the `createAgentSession()` call inside `runSubprocess()` did not forward `rules`, `preloadedExtensions`, or the discovered `.omp/tools/` paths, so each subagent re-ran `loadCapability()`, `loadSessionExtensions()`, and the full `.omp/tools/` walk. The toolsession now caches `session.rules`, `session.extensionsResult`, and `session.customToolPaths`; `runSubprocess()` threads them through; and `createAgentSession()` accepts a new `preloadedCustomToolPaths` option backed by a new exported `discoverCustomToolPaths()` helper. Custom-tool factories are still re-bound per session so each `CustomToolAPI` (cwd, exec, pushPendingAction, UI) targets the right session — only the FS scan is skipped. `preloadedExtensions` is shallow-cloned inside the SDK so inline-extension augmentation (autoresearch + custom-tools wrapper) cannot bleed back into the caller's array ([#2190](https://github.com/can1357/oh-my-pi/issues/2190)). ## [15.10.8] - 2026-06-09 diff --git a/packages/coding-agent/src/extensibility/custom-tools/loader.ts b/packages/coding-agent/src/extensibility/custom-tools/loader.ts index a25c9a0f2..8f30fd6bd 100644 --- a/packages/coding-agent/src/extensibility/custom-tools/loader.ts +++ b/packages/coding-agent/src/extensibility/custom-tools/loader.ts @@ -66,8 +66,10 @@ async function loadTool( } } -/** Tool path with optional source metadata */ -interface ToolPathWithSource { +/** Tool path with optional source metadata, suitable for forwarding from a + * parent session to a subagent so the subagent can re-bind tools to its own + * `CustomToolAPI` without redoing the filesystem scan. */ +export interface ToolPathWithSource { path: string; source?: { provider: string; providerName: string; level: "user" | "project" }; } @@ -189,26 +191,19 @@ export async function loadCustomTools( } /** - * Discover and load tools from standard locations via capability system: - * 1. User and project tools discovered by capability providers - * 2. Installed plugins (~/.omp/plugins/node_modules/*) - * 3. Explicitly configured paths from settings or CLI + * Collect the absolute tool-source paths to load, without importing or + * binding factories. Hot path on session startup — the scan walks + * `.omp/tools/`, `.claude/tools/`, the plugin tree, and any configured paths. + * + * Subagents reuse the parent's collected paths via the SDK's + * `preloadedCustomToolPaths` option, then call `loadCustomTools` themselves + * so each session re-binds factories with its own session-scoped + * `CustomToolAPI` (cwd, exec, pushPendingAction, UI). * * @param configuredPaths - Explicit paths from settings.json and CLI --tool flags * @param cwd - Current working directory - * @param builtInToolNames - Names of built-in tools to check for conflicts */ -export async function discoverAndLoadCustomTools( - configuredPaths: string[], - cwd: string, - builtInToolNames: string[], - pushPendingAction?: (action: { - label: string; - sourceToolName: string; - apply(reason: string): Promise>; - reject?(reason: string): Promise | undefined>; - }) => void, -) { +export async function discoverCustomToolPaths(configuredPaths: string[], cwd: string): Promise { const allPathsWithSources: ToolPathWithSource[] = []; const seen = new Set(); @@ -241,5 +236,34 @@ export async function discoverAndLoadCustomTools( addPath(resolvePath(configPath, cwd), { provider: "config", providerName: "Config", level: "project" }); } - return loadCustomTools(allPathsWithSources, cwd, builtInToolNames, pushPendingAction); + return allPathsWithSources; +} + +/** + * Discover and load tools from standard locations via capability system: + * 1. User and project tools discovered by capability providers + * 2. Installed plugins (~/.omp/plugins/node_modules/*) + * 3. Explicitly configured paths from settings or CLI + * + * Composed of {@link discoverCustomToolPaths} (FS scan) + {@link loadCustomTools} + * (per-session binding). Subagents skip the first step and just call + * `loadCustomTools` against the parent's collected paths. + * + * @param configuredPaths - Explicit paths from settings.json and CLI --tool flags + * @param cwd - Current working directory + * @param builtInToolNames - Names of built-in tools to check for conflicts + */ +export async function discoverAndLoadCustomTools( + configuredPaths: string[], + cwd: string, + builtInToolNames: string[], + pushPendingAction?: (action: { + label: string; + sourceToolName: string; + apply(reason: string): Promise>; + reject?(reason: string): Promise | undefined>; + }) => void, +) { + const pathsWithSources = await discoverCustomToolPaths(configuredPaths, cwd); + return loadCustomTools(pathsWithSources, cwd, builtInToolNames, pushPendingAction); } diff --git a/packages/coding-agent/src/sdk.ts b/packages/coding-agent/src/sdk.ts index c5db5814a..95b4cf3b2 100644 --- a/packages/coding-agent/src/sdk.ts +++ b/packages/coding-agent/src/sdk.ts @@ -62,13 +62,8 @@ import { type LoadedCustomCommand, loadCustomCommands as loadCustomCommandsInternal, } from "./extensibility/custom-commands"; -import { discoverAndLoadCustomTools } from "./extensibility/custom-tools"; -import type { - CustomTool, - CustomToolContext, - CustomToolSessionEvent, - LoadedCustomTool, -} from "./extensibility/custom-tools/types"; +import { discoverCustomToolPaths, loadCustomTools, type ToolPathWithSource } from "./extensibility/custom-tools"; +import type { CustomTool, CustomToolContext, CustomToolSessionEvent } from "./extensibility/custom-tools/types"; import { discoverAndLoadExtensions, type ExtensionContext, @@ -347,13 +342,17 @@ export interface CreateAgentSessionOptions { */ preloadedExtensions?: LoadExtensionsResult; /** - * Pre-loaded custom tools from `.omp/tools/`, `.claude/tools/`, plugins, etc. - * When provided, the filesystem-scan inside `discoverAndLoadCustomTools()` is - * skipped — subagents inherit the parent's discovery result. MCP/image/tts/ - * web-search tools are still resolved per-session because they depend on the - * session's own model and tool selection. + * Pre-discovered custom-tool source paths from `.omp/tools/`, `.claude/tools/`, + * plugins, etc. When provided, the filesystem-scan inside + * `discoverCustomToolPaths()` is skipped — subagents inherit the parent's + * scan result and call `loadCustomTools()` themselves so each session binds + * tools to its OWN `CustomToolAPI` (cwd, exec, pushPendingAction, UI). + * + * Forwarding the loaded `LoadedCustomTool[]` instances directly would reuse + * the parent's session-bound API and route tool execution back through the + * parent — wrong for isolated tasks and for pending-action routing. */ - preloadedCustomTools?: LoadedCustomTool[]; + preloadedCustomToolPaths?: ToolPathWithSource[]; /** Shared event bus for tool/extension communication. Default: creates new bus. */ eventBus?: EventBus; @@ -1532,27 +1531,28 @@ export async function createAgentSession(options: CreateAgentSessionOptions = {} } // Discover custom tools from `.omp/tools/`, `.claude/tools/`, plugins, etc. - // Subagents reuse the parent's discovery via `preloadedCustomTools` so the - // filesystem scan only runs once per process tree. MCP/image/tts/web-search - // tools above are still resolved per-session because they depend on the - // session's own model and tool selection. + // Subagents reuse the parent's scan via `preloadedCustomToolPaths` to skip + // the FS walk, but ALWAYS re-call `loadCustomTools` here so factories bind + // to THIS session's `CustomToolAPI` (cwd, exec, pushPendingAction, UI). + // Forwarding the parent's `LoadedCustomTool[]` directly would route tool + // execution back through the parent — wrong for isolated tasks and for + // pending-action queueing. const builtInToolNames = builtinTools.map(t => t.name); - const loadedCustomTools: LoadedCustomTool[] = - options.preloadedCustomTools ?? - (await logger.time("discoverAndLoadCustomTools", async () => { - const result = await discoverAndLoadCustomTools([], cwd, builtInToolNames, action => - queueResolveHandler(toolSession, action), - ); - for (const { path, error } of result.errors) { - logger.error("Custom tool load failed", { path, error }); - } - return result.tools; - })); - if (loadedCustomTools.length > 0) { - customTools.push(...loadedCustomTools.map(loaded => loaded.tool)); + const customToolPaths: ToolPathWithSource[] = + options.preloadedCustomToolPaths ?? + (await logger.time("discoverCustomToolPaths", () => discoverCustomToolPaths([], cwd))); + const customToolsLoadResult = await logger.time("loadCustomTools", () => + loadCustomTools(customToolPaths, cwd, builtInToolNames, action => queueResolveHandler(toolSession, action)), + ); + for (const { path, error } of customToolsLoadResult.errors) { + logger.error("Custom tool load failed", { path, error }); } - // Forward the discovered tools to subagents so they skip the FS scan. - toolSession.loadedCustomTools = loadedCustomTools; + if (customToolsLoadResult.tools.length > 0) { + customTools.push(...customToolsLoadResult.tools.map(loaded => loaded.tool)); + } + // Forward the path list (NOT the loaded tools) to subagents so they + // re-bind under their own `CustomToolAPI` while skipping the FS scan. + toolSession.customToolPaths = customToolPaths; const inlineExtensions: ExtensionFactory[] = options.extensions ? [...options.extensions] : []; inlineExtensions.push((await import("./autoresearch")).createAutoresearchExtension); diff --git a/packages/coding-agent/src/task/executor.ts b/packages/coding-agent/src/task/executor.ts index 6c3b3c9c6..a95a92a25 100644 --- a/packages/coding-agent/src/task/executor.ts +++ b/packages/coding-agent/src/task/executor.ts @@ -14,7 +14,8 @@ import { resolveModelOverrideWithAuthFallback } from "../config/model-resolver"; import type { PromptTemplate } from "../config/prompt-templates"; import { Settings } from "../config/settings"; import { SETTINGS_SCHEMA, type SettingPath } from "../config/settings-schema"; -import type { CustomTool, LoadedCustomTool } from "../extensibility/custom-tools/types"; +import type { ToolPathWithSource } from "../extensibility/custom-tools"; +import type { CustomTool } from "../extensibility/custom-tools/types"; import { runExtensionCompact, runExtensionSetModel } from "../extensibility/extensions/compact-handler"; import { getSessionSlashCommands } from "../extensibility/extensions/get-commands-handler"; import type { LoadExtensionsResult } from "../extensibility/extensions/types"; @@ -196,8 +197,12 @@ export interface ExecutorOptions { rules?: Rule[]; /** Parent-loaded extensions, forwarded via `preloadedExtensions` to skip discovery. */ preloadedExtensions?: LoadExtensionsResult; - /** Parent-discovered custom tools, forwarded to skip the `.omp/tools/` scan. */ - preloadedCustomTools?: LoadedCustomTool[]; + /** + * Parent's discovered custom-tool source paths. Forwarded to skip the + * `.omp/tools/` FS scan in the subagent; the subagent then re-binds each + * tool against its own `CustomToolAPI` (cwd, exec, pushPendingAction, UI). + */ + preloadedCustomToolPaths?: ToolPathWithSource[]; mcpManager?: MCPManager; authStorage?: AuthStorage; modelRegistry?: ModelRegistry; @@ -1294,7 +1299,7 @@ export async function runSubprocess(options: ExecutorOptions): Promise { const subagentPrompt = prompt.render(subagentSystemPromptTemplate, { agent: agent.systemPrompt, diff --git a/packages/coding-agent/src/task/index.ts b/packages/coding-agent/src/task/index.ts index 6d3157b25..8a2464edb 100644 --- a/packages/coding-agent/src/task/index.ts +++ b/packages/coding-agent/src/task/index.ts @@ -992,7 +992,7 @@ export class TaskTool implements AgentTool { + let tmp: string; + let toolPath: string; + + beforeAll(async () => { + tmp = await fs.mkdtemp(path.join(os.tmpdir(), "pi-custom-tool-binding-")); + toolPath = path.join(tmp, "echo-cwd.ts"); + // Factory exposes the API it was bound to so the test can inspect it. + await fs.writeFile( + toolPath, + [ + "export default function (api) {", + " return {", + " name: 'echo_cwd_' + api.cwd.replace(/[^a-z0-9]/gi, '_'),", + " description: 'returns the cwd the factory was bound to',", + " params: api.typebox.Type.Object({}),", + " async execute() { return { content: [{ type: 'text', text: api.cwd }] }; },", + " __boundApi: api,", + " };", + "}", + ].join("\n"), + ); + }); + + afterAll(async () => { + await fs.rm(tmp, { recursive: true, force: true }); + }); + + it("binds each load to the cwd passed to loadCustomTools", async () => { + const paths: ToolPathWithSource[] = [{ path: toolPath }]; + const parentResult = await loadCustomTools(paths, "/tmp/parent-cwd", []); + const subagentResult = await loadCustomTools(paths, "/tmp/subagent-cwd", []); + + expect(parentResult.errors).toEqual([]); + expect(subagentResult.errors).toEqual([]); + expect(parentResult.tools).toHaveLength(1); + expect(subagentResult.tools).toHaveLength(1); + + const parentApi = (parentResult.tools[0]?.tool as unknown as { __boundApi: CustomToolAPI }).__boundApi; + const subagentApi = (subagentResult.tools[0]?.tool as unknown as { __boundApi: CustomToolAPI }).__boundApi; + + expect(parentApi.cwd).toBe("/tmp/parent-cwd"); + expect(subagentApi.cwd).toBe("/tmp/subagent-cwd"); + expect(subagentApi).not.toBe(parentApi); + // Different tool instances — a session must never see the other's tool. + expect(subagentResult.tools[0]?.tool).not.toBe(parentResult.tools[0]?.tool); + }); + + it("routes pushPendingAction to the loader's own callback, not a shared one", async () => { + const parentLog: string[] = []; + const subagentLog: string[] = []; + + const parentResult = await loadCustomTools([{ path: toolPath }], "/tmp/parent-cwd", [], action => + parentLog.push(`parent:${action.label}`), + ); + const subagentResult = await loadCustomTools([{ path: toolPath }], "/tmp/subagent-cwd", [], action => + subagentLog.push(`subagent:${action.label}`), + ); + + const parentApi = (parentResult.tools[0]?.tool as unknown as { __boundApi: CustomToolAPI }).__boundApi; + const subagentApi = (subagentResult.tools[0]?.tool as unknown as { __boundApi: CustomToolAPI }).__boundApi; + + // Cast: the test fixture exposes the runtime API verbatim. + parentApi.pushPendingAction({ + label: "ping", + sourceToolName: "echo", + apply: async () => ({ content: [] }), + }); + subagentApi.pushPendingAction({ + label: "ping", + sourceToolName: "echo", + apply: async () => ({ content: [] }), + }); + + expect(parentLog).toEqual(["parent:ping"]); + expect(subagentLog).toEqual(["subagent:ping"]); + }); +}); diff --git a/packages/coding-agent/test/sdk-preloaded-extensions-isolation.test.ts b/packages/coding-agent/test/sdk-preloaded-extensions-isolation.test.ts index dbe533a1b..f940c56ce 100644 --- a/packages/coding-agent/test/sdk-preloaded-extensions-isolation.test.ts +++ b/packages/coding-agent/test/sdk-preloaded-extensions-isolation.test.ts @@ -62,7 +62,7 @@ describe("createAgentSession preloadedExtensions isolation (issue #2190)", () => skipPythonPreflight: true, skills: [], rules: [], - preloadedCustomTools: [], + preloadedCustomToolPaths: [], contextFiles: [], promptTemplates: [], }); diff --git a/packages/coding-agent/test/task/executor-pass-through.test.ts b/packages/coding-agent/test/task/executor-pass-through.test.ts index 629bf0830..a516775ed 100644 --- a/packages/coding-agent/test/task/executor-pass-through.test.ts +++ b/packages/coding-agent/test/task/executor-pass-through.test.ts @@ -7,7 +7,7 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; import type { Rule } from "@oh-my-pi/pi-coding-agent/capability/rule"; import type { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; -import type { LoadedCustomTool } from "@oh-my-pi/pi-coding-agent/extensibility/custom-tools/types"; +import type { ToolPathWithSource } from "@oh-my-pi/pi-coding-agent/extensibility/custom-tools"; import type { LoadExtensionsResult } from "@oh-my-pi/pi-coding-agent/extensibility/extensions/types"; import type { CreateAgentSessionResult } from "@oh-my-pi/pi-coding-agent/sdk"; import * as sdkModule from "@oh-my-pi/pi-coding-agent/sdk"; @@ -94,7 +94,7 @@ describe("runSubprocess parent-discovery pass-through (issue #2190)", () => { vi.restoreAllMocks(); }); - it("forwards rules, preloadedExtensions, and preloadedCustomTools to createAgentSession", async () => { + it("forwards rules, preloadedExtensions, and preloadedCustomToolPaths to createAgentSession", async () => { const session = yieldEmittingSession(); const spy = vi.spyOn(sdkModule, "createAgentSession").mockResolvedValue(createSessionResult(session)); @@ -104,15 +104,15 @@ describe("runSubprocess parent-discovery pass-through (issue #2190)", () => { errors: [], runtime: { sentinel: "runtime" } as unknown, } as unknown as LoadExtensionsResult; - const preloadedCustomTools: LoadedCustomTool[] = [ - { path: "tools/x.ts", resolvedPath: "/abs/tools/x.ts", tool: { name: "x" } } as unknown as LoadedCustomTool, + const preloadedCustomToolPaths: ToolPathWithSource[] = [ + { path: "tools/x.ts", source: { provider: "config", providerName: "Config", level: "project" } }, ]; const result = await runSubprocess({ ...baseOptions, rules, preloadedExtensions, - preloadedCustomTools, + preloadedCustomToolPaths, }); expect(result.exitCode).toBe(0); @@ -121,7 +121,7 @@ describe("runSubprocess parent-discovery pass-through (issue #2190)", () => { // Identity, not equality: passing a clone would defeat the perf fix. expect(forwarded?.rules).toBe(rules); expect(forwarded?.preloadedExtensions).toBe(preloadedExtensions); - expect(forwarded?.preloadedCustomTools).toBe(preloadedCustomTools); + expect(forwarded?.preloadedCustomToolPaths).toBe(preloadedCustomToolPaths); }); it("forwards undefined when the parent has not pre-discovered state", async () => { @@ -134,6 +134,6 @@ describe("runSubprocess parent-discovery pass-through (issue #2190)", () => { const forwarded = spy.mock.calls[0]?.[0]; expect(forwarded?.rules).toBeUndefined(); expect(forwarded?.preloadedExtensions).toBeUndefined(); - expect(forwarded?.preloadedCustomTools).toBeUndefined(); + expect(forwarded?.preloadedCustomToolPaths).toBeUndefined(); }); });