From a02f88994bf4d4994a68bdf9536a4b1cbf76e181 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 13 Aug 2026 21:59:48 +0000 Subject: [PATCH] fix(tools): preserve workspace-relative path in read hashline headers formatReadHashlineHeader collapsed every relative in-workspace path to its basename, so reading a nested file (e.g. src/settings.json) emitted [settings.json#tag]. When a same-basename file existed at the session cwd, a verbatim follow-up edit resolved against the cwd file; Patcher.prepare only runs snapshot-tag path recovery when the authored path is missing, so the valid edit was deterministically rejected with "hash is not from this session". Keep the workspace-relative path, which names the file uniquely and stays directly resolvable against cwd. Out-of-workspace absolute paths remain shortened; root-level files are unchanged. Fixes #8482 --- packages/coding-agent/CHANGELOG.md | 1 + .../coding-agent/src/tools/read-format.ts | 21 +++++++++--------- .../test/read-edit-out-of-cwd.test.ts | 22 +++++++++++++------ .../test/read-multi-range.test.ts | 9 ++++---- .../coding-agent/test/read-summary.test.ts | 2 +- 5 files changed, 33 insertions(+), 22 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 27e76d274..2a144bcb7 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -6,6 +6,7 @@ - Fixed the parent TUI stalling after a subagent submits its result until terminal focus or resize wakes the event loop ([#8462](https://github.com/can1357/oh-my-pi/issues/8462)). - Fixed `omp update` routing foreign npm/bun bin-directory alias symlinks through the package manager, causing npm EEXIST instead of updating the aliased standalone binary ([#8468](https://github.com/can1357/oh-my-pi/issues/8468)). +- Fixed `read` hashline headers collapsing nested in-workspace paths to the bare basename, which let a same-basename file at the session cwd capture a verbatim follow-up `edit` and deterministically reject it with `hash is not from this session`. Headers now retain the workspace-relative path (e.g. `[src/settings.json#0063]`) ([#8482](https://github.com/can1357/oh-my-pi/issues/8482)). ## [17.3.1] - 2026-08-13 diff --git a/packages/coding-agent/src/tools/read-format.ts b/packages/coding-agent/src/tools/read-format.ts index 0f5167610..9255ccf58 100644 --- a/packages/coding-agent/src/tools/read-format.ts +++ b/packages/coding-agent/src/tools/read-format.ts @@ -37,16 +37,17 @@ export interface HashlineHeaderContext { } export function formatReadHashlineHeader(displayPath: string, tag: string): string { - // In-workspace reads collapse to the bare filename for brevity: the edit - // tool's snapshot-tag recovery rebinds a bare `[name#tag]` onto the in-tree - // file it uniquely names. Out-of-workspace reads can't lean on that — - // recovery refuses to redirect a write outside the cwd/sandbox - // (HashlineFilesystem.allowTagPathRecovery) — so an absolute displayPath - // must stay directly resolvable, otherwise the basename resolves against - // cwd, misses, and the edit fails with "File not found" (e.g. ~/.claude/*). - // `shortenPath` keeps `~/.claude/...` (round-trips through resolveToCwd's ~ - // expansion) instead of leaking the full home path into the read output. - const anchor = path.isAbsolute(displayPath) ? shortenPath(displayPath) : path.basename(displayPath); + // In-workspace reads keep their workspace-relative path (e.g. + // `src/settings.json`), not just the basename: collapsing to the bare name + // made a header ambiguous whenever another same-named file exists at cwd — + // the edit tool would resolve the bare name against cwd, hit the wrong + // file, and reject the valid edit via the snapshot-tag guard (the authored + // path exists, so Patcher's tag-path recovery never runs). The relative + // path stays directly resolvable against cwd and names the file uniquely. + // Out-of-workspace reads use an absolute displayPath; `shortenPath` keeps + // `~/.claude/...` (round-trips through resolveToCwd's ~ expansion) instead + // of leaking the full home path into the read output. + const anchor = path.isAbsolute(displayPath) ? shortenPath(displayPath) : displayPath; return formatHashlineHeader(anchor, tag); } diff --git a/packages/coding-agent/test/read-edit-out-of-cwd.test.ts b/packages/coding-agent/test/read-edit-out-of-cwd.test.ts index cf4bf31d0..d7dbaea13 100644 --- a/packages/coding-agent/test/read-edit-out-of-cwd.test.ts +++ b/packages/coding-agent/test/read-edit-out-of-cwd.test.ts @@ -96,16 +96,24 @@ describe("read → edit round-trip for out-of-cwd files", () => { expect(await Bun.file(outFile).text()).toBe("ALPHA\nbeta\n"); }); - it("still collapses an in-cwd read header to the bare filename", async () => { - const inFile = path.join(cwdDir, "src", "settings.json"); - await fs.mkdir(path.dirname(inFile), { recursive: true }); - await fs.writeFile(inFile, "alpha\nbeta\n"); + it("keeps an in-cwd relative path so an existing basename cannot capture a follow-up edit", async () => { + const rootFile = path.join(cwdDir, "settings.json"); + const nestedFile = path.join(cwdDir, "src", "settings.json"); + await fs.mkdir(path.dirname(nestedFile), { recursive: true }); + await Promise.all([fs.writeFile(rootFile, "root\n"), fs.writeFile(nestedFile, "alpha\nbeta\n")]); const session = createSession(cwdDir); - const header = textOutput(await new ReadTool(session).execute("read-in", { path: inFile })).split("\n")[0]; + const header = textOutput(await new ReadTool(session).execute("read-in", { path: nestedFile })).split("\n")[0]; - expect(header).toMatch(/^\[settings\.json#[0-9A-F]{4}\]$/); - expect(header).not.toContain("src"); + // The header must retain the workspace-relative directory, not collapse + // to the bare `settings.json`, or the edit below resolves against the + // existing cwd file and the snapshot-tag guard rejects the valid edit. + expect(header).toBe(`[${path.join("src", "settings.json")}#${header.slice(-5, -1)}]`); + + await executeHashlineSingle(editOptions(session, `${header}\nPUT 1.=1:\n+ALPHA\n`)); + + expect(await Bun.file(nestedFile).text()).toBe("ALPHA\nbeta\n"); + expect(await Bun.file(rootFile).text()).toBe("root\n"); }); it("recovers a missing cwd path from the active approved local plan", async () => { diff --git a/packages/coding-agent/test/read-multi-range.test.ts b/packages/coding-agent/test/read-multi-range.test.ts index 6e0267da4..5eb2d8e40 100644 --- a/packages/coding-agent/test/read-multi-range.test.ts +++ b/packages/coding-agent/test/read-multi-range.test.ts @@ -53,7 +53,7 @@ describe("read tool multi-range selector", () => { await removeWithRetries(tmpDir); }); - it("uses only the filename in hashline headers for nested files", async () => { + it("keeps the workspace-relative path in hashline headers for nested files", async () => { const filePath = path.join(tmpDir, "src", "nested", "numbered.txt"); await fs.mkdir(path.dirname(filePath), { recursive: true }); await fs.writeFile(filePath, "alpha\nbeta\n"); @@ -62,8 +62,9 @@ describe("read tool multi-range selector", () => { const text = textOutput(await tool.execute("call-filename-header", { path: filePath })); const firstLine = text.split("\n")[0]; - expect(firstLine).toMatch(/^\[numbered\.txt#[0-9A-F]{4}\]$/); - expect(firstLine).not.toContain("src"); + // A same-basename file elsewhere in the tree must not capture a + // follow-up edit, so the header retains the workspace-relative path. + expect(firstLine).toBe(`[${path.join("src", "nested", "numbered.txt")}#${firstLine.slice(-5, -1)}]`); }); it("returns both ranges separated by an elision marker", async () => { @@ -75,7 +76,7 @@ describe("read tool multi-range selector", () => { const result = await tool.execute("call-multi", { path: `${filePath}:3-5,20-22` }); const text = textOutput(result); const firstLine = text.split("\n")[0]; - expect(firstLine).toMatch(/^\[numbered\.txt#[0-9A-F]{4}\]$/); + expect(firstLine).toMatch(/^\[src\/numbered\.txt#[0-9A-F]{4}\]$/); expect(text).toContain("line 3"); expect(text).toContain("line 4"); diff --git a/packages/coding-agent/test/read-summary.test.ts b/packages/coding-agent/test/read-summary.test.ts index 527e727ea..1011da8cc 100644 --- a/packages/coding-agent/test/read-summary.test.ts +++ b/packages/coding-agent/test/read-summary.test.ts @@ -74,7 +74,7 @@ describe("read summary", () => { const result = await tool.execute("read-summary-ts", { path: fixture }); const text = textOutput(result); const firstLine = text.split("\n")[0]; - expect(firstLine).toMatch(/^\[fixture\.ts#[0-9A-F]{4}\]$/); + expect(firstLine).toMatch(/^\[src\/fixture\.ts#[0-9A-F]{4}\]$/); expect(text).toContain("export function alpha(value: string): string { … }"); expect(text).toContain("export function beta(): number { … }");