Skip to content

Commit a96eeab

Browse files
committed
fix(read_file): accept 'limit' as an alias for 'length'
A caller that names the page size 'limit' (Claude's Read tool naming) had the argument silently dropped: the zod schema only knew 'length', so the whole file came back capped only by fileReadLineLimit. Accept 'limit' as an alias, let the configured fileReadLineLimit apply when neither name is given (the schema default was shadowing it), and document the alias in the tool description. Add a regression test covering limit, offset+limit, the length precedence and the config fallback.
1 parent 092ce0b commit a96eeab

4 files changed

Lines changed: 168 additions & 2 deletions

File tree

src/handlers/filesystem-handlers.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ export async function handleReadFile(args: unknown): Promise<ServerResult> {
101101
const options: ReadOptions = {
102102
isUrl: parsed.isUrl,
103103
offset: parsed.offset ?? 0,
104-
length: parsed.length ?? defaultLimit,
104+
// `limit` is accepted as an alias for `length` (see ReadFileArgsSchema);
105+
// the configured fileReadLineLimit applies when neither is given.
106+
length: parsed.length ?? parsed.limit ?? defaultLimit,
105107
sheet: sheetParam,
106108
range: parsed.range
107109
};

src/server.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,7 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
372372
- 'length' (max lines to read, default: configurable via 'fileReadLineLimit' setting, initially 1000)
373373
* Used with positive offsets for range reading
374374
* Ignored when offset is negative (reads all requested tail lines)
375+
* 'limit' is accepted as an alias for 'length' (some clients name it that way)
375376
376377
Examples:
377378
- offset: 0, length: 10 → First 10 lines

src/tools/schemas.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,13 @@ export const ReadFileArgsSchema = z.object({
5757
path: z.string(),
5858
isUrl: z.boolean().optional().default(false),
5959
offset: z.number().optional().default(0),
60-
length: z.number().optional().default(1000),
60+
// No schema default: the handler applies the configured `fileReadLineLimit`
61+
// when neither `length` nor its `limit` alias is supplied.
62+
length: z.number().optional(),
63+
// Alias for `length`. Some clients and models call the page size `limit`
64+
// (Claude's Read tool naming); without this they are silently ignored and the
65+
// whole file comes back. `length` wins when both are present.
66+
limit: z.number().optional(),
6167
sheet: z.string().optional(), // String only for MCP client compatibility (Cursor doesn't support union types in JSON Schema)
6268
range: z.string().optional(),
6369
options: z.record(z.any()).optional(),

test/test-read-file-limit-alias.js

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/**
2+
* Regression test for `read_file` ignoring the per-call page size when the
3+
* caller names it `limit` instead of `length`.
4+
*
5+
* Issue: wonderwhy-er/DesktopCommanderMCP#686 — a caller sending
6+
* `{ offset: 0, limit: 3 }` got the whole file back (capped only by
7+
* fileReadLineLimit), because `limit` was not part of the argument schema and
8+
* was silently dropped. The tool now accepts `limit` as an alias for `length`.
9+
*
10+
* This test checks:
11+
* 1. `limit` alone limits the returned lines.
12+
* 2. `offset` + `limit` reads the requested window.
13+
* 3. `length` still works and wins when both names are present.
14+
* 4. The configured fileReadLineLimit applies when neither is supplied.
15+
*/
16+
17+
import { configManager } from '../dist/config-manager.js';
18+
import { handleReadFile } from '../dist/handlers/filesystem-handlers.js';
19+
import fs from 'fs/promises';
20+
import path from 'path';
21+
import { fileURLToPath } from 'url';
22+
23+
const __filename = fileURLToPath(import.meta.url);
24+
const __dirname = path.dirname(__filename);
25+
const TEST_FILE = path.join(__dirname, 'test-read-file-limit-alias.txt');
26+
const TOTAL_LINES = 50;
27+
28+
/**
29+
* Setup: allowed directory, numbered test file, known read limit.
30+
*/
31+
async function setup() {
32+
console.log('🔧 Setting up read_file limit alias test...');
33+
34+
const originalConfig = await configManager.getConfig();
35+
await configManager.setValue('allowedDirectories', [__dirname]);
36+
// Keep a non-default limit so "no page size given" is distinguishable from
37+
// "page size applied".
38+
await configManager.setValue('fileReadLineLimit', 7);
39+
40+
const content = Array.from(
41+
{ length: TOTAL_LINES },
42+
(_, i) => `Line ${i + 1}: alias test content`
43+
).join('\n');
44+
await fs.writeFile(TEST_FILE, content, 'utf8');
45+
46+
console.log(`✓ Created ${TOTAL_LINES}-line test file, fileReadLineLimit=7`);
47+
return originalConfig;
48+
}
49+
50+
async function teardown(originalConfig) {
51+
console.log('🧹 Cleaning up read_file limit alias test...');
52+
await configManager.updateConfig(originalConfig);
53+
try {
54+
await fs.rm(TEST_FILE, { force: true });
55+
console.log('✓ Test file cleaned up');
56+
} catch (error) {
57+
console.log('⚠️ Warning: Could not clean up test file:', error.message);
58+
}
59+
}
60+
61+
/** Count the numbered body lines in a read_file result. */
62+
function countBodyLines(result) {
63+
return result.content[0].text
64+
.split('\n')
65+
.filter(line => line.startsWith('Line ')).length;
66+
}
67+
68+
async function runAllTests() {
69+
console.log('🧪 Testing read_file page-size arguments (limit alias)');
70+
let allTestsPassed = true;
71+
let originalConfig;
72+
73+
try {
74+
originalConfig = await setup();
75+
76+
const cases = [
77+
{
78+
name: 'limit: 3 returns 3 lines',
79+
args: { path: TEST_FILE, limit: 3 },
80+
expected: 3,
81+
},
82+
{
83+
name: 'offset: 0, limit: 3 returns 3 lines',
84+
args: { path: TEST_FILE, offset: 0, limit: 3 },
85+
expected: 3,
86+
},
87+
{
88+
name: 'offset: 10, limit: 5 returns 5 lines',
89+
args: { path: TEST_FILE, offset: 10, limit: 5 },
90+
expected: 5,
91+
},
92+
{
93+
name: 'length: 4 still returns 4 lines',
94+
args: { path: TEST_FILE, length: 4 },
95+
expected: 4,
96+
},
97+
{
98+
name: 'length wins when both length and limit are given',
99+
args: { path: TEST_FILE, length: 6, limit: 2 },
100+
expected: 6,
101+
},
102+
{
103+
name: 'no page size falls back to fileReadLineLimit (7)',
104+
args: { path: TEST_FILE },
105+
expected: 7,
106+
},
107+
];
108+
109+
for (const testCase of cases) {
110+
console.log(`\n 🧪 ${testCase.name}`);
111+
try {
112+
const result = await handleReadFile(testCase.args);
113+
if (result.isError) {
114+
console.log(` ❌ Error: ${result.content[0].text}`);
115+
allTestsPassed = false;
116+
continue;
117+
}
118+
const actual = countBodyLines(result);
119+
if (actual === testCase.expected) {
120+
console.log(` ✅ PASS: ${actual} line(s)`);
121+
} else {
122+
console.log(` ❌ FAIL: expected ${testCase.expected} line(s), got ${actual}`);
123+
allTestsPassed = false;
124+
}
125+
} catch (error) {
126+
console.log(` ❌ Exception: ${error.message}`);
127+
allTestsPassed = false;
128+
}
129+
}
130+
131+
console.log(
132+
`\n🎯 Overall result: ${allTestsPassed ? '✅ ALL TESTS PASSED!' : '❌ SOME TESTS FAILED'}`
133+
);
134+
} catch (error) {
135+
console.error('❌ Test setup/execution failed:', error.message);
136+
allTestsPassed = false;
137+
} finally {
138+
if (originalConfig) {
139+
await teardown(originalConfig);
140+
}
141+
}
142+
143+
return allTestsPassed;
144+
}
145+
146+
export default runAllTests;
147+
148+
if (import.meta.url === `file://${process.argv[1]}`) {
149+
runAllTests()
150+
.then(success => {
151+
process.exit(success ? 0 : 1);
152+
})
153+
.catch(error => {
154+
console.error('❌ Unhandled error:', error);
155+
process.exit(1);
156+
});
157+
}

0 commit comments

Comments
 (0)