diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 45ae50508..c64fe3719 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -139,6 +139,10 @@ - Fixed image paste failing on Wayland-only Linux sessions by reading PNG clipboard payloads through `wl-paste` before falling back to the native bridge ([#7316](https://github.com/can1357/oh-my-pi/issues/7316)). +### Fixed + +- Fixed prewalk switching to the fast model during read-only investigation: `xd://` devices are dispatched through the `write` tool, so a read-only call such as an `lsp` navigation counted as the first edit/write and armed the one-way hand-off mid-planning. Device dispatches now carry the wrapped tool's approval tier and only trigger the switch at a `write`/`exec` tier — read-only `lsp`, `debug` inspection, and internal-URL `ast_edit` calls no longer downgrade the model ([#7312](https://github.com/can1357/oh-my-pi/issues/7312)). + ## [17.2.3] - 2026-08-01 ### Changed diff --git a/packages/coding-agent/src/session/prewalk.ts b/packages/coding-agent/src/session/prewalk.ts index bfa697d24..8ff9c395f 100644 --- a/packages/coding-agent/src/session/prewalk.ts +++ b/packages/coding-agent/src/session/prewalk.ts @@ -1,6 +1,6 @@ import type { Agent, AgentMessage, AgentToolResult, AgentTurnEndContext } from "@oh-my-pi/pi-agent-core"; import { invalidateMessageCache } from "@oh-my-pi/pi-agent-core/compaction"; -import type { Model } from "@oh-my-pi/pi-ai"; +import type { Model, ToolResultMessage } from "@oh-my-pi/pi-ai"; import { prompt } from "@oh-my-pi/pi-utils"; import type { LocalProtocolOptions } from "../internal-urls"; import { resolveApprovedPlan } from "../plan-mode/approved-plan"; @@ -25,6 +25,28 @@ const PREWALK_ACTION_TOOLS: Record = { }; const PLAN_YOLO_HANDOFF_MESSAGE_TYPE = "plan-yolo-handoff"; +/** + * Whether a completed tool result is the first workspace-mutating action that + * arms the prewalk hand-off. A direct `edit`/`write` call always counts; a + * `write` that dispatched an `xd://` device (e.g. `lsp`, `ast_edit`, `debug`) + * counts only when the wrapped tool resolved to a `write`/`exec` approval tier. + * Read-only device calls — LSP navigation, `debug` inspection, `ast_edit` on + * internal URLs, help lookups — leave the tier `read` (or absent) and must not + * switch the model mid-investigation (issue #7312). + */ +function isPrewalkImplementationAction(result: ToolResultMessage): boolean { + if (!PREWALK_ACTION_TOOLS[result.toolName]) return false; + const details = result.details; + // A direct filesystem edit/write carries no `xd://` dispatch metadata. + if (!details || typeof details !== "object" || !("xdev" in details) || !details.xdev) return true; + const xdev = details.xdev; + // Device dispatch: switch only on a genuine mutation tier. An absent tier + // (help lookup, unresolved approval) declines the switch, matching the + // reporter's "stay on the large model a couple turns longer" preference. + if (typeof xdev !== "object" || !("tier" in xdev)) return false; + return xdev.tier === "write" || xdev.tier === "exec"; +} + /** Capabilities the prewalk coordinator borrows from its owning session. */ export interface PrewalkCoordinatorHost { agent: Agent; @@ -100,7 +122,7 @@ export class PrewalkCoordinator { const todoGateOpen = this.#todoSeen || !this.#host.getActiveToolNames().includes("todo"); const action = todoGateOpen - ? context.toolResults.find(result => PREWALK_ACTION_TOOLS[result.toolName]) + ? context.toolResults.find(result => isPrewalkImplementationAction(result)) : undefined; if (!action) { if (!this.#planInjected) { diff --git a/packages/coding-agent/src/tools/xdev.ts b/packages/coding-agent/src/tools/xdev.ts index 073a412a2..96a89c042 100644 --- a/packages/coding-agent/src/tools/xdev.ts +++ b/packages/coding-agent/src/tools/xdev.ts @@ -36,6 +36,7 @@ import type { RenderResultOptions } from "../extensibility/custom-tools/types"; import { XD_URL_PREFIX } from "../internal-urls/xd-protocol"; import type { Theme } from "../modes/theme/theme"; import { truncateHeadBytes } from "../session/streaming-output"; +import { resolveToolTier, type ToolTier } from "./approval"; import { renderDefaultToolExecution } from "./default-renderer"; import type { Tool } from "./index"; import { replaceTabs } from "./render-utils"; @@ -88,6 +89,13 @@ export interface XdevDispatch { mode: "help" | "execute"; /** Validated inner args, kept for renderer delegation on result rebuilds. */ args?: Record; + /** + * Approval tier of the wrapped tool for {@link args} (`read` = no workspace + * mutation). Absent for `help` dispatches and calls whose tier could not be + * resolved. Consumed by the prewalk coordinator to skip read-only device + * calls when deciding the model hand-off (issue #7312). + */ + tier?: ToolTier; /** Details object returned by the wrapped tool, when executed. */ inner?: unknown; } @@ -415,7 +423,18 @@ export async function dispatchXdevTool( } const validated = parseDeviceArgs(canonical as AiTool, content, toolCallId, () => renderDocs(canonical)); - xdev = { ...xdev, args: validated }; + // Record the wrapped tool's approval tier so the prewalk coordinator can + // tell a read-only device call (e.g. `lsp` navigation) from a real + // workspace mutation without re-decoding the payload. Best-effort: a + // throwing approval leaves the tier absent (prewalk then declines to + // switch), unlike the write gate which fails closed to `exec`. + let tier: ToolTier | undefined; + try { + tier = resolveToolTier(canonical, validated); + } catch { + tier = undefined; + } + xdev = { ...xdev, args: validated, tier }; const innerOnUpdate: AgentToolUpdateCallback | undefined = onUpdate ? partial => onUpdate({ diff --git a/packages/coding-agent/test/agent-session-prewalk.test.ts b/packages/coding-agent/test/agent-session-prewalk.test.ts index 648b4ed94..9257a76ed 100644 --- a/packages/coding-agent/test/agent-session-prewalk.test.ts +++ b/packages/coding-agent/test/agent-session-prewalk.test.ts @@ -400,6 +400,135 @@ describe("AgentSession prewalk", () => { expect(session.model?.id).toBe(primary.id); }); + it("does not switch on a read-only xd:// device dispatched through write (issue #7312)", async () => { + const primary = modelOrThrow("claude-sonnet-4-5"); + const target = modelOrThrow("claude-sonnet-4-6"); + const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml")); + + // A read-only lsp navigation is dispatched as `write xd://lsp`; the write + // result carries the wrapped tool's read tier. Like a bash step, it must + // not arm the hand-off — the model keeps reasoning about code shape on the + // strong model. Mirrors the bounded-continuation flow: one continuation, + // four turns, all primary, then a clean stop. + const readDeviceWrite: AgentTool = + { + name: "write", + label: "Write", + description: "Dispatch a read-only device", + parameters: writeToolSchema, + async execute() { + return { + content: [{ type: "text", text: "references" }], + details: { xdev: { tool: "lsp", mode: "execute", tier: "read" } }, + }; + }, + }; + const mock = createMockModel({ + responses: [ + toolCall("t1", "record"), + toolCall("t2", "write"), + { content: [{ type: "text", text: "Still planning." }], stopReason: "stop" }, + { content: [{ type: "text", text: "Done planning." }], stopReason: "stop" }, + ], + }); + const requested: string[] = []; + const agent = new Agent({ + getApiKey: () => "test-key", + initialState: { + model: primary, + systemPrompt: ["Test"], + tools: [recordTool as AgentTool, readDeviceWrite as AgentTool], + messages: [], + thinkingLevel: Effort.Medium, + }, + convertToLlm, + streamFn: (model, context, options) => { + requested.push(`${model.provider}/${model.id}`); + return mock.stream(model, context, options); + }, + }); + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated({ "compaction.enabled": false }), + modelRegistry, + toolRegistry: new Map([ + [recordTool.name, recordTool as AgentTool], + [readDeviceWrite.name, readDeviceWrite as AgentTool], + ]), + prewalk: { target }, + }); + + await session.prompt("investigate the code shape"); + + expect(requested).toEqual(Array(4).fill(`${primary.provider}/${primary.id}`)); + expect(session.model?.id).toBe(primary.id); + }); + + it("switches on a write-tier xd:// device dispatched through write (issue #7312)", async () => { + const primary = modelOrThrow("claude-sonnet-4-5"); + const target = modelOrThrow("claude-sonnet-4-6"); + const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml")); + + // An lsp rename is a write-tier device call — it must arm the hand-off + // just like a direct edit/write: the write turn stays on the strong model, + // the next turn runs on the target. + const writeDeviceWrite: AgentTool< + typeof writeToolSchema, + { xdev: { tool: string; mode: string; tier: string } } + > = { + name: "write", + label: "Write", + description: "Dispatch a write-tier device", + parameters: writeToolSchema, + async execute() { + return { + content: [{ type: "text", text: "renamed" }], + details: { xdev: { tool: "lsp", mode: "execute", tier: "write" } }, + }; + }, + }; + const mock = createMockModel({ + responses: [toolCall("t1", "record"), toolCall("t2", "write"), { content: ["done"] }], + }); + const requested: string[] = []; + const agent = new Agent({ + getApiKey: () => "test-key", + initialState: { + model: primary, + systemPrompt: ["Test"], + tools: [recordTool as AgentTool, writeDeviceWrite as AgentTool], + messages: [], + thinkingLevel: Effort.Medium, + }, + convertToLlm, + streamFn: (model, context, options) => { + requested.push(`${model.provider}/${model.id}`); + return mock.stream(model, context, options); + }, + }); + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated({ "compaction.enabled": false }), + modelRegistry, + toolRegistry: new Map([ + [recordTool.name, recordTool as AgentTool], + [writeDeviceWrite.name, writeDeviceWrite as AgentTool], + ]), + prewalk: { target }, + }); + + await session.prompt("rename the symbol"); + + expect(requested).toEqual([ + `${primary.provider}/${primary.id}`, + `${primary.provider}/${primary.id}`, + `${target.provider}/${target.id}`, + ]); + expect(session.model?.id).toBe(target.id); + }); + it("re-arms continuation after tool progress between prose turns", async () => { // Regression: a normal prewalk can split planning across several turns: // prose plan, todo init, then prose before implementation. Each tool diff --git a/packages/coding-agent/test/write-xdev-dispatch.test.ts b/packages/coding-agent/test/write-xdev-dispatch.test.ts index a55192c2c..b20dd232f 100644 --- a/packages/coding-agent/test/write-xdev-dispatch.test.ts +++ b/packages/coding-agent/test/write-xdev-dispatch.test.ts @@ -96,6 +96,9 @@ describe("read and write route xd:// device URLs", () => { expect(previewResult.isError).toBeUndefined(); expect(previewResult.details?.xdev?.tool).toBe("ast_edit"); expect(previewResult.details?.xdev?.mode).toBe("execute"); + // The dispatch records the wrapped tool's approval tier so prewalk can + // tell a mutation from a read-only device call (issue #7312). + expect(previewResult.details?.xdev?.tier).toBe("write"); const previewText = previewResult.content.find(entry => entry.type === "text")?.text ?? ""; expect(previewText).toContain("modernWrap"); @@ -109,6 +112,25 @@ describe("read and write route xd:// device URLs", () => { } }); + it("records a read tier on the dispatch of a read-only device", async () => { + const readDevice: AgentTool = { + name: "peek", + label: "Peek", + description: "Read-only device", + parameters: type({ q: "string" }), + approval: () => "read", + async execute() { + return { content: [{ type: "text", text: "peeked" }] }; + }, + }; + const xdev = createTestXdevState([readDevice]); + const write = new WriteTool(xdevSession(process.cwd(), { xdev })); + + const result = await write.execute("write-xdev-read", { path: "xd://peek", content: JSON.stringify({ q: "x" }) }); + expect(result.isError).toBeUndefined(); + expect(result.details?.xdev).toMatchObject({ tool: "peek", mode: "execute", tier: "read" }); + }); + it("rejects near-miss xd addresses before filesystem fallback", async () => { const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "write-xdev-near-miss-")); try {