fix(session): resolve wire-name aliases in hasBuiltInTool
The renderer-provenance gate (PR #7774) keys the built-in tool renderer registry on `hasBuiltInTool(name)`. In apply_patch mode the built-in `edit` tool presents on the wire as `apply_patch` (`customWireName`), and tool cards render the streamed call under that wire name. `hasBuiltInTool` only checked literal registry names, so `hasBuiltInTool("apply_patch")` was false and the edit renderer/preview was skipped. Resolve a built-in tool's `customWireName` alias to its built-in owner, while still letting an extension that registers the literal alias name shadow it (the agent loop routes exact-name matches ahead of wire aliases). Fixes #8184
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed the edit tool showing no diff preview in `apply_patch` mode: the built-in `edit` tool presents on the wire as `apply_patch`, but the renderer-provenance gate did not resolve that alias to its built-in owner, so the edit renderer was skipped ([#8184](https://github.com/can1357/oh-my-pi/issues/8184)).
|
||||
|
||||
## [17.2.12] - 2026-08-08
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -357,9 +357,23 @@ export class SessionTools {
|
||||
return this.#toolRegistry.get(name);
|
||||
}
|
||||
|
||||
/** Whether a registry entry came from a built-in factory. */
|
||||
/**
|
||||
* Whether a registry entry came from a built-in factory.
|
||||
*
|
||||
* Resolves `customWireName` aliases too: a built-in tool may present on the
|
||||
* wire under a different name (e.g. `edit` exposes itself as `apply_patch` in
|
||||
* apply_patch mode), and tool cards render the call under that wire name. An
|
||||
* extension registering the literal alias name shadows it — the agent loop
|
||||
* routes exact-name matches ahead of wire aliases — so a registered non-built-in
|
||||
* tool with that name wins and the alias no longer counts as built-in.
|
||||
*/
|
||||
hasBuiltInTool(name: string): boolean {
|
||||
return this.#builtInToolNames.has(name);
|
||||
if (this.#builtInToolNames.has(name)) return true;
|
||||
if (this.#toolRegistry.has(name)) return false;
|
||||
for (const builtInName of this.#builtInToolNames) {
|
||||
if (this.#toolRegistry.get(builtInName)?.customWireName === name) return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
/** Names of every registered tool. */
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { type } from "@oh-my-pi/omptype";
|
||||
import { Agent, type AgentTool } from "@oh-my-pi/pi-agent-core";
|
||||
import { createMockModel } from "@oh-my-pi/pi-ai/providers/mock";
|
||||
import { buildModel } from "@oh-my-pi/pi-catalog/build";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
function makeTool(name: string, customWireName?: string): AgentTool {
|
||||
return {
|
||||
name,
|
||||
label: name,
|
||||
description: `${name} tool`,
|
||||
parameters: type({}),
|
||||
...(customWireName ? { customWireName } : {}),
|
||||
async execute() {
|
||||
return { content: [{ type: "text", text: name }] };
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
async function withSession(
|
||||
tools: readonly AgentTool[],
|
||||
builtInToolNames: readonly string[],
|
||||
run: (session: AgentSession) => void,
|
||||
): Promise<void> {
|
||||
const tempDir = TempDir.createSync("@apply-patch-preview-");
|
||||
const authStorage = await AuthStorage.create(path.join(tempDir.path(), "auth.db"));
|
||||
authStorage.setRuntimeApiKey("anthropic", "test-key");
|
||||
const settings = Settings.isolated({ "compaction.enabled": false });
|
||||
const model = buildModel({
|
||||
id: "mock",
|
||||
name: "mock",
|
||||
api: "openai-responses",
|
||||
provider: "openai",
|
||||
baseUrl: "https://example.invalid",
|
||||
reasoning: false,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 8192,
|
||||
maxTokens: 2048,
|
||||
});
|
||||
const agent = new Agent({
|
||||
getApiKey: () => "test-key",
|
||||
initialState: { model, systemPrompt: ["initial"], tools: [...tools] },
|
||||
streamFn: createMockModel({ responses: [{ content: ["ok"] }] }).stream,
|
||||
});
|
||||
const session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(tempDir.path()),
|
||||
settings,
|
||||
modelRegistry: new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml")),
|
||||
toolRegistry: new Map<string, AgentTool>(tools.map(tool => [tool.name, tool])),
|
||||
builtInToolNames: [...builtInToolNames],
|
||||
rebuildSystemPrompt: async toolNames => ({ systemPrompt: [toolNames.join(",")] }),
|
||||
});
|
||||
try {
|
||||
run(session);
|
||||
} finally {
|
||||
await session.dispose();
|
||||
authStorage.close();
|
||||
tempDir.removeSync();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Regression #8184: the built-in `edit` tool presents on the wire as
|
||||
* `apply_patch` in apply_patch mode (`customWireName`). Tool cards render the
|
||||
* streamed call under that wire name, and the renderer registry is gated behind
|
||||
* `hasBuiltInTool(name)`. If the alias does not resolve to its built-in owner,
|
||||
* the edit renderer/preview is skipped for apply_patch-mode edits.
|
||||
*/
|
||||
describe("AgentSession.hasBuiltInTool wire-name aliases", () => {
|
||||
it("resolves a built-in tool's customWireName alias to built-in provenance", async () => {
|
||||
// Mirrors `edit` in apply_patch mode: internal name `edit`, wire name
|
||||
// `apply_patch`.
|
||||
const edit = makeTool("edit", "apply_patch");
|
||||
await withSession([edit], ["edit"], session => {
|
||||
expect(session.hasBuiltInTool("edit")).toBe(true);
|
||||
// The wire alias must resolve to its built-in owner so the edit
|
||||
// renderer/preview is used for apply_patch-mode cards.
|
||||
expect(session.hasBuiltInTool("apply_patch")).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
it("lets an extension registering the literal alias name shadow the built-in", async () => {
|
||||
// The agent loop routes exact-name matches ahead of wire aliases, so a
|
||||
// non-built-in tool registered under the literal `apply_patch` name wins;
|
||||
// it must not reuse the built-in edit renderer.
|
||||
const edit = makeTool("edit", "apply_patch");
|
||||
const shadow = makeTool("apply_patch");
|
||||
await withSession([edit, shadow], ["edit"], session => {
|
||||
expect(session.hasBuiltInTool("apply_patch")).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user