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
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
|
||||
@@ -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 () => {
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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 { … }");
|
||||
|
||||
Reference in New Issue
Block a user