Skip to content

Commit 33463ef

Browse files
ceilf6claude
andcommitted
fix(mcp-file): clamp list_directory maxDepth and add maxEntries budget
- Reject non-positive-integer maxDepth/maxEntries with a clear error - Clamp maxDepth to 10 and maxEntries to 2000 - Stop collecting once the entry budget is exhausted; keep counting and report truncated + omittedEntries, mirroring the filesense navigate truncation pattern - Add regression tests covering depth clamp, invalid inputs, flat and recursive truncation, and the maxEntries hard limit Closes ceilf6#274 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent d3314c2 commit 33463ef

3 files changed

Lines changed: 544 additions & 15 deletions

File tree

Lines changed: 322 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,322 @@
1+
# mcp-file list_directory maxDepth Clamp + Entry Budget Implementation Plan
2+
3+
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
4+
5+
**Goal:** Clamp `maxDepth`, add a `maxEntries` budget with truncation metadata to the `list_directory` tool so it can never produce unbounded MCP responses (Issue #274).
6+
7+
**Architecture:** All logic lives in `packages/mcp-file/src/tools/list-directory.ts`. Input normalization rejects non-positive-integer `maxDepth`/`maxEntries` and clamps them to hard limits (depth ≤ 10, entries ≤ 2000). Traversal carries a shared mutable budget; once exhausted it stops collecting but keeps counting omitted entries (within the same depth/ignore rules) so the result can report `truncated: true` plus `omittedEntries`, mirroring the filesense `navigate` pattern and the `search_code` `truncated` flag. `server.ts` passes args straight through, so no wiring changes are needed.
8+
9+
**Tech Stack:** TypeScript, Node `fs` sync APIs, vitest.
10+
11+
**GitNexus impact:** `listDirectory` and `listRecursive` are both LOW risk — sole upstream consumer is `packages/mcp-file/src/server.ts`; no execution flows affected.
12+
13+
---
14+
15+
### Task 1: Regression tests for clamp + budget (failing first)
16+
17+
**Files:**
18+
- Create: `packages/mcp-file/src/tools/list-directory.test.ts`
19+
20+
- [ ] **Step 1: Write the failing tests**
21+
22+
```typescript
23+
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs';
24+
import { tmpdir } from 'node:os';
25+
import { join } from 'node:path';
26+
import { afterEach, describe, expect, it } from 'vitest';
27+
import { listDirectory } from './list-directory.js';
28+
29+
let roots: string[] = [];
30+
31+
function makeRoot(): string {
32+
const root = mkdtempSync(join(tmpdir(), 'mcp-file-list-'));
33+
roots.push(root);
34+
return root;
35+
}
36+
37+
/** Creates root/d1/d2/.../d{depth}, each level containing marker.txt */
38+
function makeDeepFixture(root: string, depth: number): void {
39+
let current = root;
40+
for (let i = 1; i <= depth; i += 1) {
41+
current = join(current, `d${i}`);
42+
mkdirSync(current);
43+
writeFileSync(join(current, 'marker.txt'), `level ${i}`, 'utf-8');
44+
}
45+
}
46+
47+
/** Creates `count` flat files file-0000.txt ... in root/flat */
48+
function makeFlatFixture(root: string, count: number): void {
49+
const dir = join(root, 'flat');
50+
mkdirSync(dir);
51+
for (let i = 0; i < count; i += 1) {
52+
writeFileSync(join(dir, `file-${String(i).padStart(4, '0')}.txt`), 'x', 'utf-8');
53+
}
54+
}
55+
56+
afterEach(() => {
57+
for (const root of roots) {
58+
rmSync(root, { recursive: true, force: true });
59+
}
60+
roots = [];
61+
});
62+
63+
describe('listDirectory maxDepth clamping', () => {
64+
it('clamps oversized maxDepth to the hard limit of 10', () => {
65+
const root = makeRoot();
66+
makeDeepFixture(root, 13);
67+
68+
const result = listDirectory({ path: '.', recursive: true, maxDepth: 999999 }, root);
69+
expect(result.success).toBe(true);
70+
const paths = (result.entries ?? []).map((entry) => entry.path);
71+
// maxDepth=10 lists items down to call depth 10, i.e. contents of d10 (d11 + its marker)
72+
expect(paths).toContain(join('d1', 'd2', 'd3', 'd4', 'd5', 'd6', 'd7', 'd8', 'd9', 'd10', 'd11'));
73+
expect(paths.some((p) => p.endsWith(join('d11', 'd12')))).toBe(false);
74+
expect(paths.some((p) => p.endsWith(join('d12', 'marker.txt')))).toBe(false);
75+
});
76+
77+
it.each([
78+
['zero', 0],
79+
['negative', -3],
80+
['non-integer', 2.5],
81+
['Infinity', Number.POSITIVE_INFINITY],
82+
['NaN', Number.NaN],
83+
])('rejects %s maxDepth with an error', (_label, maxDepth) => {
84+
const root = makeRoot();
85+
makeDeepFixture(root, 2);
86+
87+
const result = listDirectory({ path: '.', recursive: true, maxDepth }, root);
88+
expect(result.success).toBe(false);
89+
expect(result.error).toMatch(/maxDepth/);
90+
});
91+
92+
it('keeps the default depth of 3 when maxDepth is omitted', () => {
93+
const root = makeRoot();
94+
makeDeepFixture(root, 6);
95+
96+
const result = listDirectory({ path: '.', recursive: true }, root);
97+
expect(result.success).toBe(true);
98+
const paths = (result.entries ?? []).map((entry) => entry.path);
99+
expect(paths).toContain(join('d1', 'd2', 'd3', 'd4'));
100+
expect(paths.some((p) => p.endsWith(join('d4', 'd5')))).toBe(false);
101+
});
102+
});
103+
104+
describe('listDirectory entry budget', () => {
105+
it('truncates at maxEntries and reports omitted count', () => {
106+
const root = makeRoot();
107+
makeFlatFixture(root, 10);
108+
109+
const result = listDirectory({ path: 'flat', maxEntries: 3 }, root);
110+
expect(result.success).toBe(true);
111+
expect(result.entries).toHaveLength(3);
112+
expect(result.truncated).toBe(true);
113+
expect(result.omittedEntries).toBe(7);
114+
});
115+
116+
it('reports truncated: false when under budget', () => {
117+
const root = makeRoot();
118+
makeFlatFixture(root, 5);
119+
120+
const result = listDirectory({ path: 'flat', maxEntries: 100 }, root);
121+
expect(result.success).toBe(true);
122+
expect(result.entries).toHaveLength(5);
123+
expect(result.truncated).toBe(false);
124+
expect(result.omittedEntries).toBeUndefined();
125+
});
126+
127+
it('counts omitted entries across recursive subdirectories', () => {
128+
const root = makeRoot();
129+
makeDeepFixture(root, 4); // 4 dirs + 4 markers = 8 entries total
130+
131+
const result = listDirectory({ path: '.', recursive: true, maxDepth: 10, maxEntries: 2 }, root);
132+
expect(result.success).toBe(true);
133+
expect(result.entries).toHaveLength(2);
134+
expect(result.truncated).toBe(true);
135+
expect(result.omittedEntries).toBe(6);
136+
});
137+
138+
it('clamps oversized maxEntries to the hard limit of 2000', () => {
139+
const root = makeRoot();
140+
makeFlatFixture(root, 2005);
141+
142+
const result = listDirectory({ path: 'flat', maxEntries: 999999 }, root);
143+
expect(result.success).toBe(true);
144+
expect(result.entries).toHaveLength(2000);
145+
expect(result.truncated).toBe(true);
146+
expect(result.omittedEntries).toBe(5);
147+
});
148+
149+
it.each([
150+
['zero', 0],
151+
['negative', -1],
152+
['non-integer', 1.5],
153+
])('rejects %s maxEntries with an error', (_label, maxEntries) => {
154+
const root = makeRoot();
155+
makeFlatFixture(root, 1);
156+
157+
const result = listDirectory({ path: 'flat', maxEntries }, root);
158+
expect(result.success).toBe(false);
159+
expect(result.error).toMatch(/maxEntries/);
160+
});
161+
});
162+
```
163+
164+
- [ ] **Step 2: Run tests to verify they fail**
165+
166+
Run: `pnpm --filter @frontagent/mcp-file test -- list-directory`
167+
Expected: FAIL — clamp tests see entries below depth 10 / invalid-input tests get `success: true`; budget tests find `truncated` undefined.
168+
169+
### Task 2: Implement clamp + budget in list-directory.ts
170+
171+
**Files:**
172+
- Modify: `packages/mcp-file/src/tools/list-directory.ts`
173+
174+
- [ ] **Step 1: Implement normalization, budget traversal, result metadata, schema update**
175+
176+
Add constants and a validator above `listDirectory`:
177+
178+
```typescript
179+
const DEFAULT_MAX_DEPTH = 3;
180+
const MAX_DEPTH_LIMIT = 10;
181+
const DEFAULT_MAX_ENTRIES = 2000;
182+
const MAX_ENTRIES_LIMIT = 2000;
183+
184+
interface TraversalBudget {
185+
remaining: number;
186+
omitted: number;
187+
}
188+
189+
function normalizePositiveInteger(
190+
value: number | undefined,
191+
name: string,
192+
defaultValue: number,
193+
limit: number,
194+
): { ok: true; value: number } | { ok: false; error: string } {
195+
if (value === undefined) {
196+
return { ok: true, value: defaultValue };
197+
}
198+
if (!Number.isInteger(value) || value < 1) {
199+
return { ok: false, error: `${name} must be a positive integer, got: ${value}` };
200+
}
201+
return { ok: true, value: Math.min(value, limit) };
202+
}
203+
```
204+
205+
Extend the interfaces:
206+
207+
```typescript
208+
export interface ListDirectoryParams {
209+
path: string;
210+
recursive?: boolean;
211+
includeHidden?: boolean;
212+
maxDepth?: number;
213+
maxEntries?: number;
214+
}
215+
216+
export interface ListDirectoryResult {
217+
success: boolean;
218+
entries?: FileInfo[];
219+
truncated?: boolean;
220+
omittedEntries?: number;
221+
error?: string;
222+
}
223+
```
224+
225+
In `listDirectory`, normalize before traversal and thread the budget through:
226+
227+
```typescript
228+
const { path: dirPath, recursive = false, includeHidden = false } = params;
229+
230+
const maxDepth = normalizePositiveInteger(params.maxDepth, 'maxDepth', DEFAULT_MAX_DEPTH, MAX_DEPTH_LIMIT);
231+
if (!maxDepth.ok) {
232+
return { success: false, error: maxDepth.error };
233+
}
234+
const maxEntries = normalizePositiveInteger(params.maxEntries, 'maxEntries', DEFAULT_MAX_ENTRIES, MAX_ENTRIES_LIMIT);
235+
if (!maxEntries.ok) {
236+
return { success: false, error: maxEntries.error };
237+
}
238+
```
239+
240+
and in the `try` block:
241+
242+
```typescript
243+
const budget: TraversalBudget = { remaining: maxEntries.value, omitted: 0 };
244+
const entries = listRecursive(
245+
safePath.fullPath,
246+
getRealProjectRoot(projectRoot),
247+
recursive,
248+
includeHidden,
249+
0,
250+
maxDepth.value,
251+
budget,
252+
);
253+
return {
254+
success: true,
255+
entries,
256+
truncated: budget.omitted > 0,
257+
...(budget.omitted > 0 ? { omittedEntries: budget.omitted } : {}),
258+
};
259+
```
260+
261+
In `listRecursive`, add the `budget: TraversalBudget` parameter; replace the unconditional `entries.push(fileInfo)` with budget accounting (only build/stat-decorate the FileInfo when within budget; otherwise count it as omitted) while still recursing into subdirectories so omitted entries are counted:
262+
263+
```typescript
264+
if (budget.remaining > 0) {
265+
budget.remaining -= 1;
266+
const fileInfo: FileInfo = {
267+
name: item,
268+
path: relativePath,
269+
type: itemStat.isDirectory() ? 'directory' : 'file',
270+
};
271+
if (itemStat.isFile()) {
272+
fileInfo.size = itemStat.size;
273+
fileInfo.modifiedAt = itemStat.mtime.toISOString();
274+
}
275+
entries.push(fileInfo);
276+
} else {
277+
budget.omitted += 1;
278+
}
279+
```
280+
281+
Update `listDirectorySchema.inputSchema.properties`:
282+
283+
```typescript
284+
maxDepth: {
285+
type: 'number',
286+
description: '递归时的最大深度,默认 3,必须是正整数,上限 10',
287+
default: 3,
288+
},
289+
maxEntries: {
290+
type: 'number',
291+
description: '返回条目数预算,默认 2000,必须是正整数,上限 2000;超出时 truncated 为 true 并返回 omittedEntries',
292+
default: 2000,
293+
},
294+
```
295+
296+
- [ ] **Step 2: Run the focused tests**
297+
298+
Run: `pnpm --filter @frontagent/mcp-file test -- list-directory`
299+
Expected: PASS (all new tests green).
300+
301+
- [ ] **Step 3: Run the full mcp-file package tests**
302+
303+
Run: `pnpm --filter @frontagent/mcp-file test`
304+
Expected: PASS — no regressions in sibling tool tests.
305+
306+
- [ ] **Step 4: Commit**
307+
308+
```bash
309+
git add packages/mcp-file/src/tools/list-directory.ts packages/mcp-file/src/tools/list-directory.test.ts docs/superpowers/plans/2026-06-11-mcp-file-list-directory-budget.md
310+
git commit -m "fix(mcp-file): clamp list_directory maxDepth and add maxEntries budget"
311+
```
312+
313+
### Task 3: Gates before PR
314+
315+
- [ ] **Step 1: Run precommit gate**
316+
317+
Run: `pnpm quality:precommit`
318+
Expected: PASS.
319+
320+
- [ ] **Step 2: Run GitNexus detect_changes**
321+
322+
Run `gitnexus_detect_changes()` (scope: all) — verify only `listDirectory`/`listRecursive`/schema symbols in `packages/mcp-file` are affected.

0 commit comments

Comments
 (0)