fix: stabilize apply_patch custom tool integration

This commit is contained in:
can1357
2026-04-24 00:25:21 +02:00
parent 2a367bf043
commit 601c029c20
12 changed files with 227 additions and 73 deletions
+5 -1
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Changed
- Changed tool dispatch to match model-returned tool calls by either internal tool name or custom wire name, enabling custom OpenAI tool names such as `apply_patch`.
## [14.0.1] - 2026-04-08
### Added
@@ -316,4 +320,4 @@ Initial release under @oh-my-pi scope. See previous releases at [badlogic/pi-mon
- `Agent` constructor now has all options optional (empty options use defaults).
- `queueMessage()` is now synchronous (no longer returns a Promise).
- `queueMessage()` is now synchronous (no longer returns a Promise).
+1
View File
@@ -5,6 +5,7 @@
### Added
- Added `isCopilotTransientModelError()` and `callWithCopilotModelRetry()` helpers in `utils/retry` that detect GitHub Copilot's intermittent `HTTP 400 model_not_supported` responses for preview models (`gpt-5.3-codex`, `gpt-5.4`, `gpt-5.4-mini`, ...) and retry the request up to three times with backoff. OpenAI Responses, OpenAI Completions, and Anthropic provider paths now participate in this retry when the model is served through Copilot.
- Added OpenAI Responses custom-tool grammar support for Codex-style `apply_patch` calls, including freeform streaming, history replay, and forced tool-choice mapping to the custom wire name.
### Changed
@@ -56,7 +56,12 @@ import {
transformRequestBody,
} from "./openai-codex/request-transformer";
import { parseCodexError } from "./openai-codex/response-handler";
import { encodeTextSignatureV1, mapOpenAIResponsesStopReason, parseTextSignature } from "./openai-responses-shared";
import {
encodeResponsesToolCallId,
encodeTextSignatureV1,
mapOpenAIResponsesStopReason,
parseTextSignature,
} from "./openai-responses-shared";
import { transformMessages } from "./transform-messages";
export interface OpenAICodexResponsesOptions extends StreamOptions {
@@ -343,19 +348,33 @@ function extractCodexWebSocketHandshakeHeaders(socket: WebSocket, openEvent?: Ev
);
}
function normalizeCodexToolChoice(choice: ToolChoice | undefined): string | Record<string, unknown> | undefined {
/** @internal Exported for tests. */
export function normalizeCodexToolChoice(
choice: ToolChoice | undefined,
tools: Tool[] = [],
model?: Model<"openai-codex-responses">,
): string | Record<string, unknown> | undefined {
if (!choice) return undefined;
if (typeof choice === "string") return choice;
const allowFreeform = model ? supportsFreeformApplyPatchCodex(model) : false;
const mapName = (name: string): Record<string, string> => {
const customTool = allowFreeform
? tools.find(tool => tool.customFormat && (tool.name === name || tool.customWireName === name))
: undefined;
return customTool
? { type: "custom", name: customTool.customWireName ?? customTool.name }
: { type: "function", name };
};
if (choice.type === "function") {
if ("function" in choice && choice.function?.name) {
return { type: "function", name: choice.function.name };
return mapName(choice.function.name);
}
if ("name" in choice && choice.name) {
return { type: "function", name: choice.name };
return mapName(choice.name);
}
}
if (choice.type === "tool" && choice.name) {
return { type: "function", name: choice.name };
return mapName(choice.name);
}
return undefined;
}
@@ -501,7 +520,7 @@ async function buildTransformedCodexRequestBody(
if (context.tools && context.tools.length > 0) {
params.tools = convertTools(context.tools, model);
if (options?.toolChoice) {
const toolChoice = normalizeCodexToolChoice(options.toolChoice);
const toolChoice = normalizeCodexToolChoice(options.toolChoice, context.tools, model);
if (toolChoice) {
params.tool_choice = toolChoice;
}
@@ -854,7 +873,7 @@ function createOutputBlockForItem(item: CodexEventItem): CodexOutputBlock | null
if (item.type === "function_call") {
return {
type: "toolCall",
id: `${item.call_id}|${item.id}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
name: item.name,
arguments: {},
partialJson: item.arguments || "",
@@ -866,7 +885,7 @@ function createOutputBlockForItem(item: CodexEventItem): CodexOutputBlock | null
// accumulation buffer for the raw input string.
return {
type: "toolCall",
id: `${item.call_id}|${item.id ?? ""}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
name: item.name,
arguments: { input: item.input ?? "" },
customWireName: item.name,
@@ -1053,7 +1072,7 @@ function handleOutputItemDone(
if (item.type === "function_call") {
const toolCall: ToolCall = {
type: "toolCall",
id: `${item.call_id}|${item.id}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
name: item.name,
arguments: parseStreamingJson(item.arguments || "{}"),
};
@@ -1069,7 +1088,7 @@ function handleOutputItemDone(
: (item.input ?? "");
const toolCall: ToolCall = {
type: "toolCall",
id: `${item.call_id}|${item.id ?? ""}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
name: item.name,
arguments: { input: rawInput },
customWireName: item.name,
@@ -54,6 +54,11 @@ export function parseTextSignature(
return { id: signature };
}
export function encodeResponsesToolCallId(callId: string, itemId: string | null | undefined): string {
const stableItemId = itemId && itemId.length > 0 ? itemId : `fc_${Bun.hash(callId).toString(36)}`;
return `${callId}|${stableItemId}`;
}
export function normalizeResponsesToolCallIdForTransform(
id: string,
model?: Model<Api>,
@@ -310,7 +315,7 @@ export async function processResponsesStream<TApi extends Api>(
currentItem = item;
currentBlock = {
type: "toolCall",
id: `${item.call_id}|${item.id}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
name: item.name,
arguments: {},
partialJson: item.arguments || "",
@@ -321,7 +326,7 @@ export async function processResponsesStream<TApi extends Api>(
currentItem = item;
currentBlock = {
type: "toolCall",
id: `${item.call_id}|${item.id ?? ""}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
// Preserve the raw wire name (e.g. `apply_patch`). The agent-loop
// dispatcher matches it against both `Tool.name` and
// `Tool.customWireName`, so this stays wire-accurate through
@@ -494,7 +499,7 @@ export async function processResponsesStream<TApi extends Api>(
: parseStreamingJson(item.arguments || "{}");
const toolCall: ToolCall = {
type: "toolCall",
id: `${item.call_id}|${item.id}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
name: item.name,
arguments: args,
};
@@ -507,7 +512,7 @@ export async function processResponsesStream<TApi extends Api>(
: (item.input ?? "");
const toolCall: ToolCall = {
type: "toolCall",
id: `${item.call_id}|${item.id ?? ""}`,
id: encodeResponsesToolCallId(item.call_id, item.id),
name: item.name,
arguments: { input: rawInput },
customWireName: item.name,
+19 -2
View File
@@ -40,7 +40,7 @@ import {
import { parseGitHubCopilotApiKey } from "../utils/oauth/github-copilot";
import { callWithCopilotModelRetry } from "../utils/retry";
import { adaptSchemaForStrict, NO_STRICT } from "../utils/schema";
import { mapToOpenAIResponsesToolChoice } from "../utils/tool-choice";
import { mapToOpenAIResponsesToolChoice, type OpenAIResponsesToolChoice } from "../utils/tool-choice";
import {
buildCopilotDynamicHeaders,
hasCopilotVisionInput,
@@ -391,7 +391,7 @@ function buildParams(
if (context.tools) {
params.tools = convertTools(context.tools, supportsStrictMode(model), model);
if (options?.toolChoice) {
params.tool_choice = mapToOpenAIResponsesToolChoice(options.toolChoice);
params.tool_choice = mapOpenAIResponsesToolChoiceForTools(options.toolChoice, context.tools, model);
}
// The apply_patch spec §1 marks only `apply_patch` itself as
// `supports_parallel_tool_calls = false`. OpenAI's Responses API
@@ -544,6 +544,23 @@ export function supportsFreeformApplyPatch(model: Model<"openai-responses">): bo
return model.applyPatchToolType === "freeform";
}
/** @internal Exported for tests. */
export function mapOpenAIResponsesToolChoiceForTools(
choice: ToolChoice | undefined,
tools: Tool[],
model: Model<"openai-responses">,
): OpenAIResponsesToolChoice {
const mapped = mapToOpenAIResponsesToolChoice(choice);
if (!mapped || typeof mapped === "string" || mapped.type !== "function" || !supportsFreeformApplyPatch(model)) {
return mapped;
}
const customTool = tools.find(
tool => tool.customFormat && (tool.name === mapped.name || tool.customWireName === mapped.name),
);
return customTool ? { type: "custom", name: customTool.customWireName ?? customTool.name } : mapped;
}
/** @internal Exported for tests. */
export function convertTools(tools: Tool[], strictMode: boolean, model: Model<"openai-responses">): OpenAITool[] {
const allowFreeform = supportsFreeformApplyPatch(model);
+7 -1
View File
@@ -12,7 +12,13 @@ export type OpenAICompletionsToolChoice =
| undefined;
/** OpenAI Responses API tool choice format (flat structure) */
export type OpenAIResponsesToolChoice = "auto" | "none" | "required" | { type: "function"; name: string } | undefined;
export type OpenAIResponsesToolChoice =
| "auto"
| "none"
| "required"
| { type: "function"; name: string }
| { type: "custom"; name: string }
| undefined;
/** Anthropic-compatible tool choice format */
export type AnthropicToolChoice = "auto" | "none" | "any" | { type: "tool"; name: string } | undefined;
+131 -43
View File
@@ -1,5 +1,13 @@
import { describe, expect, test } from "bun:test";
import { convertTools, supportsFreeformApplyPatch } from "@oh-my-pi/pi-ai/providers/openai-responses";
import {
convertTools as convertCodexTools,
normalizeCodexToolChoice,
} from "@oh-my-pi/pi-ai/providers/openai-codex-responses";
import {
convertTools,
mapOpenAIResponsesToolChoiceForTools,
supportsFreeformApplyPatch,
} from "@oh-my-pi/pi-ai/providers/openai-responses";
import {
appendResponsesToolResultMessages,
convertResponsesAssistantMessage,
@@ -7,6 +15,7 @@ import {
} from "@oh-my-pi/pi-ai/providers/openai-responses-shared";
import type { AssistantMessage, Model, Tool, ToolResultMessage } from "@oh-my-pi/pi-ai/types";
import { Type } from "@sinclair/typebox";
import type { ResponseStreamEvent } from "openai/resources/responses/responses";
const GRAMMAR = 'start: "*** Begin Patch" LF';
@@ -26,6 +35,22 @@ function makeModel(overrides: Partial<Model<"openai-responses">> = {}): Model<"o
};
}
function makeCodexModel(overrides: Partial<Model<"openai-codex-responses">> = {}): Model<"openai-codex-responses"> {
return {
id: "gpt-5",
name: "GPT-5",
api: "openai-codex-responses",
provider: "openai-codex",
baseUrl: "https://chatgpt.com/backend-api",
reasoning: true,
input: ["text"],
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
contextWindow: 272000,
maxTokens: 128000,
...overrides,
};
}
const editTool: Tool = {
name: "edit",
customWireName: "apply_patch",
@@ -97,9 +122,42 @@ describe("convertTools: freeform emission", () => {
});
});
describe("tool choice mapping: freeform emission", () => {
const freeformModel = makeModel({ applyPatchToolType: "freeform" });
test("forced internal edit choice targets custom wire name", () => {
expect(mapOpenAIResponsesToolChoiceForTools({ type: "tool", name: "edit" }, [editTool], freeformModel)).toEqual({
type: "custom",
name: "apply_patch",
});
});
test("regular forced choices remain function choices", () => {
expect(
mapOpenAIResponsesToolChoiceForTools({ type: "tool", name: "read_file" }, [plainTool], freeformModel),
).toEqual({
type: "function",
name: "read_file",
});
});
test("codex backend forced internal edit choice targets custom wire name", () => {
expect(
normalizeCodexToolChoice(
{ type: "tool", name: "edit" },
[editTool],
makeCodexModel({ applyPatchToolType: "freeform" }),
),
).toEqual({
type: "custom",
name: "apply_patch",
});
});
});
describe("custom_tool_call stream receive", () => {
async function* makeStream(events: unknown[]): AsyncIterable<any> {
for (const e of events) yield e;
async function* makeStream(events: unknown[]): AsyncIterable<ResponseStreamEvent> {
for (const e of events) yield e as ResponseStreamEvent;
}
test("aggregates delta events into a ToolCall with input arg", async () => {
@@ -189,56 +247,86 @@ describe("custom_tool_call stream receive", () => {
expect(endEvent?.toolCall.name).toBe("apply_patch");
expect(endEvent?.toolCall.customWireName).toBe("apply_patch");
});
test("synthesizes a non-empty item id when custom output item id is absent", async () => {
const output: AssistantMessage = {
role: "assistant",
content: [],
timestamp: Date.now(),
provider: "openai",
model: "gpt-5",
api: "openai-responses",
usage: {
input: 0,
output: 0,
cacheRead: 0,
cacheWrite: 0,
totalTokens: 0,
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
},
stopReason: "stop",
};
const emitted: unknown[] = [];
const stream = {
push: (e: unknown) => emitted.push(e),
end: () => {},
} as never;
await processResponsesStream(
makeStream([
{
type: "response.output_item.added",
item: {
type: "custom_tool_call",
call_id: "call_missing_item",
name: "apply_patch",
input: "",
},
},
{
type: "response.output_item.done",
item: {
type: "custom_tool_call",
call_id: "call_missing_item",
name: "apply_patch",
input: "*** Begin Patch\n*** End Patch\n",
},
},
]),
output,
stream,
makeModel(),
);
const block = output.content[0];
expect(block?.type).toBe("toolCall");
expect((block as { id: string }).id).toStartWith("call_missing_item|fc_");
const endEvent = emitted.find(
(e): e is { type: string; toolCall: { id: string } } =>
!!e && typeof e === "object" && (e as { type?: string }).type === "toolcall_end",
);
expect(endEvent?.toolCall.id).toStartWith("call_missing_item|fc_");
});
});
describe("codex-backend convertTools (chatgpt.com/backend-api)", () => {
// Dynamic import: loading the codex provider pulls in heavy SDK code we
// don't want mixed into module-resolve for unrelated tests.
async function getCodexConvertTools() {
const mod = (await import("@oh-my-pi/pi-ai/providers/openai-codex-responses")) as {
convertTools: (tools: Tool[], model: Model<"openai-codex-responses">) => Array<Record<string, unknown>>;
};
return mod.convertTools;
}
function makeCodexModel(overrides: Partial<Model<"openai-codex-responses">> = {}): Model<"openai-codex-responses"> {
return {
id: "gpt-5",
name: "GPT-5",
api: "openai-codex-responses",
provider: "openai-codex",
baseUrl: "https://chatgpt.com/backend-api",
reasoning: true,
input: ["text"],
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
contextWindow: 272000,
maxTokens: 128000,
...overrides,
};
}
test("edit tool with customFormat becomes a custom grammar tool when flag is set", async () => {
const codexConvertTools = await getCodexConvertTools();
const [out] = codexConvertTools([editTool], makeCodexModel({ applyPatchToolType: "freeform" }));
test("edit tool with customFormat becomes a custom grammar tool when flag is set", () => {
const [out] = convertCodexTools([editTool], makeCodexModel({ applyPatchToolType: "freeform" }));
expect(out.type).toBe("custom");
expect(out.name).toBe("apply_patch");
if (out.type !== "custom") throw new Error("Expected custom tool payload");
expect(out.format).toEqual({ type: "grammar", syntax: "lark", definition: GRAMMAR });
});
test("wire shape matches direct-OpenAI convertTools (single serializer contract)", async () => {
const codexConvertTools = await getCodexConvertTools();
const [codexOut] = codexConvertTools([editTool], makeCodexModel({ applyPatchToolType: "freeform" }));
const [openaiOut] = convertTools(
[editTool],
false,
makeModel({ applyPatchToolType: "freeform" }),
) as unknown as Array<Record<string, unknown>>;
expect(codexOut).toEqual(openaiOut);
test("wire shape matches direct-OpenAI convertTools (single serializer contract)", () => {
const [codexOut] = convertCodexTools([editTool], makeCodexModel({ applyPatchToolType: "freeform" }));
const [openaiOut] = convertTools([editTool], false, makeModel({ applyPatchToolType: "freeform" }));
expect(codexOut).toEqual(openaiOut as unknown as typeof codexOut);
});
test("falls back to function tool when flag is absent", async () => {
const codexConvertTools = await getCodexConvertTools();
const [out] = codexConvertTools([editTool], makeCodexModel());
test("falls back to function tool when flag is absent", () => {
const [out] = convertCodexTools([editTool], makeCodexModel());
expect(out.type).toBe("function");
expect(out.name).toBe("edit");
});
+5
View File
@@ -1,6 +1,11 @@
# Changelog
## [Unreleased]
### Added
- Added an `apply_patch` edit mode that accepts Codex `*** Begin Patch` envelopes, shares patch-mode execution and diagnostics, and renders streaming per-file diffs in the TUI.
### Changed
- Tightened the contract for `SearchParams.recency` in `web/search/providers/base.ts`: providers MUST interpret recency as a pure time filter and MUST NOT use it as an implicit signal to change topic scope, content domain, or ranking strategy.
+7 -6
View File
@@ -449,7 +449,8 @@ export const editToolRenderer = {
const firstApplyPatchEntry = applyPatchSummary?.entries[0];
// Extract path from first edit entry when top-level path is absent (new schema)
const firstEdit = Array.isArray(args.edits) && args.edits.length > 0 ? args.edits[0] : undefined;
const rawPath = args.file_path || args.path || filePathFromEditEntry(firstEdit?.path) || firstApplyPatchEntry?.path || "";
const rawPath =
args.file_path || args.path || filePathFromEditEntry(firstEdit?.path) || firstApplyPatchEntry?.path || "";
const rename = args.rename || firstEdit?.rename || firstEdit?.move || firstApplyPatchEntry?.rename;
const op = args.op || firstEdit?.op || firstApplyPatchEntry?.op;
const { description } = formatEditDescription(rawPath, uiTheme, { rename });
@@ -486,7 +487,7 @@ export const editToolRenderer = {
}
const perFileResults = result.details?.perFileResults;
const totalFiles = Array.isArray(args?.edits) ? countEditFiles(args!.edits as any[]) : 0;
const totalFiles = args?.edits ? countEditFiles(args.edits) : 0;
if (perFileResults && (perFileResults.length > 1 || totalFiles > 1)) {
return renderMultiFileResult(perFileResults, totalFiles, options, uiTheme);
}
@@ -506,15 +507,15 @@ function renderSingleFileResult(
): Component {
const details = result.details;
const isError = result.isError ?? (details && "isError" in details ? details.isError : false);
const firstEdit = Array.isArray(args?.edits) && args!.edits.length > 0 ? args!.edits[0] : undefined;
const firstEdit = args?.edits?.[0];
const rawPath =
args?.file_path ||
args?.path ||
filePathFromEditEntry((firstEdit as any)?.path) ||
filePathFromEditEntry(firstEdit?.path) ||
(details && "path" in details ? details.path : "") ||
"";
const op = args?.op || (firstEdit as any)?.op || details?.op;
const rename = args?.rename || (firstEdit as any)?.rename || details?.move;
const op = args?.op || firstEdit?.op || details?.op;
const rename = args?.rename || firstEdit?.rename || firstEdit?.move || details?.move;
const { language } = formatEditDescription(rawPath, uiTheme, { rename });
const metadataLine =
@@ -206,7 +206,7 @@ export class ToolExecutionComponent extends Container {
try {
const first = expandApplyPatchToEntries({ input })[0];
if (!first?.path) return;
computePatchDiff(first, this.#cwd, {
computePatchDiff({ ...first, op: first.op ?? "update" }, this.#cwd, {
fuzzyThreshold: this.#editFuzzyThreshold,
allowFuzzy: this.#editAllowFuzzy,
}).then(result => {
@@ -4,7 +4,6 @@ import * as os from "node:os";
import * as path from "node:path";
import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution";
import * as themeModule from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
import { editToolRenderer } from "@oh-my-pi/pi-coding-agent/edit/renderer";
import { toolRenderers } from "@oh-my-pi/pi-coding-agent/tools/renderers";
import type { TUI } from "@oh-my-pi/pi-tui";
@@ -17,7 +16,7 @@ async function getUiTheme() {
describe("apply_patch rendering", () => {
it("registers apply_patch to use the edit renderer", () => {
expect(toolRenderers.apply_patch).toBe(editToolRenderer);
expect(toolRenderers.apply_patch).toBe(toolRenderers.edit);
});
it("renders apply_patch results through edit UI instead of generic fallback", async () => {
@@ -65,7 +64,7 @@ describe("apply_patch rendering", () => {
"*** End Patch",
].join("\n");
const component = editToolRenderer.renderCall({ input }, { expanded: false, isPartial: true }, uiTheme);
const component = toolRenderers.apply_patch.renderCall({ input }, { expanded: false, isPartial: true }, uiTheme);
const rendered = Bun.stripANSI(component.render(160).join("\n"));
expect(rendered).toContain("src/first.ts");
@@ -77,7 +76,11 @@ describe("apply_patch rendering", () => {
const uiTheme = await getUiTheme();
const malformedInput = ["*** Begin Patch", "*** Update File: src/bad.ts", "*** End Patch"].join("\n");
const component = editToolRenderer.renderCall({ input: malformedInput }, { expanded: false, isPartial: true }, uiTheme);
const component = toolRenderers.apply_patch.renderCall(
{ input: malformedInput },
{ expanded: false, isPartial: true },
uiTheme,
);
const rendered = Bun.stripANSI(component.render(160).join("\n"));
expect(rendered).toContain("src/bad.ts");
+6 -1
View File
@@ -13,6 +13,11 @@ declare module "*.py" {
export default content;
}
declare module "*.lark" {
const content: string;
export default content;
}
// turndown-plugin-gfm has no published types
declare module "turndown-plugin-gfm" {
import type TurndownService from "turndown";
@@ -20,4 +25,4 @@ declare module "turndown-plugin-gfm" {
export const tables: TurndownService.Plugin;
export const strikethrough: TurndownService.Plugin;
export const taskListItems: TurndownService.Plugin;
}
}