From bfd1e64bf68358610ca91a1a7585f22860714f12 Mon Sep 17 00:00:00 2001 From: roboomp Date: Thu, 6 Aug 2026 03:40:10 +0000 Subject: [PATCH] fix(read): normalized recovery paths Used the resolved cwd-relative path in summary recovery selectors, PDF image handles, and notebook diagnostics. Fixes #7788 --- packages/coding-agent/CHANGELOG.md | 4 ++++ packages/coding-agent/src/tools/read.ts | 9 +++---- .../coding-agent/test/read-summary.test.ts | 24 ++++++++++++++++--- .../test/tools/read-pdf-images.test.ts | 6 ++--- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 456e88609..dc93bad66 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Normalized resolved file paths in `read` summary recovery selectors, PDF image handles, and notebook errors so suffix-matched input does not teach agents malformed follow-up paths ([#7788](https://github.com/can1357/oh-my-pi/issues/7788)). + ## [17.2.9] - 2026-08-05 ### Breaking Changes diff --git a/packages/coding-agent/src/tools/read.ts b/packages/coding-agent/src/tools/read.ts index 1a9140064..30584b7ff 100644 --- a/packages/coding-agent/src/tools/read.ts +++ b/packages/coding-agent/src/tools/read.ts @@ -2486,6 +2486,7 @@ export class ReadTool implements AgentTool { const imageMetadata = await readImageMetadata(absolutePath); const mimeType = imageMetadata?.mimeType; const ext = path.extname(absolutePath).toLowerCase(); + const resolvedDisplayPath = formatPathRelativeToCwd(absolutePath, this.session.cwd); const shouldConvertWithMarkit = CONVERTIBLE_EXTENSIONS.has(ext); // Profiler reports (macOS `sample` call trees, V8 `.cpuprofile` JSON): @@ -2530,7 +2531,7 @@ export class ReadTool implements AgentTool { fileSize, })); } else if (isNotebookPath(absolutePath) && !isRawSelector(parsed)) { - const notebookText = await readEditableNotebookText(absolutePath, localReadPath); + const notebookText = await readEditableNotebookText(absolutePath, resolvedDisplayPath); if (isMultiRange(parsed) && parsed.kind === "lines") { return this.#buildInMemoryMultiRangeResult(notebookText, parsed.ranges, { details: { resolvedPath: absolutePath }, @@ -2549,7 +2550,7 @@ export class ReadTool implements AgentTool { const result = await convertFileWithMarkit(absolutePath, signal); if (result.ok) { const renderedContent = - ext === ".pdf" ? rewritePdfImagePlaceholders(result.content, localReadPath) : result.content; + ext === ".pdf" ? rewritePdfImagePlaceholders(result.content, resolvedDisplayPath) : result.content; // Route the converted markdown through the in-memory text builder // so line-range selectors (`file.pdf:50-100`, `:5-16,40-80`) and // raw mode apply against the converted output. Without this, @@ -2592,7 +2593,7 @@ export class ReadTool implements AgentTool { return toolResult({ resolvedPath: absolutePath, suffixResolution }) .text( prependSuffixResolutionNotice( - `[Cannot read binary file '${formatPathRelativeToCwd(absolutePath, this.session.cwd)}' (${formatBytes(fileSize)}); not valid UTF-8 text. Use ':raw' to read bytes verbatim.]`, + `[Cannot read binary file '${resolvedDisplayPath}' (${formatBytes(fileSize)}); not valid UTF-8 text. Use ':raw' to read bytes verbatim.]`, suffixResolution, ), ) @@ -2609,7 +2610,7 @@ export class ReadTool implements AgentTool { if (summary?.parsed && summary.elided) { const renderedSummary = this.#renderSummary(summary); const footer = formatSummaryElisionFooter( - localReadPath, + resolvedDisplayPath, renderedSummary.elidedRanges, renderedSummary.elidedLines, ); diff --git a/packages/coding-agent/test/read-summary.test.ts b/packages/coding-agent/test/read-summary.test.ts index b20a46ae4..6a2080860 100644 --- a/packages/coding-agent/test/read-summary.test.ts +++ b/packages/coding-agent/test/read-summary.test.ts @@ -82,6 +82,24 @@ describe("read summary", () => { expect(result.details?.summary?.elidedSpans).toBe(2); }); + it("uses the resolved path in elision recovery selectors after suffix matching", async () => { + const fixture = path.join(tmpDir, "project", "src", "fixture.ts"); + await fs.mkdir(path.dirname(fixture), { recursive: true }); + await fs.writeFile( + fixture, + "export function alpha(value: string): string {\n\tconst clean = value.trim();\n\tconst label = clean || 'alpha';\n\treturn label.toUpperCase();\n}\n\nexport function beta(): number {\n\tconst one = 1;\n\tconst two = 2;\n\treturn one + two;\n}\n", + ); + const malformed = "src/fixture.ts"; + + const tool = new ReadTool(createSession(tmpDir)); + const result = await tool.execute("read-summary-suffix-path", { path: malformed }); + const text = textOutput(result); + + expect(result.details?.suffixResolution?.to).toBe("project/src/fixture.ts"); + expect(text).toContain("with project/src/fixture.ts:1-5,7-11]"); + expect(text).not.toContain(`with ${malformed}:`); + }); + it("summarizes Markdown only when prose summaries are enabled", async () => { const fixture = path.join(tmpDir, "fixture.md"); await fs.writeFile( @@ -316,9 +334,9 @@ describe("read summary", () => { expect(result.details?.summary?.elidedSpans).toBe(2); expect(result.details?.summary?.elidedLines).toBeGreaterThan(0); expect(text).toContain("ln elided"); - expect(text).toContain(`${fixture}:1-5,7-11`); - expect(text).not.toContain(`${fixture}:raw`); - expect(text).not.toContain(`${fixture}:1-9999`); + expect(text).toContain("footer.ts:1-5,7-11"); + expect(text).not.toContain("footer.ts:raw"); + expect(text).not.toContain("footer.ts:1-9999"); // Footer must be the LAST block of output so the recovery hint sits // next to the structural summary it describes. expect(text.trimEnd().endsWith("]")).toBe(true); diff --git a/packages/coding-agent/test/tools/read-pdf-images.test.ts b/packages/coding-agent/test/tools/read-pdf-images.test.ts index 57b08d5e4..17cca4c7e 100644 --- a/packages/coding-agent/test/tools/read-pdf-images.test.ts +++ b/packages/coding-agent/test/tools/read-pdf-images.test.ts @@ -125,8 +125,8 @@ describe("read PDF image extraction", () => { .join("\n"); expect(text).not.toContain("