diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index b5879cf1f..348776bbe 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -84,6 +84,9 @@ ### Fixed - Fixed `defaultThinkingLevel: auto` skipping classification for user-invoked `/skill:` turns, which left the effort stuck on pending `auto`; user-attributed skill prompts now classify like any user turn while agent/autoload injections stay excluded ([#8554](https://github.com/can1357/oh-my-pi/issues/8554)). +### Fixed + +- Fixed custom-tool and other directory discovery recursing into subtrees despite a non-recursive default: `loadFilesFromDir` built a top-level-only glob pattern but never forwarded its `recursive` flag to the native glob (which defaults to recursive), so `~/.codex/tools` scans descended into a Python venv's `site-packages` and imported browser-only frontend assets as tools, crashing startup on an unhandled `window` rejection ([#8552](https://github.com/can1357/oh-my-pi/issues/8552)). ## [17.3.4] - 2026-08-14 diff --git a/packages/coding-agent/src/discovery/helpers.ts b/packages/coding-agent/src/discovery/helpers.ts index 01456ca83..7a14a9169 100644 --- a/packages/coding-agent/src/discovery/helpers.ts +++ b/packages/coding-agent/src/discovery/helpers.ts @@ -512,6 +512,11 @@ export async function loadFilesFromDir( gitignore: true, hidden: false, fileType: FileType.File, + // Thread the caller's non-recursive intent explicitly: the native glob + // defaults `recursive` to true and rewrites `*.{ts,js}` -> `**/*.{ts,js}`, + // which would walk the entire subtree (e.g. a venv's site-packages under + // ~/.codex/tools) and import arbitrary frontend assets as tools (#8552). + recursive, }); matches = result.matches; } catch { diff --git a/packages/coding-agent/test/discovery/helpers.test.ts b/packages/coding-agent/test/discovery/helpers.test.ts index 422bc57b3..7755948ba 100644 --- a/packages/coding-agent/test/discovery/helpers.test.ts +++ b/packages/coding-agent/test/discovery/helpers.test.ts @@ -1,5 +1,11 @@ -import { describe, expect, test } from "bun:test"; -import { parseFrontmatter } from "@oh-my-pi/pi-utils"; +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { clearCache } from "@oh-my-pi/pi-coding-agent/capability/fs"; +import type { LoadContext } from "@oh-my-pi/pi-coding-agent/capability/types"; +import { loadFilesFromDir } from "@oh-my-pi/pi-coding-agent/discovery/helpers"; +import { parseFrontmatter, removeSyncWithRetries } from "@oh-my-pi/pi-utils"; describe("parseFrontmatter", () => { const parse = (content: string) => parseFrontmatter(content, { source: "tests:frontmatter", level: "off" }); @@ -147,3 +153,53 @@ Body content`; expect(result.body).toBe("Body content"); }); }); + +describe("loadFilesFromDir recursion", () => { + let tempDir!: string; + let ctx!: LoadContext; + + const write = (rel: string, content: string) => { + const full = path.join(tempDir, rel); + fs.mkdirSync(path.dirname(full), { recursive: true }); + fs.writeFileSync(full, content); + }; + + beforeEach(() => { + clearCache(); + tempDir = fs.mkdtempSync(path.join(os.tmpdir(), "pi-loadfiles-recursion-")); + ctx = { cwd: tempDir, home: tempDir, repoRoot: tempDir }; + // Top-level tool plus a Python-venv-style frontend asset nested below it, + // mirroring the ~/.codex/tools/mineru/Lib/site-packages layout from #8552. + write("my-tool.ts", "export default () => ({});\n"); + write( + path.join("mineru", "Lib", "site-packages", "gradio", "assets", "svelte", "media-query-D37ajmZt.js"), + "window.matchMedia;\n", + ); + }); + + afterEach(() => { + clearCache(); + removeSyncWithRetries(tempDir); + }); + + const names = (dir: string, recursive?: boolean) => + loadFilesFromDir<{ name: string }>(ctx, dir, "test", "user", { + extensions: ["ts", "js"], + recursive, + transform: (_name, _content, filePath) => ({ name: path.relative(dir, filePath) }), + }).then(r => r.items.map(i => i.name).sort()); + + // Regression for #8552: the non-recursive default must NOT descend into the + // venv subtree. The native glob defaults recursive=true, so before the fix + // `*.{ts,js}` was rewritten to `**/*.{ts,js}` and imported the Svelte asset. + test("default scan stays top-level and skips the venv subtree", async () => { + expect(await names(tempDir)).toEqual(["my-tool.ts"]); + }); + + test("recursive:true still walks the whole subtree", async () => { + expect(await names(tempDir, true)).toEqual([ + path.join("mineru", "Lib", "site-packages", "gradio", "assets", "svelte", "media-query-D37ajmZt.js"), + "my-tool.ts", + ]); + }); +});