Skip to content

Commit 6607eec

Browse files
BYKclaude
andauthored
perf(init): tighten list-dir hot loop + emit POSIX paths (#815)
## Summary Focused code-audit cleanup of `src/lib/init/tools/list-dir.ts`. No behavior changes on POSIX hosts; Windows hosts now get a consistent wire contract. ## What changed ### Correctness: emit POSIX-separator paths `path.relative(cwd, ...)` returns native separators, so Windows hosts were sending paths like \`src\\app.ts\` on the Mastra wire. Two downstream callers had \`replaceAll(\"\\\\\", \"/\")\` patches to cope — one (\`workflow-inputs.ts::preReadCommonFiles\`, the direct list-dir consumer) is cleaned up here; the other (\`formatters.ts\`, normalizing data from a different source) stays as-is. The existing \`filesystem-tools.test.ts\` already asserted \`src/app.ts\` (POSIX) and would have failed on Windows pre-fix. ### Perf: prune the hot loop Per-entry changes in \`walkDirectory\`: 1. \`dir + NATIVE_SEP + entry.name\` instead of \`path.join(...)\` — manual concat is ~10× faster in V8 since inputs are already clean. 2. \`abs.slice(state.cwdPrefixLen)\` instead of \`path.relative(...)\` — O(1) slice instead of a parse-and-allocate call, since \`dir\` always starts with \`cwd + sep\`. 3. \`cwdPrefixLen\` cached once at walk entry rather than recomputed per dirent. 4. Dropped the redundant mid-loop \`reachedWalkLimit\` call (the function-entry guard handles \`depth\`; an inline \`state.entries.length >= state.maxEntries\` check handles the cap). 5. Same manual-concat fix applied to the recursion call site. Microbench on a 500-entry recursive listing (30 runs, 3 warmup): | Version | p50 | |---|---:| | Before | 1.705 ms | | After | **1.130 ms** | −34% at p50. Absolute numbers are small in both cases because list-dir isn't on a hot path (called once per wizard step), but cleanup stacked on top of the correctness fix. ## New test coverage Added \`test/lib/init/tools/list-dir.test.ts\` with 11 dedicated tests. Pre-PR, list-dir was covered by a single smoke test in \`filesystem-tools.test.ts\`. ## Test plan - [x] \`bunx tsc --noEmit\` — clean - [x] \`bun run lint\` — clean (1 pre-existing markdown.ts warning, unrelated) - [x] \`bun test test/lib/init/\` — **135 pass, 0 fail** (+11 new) - [x] \`bun test --timeout 15000 test/lib test/commands test/types\` — **5664 pass, 0 fail** - [x] \`bun test test/isolated\` — 138 pass - [x] Microbench before/after — −34% p50 on 500-entry fixture 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent e1765bd commit 6607eec

7 files changed

Lines changed: 267 additions & 41 deletions

File tree

AGENTS.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1035,7 +1035,7 @@ mock.module("./some-module", () => ({
10351035
* **AuthError constructor takes reason first, message second**: \`AuthError(reason: AuthErrorReason, message?: string)\` where \`AuthErrorReason\` is \`"not\_authenticated" | "expired" | "invalid"\`. Easy to accidentally swap args as \`new AuthError("Token expired", "expired")\` — the string \`"Token expired"\` gets assigned as \`reason\` (invalid enum value). Tests aren't type-checked (tsconfig excludes them), so TypeScript won't catch this. Correct: \`new AuthError("expired", "Token expired")\`. Default messages exist for each reason, so the second arg is often unnecessary.
10361036
10371037
<!-- lore:019db4d8-2b12-7621-a157-12d34789fa43 -->
1038-
* **DEFAULT\_SKIP\_DIRS prunes dist/build/.next — narrow for build-output scans**: \*\*scan module walker gotchas\*\*: (1) \`DEFAULT\_SKIP\_DIRS\` in \`src/lib/scan/ignore.ts\` prunes broadly: \`node\_modules\`, VCS, plus \`dist\`, \`build\`, \`target\`, \`.next\`, \`.nuxt\`, \`.output\`, \`vendor\`, \`.gradle\`, \`.bundle\`, \`coverage\`, \`.cache\`, \`.turbo\`. When walker starts AT \`dist/\`, skip list doesn't apply to cwd itself; recursing from parent prunes. For \`sourcemap/inject.ts\`, pass narrowed \`alwaysSkipDirs: \["node\_modules"]\` + \`respectGitignore: false\` to scan build outputs. (2) When \`extensions\` is set, walker skips binary NUL-sniff entirely. (3) walkFiles migration checklist — verify 5 knobs: \`respectGitignore\` (default true), \`hidden\` (default true, include), \`alwaysSkipDirs\` (default broad), \`extensions\` (skips NUL-sniff), \`followSymlinks\` (default false). DFS order differs from files-first-then-dirs — verify no ordering contract at call sites.
1038+
* **DEFAULT\_SKIP\_DIRS prunes dist/build/.next — narrow for build-output scans**: \*\*scan module walkFiles migration gotchas\*\*: (1) \`DEFAULT\_SKIP\_DIRS\` in \`src/lib/scan/ignore.ts\` prunes broadly: \`node\_modules\`, VCS, \`dist\`, \`build\`, \`target\`, \`.next\`, \`.nuxt\`, \`.output\`, \`vendor\`, \`.gradle\`, \`.bundle\`, \`coverage\`, \`.cache\`, \`.turbo\`. Walker starting AT \`dist/\` doesn't prune cwd itself. For \`sourcemap/inject.ts\`, pass narrowed \`alwaysSkipDirs: \["node\_modules"]\` + \`respectGitignore: false\`. (2) \`walkFiles\` defaults \`maxFileSize: 256KB\` — silently skips large JS bundles (webpack/rollup/Next.js chunks routinely exceed this). Pass \`maxFileSize: Number.POSITIVE\_INFINITY\` when replacing unlimited \`readdir\` loops. (3) \`walkFiles\` enforces \`path.isAbsolute(opts.cwd)\` and throws on relative paths. CLI users pass \`./dist\`; callers must \`resolvePath(dir)\` before handing to walker. (4) When \`extensions\` is set, walker skips binary NUL-sniff. (5) Migration checklist: verify \`respectGitignore\`, \`hidden\`, \`alwaysSkipDirs\`, \`extensions\`, \`followSymlinks\`, \`maxFileSize\`, absolute-cwd. DFS order differs from files-first-then-dirs.
10391039
10401040
<!-- lore:019dafc7-8ee7-7249-9505-5b32e66fb03c -->
10411041
* **mapFilesConcurrent skips null but not empty arrays — callers must return null for no-op**: Scan/formatter off-by-ones: (1) \`mapFilesConcurrent\` in \`src/lib/scan/concurrent.ts\` filters \`null\` but NOT empty arrays — fires \`onResult\` per empty result. Callers (e.g. \`processEntry\` in \`dsn/code-scanner.ts\`) must return \`null\` (not \`\[]\`) for no-op files (~99% of 10k-file walks). Stream variant filters both. (2) \`collectGlob\`/\`collectGrep\` must NOT forward \`maxResults\` to underlying iterator — collector drains uncapped, sets \`truncated=true\` on overshoot; forwarding stops iterator at exactly N without \`truncated\`. (3) \`filterFields\` in \`formatters/json.ts\` uses dot-notation — property tests must use \`\[a-zA-Z0-9\_]\` charset to avoid ambiguous keys with dots.

src/lib/constants.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,14 @@ export const DEFAULT_SENTRY_HOST = "sentry.io";
2424
/** Default Sentry SaaS URL (control silo for OAuth and region discovery) */
2525
export const DEFAULT_SENTRY_URL = `https://${DEFAULT_SENTRY_HOST}`;
2626

27+
/**
28+
* Name of the JavaScript package directory — used as both a skip target
29+
* when walking project trees (DSN scanner, sourcemap discovery, init
30+
* wizard) and as a path segment when detecting how the CLI itself was
31+
* installed (upgrade detection).
32+
*/
33+
export const NODE_MODULES_DIRNAME = "node_modules";
34+
2735
/** Matches strings that already start with http:// or https:// */
2836
const HAS_PROTOCOL_RE = /^https?:\/\//i;
2937

src/lib/init/tools/list-dir.ts

Lines changed: 45 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,43 +1,50 @@
11
import fs from "node:fs";
22
import path from "node:path";
3+
import { NODE_MODULES_DIRNAME } from "../../constants.js";
4+
import { normalizePath } from "../../scan/index.js";
35
import type { DirEntry, ListDirPayload, ToolResult } from "../types.js";
46
import { safePath } from "./shared.js";
57
import type { InitToolDefinition } from "./types.js";
68

9+
const NATIVE_SEP = path.sep;
10+
711
/**
812
* List files and directories within the workflow sandbox.
13+
*
14+
* Paths in the result are POSIX-normalized (`/`-separated) regardless
15+
* of host OS, so the Mastra agent sees a consistent wire shape.
916
*/
1017
export async function listDir(payload: ListDirPayload): Promise<ToolResult> {
1118
const { cwd, params } = payload;
1219
const targetPath = safePath(cwd, params.path);
1320
const maxDepth = params.maxDepth ?? 3;
1421
const maxEntries = params.maxEntries ?? 500;
1522
const recursive = params.recursive ?? false;
16-
const walkState = {
23+
const state: WalkState = {
1724
cwd,
18-
entries: [] as DirEntry[],
25+
// Cached prefix length used to turn an absolute native path into a
26+
// cwd-relative POSIX path via `abs.slice(cwdPrefixLen)` — O(1) and
27+
// avoids the per-entry `path.relative` allocation.
28+
cwdPrefixLen: cwd.length + 1,
29+
entries: [],
1930
maxDepth,
2031
maxEntries,
2132
recursive,
2233
};
2334

24-
await walkDirectory(targetPath, 0, walkState);
25-
const { entries } = walkState;
26-
return { ok: true, data: { entries } };
35+
await walkDirectory(targetPath, 0, state);
36+
return { ok: true, data: { entries: state.entries } };
2737
}
2838

2939
type WalkState = {
3040
cwd: string;
41+
cwdPrefixLen: number;
3142
entries: DirEntry[];
3243
maxDepth: number;
3344
maxEntries: number;
3445
recursive: boolean;
3546
};
3647

37-
function reachedWalkLimit(state: WalkState, depth: number): boolean {
38-
return state.entries.length >= state.maxEntries || depth > state.maxDepth;
39-
}
40-
4148
async function readDirEntries(dir: string): Promise<fs.Dirent[]> {
4249
try {
4350
return await fs.promises.readdir(dir, { withFileTypes: true });
@@ -46,64 +53,68 @@ async function readDirEntries(dir: string): Promise<fs.Dirent[]> {
4653
}
4754
}
4855

56+
function shouldRecurseInto(entry: fs.Dirent, state: WalkState): boolean {
57+
return (
58+
state.recursive &&
59+
entry.isDirectory() &&
60+
!entry.isSymbolicLink() &&
61+
!entry.name.startsWith(".") &&
62+
entry.name !== NODE_MODULES_DIRNAME
63+
);
64+
}
65+
66+
/**
67+
* Build a `DirEntry` for `entry` sitting inside `dir`. Returns
68+
* `undefined` for symlinks that escape the sandbox.
69+
*
70+
* `dir` is absolute native-separator, guaranteed to start with
71+
* `state.cwd + sep`, so `abs.slice(state.cwdPrefixLen)` yields the
72+
* cwd-relative path without a `path.relative` allocation.
73+
*/
4974
function toDirEntry(
50-
cwd: string,
75+
state: WalkState,
5176
dir: string,
5277
entry: fs.Dirent
5378
): DirEntry | undefined {
54-
const relPath = path.relative(cwd, path.join(dir, entry.name));
79+
const abs = dir + NATIVE_SEP + entry.name;
80+
const relNative = abs.slice(state.cwdPrefixLen);
5581

5682
if (entry.isSymbolicLink()) {
5783
try {
58-
safePath(cwd, relPath);
84+
safePath(state.cwd, relNative);
5985
} catch {
6086
return;
6187
}
6288
}
6389

6490
return {
6591
name: entry.name,
66-
path: relPath,
92+
path: normalizePath(relNative),
6793
type: entry.isDirectory() ? "directory" : "file",
6894
};
6995
}
7096

71-
function shouldRecurseInto(entry: fs.Dirent, state: WalkState): boolean {
72-
return (
73-
state.recursive &&
74-
entry.isDirectory() &&
75-
!entry.isSymbolicLink() &&
76-
!entry.name.startsWith(".") &&
77-
entry.name !== "node_modules"
78-
);
79-
}
80-
8197
async function walkDirectory(
8298
dir: string,
8399
depth: number,
84100
state: WalkState
85101
): Promise<void> {
86-
if (reachedWalkLimit(state, depth)) {
102+
if (depth > state.maxDepth || state.entries.length >= state.maxEntries) {
87103
return;
88104
}
89105

90-
const dirEntries = await readDirEntries(dir);
91-
for (const entry of dirEntries) {
92-
if (reachedWalkLimit(state, depth)) {
106+
for (const entry of await readDirEntries(dir)) {
107+
if (state.entries.length >= state.maxEntries) {
93108
return;
94109
}
95-
96-
const nextEntry = toDirEntry(state.cwd, dir, entry);
110+
const nextEntry = toDirEntry(state, dir, entry);
97111
if (!nextEntry) {
98112
continue;
99113
}
100-
101114
state.entries.push(nextEntry);
102-
if (!shouldRecurseInto(entry, state)) {
103-
continue;
115+
if (shouldRecurseInto(entry, state)) {
116+
await walkDirectory(dir + NATIVE_SEP + entry.name, depth + 1, state);
104117
}
105-
106-
await walkDirectory(path.join(dir, entry.name), depth + 1, state);
107118
}
108119
}
109120

src/lib/init/workflow-inputs.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -103,9 +103,10 @@ export async function preReadCommonFiles(
103103
directory: string,
104104
dirListing: DirEntry[]
105105
): Promise<Record<string, string | null>> {
106-
const listingPaths = new Set(
107-
dirListing.map((entry) => entry.path.replaceAll("\\", "/"))
108-
);
106+
// `listDir` emits POSIX-normalized paths regardless of host OS,
107+
// so `COMMON_CONFIG_FILES` (POSIX) membership checks don't need
108+
// any per-path separator translation.
109+
const listingPaths = new Set(dirListing.map((entry) => entry.path));
109110
const toRead = COMMON_CONFIG_FILES.filter((filePath) =>
110111
listingPaths.has(filePath)
111112
);

src/lib/sourcemap/inject.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
import { readFile, stat } from "node:fs/promises";
99
import { resolve as resolvePath } from "node:path";
10+
import { NODE_MODULES_DIRNAME } from "../constants.js";
1011
import { walkFiles } from "../scan/index.js";
1112
import { EXISTING_DEBUGID_RE, injectDebugId } from "./debug-id.js";
1213

@@ -110,7 +111,7 @@ async function findCompanionMap(jsPath: string): Promise<string | undefined> {
110111
* dropping large JS files would skip debug-ID injection on the
111112
* exact bundles users care about most.
112113
*/
113-
const SOURCEMAP_SKIP_DIRS: readonly string[] = ["node_modules"];
114+
const SOURCEMAP_SKIP_DIRS: readonly string[] = [NODE_MODULES_DIRNAME];
114115

115116
async function discoverFilePairs(
116117
dir: string,

src/lib/upgrade.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ import {
2424
KNOWN_CURL_DIRS,
2525
releaseLock,
2626
} from "./binary.js";
27-
import { CLI_VERSION } from "./constants.js";
27+
import { CLI_VERSION, NODE_MODULES_DIRNAME } from "./constants.js";
2828
import { getInstallInfo, setInstallInfo } from "./db/install-info.js";
2929
import type { ReleaseChannel } from "./db/release-channel.js";
3030
import { attemptDeltaUpgrade, type DeltaResult } from "./delta-upgrade.js";
@@ -237,7 +237,7 @@ export function detectPackageManagerFromPath(): PackageManager | null {
237237
}
238238

239239
const segments = scriptPath.split(sep);
240-
if (!segments.includes("node_modules")) {
240+
if (!segments.includes(NODE_MODULES_DIRNAME)) {
241241
return null;
242242
}
243243

0 commit comments

Comments
 (0)