fix(agent): skip prewalk switch on read-only xd:// device calls
xd:// devices dispatch through the write tool, so a read-only call such as an lsp navigation matched PREWALK_ACTION_TOOLS and armed the one-way model hand-off while still reasoning about code shape. dispatchXdevTool now records the wrapped tool's approval tier on the XdevDispatch, and the prewalk coordinator only treats a device write as an implementation action at write/exec tier. Direct edit/write are unchanged. Fixes #7312
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### 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
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user