Skip to content

Commit ce1cd26

Browse files
committed
refactor: align agent tool params with upstream pi
1 parent 181a50e commit ce1cd26

11 files changed

Lines changed: 164 additions & 407 deletions

docs/pi.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,7 @@ if (sandboxRoot) {
503503

504504
- Refusal magic string scrubbing
505505
- Turn validation for consecutive roles
506-
- Claude Code parameter compatibility
506+
- Strict upstream Pi tool parameter validation
507507

508508
### Google/Gemini
509509

src/agents/pi-tool-definition-adapter.logging.test.ts

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ vi.mock("../logger.js", () => ({
1313
}));
1414

1515
let toToolDefinitions: typeof import("./pi-tool-definition-adapter.js").toToolDefinitions;
16-
let wrapToolParamNormalization: typeof import("./pi-tools.params.js").wrapToolParamNormalization;
17-
let CLAUDE_PARAM_GROUPS: typeof import("./pi-tools.params.js").CLAUDE_PARAM_GROUPS;
16+
let wrapToolParamValidation: typeof import("./pi-tools.params.js").wrapToolParamValidation;
17+
let REQUIRED_PARAM_GROUPS: typeof import("./pi-tools.params.js").REQUIRED_PARAM_GROUPS;
1818
let logError: typeof import("../logger.js").logError;
1919

2020
type ToolExecute = ReturnType<
@@ -25,7 +25,7 @@ const extensionContext = {} as Parameters<ToolExecute>[4];
2525
describe("pi tool definition adapter logging", () => {
2626
beforeAll(async () => {
2727
({ toToolDefinitions } = await import("./pi-tool-definition-adapter.js"));
28-
({ wrapToolParamNormalization, CLAUDE_PARAM_GROUPS } = await import("./pi-tools.params.js"));
28+
({ wrapToolParamValidation, REQUIRED_PARAM_GROUPS } = await import("./pi-tools.params.js"));
2929
({ logError } = await import("../logger.js"));
3030
});
3131

@@ -41,16 +41,20 @@ describe("pi tool definition adapter logging", () => {
4141
description: "edits files",
4242
parameters: Type.Object({
4343
path: Type.String(),
44-
oldText: Type.String(),
45-
newText: Type.String(),
44+
edits: Type.Array(
45+
Type.Object({
46+
oldText: Type.String(),
47+
newText: Type.String(),
48+
}),
49+
),
4650
}),
4751
execute: async () => ({
4852
content: [{ type: "text" as const, text: "ok" }],
4953
details: { ok: true },
5054
}),
5155
} satisfies AgentTool;
5256

53-
const tool = wrapToolParamNormalization(baseTool, CLAUDE_PARAM_GROUPS.edit);
57+
const tool = wrapToolParamValidation(baseTool, REQUIRED_PARAM_GROUPS.edit);
5458
const [def] = toToolDefinitions([tool]);
5559
if (!def) {
5660
throw new Error("missing tool definition");
@@ -60,7 +64,7 @@ describe("pi tool definition adapter logging", () => {
6064

6165
expect(logError).toHaveBeenCalledWith(
6266
expect.stringContaining(
63-
'[tools] edit failed: Missing required parameters: oldText alias, newText alias (received: path). Supply correct parameters before retrying. raw_params={"path":"notes.txt"}',
67+
'[tools] edit failed: Missing required parameter: edits (received: path). Supply correct parameters before retrying. raw_params={"path":"notes.txt"}',
6468
),
6569
);
6670
});
@@ -86,7 +90,7 @@ describe("pi tool definition adapter logging", () => {
8690
execute,
8791
} satisfies AgentTool;
8892

89-
const tool = wrapToolParamNormalization(baseTool, CLAUDE_PARAM_GROUPS.edit);
93+
const tool = wrapToolParamValidation(baseTool, REQUIRED_PARAM_GROUPS.edit);
9094
const [def] = toToolDefinitions([tool]);
9195
if (!def) {
9296
throw new Error("missing tool definition");

src/agents/pi-tools.create-openclaw-coding-tools.adds-claude-style-aliases-schemas-without-dropping-e.test.ts

Lines changed: 8 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,12 @@
11
import type { AgentTool } from "@mariozechner/pi-agent-core";
22
import { Type } from "@sinclair/typebox";
33
import { describe, expect, it, vi } from "vitest";
4-
import {
5-
patchToolSchemaForClaudeCompatibility,
6-
wrapToolParamNormalization,
7-
} from "./pi-tools.params.js";
4+
import { REQUIRED_PARAM_GROUPS, wrapToolParamValidation } from "./pi-tools.params.js";
85
import { cleanToolSchemaForGemini } from "./pi-tools.schema.js";
96

107
describe("createOpenClawCodingTools", () => {
11-
describe("Claude/Gemini alias support", () => {
12-
it("adds Claude-style aliases to schemas without dropping metadata", () => {
13-
const base: AgentTool = {
14-
name: "write",
15-
label: "write",
16-
description: "test",
17-
parameters: Type.Object({
18-
path: Type.String({ description: "Path" }),
19-
content: Type.String({ description: "Body" }),
20-
}),
21-
execute: vi.fn(),
22-
};
23-
24-
const patched = patchToolSchemaForClaudeCompatibility(base);
25-
const params = patched.parameters as {
26-
additionalProperties?: unknown;
27-
properties?: Record<string, unknown>;
28-
required?: string[];
29-
};
30-
const props = params.properties ?? {};
31-
32-
expect(props.file_path).toEqual(props.path);
33-
expect(params.additionalProperties).toBe(false);
34-
expect(params.required ?? []).not.toContain("path");
35-
expect(params.required ?? []).not.toContain("file_path");
36-
});
37-
38-
it("normalizes file_path to path and enforces required groups at runtime", async () => {
8+
describe("Gemini cleanup and strict param validation", () => {
9+
it("enforces canonical path/content at runtime", async () => {
3910
const execute = vi.fn(async (_id, args) => args);
4011
const tool: AgentTool = {
4112
name: "write",
@@ -48,12 +19,9 @@ describe("createOpenClawCodingTools", () => {
4819
execute,
4920
};
5021

51-
const wrapped = wrapToolParamNormalization(tool, [
52-
{ keys: ["path", "file_path"], label: "path (path or file_path)" },
53-
{ keys: ["content"], label: "content" },
54-
]);
22+
const wrapped = wrapToolParamValidation(tool, REQUIRED_PARAM_GROUPS.write);
5523

56-
await wrapped.execute("tool-1", { file_path: "foo.txt", content: "x" });
24+
await wrapped.execute("tool-1", { path: "foo.txt", content: "x" });
5725
expect(execute).toHaveBeenCalledWith(
5826
"tool-1",
5927
{ path: "foo.txt", content: "x" },
@@ -67,14 +35,14 @@ describe("createOpenClawCodingTools", () => {
6735
await expect(wrapped.execute("tool-2", { content: "x" })).rejects.toThrow(
6836
/Supply correct parameters before retrying\./,
6937
);
70-
await expect(wrapped.execute("tool-3", { file_path: " ", content: "x" })).rejects.toThrow(
38+
await expect(wrapped.execute("tool-3", { path: " ", content: "x" })).rejects.toThrow(
7139
/Missing required parameter/,
7240
);
73-
await expect(wrapped.execute("tool-3", { file_path: " ", content: "x" })).rejects.toThrow(
41+
await expect(wrapped.execute("tool-3", { path: " ", content: "x" })).rejects.toThrow(
7442
/Supply correct parameters before retrying\./,
7543
);
7644
await expect(wrapped.execute("tool-4", {})).rejects.toThrow(
77-
/Missing required parameters: path \(path or file_path\), content/,
45+
/Missing required parameters: path, content/,
7846
);
7947
await expect(wrapped.execute("tool-4", {})).rejects.toThrow(
8048
/Supply correct parameters before retrying\./,

src/agents/pi-tools.create-openclaw-coding-tools.adds-claude-style-aliases-schemas-without-dropping-f.test.ts

Lines changed: 54 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -8,26 +8,25 @@ import { createOpenClawCodingTools } from "./pi-tools.js";
88
import { expectReadWriteEditTools } from "./test-helpers/pi-tools-fs-helpers.js";
99

1010
describe("createOpenClawCodingTools", () => {
11-
it("accepts Claude Code parameter aliases for read/write/edit", async () => {
12-
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-alias-"));
11+
it("accepts canonical parameters for read/write/edit", async () => {
12+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-canonical-"));
1313
try {
1414
const tools = createOpenClawCodingTools({ workspaceDir: tmpDir });
1515
const { readTool, writeTool, editTool } = expectReadWriteEditTools(tools);
1616

17-
const filePath = "alias-test.txt";
18-
await writeTool?.execute("tool-alias-1", {
19-
file_path: filePath,
17+
const filePath = "canonical-test.txt";
18+
await writeTool?.execute("tool-canonical-1", {
19+
path: filePath,
2020
content: "hello world",
2121
});
2222

23-
await editTool?.execute("tool-alias-2", {
24-
file_path: filePath,
25-
old_string: "world",
26-
new_string: "universe",
23+
await editTool?.execute("tool-canonical-2", {
24+
path: filePath,
25+
edits: [{ oldText: "world", newText: "universe" }],
2726
});
2827

29-
const result = await readTool?.execute("tool-alias-3", {
30-
file_path: filePath,
28+
const result = await readTool?.execute("tool-canonical-3", {
29+
path: filePath,
3130
});
3231

3332
const textBlocks = result?.content?.filter((block) => block.type === "text") as
@@ -40,62 +39,59 @@ describe("createOpenClawCodingTools", () => {
4039
}
4140
});
4241

43-
it("accepts broader file/edit alias variants", async () => {
44-
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-alias-broad-"));
42+
it("rejects legacy alias parameters", async () => {
43+
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-legacy-alias-"));
4544
try {
4645
const tools = createOpenClawCodingTools({ workspaceDir: tmpDir });
4746
const { readTool, writeTool, editTool } = expectReadWriteEditTools(tools);
4847

49-
await writeTool?.execute("tool-alias-broad-1", {
50-
file: "broad-alias.txt",
51-
content: "hello old value",
52-
});
53-
54-
await editTool?.execute("tool-alias-broad-2", {
55-
filePath: "broad-alias.txt",
56-
old_text: "old",
57-
newString: "new",
58-
});
59-
60-
const result = await readTool?.execute("tool-alias-broad-3", {
61-
file: "broad-alias.txt",
62-
});
63-
64-
const textBlocks = result?.content?.filter((block) => block.type === "text") as
65-
| Array<{ text?: string }>
66-
| undefined;
67-
const combinedText = textBlocks?.map((block) => block.text ?? "").join("\n");
68-
expect(combinedText).toContain("hello new value");
48+
await expect(
49+
writeTool?.execute("tool-legacy-write", {
50+
file: "legacy.txt",
51+
content: "hello old value",
52+
}),
53+
).rejects.toThrow(/Missing required parameter: path/);
54+
55+
await expect(
56+
editTool?.execute("tool-legacy-edit", {
57+
filePath: "legacy.txt",
58+
old_text: "old",
59+
newString: "new",
60+
}),
61+
).rejects.toThrow(/Missing required parameters: path, edits/);
62+
63+
await expect(
64+
readTool?.execute("tool-legacy-read", {
65+
file_path: "legacy.txt",
66+
}),
67+
).rejects.toThrow(/Missing required parameter: path/);
6968
} finally {
7069
await fs.rm(tmpDir, { recursive: true, force: true });
7170
}
7271
});
7372

74-
it("coerces structured content blocks for write", async () => {
73+
it("rejects structured content blocks for write", async () => {
7574
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-structured-write-"));
7675
try {
7776
const tools = createOpenClawCodingTools({ workspaceDir: tmpDir });
7877
const writeTool = tools.find((tool) => tool.name === "write");
7978
expect(writeTool).toBeDefined();
8079

81-
await writeTool?.execute("tool-structured-write", {
82-
path: "structured-write.js",
83-
content: [
84-
{ type: "text", text: "const path = require('path');\n" },
85-
{ type: "input_text", text: "const root = path.join(process.env.HOME, 'clawd');\n" },
86-
],
87-
});
88-
89-
const written = await fs.readFile(path.join(tmpDir, "structured-write.js"), "utf8");
90-
expect(written).toBe(
91-
"const path = require('path');\nconst root = path.join(process.env.HOME, 'clawd');\n",
92-
);
80+
await expect(
81+
writeTool?.execute("tool-structured-write", {
82+
path: "structured-write.js",
83+
content: [
84+
{ type: "text", text: "const path = require('path');\n" },
85+
{ type: "input_text", text: "const root = path.join(process.env.HOME, 'clawd');\n" },
86+
],
87+
}),
88+
).rejects.toThrow(/Missing required parameter: content/);
9389
} finally {
9490
await fs.rm(tmpDir, { recursive: true, force: true });
9591
}
9692
});
9793

98-
it("coerces structured old/new text blocks for edit", async () => {
94+
it("rejects structured edit payloads", async () => {
9995
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-structured-edit-"));
10096
try {
10197
const filePath = path.join(tmpDir, "structured-edit.js");
@@ -105,14 +101,17 @@ describe("createOpenClawCodingTools", () => {
105101
const editTool = tools.find((tool) => tool.name === "edit");
106102
expect(editTool).toBeDefined();
107103

108-
await editTool?.execute("tool-structured-edit", {
109-
file_path: "structured-edit.js",
110-
old_string: [{ type: "text", text: "old" }],
111-
new_string: [{ kind: "text", value: "new" }],
112-
});
113-
114-
const edited = await fs.readFile(filePath, "utf8");
115-
expect(edited).toBe("const value = 'new';\n");
104+
await expect(
105+
editTool?.execute("tool-structured-edit", {
106+
path: "structured-edit.js",
107+
edits: [
108+
{
109+
oldText: [{ type: "text", text: "old" }],
110+
newText: [{ kind: "text", value: "new" }],
111+
},
112+
],
113+
}),
114+
).rejects.toThrow(/Missing required parameter: edits/);
116115
} finally {
117116
await fs.rm(tmpDir, { recursive: true, force: true });
118117
}

src/agents/pi-tools.create-openclaw-coding-tools.adds-claude-style-aliases-schemas-without-dropping-g.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ function extractToolText(result: unknown): string {
2727
}
2828

2929
describe("createOpenClawCodingTools read behavior", () => {
30-
it("applies sandbox path guards to file_path alias", async () => {
30+
it("applies sandbox path guards to canonical path", async () => {
3131
const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sbx-"));
3232
const outsidePath = path.join(os.tmpdir(), "openclaw-outside.txt");
3333
await fs.writeFile(outsidePath, "outside", "utf8");
@@ -36,7 +36,7 @@ describe("createOpenClawCodingTools read behavior", () => {
3636
root: tmpDir,
3737
bridge: createHostSandboxFsBridge(tmpDir),
3838
});
39-
await expect(readTool.execute("sandbox-1", { file_path: outsidePath })).rejects.toThrow(
39+
await expect(readTool.execute("sandbox-1", { path: outsidePath })).rejects.toThrow(
4040
/sandbox root/i,
4141
);
4242
} finally {

src/agents/pi-tools.host-edit.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import os from "node:os";
22
import path from "node:path";
33
import type { AgentToolResult, AgentToolUpdateCallback } from "@mariozechner/pi-agent-core";
4-
import { normalizeToolParams } from "./pi-tools.params.js";
4+
import { getToolParamsRecord } from "./pi-tools.params.js";
55
import type { AnyAgentTool } from "./pi-tools.types.js";
66

77
type EditToolRecoveryOptions = {
@@ -61,12 +61,9 @@ function readEditReplacements(record: Record<string, unknown> | undefined): Edit
6161
}
6262

6363
function readEditToolParams(params: unknown): EditToolParams {
64-
const normalized = normalizeToolParams(params);
65-
const record =
66-
normalized ??
67-
(params && typeof params === "object" ? (params as Record<string, unknown>) : undefined);
64+
const record = getToolParamsRecord(params);
6865
return {
69-
pathParam: readStringParam(record, "path", "file_path", "filePath", "file"),
66+
pathParam: readStringParam(record, "path"),
7067
edits: readEditReplacements(record),
7168
};
7269
}

0 commit comments

Comments
 (0)