fix(pi-claude-cli): unblock parameterless MCP tool calls in triage
Triage with claude-sonnet-4-6 via pi-claude-cli kept looping on
fn_review_spec calls that were rejected by pi's validator with
"root: must be object". Parameterless MCP tools (schema
{type:"object", properties:{}}) emit zero input_json_delta events,
so partialJson stayed "" and the catch fell through to
finalArgs = "" — a string, which TypeBox's Type.Object({}) rightly
refuses. Default empty partialJson to {} so the call lands.
Also:
- Add a 2-step reminder loop in triage before swapping to the
fallback planning model — primary models that wrote PROMPT.md
but forgot fn_review_spec recover from a nudge, no need to pay
the cold-start tax of a new triage on a different model.
- Inject @runfusion/fusion's own pi extension into dashboard/
daemon/serve sessions and propagate the path to createFnAgent
via setHostExtensionPaths so fn_* tools register globally
without requiring `pi install npm:@runfusion/fusion`.
- Drop the "historical" qualifier from replayed tool labels —
Claude was reading "TOOL RESULT (historical Read):" as
"previous session, ignore" and looping on verification.
- Remove subprocess-lifecycle stderr debug logs that landed for
hang diagnosis — root cause is fixed, the noise can go.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -444,6 +444,40 @@ describe("createEventBridge", () => {
|
||||
expect(event.toolCall.arguments).toEqual({ path: "/foo.ts" });
|
||||
});
|
||||
|
||||
it("emits {} for parameterless MCP tool calls (no input_json_delta)", () => {
|
||||
// Parameterless MCP tools (e.g. fn_review_spec, schema
|
||||
// {type:"object", properties:{}}) emit ZERO input_json_delta events.
|
||||
// Without the empty-partialJson guard, finalArgs would fall through to
|
||||
// the raw-string fallback ("") and pi's TypeBox validator would reject
|
||||
// the call with "Validation failed for tool ...: root: must be object".
|
||||
const bridge = createBridgeWithStart();
|
||||
bridge.handleEvent({
|
||||
type: "content_block_start",
|
||||
index: 0,
|
||||
content_block: {
|
||||
type: "tool_use",
|
||||
id: "toolu_01XYZ",
|
||||
name: "mcp__custom-tools__fn_review_spec",
|
||||
},
|
||||
});
|
||||
// No content_block_delta with input_json_delta — Claude emits none for
|
||||
// parameterless tools.
|
||||
stream.push.mockClear();
|
||||
stream.events.length = 0;
|
||||
|
||||
bridge.handleEvent({
|
||||
type: "content_block_stop",
|
||||
index: 0,
|
||||
});
|
||||
|
||||
expect(stream.push).toHaveBeenCalledTimes(1);
|
||||
const event = stream.events[0] as any;
|
||||
expect(event.type).toBe("toolcall_end");
|
||||
expect(event.toolCall.arguments).toEqual({});
|
||||
// The MCP prefix should be stripped: pi sees the bare tool name.
|
||||
expect(event.toolCall.name).toBe("fn_review_spec");
|
||||
});
|
||||
|
||||
it("tracks multiple tool_use blocks independently by Claude event.index", () => {
|
||||
const bridge = createBridgeWithStart();
|
||||
|
||||
|
||||
@@ -21,7 +21,7 @@ describe("buildPrompt", () => {
|
||||
expect(buildPrompt(context)).toBe("ASSISTANT:\nHi there");
|
||||
});
|
||||
|
||||
it("produces 'TOOL RESULT (historical {claudeName}):\\n{content}' for a tool result message", () => {
|
||||
it("produces 'TOOL RESULT ({claudeName}):\\n{content}' for a tool result message", () => {
|
||||
const context = {
|
||||
messages: [
|
||||
{
|
||||
@@ -33,7 +33,7 @@ describe("buildPrompt", () => {
|
||||
} as unknown as any;
|
||||
// Pi tool name "read" should be mapped to Claude name "Read" in the label
|
||||
expect(buildPrompt(context)).toBe(
|
||||
"TOOL RESULT (historical Read):\nfile contents here",
|
||||
"TOOL RESULT (Read):\nfile contents here",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -57,7 +57,7 @@ describe("buildPrompt", () => {
|
||||
"What is in file.ts?",
|
||||
"ASSISTANT:",
|
||||
"Let me read that file.",
|
||||
"TOOL RESULT (historical Read):",
|
||||
"TOOL RESULT (Read):",
|
||||
"export const x = 1;",
|
||||
"USER:",
|
||||
"Now explain it.",
|
||||
@@ -109,7 +109,7 @@ describe("buildPrompt", () => {
|
||||
// Tool name should be mapped from pi "read" to Claude "Read"
|
||||
// Arg "path" should be mapped from pi format to Claude "file_path"
|
||||
expect(result).toContain(
|
||||
'Historical tool call (non-executable): Read args={"file_path":"/file.ts"}',
|
||||
'[Prior tool call — already executed; result follows in TOOL RESULT (Read):] args={"file_path":"/file.ts"}',
|
||||
);
|
||||
});
|
||||
|
||||
@@ -158,7 +158,7 @@ describe("buildPrompt", () => {
|
||||
const result = buildPrompt(context);
|
||||
// Pi "bash" maps to Claude "Bash"
|
||||
expect(result).toContain(
|
||||
"Historical tool call (non-executable): Bash args={}",
|
||||
"[Prior tool call — already executed; result follows in TOOL RESULT (Bash):] args={}",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -177,7 +177,7 @@ describe("buildPrompt", () => {
|
||||
} as unknown as any;
|
||||
|
||||
const result = buildPrompt(context);
|
||||
expect(result).toBe("TOOL RESULT (historical Bash):\nline 1\nline 2");
|
||||
expect(result).toBe("TOOL RESULT (Bash):\nline 1\nline 2");
|
||||
});
|
||||
|
||||
describe("tool name and argument reverse mapping", () => {
|
||||
@@ -237,7 +237,7 @@ describe("buildPrompt", () => {
|
||||
} as unknown as any;
|
||||
|
||||
const result = buildPrompt(context);
|
||||
expect(result).toContain("TOOL RESULT (historical Read):");
|
||||
expect(result).toContain("TOOL RESULT (Read):");
|
||||
});
|
||||
|
||||
it("prefixes custom (non-built-in) tool names with MCP prefix", () => {
|
||||
@@ -281,7 +281,8 @@ describe("buildPrompt", () => {
|
||||
|
||||
const result = buildPrompt(context);
|
||||
// String arguments should be serialized as JSON string
|
||||
expect(result).toContain('Read args="raw string args"');
|
||||
expect(result).toContain('TOOL RESULT (Read):');
|
||||
expect(result).toContain('args="raw string args"');
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -688,7 +689,7 @@ describe("custom tool history replay", () => {
|
||||
} as unknown as any;
|
||||
|
||||
const result = buildPrompt(context);
|
||||
expect(result).toContain("TOOL RESULT (historical Read):");
|
||||
expect(result).toContain("TOOL RESULT (Read):");
|
||||
expect(result).not.toContain("mcp__custom-tools__");
|
||||
});
|
||||
|
||||
@@ -1019,7 +1020,7 @@ describe("buildResumePrompt", () => {
|
||||
],
|
||||
};
|
||||
const result = buildResumePrompt(context) as string;
|
||||
expect(result).toContain("TOOL RESULT (historical Read):");
|
||||
expect(result).toContain("TOOL RESULT (Read):");
|
||||
expect(result).toContain("file contents here");
|
||||
expect(result).toContain("Now explain it");
|
||||
});
|
||||
|
||||
@@ -323,13 +323,24 @@ export function createEventBridge(
|
||||
partial: output,
|
||||
});
|
||||
} else if (block.type === "tool_use") {
|
||||
// Final JSON parse with fallback to raw string
|
||||
// Final JSON parse with fallback to raw string.
|
||||
// Special case: parameterless MCP tools (e.g. fn_review_spec, schema
|
||||
// `{type:"object", properties:{}}`) emit ZERO input_json_delta events,
|
||||
// so `partialJson` stays "". Without this guard we'd JSON.parse("")
|
||||
// → throw → fall through to `finalArgs = ""` (raw string), and pi's
|
||||
// TypeBox validator then rejects with "root: must be object" because
|
||||
// an empty string is not an object. Default to `{}` so the call lands.
|
||||
let finalArgs: Record<string, unknown> | string;
|
||||
try {
|
||||
const parsed = JSON.parse(block.partialJson);
|
||||
finalArgs = translateClaudeArgsToPi(block.claudeName, parsed);
|
||||
} catch {
|
||||
finalArgs = block.partialJson;
|
||||
const trimmedJson = block.partialJson.trim();
|
||||
if (trimmedJson === "") {
|
||||
finalArgs = {};
|
||||
} else {
|
||||
try {
|
||||
const parsed = JSON.parse(trimmedJson);
|
||||
finalArgs = translateClaudeArgsToPi(block.claudeName, parsed);
|
||||
} catch {
|
||||
finalArgs = block.partialJson;
|
||||
}
|
||||
}
|
||||
|
||||
// Update output.content with final arguments
|
||||
|
||||
@@ -62,7 +62,7 @@ type AnthropicContentBlock =
|
||||
* Each message is labeled with its role:
|
||||
* - USER: for user messages
|
||||
* - ASSISTANT: for assistant messages
|
||||
* - TOOL RESULT (historical {toolName}): for tool result messages
|
||||
* - TOOL RESULT ({toolName}): for tool result messages
|
||||
*/
|
||||
/** Module-level counter for placeholder images, reset per buildPrompt call. */
|
||||
let placeholderImageCount = 0;
|
||||
@@ -213,7 +213,7 @@ export function buildResumePrompt(context: PiContext): string | AnthropicContent
|
||||
const claudeToolName = msg.toolName
|
||||
? mapPiToolNameToClaude(msg.toolName)
|
||||
: "unknown";
|
||||
parts.push(`TOOL RESULT (historical ${claudeToolName}):`);
|
||||
parts.push(`TOOL RESULT (${claudeToolName}):`);
|
||||
}
|
||||
parts.push(toolResultContentToText(msg.content));
|
||||
} else if (msg.role === "user") {
|
||||
@@ -283,7 +283,7 @@ export function buildPrompt(context: PiContext): string | AnthropicContentBlock[
|
||||
const claudeToolName = message.toolName
|
||||
? mapPiToolNameToClaude(message.toolName)
|
||||
: "unknown";
|
||||
historyParts.push(`TOOL RESULT (historical ${claudeToolName}):`);
|
||||
historyParts.push(`TOOL RESULT (${claudeToolName}):`);
|
||||
}
|
||||
// Extract text portion of tool result
|
||||
historyParts.push(toolResultContentToText(message.content));
|
||||
@@ -347,7 +347,7 @@ export function buildPrompt(context: PiContext): string | AnthropicContentBlock[
|
||||
const claudeToolName = message.toolName
|
||||
? mapPiToolNameToClaude(message.toolName)
|
||||
: "unknown";
|
||||
parts.push(`TOOL RESULT (historical ${claudeToolName}):`);
|
||||
parts.push(`TOOL RESULT (${claudeToolName}):`);
|
||||
}
|
||||
parts.push(toolResultContentToText(message.content));
|
||||
}
|
||||
@@ -451,8 +451,6 @@ function rewriteCustomToolReferences(
|
||||
}
|
||||
|
||||
let result = prompt;
|
||||
let totalRewrites = 0;
|
||||
const rewritten: string[] = [];
|
||||
for (const tool of tools) {
|
||||
if (BUILT_IN_PI_TOOLS.has(tool.name)) continue;
|
||||
// \b doesn't treat `_` as a word boundary the way we want here, so anchor
|
||||
@@ -464,19 +462,7 @@ function rewriteCustomToolReferences(
|
||||
`(?<![A-Za-z0-9_])(?<!mcp__custom-tools__)${escaped}(?![A-Za-z0-9_])`,
|
||||
"g",
|
||||
);
|
||||
const before = result;
|
||||
result = result.replace(pattern, `mcp__custom-tools__${tool.name}`);
|
||||
if (result !== before) {
|
||||
const matches = before.match(pattern);
|
||||
const count = matches?.length ?? 0;
|
||||
totalRewrites += count;
|
||||
rewritten.push(`${tool.name}×${count}`);
|
||||
}
|
||||
}
|
||||
if (totalRewrites > 0) {
|
||||
console.error(
|
||||
`[pi-claude-cli] system prompt: rewrote ${totalRewrites} custom tool ref(s) [${rewritten.join(", ")}]`,
|
||||
);
|
||||
}
|
||||
return result;
|
||||
}
|
||||
@@ -580,7 +566,7 @@ function contentToText(content: string | unknown[]): string {
|
||||
: typeof rawArgs === "string"
|
||||
? JSON.stringify(rawArgs)
|
||||
: "{}";
|
||||
return `Historical tool call (non-executable): ${claudeName} args=${argsStr}`;
|
||||
return `[Prior tool call — already executed; result follows in TOOL RESULT (${claudeName}):] args=${argsStr}`;
|
||||
}
|
||||
// Unknown block types are represented as a placeholder
|
||||
return `[${String(block.type)}]`;
|
||||
|
||||
@@ -136,11 +136,6 @@ export function streamViaCli(
|
||||
});
|
||||
const getStderr = captureStderr(proc);
|
||||
const spawnTime = Date.now();
|
||||
const procPid = proc.pid;
|
||||
const traceMode = resumeSessionId ? "resume" : "new";
|
||||
console.error(
|
||||
`[pi-claude-cli] spawn pid=${procPid} model=${model.id} mode=${traceMode} effort=${effort ?? "default"} promptLen=${typeof prompt === "string" ? prompt.length : 0} systemPromptLen=${systemPrompt?.length ?? 0} mcp=${options?.mcpConfigPath ? "yes" : "no"}`,
|
||||
);
|
||||
|
||||
// Register in global process registry for teardown cleanup
|
||||
registerProcess(proc);
|
||||
@@ -231,12 +226,8 @@ export function streamViaCli(
|
||||
});
|
||||
|
||||
// Handle subprocess close -- surface crashes with stderr and exit code
|
||||
proc.on("close", (code: number | null, signal: string | null) => {
|
||||
proc.on("close", (code: number | null, _signal: string | null) => {
|
||||
clearTimeout(inactivityTimer);
|
||||
const elapsedMs = Date.now() - spawnTime;
|
||||
console.error(
|
||||
`[pi-claude-cli] close pid=${procPid} code=${code ?? "null"} signal=${signal ?? "null"} elapsedMs=${elapsedMs} broken=${broken}`,
|
||||
);
|
||||
if (broken) return; // Break-early kill, expected
|
||||
if (code !== 0 && code !== null) {
|
||||
const stderr = getStderr();
|
||||
@@ -250,9 +241,6 @@ export function streamViaCli(
|
||||
// Start inactivity timer after writing user message
|
||||
resetInactivityTimer();
|
||||
|
||||
let firstLineLoggedAt = 0;
|
||||
let lineCount = 0;
|
||||
|
||||
// Process NDJSON lines from stdout using event-based callback
|
||||
// NOTE: Using 'line' event instead of `for await` because the async
|
||||
// iterator batches lines, breaking real-time streaming to pi.
|
||||
@@ -261,36 +249,10 @@ export function streamViaCli(
|
||||
|
||||
// Reset inactivity timer on each line of output
|
||||
resetInactivityTimer();
|
||||
lineCount++;
|
||||
if (lineCount === 1) {
|
||||
firstLineLoggedAt = Date.now();
|
||||
console.error(
|
||||
`[pi-claude-cli] first-stdout-line pid=${procPid} afterMs=${firstLineLoggedAt - spawnTime}`,
|
||||
);
|
||||
}
|
||||
|
||||
const msg = parseLine(line);
|
||||
if (!msg) return;
|
||||
|
||||
// Log init system event so we can see MCP server status / model on stderr
|
||||
if (
|
||||
msg.type === "system" &&
|
||||
(msg as { subtype?: string }).subtype === "init"
|
||||
) {
|
||||
const init = msg as unknown as {
|
||||
session_id?: string;
|
||||
mcp_servers?: Array<{ name: string; status: string }>;
|
||||
model?: string;
|
||||
permissionMode?: string;
|
||||
};
|
||||
const mcps = (init.mcp_servers ?? [])
|
||||
.map((s) => `${s.name}=${s.status}`)
|
||||
.join(",");
|
||||
console.error(
|
||||
`[pi-claude-cli] init pid=${procPid} session=${init.session_id ?? "?"} model=${init.model ?? "?"} permissionMode=${init.permissionMode ?? "?"} mcp=[${mcps}]`,
|
||||
);
|
||||
}
|
||||
|
||||
if (msg.type === "stream_event") {
|
||||
// Only forward top-level events to pi's event bridge.
|
||||
// Sub-agent events (parent_tool_use_id !== null) are internal to the CLI.
|
||||
@@ -306,11 +268,6 @@ export function streamViaCli(
|
||||
msg.event.content_block?.type === "tool_use"
|
||||
) {
|
||||
const toolName = msg.event.content_block.name;
|
||||
if (toolName) {
|
||||
console.error(
|
||||
`[pi-claude-cli] tool_use pid=${procPid} name=${toolName} piKnown=${isPiKnownClaudeTool(toolName)}`,
|
||||
);
|
||||
}
|
||||
if (toolName && isPiKnownClaudeTool(toolName)) {
|
||||
// Built-in tool (Read/Write/etc.) OR custom MCP tool (mcp__custom-tools__*)
|
||||
// Internal Claude Code tools (ToolSearch, Task, etc.) are excluded
|
||||
@@ -325,9 +282,6 @@ export function streamViaCli(
|
||||
msg.event.type === "message_stop" &&
|
||||
sawBuiltInOrCustomTool
|
||||
) {
|
||||
console.error(
|
||||
`[pi-claude-cli] break-early pid=${procPid} elapsedMs=${Date.now() - spawnTime} lines=${lineCount}`,
|
||||
);
|
||||
broken = true; // Set guard BEFORE rl.close() to prevent buffered lines
|
||||
clearTimeout(inactivityTimer);
|
||||
// Pi will execute these tools. Kill subprocess to prevent CLI from executing them.
|
||||
|
||||
Reference in New Issue
Block a user