Skip to content

Commit 53cd723

Browse files
authored
refactor: rename read/write/edit file_path param to path (LibreChat-AI#250)
* feat(coding-tools)!: rename read/write/edit `file_path` param to `path` Models intermittently emit the very common `path` instead of `file_path` for read_file/write_file/edit_file, which fails Zod validation ("Received tool input did not match expected schema") and derails the agent run — the model then retries or returns empty. Observed reliably with Kimi K2 via Fireworks/ClickHouse Inference (the investigator dies right after the first tool call). Rather than coerce the alias at every execution path, align the schema with what models actually emit — and with the sibling search tools (grep_search / glob_search / list_directory), which already use `path`. Now every built-in coding tool takes its file/dir target in `path`. - ReadFileToolSchema + Local{Read,Write,Edit}FileToolSchema: `file_path` -> `path` - handlers read `input.path` - workspace-policy extractors collapse to a single `path` extractor for all tools - tests updated BREAKING CHANGE: the read_file/write_file/edit_file parameter is now `path` (was `file_path`). Consumers that construct or read these tool-call args must update. Verified end-to-end against ClickHouse Inference (kimi-k2p7-code): a real investigator loop goes 4/10 -> 10/10 with no coercion. Tool + hook suites green (the one failing suite, directToolHITLResumeScope, fails identically on clean main — pre-existing). * fix(cloudflare-ptc): rename generated read/write/edit helpers to `path` Codex (LibreChat-AI#250): the Cloudflare Sandbox programmatic runners generate in-sandbox read_file/write_file/edit_file helpers that still took `file_path` (Python defs + the JS `payload.file_path` reads), so `write_file(path=...)` failed only in the Cloudflare programmatic path after the schema switched to `path`. Rename the tool wrappers' param to `path` (the internal `_resolve`/`_is_within_workspace` utils and the code-execution temp-file locals keep `file_path` — unrelated).
1 parent 2c51596 commit 53cd723

10 files changed

Lines changed: 83 additions & 82 deletions

‎src/hooks/__tests__/createWorkspacePolicyHook.test.ts‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('createWorkspacePolicyHook', () => {
5151
const hook = createWorkspacePolicyHook({ root: workspace });
5252
const out = await call(
5353
hook,
54-
makeInput('read_file', { file_path: 'src/x.ts' })
54+
makeInput('read_file', { path: 'src/x.ts' })
5555
);
5656
expect(out.decision).toBe('allow');
5757
});
@@ -60,7 +60,7 @@ describe('createWorkspacePolicyHook', () => {
6060
const hook = createWorkspacePolicyHook({ root: workspace });
6161
const out = await call(
6262
hook,
63-
makeInput('read_file', { file_path: join(workspace, 'src/x.ts') })
63+
makeInput('read_file', { path: join(workspace, 'src/x.ts') })
6464
);
6565
expect(out.decision).toBe('allow');
6666
});
@@ -72,7 +72,7 @@ describe('createWorkspacePolicyHook', () => {
7272
});
7373
const out = await call(
7474
hook,
75-
makeInput('read_file', { file_path: join(extra, 'lib/y.ts') })
75+
makeInput('read_file', { path: join(extra, 'lib/y.ts') })
7676
);
7777
expect(out.decision).toBe('allow');
7878
});
@@ -81,7 +81,7 @@ describe('createWorkspacePolicyHook', () => {
8181
const hook = createWorkspacePolicyHook({ root: workspace });
8282
const out = await call(
8383
hook,
84-
makeInput('read_file', { file_path: '/etc/passwd' })
84+
makeInput('read_file', { path: '/etc/passwd' })
8585
);
8686
expect(out.decision).toBe('ask');
8787
expect(out.reason).toContain('/etc/passwd');
@@ -93,7 +93,7 @@ describe('createWorkspacePolicyHook', () => {
9393
const out = await call(
9494
hook,
9595
makeInput('write_file', {
96-
file_path: '/etc/foo',
96+
path: '/etc/foo',
9797
content: 'malicious',
9898
})
9999
);
@@ -107,7 +107,7 @@ describe('createWorkspacePolicyHook', () => {
107107
});
108108
const out = await call(
109109
hook,
110-
makeInput('read_file', { file_path: '/etc/passwd' })
110+
makeInput('read_file', { path: '/etc/passwd' })
111111
);
112112
expect(out.decision).toBe('deny');
113113
expect(out.reason).toContain('/etc/passwd');
@@ -121,7 +121,7 @@ describe('createWorkspacePolicyHook', () => {
121121
const out = await call(
122122
hook,
123123
makeInput('edit_file', {
124-
file_path: '/etc/foo',
124+
path: '/etc/foo',
125125
old_text: 'a',
126126
new_text: 'b',
127127
})
@@ -136,7 +136,7 @@ describe('createWorkspacePolicyHook', () => {
136136
});
137137
const out = await call(
138138
hook,
139-
makeInput('read_file', { file_path: '/etc/passwd' })
139+
makeInput('read_file', { path: '/etc/passwd' })
140140
);
141141
expect(out.decision).toBe('allow');
142142
});
@@ -184,7 +184,7 @@ describe('createWorkspacePolicyHook', () => {
184184
});
185185
const out = await call(
186186
hook,
187-
makeInput('read_file', { file_path: '/somewhere/else.ts' })
187+
makeInput('read_file', { path: '/somewhere/else.ts' })
188188
);
189189
expect(out.reason).toBe('read_file blocked from /somewhere/else.ts');
190190
});
@@ -212,7 +212,7 @@ describe('createWorkspacePolicyHook', () => {
212212
const hook = createWorkspacePolicyHook({ root: '.' });
213213
const out = await call(
214214
hook,
215-
makeInput('read_file', { file_path: resolve('.', 'src/x.ts') })
215+
makeInput('read_file', { path: resolve('.', 'src/x.ts') })
216216
);
217217
expect(out.decision).toBe('allow');
218218
});
@@ -228,7 +228,7 @@ describe('createWorkspacePolicyHook', () => {
228228
});
229229
const out = await call(
230230
hook,
231-
makeInput('read_file', { file_path: 'escape' })
231+
makeInput('read_file', { path: 'escape' })
232232
);
233233
expect(out.decision).toBe('deny');
234234
expect(out.reason).toContain('escape');
@@ -245,7 +245,7 @@ describe('createWorkspacePolicyHook', () => {
245245
});
246246
const out = await call(
247247
hook,
248-
makeInput('read_file', { file_path: join(altMount, 'file.ts') })
248+
makeInput('read_file', { path: join(altMount, 'file.ts') })
249249
);
250250
expect(out.decision).toBe('allow');
251251
});

‎src/hooks/createWorkspacePolicyHook.ts‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -166,13 +166,14 @@ function extractCompileCheckPaths(input: Record<string, unknown>): string[] {
166166
return out;
167167
}
168168

169+
// All built-in coding tools take their file/dir target in `path`.
170+
const extractPath: PathExtractor = (i) =>
171+
typeof i.path === 'string' && i.path !== '' ? [i.path] : [];
172+
169173
const DEFAULT_EXTRACTORS: Record<string, PathExtractor> = {
170-
[Constants.READ_FILE]: (i) =>
171-
typeof i.file_path === 'string' ? [i.file_path] : [],
172-
[Constants.WRITE_FILE]: (i) =>
173-
typeof i.file_path === 'string' ? [i.file_path] : [],
174-
[Constants.EDIT_FILE]: (i) =>
175-
typeof i.file_path === 'string' ? [i.file_path] : [],
174+
[Constants.READ_FILE]: extractPath,
175+
[Constants.WRITE_FILE]: extractPath,
176+
[Constants.EDIT_FILE]: extractPath,
176177
[Constants.GREP_SEARCH]: (i) =>
177178
typeof i.path === 'string' && i.path !== '' ? [i.path] : [],
178179
[Constants.GLOB_SEARCH]: (i) =>

‎src/tools/ReadFile.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,13 @@ CONSTRAINTS:
2222
export const ReadFileToolSchema = {
2323
type: 'object',
2424
properties: {
25-
file_path: {
25+
path: {
2626
type: 'string',
2727
description:
2828
'Path to the file. For skill files: "{skillName}/{path}" (e.g. "pdf-analyzer/src/utils.py"). For code execution output: the path as returned by the execution tool.',
2929
},
3030
},
31-
required: ['file_path'],
31+
required: ['path'],
3232
} as const;
3333

3434
export const ReadFileToolDefinition = {

‎src/tools/__tests__/LocalExecutionTools.test.ts‎

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -175,8 +175,8 @@ describe('local execution tools', () => {
175175
aiMessageWithToolCall(Constants.PROGRAMMATIC_TOOL_CALLING, {
176176
lang: 'py',
177177
code: [
178-
'await write_file(file_path="ptc.txt", content="from local ptc")',
179-
'contents = await read_file(file_path="ptc.txt")',
178+
'await write_file(path="ptc.txt", content="from local ptc")',
179+
'contents = await read_file(path="ptc.txt")',
180180
'print(contents)',
181181
].join('\n'),
182182
}),
@@ -205,8 +205,8 @@ describe('local execution tools', () => {
205205
messages: [
206206
aiMessageWithToolCall(Constants.PROGRAMMATIC_TOOL_CALLING, {
207207
code: [
208-
'write_file \'{"file_path":"bash-ptc.txt","content":"from bash ptc"}\'',
209-
'read_file \'{"file_path":"bash-ptc.txt"}\'',
208+
'write_file \'{"path":"bash-ptc.txt","content":"from bash ptc"}\'',
209+
'read_file \'{"path":"bash-ptc.txt"}\'',
210210
].join('\n'),
211211
}),
212212
],
@@ -335,8 +335,8 @@ describe('LocalFileCheckpointer', () => {
335335

336336
const writeTool = bundle.tools.find((tool_) => tool_.name === 'write_file');
337337
expect(writeTool).toBeDefined();
338-
await writeTool!.invoke({ file_path: 'cp.txt', content: 'first' });
339-
await writeTool!.invoke({ file_path: 'cp.txt', content: 'second' });
338+
await writeTool!.invoke({ path: 'cp.txt', content: 'first' });
339+
await writeTool!.invoke({ path: 'cp.txt', content: 'second' });
340340

341341
const restored = await bundle.checkpointer!.rewind();
342342
expect(restored).toBe(1);
@@ -352,7 +352,7 @@ describe('local read tool guards', () => {
352352

353353
const bundle = createLocalCodingToolBundle({ cwd });
354354
const readTool = bundle.tools.find((t_) => t_.name === Constants.READ_FILE);
355-
const result = await readTool!.invoke({ file_path: 'binary.bin' });
355+
const result = await readTool!.invoke({ path: 'binary.bin' });
356356
expect(String(result)).toContain('binary file');
357357
});
358358

@@ -366,7 +366,7 @@ describe('local read tool guards', () => {
366366
maxReadBytes: 1024,
367367
});
368368
const readTool = bundle.tools.find((t_) => t_.name === Constants.READ_FILE);
369-
const result = await readTool!.invoke({ file_path: 'big.txt' });
369+
const result = await readTool!.invoke({ path: 'big.txt' });
370370
expect(String(result)).toContain('exceeds the 1024-byte read cap');
371371
});
372372

@@ -380,7 +380,7 @@ describe('local read tool guards', () => {
380380
const bundle = createLocalCodingToolBundle({ cwd });
381381
const readTool = bundle.tools.find((t_) => t_.name === Constants.READ_FILE);
382382
await expect(
383-
readTool!.invoke({ file_path: 'escape/secret.txt' })
383+
readTool!.invoke({ path: 'escape/secret.txt' })
384384
).rejects.toThrow(/symlink escape/);
385385
});
386386
});
@@ -406,7 +406,7 @@ describe('local programmatic bridge auth', () => {
406406
code: [
407407
'import os, json, urllib.request, urllib.error',
408408
'url = os.environ["BRIDGE_PROBE_URL"] if "BRIDGE_PROBE_URL" in os.environ else __LIBRECHAT_TOOL_BRIDGE',
409-
'body = json.dumps({"name":"read_file","input":{"file_path":"x"}}).encode("utf-8")',
409+
'body = json.dumps({"name":"read_file","input":{"path":"x"}}).encode("utf-8")',
410410
'try:',
411411
' req = urllib.request.Request(url, data=body, headers={"Content-Type":"application/json"}, method="POST")',
412412
' urllib.request.urlopen(req, timeout=5)',
@@ -438,7 +438,7 @@ describe('local edit fuzzy matching', () => {
438438
const bundle = createLocalCodingToolBundle({ cwd });
439439
const editTool = bundle.tools.find((tt) => tt.name === 'edit_file');
440440
const result = await editTool!.invoke({
441-
file_path: 'a.ts',
441+
path: 'a.ts',
442442
// LLM emits a trailing-whitespace-stripped version.
443443
old_text:
444444
'function greet(name: string) {\n return `Hello, ${name}!`;\n}',
@@ -461,7 +461,7 @@ describe('local edit fuzzy matching', () => {
461461
const bundle = createLocalCodingToolBundle({ cwd });
462462
const editTool = bundle.tools.find((tt) => tt.name === 'edit_file');
463463
const result = await editTool!.invoke({
464-
file_path: 'a.ts',
464+
path: 'a.ts',
465465
// LLM stripped the 4-space indent
466466
old_text: 'method() {\n return 1;\n}',
467467
new_text: 'method() {\n return 42;\n}',
@@ -480,7 +480,7 @@ describe('local edit fuzzy matching', () => {
480480
const bundle = createLocalCodingToolBundle({ cwd });
481481
const editTool = bundle.tools.find((tt) => tt.name === 'edit_file');
482482
const result = await editTool!.invoke({
483-
file_path: 'a.txt',
483+
path: 'a.txt',
484484
old_text: 'second',
485485
new_text: 'SECOND',
486486
});
@@ -497,7 +497,7 @@ describe('local edit fuzzy matching', () => {
497497
const bundle = createLocalCodingToolBundle({ cwd });
498498
const editTool = bundle.tools.find((tt) => tt.name === 'edit_file');
499499
await editTool!.invoke({
500-
file_path: 'a.txt',
500+
path: 'a.txt',
501501
old_text: 'two',
502502
new_text: 'TWO',
503503
});
@@ -512,7 +512,7 @@ describe('local edit fuzzy matching', () => {
512512
await fsWriteFile(file, BOM + 'hello\n', 'utf8');
513513
const bundle = createLocalCodingToolBundle({ cwd });
514514
const writeTool = bundle.tools.find((tt) => tt.name === 'write_file');
515-
await writeTool!.invoke({ file_path: 'a.txt', content: 'goodbye\n' });
515+
await writeTool!.invoke({ path: 'a.txt', content: 'goodbye\n' });
516516
const raw = await fsReadFile(file, 'utf8');
517517
expect(raw.startsWith(BOM)).toBe(true);
518518
expect(raw.slice(1)).toBe('goodbye\n');
@@ -532,7 +532,7 @@ describe('local read attachments', () => {
532532
await fsWriteFile(file, tinyPng);
533533
const bundle = createLocalCodingToolBundle({ cwd });
534534
const readTool = bundle.tools.find((tt) => tt.name === Constants.READ_FILE);
535-
const result = await readTool!.invoke({ file_path: 'tiny.png' });
535+
const result = await readTool!.invoke({ path: 'tiny.png' });
536536
expect(String(result)).toContain('binary file');
537537
});
538538

@@ -552,7 +552,7 @@ describe('local read attachments', () => {
552552
const message = (await readTool!.invoke({
553553
id: 'call_image',
554554
name: Constants.READ_FILE,
555-
args: { file_path: 'tiny.png' },
555+
args: { path: 'tiny.png' },
556556
type: 'tool_call',
557557
})) as { content: unknown; artifact: unknown };
558558
expect(Array.isArray(message.content)).toBe(true);
@@ -589,7 +589,7 @@ describe('local read attachments', () => {
589589
maxAttachmentBytes: 100,
590590
});
591591
const readTool = bundle.tools.find((tt) => tt.name === Constants.READ_FILE);
592-
const result = await readTool!.invoke({ file_path: 'big.png' });
592+
const result = await readTool!.invoke({ path: 'big.png' });
593593
expect(String(result)).toMatch(/Refusing to embed/);
594594
});
595595

@@ -602,7 +602,7 @@ describe('local read attachments', () => {
602602
attachReadAttachments: 'images-only',
603603
});
604604
const readTool = bundle.tools.find((tt) => tt.name === Constants.READ_FILE);
605-
const result = await readTool!.invoke({ file_path: 'a.txt' });
605+
const result = await readTool!.invoke({ path: 'a.txt' });
606606
expect(String(result)).toContain('hello world');
607607
});
608608
});
@@ -662,7 +662,7 @@ describe('post-edit syntax check', () => {
662662
const message = (await writeTool!.invoke({
663663
id: 'call_w',
664664
name: 'write_file',
665-
args: { file_path: 'broken.js', content: 'function (\n' },
665+
args: { path: 'broken.js', content: 'function (\n' },
666666
type: 'tool_call',
667667
})) as { content: string; artifact: { syntax_error?: string } };
668668
expect(message.content).toContain('[syntax-check warning');
@@ -680,7 +680,7 @@ describe('post-edit syntax check', () => {
680680
writeTool!.invoke({
681681
id: 'call_w',
682682
name: 'write_file',
683-
args: { file_path: 'broken.js', content: 'function (\n' },
683+
args: { path: 'broken.js', content: 'function (\n' },
684684
type: 'tool_call',
685685
})
686686
).rejects.toThrow(/syntax check failed/);
@@ -850,7 +850,7 @@ describe('codex review fixes', () => {
850850
const result = await readTool!.invoke({
851851
id: 'c',
852852
name: Constants.READ_FILE,
853-
args: { file_path: join(parent, 'shared/lib.ts') },
853+
args: { path: join(parent, 'shared/lib.ts') },
854854
type: 'tool_call',
855855
});
856856
expect(JSON.stringify(result)).toContain('X');
@@ -2253,7 +2253,7 @@ describe('comprehensive review (round 14) — Codex P1 #37 + P2 #38/#40/#41', ()
22532253
writeTool!.invoke({
22542254
id: 'wf-strict',
22552255
name: Constants.WRITE_FILE,
2256-
args: { file_path: file, content: 'function broken( {\n' },
2256+
args: { path: file, content: 'function broken( {\n' },
22572257
type: 'tool_call',
22582258
})
22592259
).rejects.toThrow(/syntax check failed.*reverted/i);
@@ -2281,7 +2281,7 @@ describe('comprehensive review (round 14) — Codex P1 #37 + P2 #38/#40/#41', ()
22812281
writeTool!.invoke({
22822282
id: 'wf-strict-new',
22832283
name: Constants.WRITE_FILE,
2284-
args: { file_path: file, content: 'function broken( {\n' },
2284+
args: { path: file, content: 'function broken( {\n' },
22852285
type: 'tool_call',
22862286
})
22872287
).rejects.toThrow(/syntax check failed.*reverted/i);
@@ -2308,7 +2308,7 @@ describe('comprehensive review (round 14) — Codex P1 #37 + P2 #38/#40/#41', ()
23082308
id: 'ef-strict',
23092309
name: Constants.EDIT_FILE,
23102310
args: {
2311-
file_path: file,
2311+
path: file,
23122312
old_text: 'return 1;',
23132313
new_text: 'return broken(',
23142314
},

‎src/tools/__tests__/ProgrammaticToolCalling.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1503,10 +1503,10 @@ for member in team:
15031503
{ registry, runId: 'r1' },
15041504
'read_file',
15051505
'call_1',
1506-
{ file_path: '/tmp/x' }
1506+
{ path: '/tmp/x' }
15071507
);
15081508
expect(gate.denyReason).toBeUndefined();
1509-
expect(gate.input).toEqual({ file_path: '/tmp/x' });
1509+
expect(gate.input).toEqual({ path: '/tmp/x' });
15101510
});
15111511

15121512
it('applies updatedInput to the inner tool args', async () => {
@@ -1520,7 +1520,7 @@ for member in team:
15201520
// eslint-disable-next-line @typescript-eslint/explicit-function-return-type
15211521
async () => ({
15221522
decision: 'allow',
1523-
updatedInput: { file_path: '/tmp/rewritten' },
1523+
updatedInput: { path: '/tmp/rewritten' },
15241524
}),
15251525
],
15261526
});
@@ -1529,10 +1529,10 @@ for member in team:
15291529
{ registry, runId: 'r1' },
15301530
'read_file',
15311531
'call_1',
1532-
{ file_path: '/tmp/original' }
1532+
{ path: '/tmp/original' }
15331533
);
15341534
expect(gate.denyReason).toBeUndefined();
1535-
expect(gate.input).toEqual({ file_path: '/tmp/rewritten' });
1535+
expect(gate.input).toEqual({ path: '/tmp/rewritten' });
15361536
});
15371537

15381538
it('treats `ask` as fail-closed deny (HITL not reachable from bridge)', async () => {

‎src/tools/__tests__/ReadFile.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@ import { Constants } from '@/common';
99

1010
describe('ReadFile', () => {
1111
describe('schema structure', () => {
12-
it('has file_path as required string property', () => {
13-
expect(ReadFileToolSchema.properties.file_path.type).toBe('string');
14-
expect(ReadFileToolSchema.required).toContain('file_path');
12+
it('has path as required string property', () => {
13+
expect(ReadFileToolSchema.properties.path.type).toBe('string');
14+
expect(ReadFileToolSchema.required).toContain('path');
1515
});
1616

1717
it('is an object type schema', () => {

0 commit comments

Comments
 (0)