Merge PR #7314: fix(agent): skip prewalk switch on read-only xd:// device calls (@roboomp)

This commit is contained in:
can1357
2026-08-02 20:57:06 +02:00
5 changed files with 199 additions and 3 deletions
+4
View File
@@ -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
+24 -2
View File
@@ -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<string, true> = {
};
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) {
+20 -1
View File
@@ -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<string, unknown>;
/**
* 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({
@@ -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<typeof writeToolSchema, { xdev: { tool: string; mode: string; tier: string } }> =
{
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
@@ -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 {