Merge PR #8553: fix(discovery): honor non-recursive glob in loadFilesFromDir (@roboomp)
This commit is contained in:
@@ -84,6 +84,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed `defaultThinkingLevel: auto` skipping classification for user-invoked `/skill:<name>` 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
|
||||
|
||||
|
||||
@@ -512,6 +512,11 @@ export async function loadFilesFromDir<T>(
|
||||
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 {
|
||||
|
||||
@@ -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",
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user