From 285ed3c47d520dc21b83bb461abb91cca580a4e5 Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 14 Jun 2026 07:36:24 +0000 Subject: [PATCH] fix(plan-mode): normalize write target before bridge routing Unwrap bracketed [path#TAG] headers at the top of WriteTool.execute() so internal-URL detection, plan-mode guard, plan path resolution, and ACP bridge routing all see the same filesystem target. Without this, ['/data/workspaces/can1357__oh-my-pi__2472/.omp-session/2026-06-13T20-19-47-341Z_019ec2a3-fc0d-7000-b1e6-25831d3c3ec5/local/scratch.md' slipped past isInternalUrlPath() and was bridged to the editor instead of staying on disk as a session-local artifact.\n\nFixes #2472 --- .../coding-agent/src/tools/plan-mode-guard.ts | 5 +-- packages/coding-agent/src/tools/write.ts | 13 ++++++-- .../coding-agent/test/write-acp-fs.test.ts | 31 +++++++++++++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/packages/coding-agent/src/tools/plan-mode-guard.ts b/packages/coding-agent/src/tools/plan-mode-guard.ts index ed1c893c1..433f341d4 100644 --- a/packages/coding-agent/src/tools/plan-mode-guard.ts +++ b/packages/coding-agent/src/tools/plan-mode-guard.ts @@ -36,8 +36,9 @@ function isWithinRoot(absolutePath: string, root: string): boolean { * filesystem path drives both authorization and resolution. Only unwraps inputs * that match the strict hashline header shape (`[path]` or `[path#XXXX]` with a * 4-hex tag); anything else returns the original string so the downstream - * resolver surfaces the real error. */ -function unwrapHashlineHeaderPath(targetPath: string): string { + * resolver surfaces the real error. Exported for callers (e.g. `write`) that + * make scheme/bridge-routing decisions before {@link resolvePlanPath} runs. */ +export function unwrapHashlineHeaderPath(targetPath: string): string { const trimmed = targetPath.trimEnd(); if ( trimmed.length < HL_FILE_PREFIX.length + HL_FILE_SUFFIX.length || diff --git a/packages/coding-agent/src/tools/write.ts b/packages/coding-agent/src/tools/write.ts index 8712b6690..94e1fb857 100644 --- a/packages/coding-agent/src/tools/write.ts +++ b/packages/coding-agent/src/tools/write.ts @@ -35,7 +35,7 @@ import { import { invalidateFsScanAfterWrite } from "./fs-cache-invalidation"; import { type OutputMeta, outputMeta } from "./output-meta"; import { formatPathRelativeToCwd, isInternalUrlPath } from "./path-utils"; -import { enforcePlanModeWrite, resolvePlanPath } from "./plan-mode-guard"; +import { enforcePlanModeWrite, resolvePlanPath, unwrapHashlineHeaderPath } from "./plan-mode-guard"; import { cachedRenderedString, createRenderedStringCache, @@ -819,11 +819,20 @@ export class WriteTool implements AgentTool, context?: AgentToolContext, ): Promise> { + // Strip a hashline `[path#TAG]` wrapper up front so every downstream + // decision (scheme routing, internal-URL handler dispatch, plan-mode + // guard, plan path resolution, ACP bridge routing) sees the same + // filesystem target. Without this, a model that pastes a `read` + // header as the `path` arg would slip past `isInternalUrlPath` + // (which fails on a leading `[`) and the bridge router would send a + // `[local://scratch.md#ABCD]` write to the editor instead of the + // session-local sandbox. + const path = unwrapHashlineHeaderPath(rawPath); return untilAborted(signal, async () => { // Strip hashline display prefixes ([PATH#HASH] + LINE:) if the model copied them from read output const { text: cleanContent, stripped } = stripWriteContent(this.session, content); diff --git a/packages/coding-agent/test/write-acp-fs.test.ts b/packages/coding-agent/test/write-acp-fs.test.ts index d4db19fda..3b076baf3 100644 --- a/packages/coding-agent/test/write-acp-fs.test.ts +++ b/packages/coding-agent/test/write-acp-fs.test.ts @@ -100,4 +100,35 @@ describe("write tool ACP fs routing", () => { ).text(), ).toBe(planContent); }); + + it("treats bracketed `[local://...#TAG]` headers as local artifacts, not bridge writes", async () => { + const planPath = "local://PLAN.md"; + const scratchPath = "local://scratch.md"; + // Active plan file is unrelated to the scratch artifact we are writing. + const bracketedScratch = `[${scratchPath}#ABCD]`; + const scratchContent = "scratch notes\n"; + const bridge: ClientBridge = { + capabilities: { writeTextFile: true }, + writeTextFile: async () => undefined, + }; + const bridgeSpy = spyOn(bridge, "writeTextFile"); + const session = createSession(tmpDir, { + bridge, + planMode: { enabled: true, planFilePath: planPath, workflow: "parallel", reentry: false }, + }); + + await new WriteTool(session).execute("call-bracketed", { path: bracketedScratch, content: scratchContent }); + + // Bracketed local headers must not slip past the bridge router — they are + // still session-local artifacts and stay on disk under the local sandbox. + expect(bridgeSpy).not.toHaveBeenCalled(); + expect( + await Bun.file( + resolveLocalUrlToPath(scratchPath, { + getArtifactsDir: session.getArtifactsDir, + getSessionId: session.getSessionId, + }), + ).text(), + ).toBe(scratchContent); + }); });