fix: keep Claude file_path aliases validated
This commit is contained in:
@@ -40,6 +40,7 @@
|
||||
- Auto-reply: restore 300-char heartbeat ack limit and keep >300 char replies instead of dropping them; adjust long heartbeat test content accordingly.
|
||||
- Gateway: `agents.list` now honors explicit `agents.list` config without pulling stray agents from disk; GitHub Copilot CLI auth path uses the updated provider build.
|
||||
- Google: apply patched pi-ai `google-gemini-cli` function call handling (strips ids) after upgrading to pi-ai 0.43.0.
|
||||
- Tools: allow Gemini/Claude-style aliases (`file_path`, `old_string`, `new_string`) in read/write/edit schemas while still enforcing required params at runtime to avoid validation loops.
|
||||
- Auto-reply: elevated/reasoning toggles now enqueue system events so the model sees the mode change immediately.
|
||||
- Tools: keep `image` available in sandbox and fail over when image models return empty output (fixes “(no text returned)”).
|
||||
- Discord: add per-channel `autoThread` to auto-create threads for top-level messages. (#800) — thanks @davidguttman.
|
||||
|
||||
@@ -3,7 +3,8 @@ import os from "node:os";
|
||||
import path from "node:path";
|
||||
|
||||
import sharp from "sharp";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { AgentTool } from "@mariozechner/pi-agent-core";
|
||||
import type { ClawdbotConfig } from "../config/config.js";
|
||||
import { __testing, createClawdbotCodingTools } from "./pi-tools.js";
|
||||
import { createBrowserTool } from "./tools/browser-tool.js";
|
||||
@@ -355,6 +356,71 @@ describe("createClawdbotCodingTools", () => {
|
||||
}
|
||||
});
|
||||
|
||||
describe("Claude/Gemini alias support", () => {
|
||||
it("adds Claude-style aliases to schemas without dropping metadata", () => {
|
||||
const base: AgentTool = {
|
||||
name: "write",
|
||||
description: "test",
|
||||
parameters: {
|
||||
type: "object",
|
||||
required: ["path", "content"],
|
||||
properties: {
|
||||
path: { type: "string", description: "Path" },
|
||||
content: { type: "string", description: "Body" },
|
||||
},
|
||||
},
|
||||
execute: vi.fn(),
|
||||
};
|
||||
|
||||
const patched = __testing.patchToolSchemaForClaudeCompatibility(base);
|
||||
const params = patched.parameters as {
|
||||
properties?: Record<string, unknown>;
|
||||
required?: string[];
|
||||
};
|
||||
const props = params.properties ?? {};
|
||||
|
||||
expect(props.file_path).toEqual(props.path);
|
||||
expect(params.required ?? []).not.toContain("path");
|
||||
expect(params.required ?? []).not.toContain("file_path");
|
||||
});
|
||||
|
||||
it("normalizes file_path to path and enforces required groups at runtime", async () => {
|
||||
const execute = vi.fn(async (_id, args) => args);
|
||||
const tool: AgentTool = {
|
||||
name: "write",
|
||||
description: "test",
|
||||
parameters: {
|
||||
type: "object",
|
||||
required: ["path", "content"],
|
||||
properties: {
|
||||
path: { type: "string" },
|
||||
content: { type: "string" },
|
||||
},
|
||||
},
|
||||
execute,
|
||||
};
|
||||
|
||||
const wrapped = __testing.wrapToolParamNormalization(tool, [
|
||||
{ keys: ["path", "file_path"] },
|
||||
]);
|
||||
|
||||
await wrapped.execute("tool-1", { file_path: "foo.txt", content: "x" });
|
||||
expect(execute).toHaveBeenCalledWith(
|
||||
"tool-1",
|
||||
{ path: "foo.txt", content: "x" },
|
||||
undefined,
|
||||
undefined,
|
||||
);
|
||||
|
||||
await expect(
|
||||
wrapped.execute("tool-2", { content: "x" }),
|
||||
).rejects.toThrow(/Missing required parameter/);
|
||||
await expect(
|
||||
wrapped.execute("tool-3", { file_path: " ", content: "x" }),
|
||||
).rejects.toThrow(/Missing required parameter/);
|
||||
});
|
||||
});
|
||||
|
||||
it("filters tools by sandbox policy", () => {
|
||||
const sandbox = {
|
||||
enabled: true,
|
||||
|
||||
@@ -429,6 +429,96 @@ function wrapSandboxPathGuard(tool: AnyAgentTool, root: string): AnyAgentTool {
|
||||
};
|
||||
}
|
||||
|
||||
type RequiredParamGroup = {
|
||||
keys: string[];
|
||||
allowEmpty?: boolean;
|
||||
label?: string;
|
||||
};
|
||||
|
||||
const CLAUDE_PARAM_GROUPS = {
|
||||
read: [{ keys: ["path", "file_path"], label: "path (path or file_path)" }],
|
||||
write: [{ keys: ["path", "file_path"], label: "path (path or file_path)" }],
|
||||
edit: [
|
||||
{ keys: ["path", "file_path"], label: "path (path or file_path)" },
|
||||
{ keys: ["oldText", "old_string"], label: "oldText (oldText or old_string)" },
|
||||
{ keys: ["newText", "new_string"], label: "newText (newText or new_string)" },
|
||||
],
|
||||
} as const;
|
||||
|
||||
function patchToolSchemaForClaudeCompatibility(
|
||||
tool: AnyAgentTool,
|
||||
): AnyAgentTool {
|
||||
const schema =
|
||||
tool.parameters && typeof tool.parameters === "object"
|
||||
? (tool.parameters as Record<string, unknown>)
|
||||
: undefined;
|
||||
|
||||
if (!schema || !schema.properties || typeof schema.properties !== "object") {
|
||||
return tool;
|
||||
}
|
||||
|
||||
const properties = { ...(schema.properties as Record<string, unknown>) };
|
||||
const required = Array.isArray(schema.required)
|
||||
? schema.required.filter((key): key is string => typeof key === "string")
|
||||
: [];
|
||||
let changed = false;
|
||||
|
||||
const aliasPairs: Array<{ original: string; alias: string }> = [
|
||||
{ original: "path", alias: "file_path" },
|
||||
{ original: "oldText", alias: "old_string" },
|
||||
{ original: "newText", alias: "new_string" },
|
||||
];
|
||||
|
||||
for (const { original, alias } of aliasPairs) {
|
||||
if (!(original in properties)) continue;
|
||||
if (!(alias in properties)) {
|
||||
properties[alias] = properties[original];
|
||||
changed = true;
|
||||
}
|
||||
const idx = required.indexOf(original);
|
||||
if (idx !== -1) {
|
||||
required.splice(idx, 1);
|
||||
changed = true;
|
||||
}
|
||||
}
|
||||
|
||||
if (!changed) return tool;
|
||||
|
||||
return {
|
||||
...tool,
|
||||
parameters: {
|
||||
...schema,
|
||||
properties,
|
||||
...(required.length > 0 ? { required } : {}),
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function assertRequiredParams(
|
||||
record: Record<string, unknown> | undefined,
|
||||
groups: readonly RequiredParamGroup[],
|
||||
toolName: string,
|
||||
): void {
|
||||
if (!record || typeof record !== "object") {
|
||||
throw new Error(`Missing parameters for ${toolName}`);
|
||||
}
|
||||
|
||||
for (const group of groups) {
|
||||
const satisfied = group.keys.some((key) => {
|
||||
if (!(key in record)) return false;
|
||||
const value = record[key];
|
||||
if (typeof value !== "string") return false;
|
||||
if (group.allowEmpty) return true;
|
||||
return value.trim().length > 0;
|
||||
});
|
||||
|
||||
if (!satisfied) {
|
||||
const label = group.label ?? group.keys.join(" or ");
|
||||
throw new Error(`Missing required parameter: ${label}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
function createSandboxedReadTool(root: string) {
|
||||
const base = createReadTool(root);
|
||||
return wrapSandboxPathGuard(createClawdbotReadTool(base), root);
|
||||
@@ -436,12 +526,18 @@ function createSandboxedReadTool(root: string) {
|
||||
|
||||
function createSandboxedWriteTool(root: string) {
|
||||
const base = createWriteTool(root);
|
||||
return wrapSandboxPathGuard(wrapToolParamNormalization(base), root);
|
||||
return wrapSandboxPathGuard(
|
||||
wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.write),
|
||||
root,
|
||||
);
|
||||
}
|
||||
|
||||
function createSandboxedEditTool(root: string) {
|
||||
const base = createEditTool(root);
|
||||
return wrapSandboxPathGuard(wrapToolParamNormalization(base), root);
|
||||
return wrapSandboxPathGuard(
|
||||
wrapToolParamNormalization(base, CLAUDE_PARAM_GROUPS.edit),
|
||||
root,
|
||||
);
|
||||
}
|
||||
// Normalize tool parameters from Claude Code conventions to pi-coding-agent conventions.
|
||||
// Claude Code uses file_path/old_string/new_string while pi-coding-agent uses path/oldText/newText.
|
||||
@@ -471,26 +567,44 @@ function normalizeToolParams(
|
||||
}
|
||||
|
||||
// Generic wrapper to normalize parameters for any tool
|
||||
function wrapToolParamNormalization(tool: AnyAgentTool): AnyAgentTool {
|
||||
function wrapToolParamNormalization(
|
||||
tool: AnyAgentTool,
|
||||
requiredParamGroups?: readonly RequiredParamGroup[],
|
||||
): AnyAgentTool {
|
||||
const patched = patchToolSchemaForClaudeCompatibility(tool);
|
||||
return {
|
||||
...tool,
|
||||
...patched,
|
||||
execute: async (toolCallId, params, signal, onUpdate) => {
|
||||
const normalized = normalizeToolParams(params);
|
||||
const record =
|
||||
normalized ??
|
||||
(params && typeof params === "object"
|
||||
? (params as Record<string, unknown>)
|
||||
: undefined);
|
||||
if (requiredParamGroups?.length) {
|
||||
assertRequiredParams(record, requiredParamGroups, tool.name);
|
||||
}
|
||||
return tool.execute(toolCallId, normalized ?? params, signal, onUpdate);
|
||||
},
|
||||
};
|
||||
}
|
||||
function createClawdbotReadTool(base: AnyAgentTool): AnyAgentTool {
|
||||
const patched = patchToolSchemaForClaudeCompatibility(base);
|
||||
return {
|
||||
...base,
|
||||
...patched,
|
||||
execute: async (toolCallId, params, signal) => {
|
||||
const normalized = normalizeToolParams(params);
|
||||
const record =
|
||||
normalized ??
|
||||
(params && typeof params === "object"
|
||||
? (params as Record<string, unknown>)
|
||||
: undefined);
|
||||
assertRequiredParams(record, CLAUDE_PARAM_GROUPS.read, base.name);
|
||||
const result = (await base.execute(
|
||||
toolCallId,
|
||||
normalized ?? params,
|
||||
signal,
|
||||
)) as AgentToolResult<unknown>;
|
||||
const record = normalized ?? (params as Record<string, unknown>);
|
||||
const filePath =
|
||||
typeof record?.path === "string" ? String(record.path) : "<unknown>";
|
||||
const normalizedResult = await normalizeReadImageResult(result, filePath);
|
||||
@@ -501,6 +615,10 @@ function createClawdbotReadTool(base: AnyAgentTool): AnyAgentTool {
|
||||
|
||||
export const __testing = {
|
||||
cleanToolSchemaForGemini,
|
||||
normalizeToolParams,
|
||||
patchToolSchemaForClaudeCompatibility,
|
||||
wrapToolParamNormalization,
|
||||
assertRequiredParams,
|
||||
} as const;
|
||||
|
||||
function throwAbortError(): never {
|
||||
@@ -618,12 +736,22 @@ export function createClawdbotCodingTools(options?: {
|
||||
if (tool.name === "write") {
|
||||
if (sandboxRoot) return [];
|
||||
// Wrap with param normalization for Claude Code compatibility
|
||||
return [wrapToolParamNormalization(createWriteTool(workspaceRoot))];
|
||||
return [
|
||||
wrapToolParamNormalization(
|
||||
createWriteTool(workspaceRoot),
|
||||
CLAUDE_PARAM_GROUPS.write,
|
||||
),
|
||||
];
|
||||
}
|
||||
if (tool.name === "edit") {
|
||||
if (sandboxRoot) return [];
|
||||
// Wrap with param normalization for Claude Code compatibility
|
||||
return [wrapToolParamNormalization(createEditTool(workspaceRoot))];
|
||||
return [
|
||||
wrapToolParamNormalization(
|
||||
createEditTool(workspaceRoot),
|
||||
CLAUDE_PARAM_GROUPS.edit,
|
||||
),
|
||||
];
|
||||
}
|
||||
return [tool as AnyAgentTool];
|
||||
});
|
||||
|
||||
@@ -241,6 +241,13 @@ const ExecutableTokenSchema = z
|
||||
.string()
|
||||
.refine(isSafeExecutableValue, "expected safe executable name or path");
|
||||
|
||||
const ToolsAudioTranscriptionSchema = z
|
||||
.object({
|
||||
args: z.array(z.string()).optional(),
|
||||
timeoutSeconds: z.number().int().positive().optional(),
|
||||
})
|
||||
.optional();
|
||||
|
||||
const NativeCommandsSettingSchema = z.union([z.boolean(), z.literal("auto")]);
|
||||
|
||||
const ProviderCommandsSchema = z
|
||||
@@ -249,25 +256,6 @@ const ProviderCommandsSchema = z
|
||||
})
|
||||
.optional();
|
||||
|
||||
const CommandsSchema = z
|
||||
.object({
|
||||
native: NativeCommandsSettingSchema.optional().default("auto"),
|
||||
text: z.boolean().optional(),
|
||||
config: z.boolean().optional(),
|
||||
debug: z.boolean().optional(),
|
||||
restart: z.boolean().optional(),
|
||||
useAccessGroups: z.boolean().optional(),
|
||||
})
|
||||
.optional()
|
||||
.default({ native: "auto" });
|
||||
|
||||
const ToolsAudioTranscriptionSchema = z
|
||||
.object({
|
||||
args: z.array(z.string()).optional(),
|
||||
timeoutSeconds: z.number().int().positive().optional(),
|
||||
})
|
||||
.optional();
|
||||
|
||||
const TelegramTopicSchema = z.object({
|
||||
requireMention: z.boolean().optional(),
|
||||
skills: z.array(z.string()).optional(),
|
||||
@@ -732,6 +720,18 @@ const MessagesSchema = z
|
||||
})
|
||||
.optional();
|
||||
|
||||
const CommandsSchema = z
|
||||
.object({
|
||||
native: NativeCommandsSettingSchema.optional().default("auto"),
|
||||
text: z.boolean().optional(),
|
||||
config: z.boolean().optional(),
|
||||
debug: z.boolean().optional(),
|
||||
restart: z.boolean().optional(),
|
||||
useAccessGroups: z.boolean().optional(),
|
||||
})
|
||||
.optional()
|
||||
.default({ native: "auto" });
|
||||
|
||||
const HeartbeatSchema = z
|
||||
.object({
|
||||
every: z.string().optional(),
|
||||
@@ -801,7 +801,6 @@ const SandboxDockerSchema = z
|
||||
apparmorProfile: z.string().optional(),
|
||||
dns: z.array(z.string()).optional(),
|
||||
extraHosts: z.array(z.string()).optional(),
|
||||
binds: z.array(z.string()).optional(),
|
||||
})
|
||||
.optional();
|
||||
|
||||
|
||||
Reference in New Issue
Block a user