Skip to content

Commit c6b368f

Browse files
authored
refactor(pi-add-dir): use fs glob for file search (#176)
1 parent 97764bf commit c6b368f

6 files changed

Lines changed: 215 additions & 43 deletions

File tree

package-lock.json

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

packages/pi-add-dir/README.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ pi install npm:@henryqw/pi-add-dir
1919
| --- | --- | --- |
2020
| `/dir-add` | command | Add directory; no path opens input. Supports `~`. |
2121
| `/dir-ls` | command | List directories; select one to remove. |
22+
| `/dir-reload` | command | Reload external directory resources. |
2223
| `add_directory` | tool | Add a directory. |
2324
| `search_external_files` | tool | Glob-search added directories. |
2425

@@ -28,4 +29,4 @@ Search uses Node filesystem traversal, skips `.git` and `node_modules`, supports
2829

2930
## State
3031

31-
Pi session entry `add-dir:state` stores added directories; package-managed, do not edit.
32+
Pi session entry `add-dir:state` stores added directories; package-managed, do not edit. Tree navigation restores the active branch's directories and reloads resources when that set changes.

packages/pi-add-dir/extensions/add-dir-helpers.ts

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { readdirSync, readFileSync, realpathSync, statSync } from "node:fs";
2-
import { readdir } from "node:fs/promises";
2+
import { glob } from "node:fs/promises";
33
import { homedir } from "node:os";
44
import * as path from "node:path";
55

@@ -18,6 +18,11 @@ const CONTEXT_FILES = ["AGENTS.md", "CLAUDE.md"] as const;
1818
const SKILL_DIRS = [".pi/skills", ".agents/skills", ".claude/skills"] as const;
1919
const SKIPPED_SEARCH_DIRS = new Set([".git", "node_modules"]);
2020

21+
function isMissingPathError(error: unknown): boolean {
22+
if (!error || typeof error !== "object" || !("code" in error)) return false;
23+
return error.code === "ENOENT" || error.code === "ENOTDIR";
24+
}
25+
2126
export function expandUserPath(input: string): string {
2227
if (input === "~") return homedir();
2328
if (input.startsWith("~/") || input.startsWith(`~${path.sep}`)) return path.join(homedir(), input.slice(2));
@@ -29,16 +34,18 @@ export function resolveDir(input: string, cwd: string): string {
2934
const resolved = path.isAbsolute(expanded) ? expanded : path.resolve(cwd, expanded);
3035
try {
3136
return realpathSync(resolved);
32-
} catch {
33-
return path.resolve(resolved);
37+
} catch (error) {
38+
if (isMissingPathError(error)) return path.resolve(resolved);
39+
throw error;
3440
}
3541
}
3642

3743
export function dirExists(dir: string): boolean {
3844
try {
3945
return statSync(dir).isDirectory();
40-
} catch {
41-
return false;
46+
} catch (error) {
47+
if (isMissingPathError(error)) return false;
48+
throw error;
4249
}
4350
}
4451

@@ -63,7 +70,6 @@ function skillFiles(dir: string): Array<{ name: string; path: string }> {
6370

6471
for (const skillDir of SKILL_DIRS) {
6572
const fullSkillDir = path.join(dir, skillDir);
66-
if (!dirExists(fullSkillDir)) continue;
6773
try {
6874
for (const entry of readdirSync(fullSkillDir, { withFileTypes: true })) {
6975
if (!entry.isDirectory() && !entry.isSymbolicLink()) continue;
@@ -97,7 +103,6 @@ export function collectSkillPaths(dirs: AddedDir[]): string[] {
97103
const paths: string[] = [];
98104
const names = new Set<string>();
99105
for (const dir of dirs) {
100-
if (!dirExists(dir.absolutePath)) continue;
101106
for (const skill of skillFiles(dir.absolutePath)) {
102107
if (names.has(skill.name)) continue;
103108
names.add(skill.name);
@@ -146,33 +151,21 @@ export async function findFiles(
146151

147152
const matchPath = normalizedPattern.includes(path.sep);
148153
const results: string[] = [];
149-
const pending = [root];
150154

151155
signal?.throwIfAborted();
152-
while (pending.length > 0) {
156+
for await (const entry of glob("**/@(*|.*)", {
157+
cwd: root,
158+
withFileTypes: true,
159+
exclude: (entry) => entry.isSymbolicLink() || (entry.isDirectory() && SKIPPED_SEARCH_DIRS.has(entry.name)),
160+
})) {
153161
signal?.throwIfAborted();
154-
const current = pending.pop()!;
155-
let entries;
156-
try {
157-
entries = await readdir(current, { withFileTypes: true });
158-
} catch {
159-
continue;
160-
}
162+
if (!entry.isFile()) continue;
161163

162-
for (const entry of entries) {
163-
signal?.throwIfAborted();
164-
const fullPath = path.join(current, entry.name);
165-
if (entry.isDirectory()) {
166-
if (!SKIPPED_SEARCH_DIRS.has(entry.name)) pending.push(fullPath);
167-
continue;
168-
}
169-
if (!entry.isFile()) continue;
170-
171-
const candidate = matchPath ? path.relative(root, fullPath) : entry.name;
172-
if (!path.matchesGlob(candidate, normalizedPattern)) continue;
173-
results.push(fullPath);
174-
if (results.length >= maxResults) return results;
175-
}
164+
const fullPath = path.join(entry.parentPath, entry.name);
165+
const candidate = matchPath ? path.relative(root, fullPath) : entry.name;
166+
if (!path.matchesGlob(candidate, normalizedPattern)) continue;
167+
results.push(fullPath);
168+
if (results.length >= maxResults) return results;
176169
}
177170

178171
return results;

packages/pi-add-dir/extensions/add-dir.ts

Lines changed: 36 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,21 @@ export default function addDirExtension(pi: ExtensionAPI): void {
117117
}));
118118
}
119119

120-
function reconstructState(ctx: ExtensionContext): void {
120+
function reconstructState(ctx: ExtensionContext): boolean {
121121
currentCwd = ctx.cwd;
122122
const stateEntry = [...ctx.sessionManager.getBranch()]
123123
.reverse()
124124
.find((entry) => entry.type === "custom" && entry.customType === STATE_TYPE);
125-
addedDirs = stateEntry?.type === "custom" ? readState(stateEntry.data) : [];
125+
const nextDirs = stateEntry?.type === "custom" ? readState(stateEntry.data) : [];
126+
const changed =
127+
addedDirs.length !== nextDirs.length ||
128+
addedDirs.some(
129+
(dir, index) =>
130+
dir.absolutePath !== nextDirs[index]?.absolutePath || dir.label !== nextDirs[index]?.label,
131+
);
132+
addedDirs = nextDirs;
126133
updateWidget(ctx);
134+
return changed;
127135
}
128136

129137
function persistState(): void {
@@ -138,17 +146,24 @@ export default function addDirExtension(pi: ExtensionAPI): void {
138146
const input = dirPath.trim();
139147
if (!input) return { ok: false, message: "Directory path must not be blank.", hasNewSkills: false };
140148

141-
const absolutePath = resolveDir(input, cwd);
142-
if (!dirExists(absolutePath)) {
143-
return { ok: false, message: `Directory does not exist: ${absolutePath}`, hasNewSkills: false };
149+
let absolutePath: string;
150+
try {
151+
absolutePath = resolveDir(input, cwd);
152+
if (!dirExists(absolutePath)) {
153+
return { ok: false, message: `Directory does not exist: ${absolutePath}`, hasNewSkills: false };
154+
}
155+
} catch (error) {
156+
const message = error instanceof Error ? error.message : String(error);
157+
return { ok: false, message: `Cannot access directory: ${message}`, hasNewSkills: false };
144158
}
145159
if (addedDirs.some((dir) => dir.absolutePath === absolutePath)) {
146160
return { ok: false, message: `Already added: ${absolutePath}`, hasNewSkills: false };
147161
}
148-
if (isWithinDir(resolveDir(cwd, cwd), absolutePath)) {
162+
const cwdPath = resolveDir(cwd, cwd);
163+
if (isWithinDir(cwdPath, absolutePath) || isWithinDir(absolutePath, cwdPath)) {
149164
return {
150165
ok: false,
151-
message: "Directory is already in current working directory scope.",
166+
message: "Directory overlaps current working directory scope.",
152167
hasNewSkills: false,
153168
};
154169
}
@@ -195,14 +210,26 @@ export default function addDirExtension(pi: ExtensionAPI): void {
195210
return skillPaths.length > 0 ? { skillPaths } : undefined;
196211
});
197212

198-
pi.on("session_start", async (_event, ctx) => reconstructState(ctx));
199-
pi.on("session_tree", async (_event, ctx) => reconstructState(ctx));
213+
pi.on("session_start", async (_event, ctx) => {
214+
reconstructState(ctx);
215+
});
216+
pi.on("session_tree", async (_event, ctx) => {
217+
if (reconstructState(ctx)) pi.sendUserMessage("/dir-reload", { expandPromptTemplates: true });
218+
});
200219

201220
pi.on("before_agent_start", async (event) => {
202221
if (addedDirs.length === 0) return;
203222
return { systemPrompt: event.systemPrompt + buildContextInjection(addedDirs) };
204223
});
205224

225+
pi.registerCommand("dir-reload", {
226+
description: "Reload external directory resources",
227+
handler: async (_args, ctx) => {
228+
await ctx.reload();
229+
return;
230+
},
231+
});
232+
206233
pi.registerCommand("dir-add", {
207234
description: "Add an external directory to this session",
208235
handler: async (args, ctx) => {

packages/pi-add-dir/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@henryqw/pi-add-dir",
3-
"version": "0.1.10",
3+
"version": "0.1.11",
44
"description": "Add external directories to a Pi session with context, skills, and file search.",
55
"keywords": [
66
"pi-package",

packages/pi-add-dir/test/add-dir.test.ts

Lines changed: 152 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,58 @@
11
import assert from "node:assert/strict";
2-
import { mkdtemp, mkdir, rm, writeFile } from "node:fs/promises";
2+
import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises";
33
import { tmpdir } from "node:os";
44
import { join } from "node:path";
55
import test from "node:test";
6+
import type { ExtensionAPI, ExtensionCommandContext, ExtensionContext } from "@earendil-works/pi-coding-agent";
67
import { buildContextInjection, collectSkillPaths, findFiles, scanDirContext } from "../extensions/add-dir-helpers.ts";
8+
import addDirExtension from "../extensions/add-dir.ts";
9+
10+
interface RegisteredTool {
11+
name: string;
12+
execute: (...args: any[]) => Promise<any>;
13+
}
14+
15+
interface RegisteredCommand {
16+
handler: (args: string, ctx: ExtensionCommandContext) => Promise<void>;
17+
}
18+
19+
function loadExtension(): {
20+
commands: Map<string, RegisteredCommand>;
21+
handlers: Map<string, (event: unknown, ctx: ExtensionContext) => unknown>;
22+
sentMessages: Array<{ content: string; expandPromptTemplates?: boolean }>;
23+
tools: Map<string, RegisteredTool>;
24+
} {
25+
const commands = new Map<string, RegisteredCommand>();
26+
const handlers = new Map<string, (event: unknown, ctx: ExtensionContext) => unknown>();
27+
const sentMessages: Array<{ content: string; expandPromptTemplates?: boolean }> = [];
28+
const tools = new Map<string, RegisteredTool>();
29+
addDirExtension({
30+
on(event: string, handler: (event: unknown, ctx: ExtensionContext) => unknown) {
31+
handlers.set(event, handler);
32+
},
33+
registerCommand(name: string, command: RegisteredCommand) {
34+
commands.set(name, command);
35+
},
36+
registerTool(tool: RegisteredTool) {
37+
tools.set(tool.name, tool);
38+
},
39+
appendEntry() {},
40+
sendUserMessage(content: string, options?: { expandPromptTemplates?: boolean }) {
41+
sentMessages.push({ content, expandPromptTemplates: options?.expandPromptTemplates });
42+
},
43+
} as unknown as ExtensionAPI);
44+
return { commands, handlers, sentMessages, tools };
45+
}
46+
47+
function extensionContext(cwd: string, getBranch: () => unknown[], reload = async () => {}): ExtensionContext {
48+
return {
49+
cwd,
50+
hasUI: false,
51+
sessionManager: { getBranch },
52+
ui: { setWidget() {} },
53+
reload,
54+
} as unknown as ExtensionContext;
55+
}
756

857
test("registers external skills without duplicating Pi's skill prompt", async () => {
958
const dir = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
@@ -24,8 +73,110 @@ test("registers external skills without duplicating Pi's skill prompt", async ()
2473
}
2574
});
2675

76+
test("finds files recursively while skipping dependency and Git trees", async () => {
77+
const dir = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
78+
try {
79+
const matches = [join(dir, "src", "main.ts"), join(dir, ".hidden", "config.ts")];
80+
for (const file of [...matches, join(dir, "node_modules", "ignored.ts"), join(dir, ".git", "ignored.ts")]) {
81+
await mkdir(join(file, ".."), { recursive: true });
82+
await writeFile(file, "");
83+
}
84+
85+
assert.deepEqual((await findFiles(dir, "*.ts", 10)).sort(), matches.sort());
86+
} finally {
87+
await rm(dir, { recursive: true, force: true });
88+
}
89+
});
90+
91+
test("ignores symbolic links and their descendants when finding files", async () => {
92+
const root = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
93+
const target = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
94+
try {
95+
const included = join(root, "src", "main.ts");
96+
await mkdir(join(included, ".."), { recursive: true });
97+
await mkdir(join(target, "nested"), { recursive: true });
98+
await writeFile(included, "");
99+
await writeFile(join(target, "nested", "linked.ts"), "");
100+
await symlink(target, join(root, "linked"), process.platform === "win32" ? "junction" : "dir");
101+
102+
assert.deepEqual(await findFiles(root, "*.ts", 10), [included]);
103+
} finally {
104+
await rm(root, { recursive: true, force: true });
105+
await rm(target, { recursive: true, force: true });
106+
}
107+
});
108+
27109
test("returns no results when an external directory disappears", async () => {
28110
const dir = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
29111
await rm(dir, { recursive: true, force: true });
30112
assert.deepEqual(await findFiles(dir, "*.ts", 1), []);
31113
});
114+
115+
test("rejects an ancestor of the current working directory", async () => {
116+
const root = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
117+
try {
118+
const cwd = join(root, "project");
119+
await mkdir(cwd);
120+
const { tools } = loadExtension();
121+
const addDirectory = tools.get("add_directory")!;
122+
123+
await assert.rejects(
124+
addDirectory.execute("call", { path: root }, undefined, undefined, extensionContext(cwd, () => [])),
125+
/overlaps current working directory scope/,
126+
);
127+
} finally {
128+
await rm(root, { recursive: true, force: true });
129+
}
130+
});
131+
132+
test("queues a resource reload when tree navigation changes added directories", async () => {
133+
const external = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
134+
try {
135+
let branch: unknown[] = [
136+
{
137+
type: "custom",
138+
customType: "add-dir:state",
139+
data: { dirs: [{ absolutePath: external, label: "external" }] },
140+
},
141+
];
142+
let reloads = 0;
143+
const { commands, handlers, sentMessages } = loadExtension();
144+
const ctx = extensionContext(process.cwd(), () => branch, async () => {
145+
reloads += 1;
146+
});
147+
148+
await handlers.get("session_start")!({}, ctx);
149+
await handlers.get("session_tree")!({}, ctx);
150+
assert.deepEqual(sentMessages, []);
151+
152+
branch = [];
153+
await handlers.get("session_tree")!({}, ctx);
154+
assert.deepEqual(sentMessages, [{ content: "/dir-reload", expandPromptTemplates: true }]);
155+
await commands.get("dir-reload")!.handler("", ctx as ExtensionCommandContext);
156+
assert.equal(reloads, 1);
157+
} finally {
158+
await rm(external, { recursive: true, force: true });
159+
}
160+
});
161+
162+
test("reports non-missing filesystem failures", { skip: process.platform === "win32" }, async () => {
163+
const root = await mkdtemp(join(tmpdir(), "pi-add-dir-"));
164+
try {
165+
const loop = join(root, "loop");
166+
await symlink("loop", loop);
167+
const { tools } = loadExtension();
168+
169+
await assert.rejects(
170+
tools.get("add_directory")!.execute(
171+
"call",
172+
{ path: loop },
173+
undefined,
174+
undefined,
175+
extensionContext(process.cwd(), () => []),
176+
),
177+
/Cannot access directory:.*(?:ELOOP|too many symbolic links)/i,
178+
);
179+
} finally {
180+
await rm(root, { recursive: true, force: true });
181+
}
182+
});

0 commit comments

Comments
 (0)