diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 979907498..827fcd811 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -117,6 +117,7 @@ - Clarified that the production collab relay source and binaries are not currently published, and documented the source-available local protocol relay ([#8165](https://github.com/can1357/oh-my-pi/issues/8165)). - Fixed snapcompact compaction shipping its redundant frame archive out of `SessionMaintenance.compact()` on both the manual RPC response (which hard-failed protocol v1 with a transport error after the compaction had already persisted) and the `auto_compaction_end` event payload (which forced the shrink ladder on every unattended pass); the archive is now stripped from both exits while the persisted compaction entry keeps it ([#8168](https://github.com/can1357/oh-my-pi/issues/8168)). +- 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 diff --git a/packages/coding-agent/src/session/session-tools.ts b/packages/coding-agent/src/session/session-tools.ts index fcd339e97..682fa2b9d 100644 --- a/packages/coding-agent/src/session/session-tools.ts +++ b/packages/coding-agent/src/session/session-tools.ts @@ -379,9 +379,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; } /** Updates source provenance when a live registry entry is replaced or restored. */ diff --git a/packages/coding-agent/test/apply-patch-preview-renderer.test.ts b/packages/coding-agent/test/apply-patch-preview-renderer.test.ts new file mode 100644 index 000000000..96e482351 --- /dev/null +++ b/packages/coding-agent/test/apply-patch-preview-renderer.test.ts @@ -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 { + 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(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); + }); + }); +});