fix(diff): decode git-quoted paths from the PR diff

[aliasunder]

Summary

A PR that touches a file with a non-ASCII name got a worse review. The same applied to a name with a " or a \. GitHub's diff C-quotes those paths ("b/nn/0016_\303\245-f\303\270de.md"). parse-diff removes the quotes but keeps the escapes, so the action worked with a path that does not exist:

• The workspace read failed, so the file went to the model diff-only.
• Diff exclusion matched the escaped string.
• Inline comments named a path GitHub rejects, so they fell back to an issue comment.
• The annotated diff header showed the escaped form.

This PR decodes the quoted paths once, right after parse-diff, so every later step sees the real path.

• Decoding rule. A backslash in a parse-diff path means git quoted it. Git quotes every path that contains a backslash, even with core.quotePath=false, and every character it quotes becomes a backslash escape. The decoder accepts the escapes git's unquote_c_style accepts: \a \b \t \n \v \f \r \" \\ and \ooo with a first digit of 0-3. The resulting bytes decode as UTF-8, and node:buffer's isUtf8 validates them.
• Rejected input. The decoder does not guess a path. It rejects an unknown escape and bytes that are not UTF-8. The path stays as received, except that a raw line break becomes git's octal escape, and the action logs a warning with the reason. A replacement-character decode would name a file that does not exist.
• Findings on a rejected path. A rejected quoted path keeps its escapes, so it names no file GitHub knows. GitHub rejects a whole review when one inline comment names such a path, and every inline finding then falls back to an issue comment. The file stays in the diff and the prompt, but it gets no commentable lines. Its findings post as standalone comments with a location note saying the file's path cannot take an inline comment, and the rest of the review still posts inline.
• Line breaks. A path that contains a line break is also rejected, and the warning names the code point. The rejected characters are LF, CR, VT, FF, NEL (U+0085), U+2028, and U+2029. Paths are rendered raw in the annotated diff header and in markdown, so a decoded newline would let a filename forge a header line or split a table row. A quoted path keeps its escaped form, which breaks no line. The check runs on unquoted paths too, so it does not depend on GitHub quoting every line break, and a raw line break in a rejected path is written as git's octal escape of its UTF-8 bytes (U+2028 becomes \342\200\250).
• Other control characters decode. A tab, NUL, ESC, DEL, or other control character breaks no line, so a file named with one gets its workspace read and inline comments. The logger's JSON escapes every C0 character, and DEL prints as an invisible byte. Markdown and the prompt keep each one inside its line. A workspace read the filesystem refuses falls back to the diff alone.

Changes

• src/diff/quoted-paths.ts (new): decodeQuotedPath returns unquoted, decoded, or rejected with a reason. decodeQuotedFilePaths maps parse-diff files, decodes from and to, and passes /dev/null and an absent path through unchanged. It returns the files with the set of rejected paths, each spelled as the diff spelled it with any raw line break escaped. Each distinct path decodes once, so a decoded path logs once at debug and a rejected path logs once at warn.
• src/orchestrate.ts: wraps the single parseDiff call in decodeQuotedFilePaths. A file whose new path was rejected is left out of the commentable-lines map, with a debug log naming the path. Its findings render with their own location note instead of the beyond-diff one. The note is chosen by comparing normalized paths, the same way the unknown-file filter matches, so a finding spelled ./a\rb.ts still gets it. Excluded diff paths are compared to priority docs without a second posix.normalize, because exclusion already normalizes them.
• src/review/comment-mapping.ts: renderRejectedPathFinding renders a finding in a changed file whose path cannot take an inline comment. The finding location line and the max-findings cap note render each path with renderCodeSpan.
• src/review/markdown.ts (new): renderCodeSpan wraps a path in an inline code span that a backtick in the path cannot close. It writes each line break as its octal escape, because a blank line ends a code span. The delimiter is one backtick longer than the path's longest backtick run, and a path that starts or ends with a backtick or a space gets one space of padding on each side. Git never quotes a backtick, so such a path reaches these comments as written.
• src/review/context-notes.ts: the priority-doc, related-file, diff-exclusion, and conventions notes render paths with renderCodeSpan.
• src/review/review-summary.ts: the job-summary path lists escape line breaks, then backslashes, then backticks and pipes. Workspace-scan paths (related files, mention-matched docs, cap-excluded paths) never pass the decoder's line-break check, so a raw newline would split the table row; it is written as its octal escape first. A decoded backslash right before a pipe would otherwise cancel the pipe's escape and split the table cell. Two backticks in one cell, which git never quotes, would otherwise open a code span. The PR line renders the head and base branch names with renderCodeSpan, because a branch name can contain a backtick.
• src/diff/__tests__/quoted-paths.test.ts (new): decoder tests.
  • Decoded cases: two-byte and three-byte UTF-8, \", \\, raw non-ASCII text beside an escape, \\ directly before octal escapes, \\ followed by octal-looking digits, and a lone trailing backslash.
  • Unquoted cases: plain, spaced, and /dev/null.
  • Malformed cases: an unknown escape, octal above one byte, octal with too few digits, and invalid UTF-8.
  • Control characters that break no line decode: \t, \a, \b, an octal NUL, an octal ESC, and an octal DEL.
  • Line-break cases are rejected: \n, \r, \v, \f, U+0085, U+2028, and U+2029, plus an unquoted path holding a raw U+2028.
  • File-level cases: a quoted rename through real parse-diff, a path ending in a backslash through both parse-diff header forms, added and deleted files, an absent old path, an absent new path, the rejected-path set, a rename whose old path alone is rejected, and the debug and warn logs.
• src/__tests__/orchestrate.test.ts:
  • A quoted non-ASCII diff reaches the changed-file read under its decoded path. A finding on that path posts as an inline comment anchored to it.
  • A filename with an escaped newline and a fake === forged.ts === header leaves exactly one header line in the annotated diff. An unquoted filename with a raw U+2028 does the same when the diff is split on every line-break character.
  • A diff with one normal file and one whose path decodes to a carriage return posts the normal file's finding inline and the rejected file's finding as a standalone comment with the rejected-path location note. No inline comment carries the escaped path.
  • A finding that spells the rejected path with a leading ./ still posts with the rejected-path note.
  • The escaped changed-file path test now expects the decoded docs/a"b.md. The prompt escapes the quote in path attributes, so the model reports docs/a"b.md, and the finding still posts inline under the decoded path.
  • An excluded diff path written with a doubled slash still removes its priority doc from the priority-doc read and reaches the scan exclusions normalized, beside the conventions file.
• src/review/__tests__/comment-mapping.test.ts: the rejected-path renderer's full body, for a plain path and for a path with a backtick.
• src/review/__tests__/review-summary.test.ts: a backslash before a pipe renders as \\\|, a path with a newline and pipes stays on one table row, two paths with backticks in one cell render escaped, and a branch name with a backtick keeps its code span whole.
• AGENTS.md: the diff/ structure line lists path decoding, and the review/ line lists markdown code spans.
• README.md: the feature list and step 9 of How it works name findings in a changed file whose diff path cannot take an inline comment among the standalone comments, along with every in-diff finding when GitHub rejects the inline review. The Unknown-file filter section says a diff header prints the decoded path, and that a path whose decoding was rejected keeps its escaped spelling.
• src/review/__tests__/markdown.test.ts (new): code spans for a plain path, one backtick, a two-backtick run, an edge backtick at either end, an edge space at either end, and a path holding a blank line.
• src/review/__tests__/context-notes.test.ts: a diff-excluded path with a backtick keeps its code span whole.

Testing

• npm test: 899 passed, after merging main. npm run lint and npm run build pass.
• Mutation checks, each restored with git checkout:
  • Removing the decode from orchestrate.ts fails the orchestrate test, because changedPaths gets nn/0016_\\303\\245-f\\303\\270de.md.
  • Removing the UTF-8 check fails both invalid-UTF-8 tests, because the result becomes caf�.md.
  • Widening the octal range to [0-7]{3} fails the above-one-byte test.
  • Dropping the \" escape fails the escaped-quote test.
  • Removing the no-backslash short-circuit fails the three unquoted tests and the no-logging test.
  • Disabling the line-break rejection fails 12 tests: the seven line-break cases, the unquoted U+2028 case, the newline warn test, the rename test, the orchestrate header test, and the orchestrate rejected-path test.
  • Keeping a rejected path's raw line break unescaped fails the unquoted U+2028 file-level test and the raw-separator orchestrate header test.
  • Widening the rejected set back to every control character (\p{Cc}) fails all six break-no-line decode cases, tab included.
  • Skipping the check for unquoted paths fails the unquoted U+2028 case.
  • Always setting to fails the absent-new-path test, because the file gains to: "".
  • Dropping the \a escape fails the bell case.
  • Removing posix.normalize from exclusion's classification path fails the doubled-slash test, because assets/guide.md stays in the priority-doc read.
  • Escaping pipes before backslashes in the job-summary list fails both pipe-escape tests.
  • Removing the lone-trailing-backslash branch fails both trailing-backslash cases and the header-forms test.
  • Keeping rejected paths in the commentable-lines map fails the rejected-path orchestrate test, because the posted review gains an inline comment on a\rb.ts.
  • Rendering rejected-path findings with the beyond-diff note fails the rejected-path orchestrate test.
  • Comparing the raw finding path against the rejected paths fails the leading-./ orchestrate test.
  • Skipping the line-break escape in the job-summary list and in renderCodeSpan fails the one-row table test and the blank-line code-span test.
  • Fixing the code-span delimiter at one backtick fails 6 tests: the four backtick cases in the code-span suite, the backtick rejected-path body, and the backtick diff-exclusion note.
• The parse-diff 0.12.0 probe ran on literal diffs. It keeps escapes in from and to for every quoted case:
  • added: from: "/dev/null", to: "nn/0016_\\303\\245-f\\303\\270de.md"
  • rename with edits and pure rename: from: "docs/\\341\\213\\265.md", to: "docs/\\341\\213\\265-new.md". Pure renames take their paths from the diff --git line, because parse-diff ignores rename from and rename to.
  • deleted: from: "nn/\\303\\245.md", to: "/dev/null"
  • escaped quote: from and to are both "say \\\"hi\\\".md"
  • A path ending in \ comes back as ends-with\ with a lone trailing backslash, because parse-diff's closing-quote regex drops the escaped backslash. The decoder reads that lone trailing backslash as the escaped backslash parse-diff dropped. The diff --git line keeps both backslashes, so a hunk-less rename decodes to the same path.
  • The index field holds only blob hashes and the mode, and nothing reads it.
• A local git repo confirmed the quoting:
  • Git quotes back\slash.md, say "hi".md, a tab, a newline, a C0 byte, and DEL under both core.quotePath settings.
  • It quotes å-føde.md, ድ.md, U+0085, and U+2028 only with the default, as octal escapes.
  • A name with a space, a |, or a backtick is never quoted.

🤖 Generated with Claude Code

[umm-actually]

umm-actually re-reviewed at cec2d4b

No new findings (13 tracked finding(s) across all runs).

---

umm-actually · deepseek/deepseek-v4.1-flash

[umm-actually]

Document the standalone route for paths that cannot take an inline comment
Low severity · conventions · medium confidence

README.md:13 — beyond the diff's line ranges, in code the changes touch or depend on.

This PR adds a third way a finding surfaces — a finding in a changed file whose diff path cannot take an inline comment posts as a standalone issue comment with its own location note — but the README still enumerates only an inline review plus beyond-diff standalone comments. A reader cannot account for why a changed file's finding appeared outside the diff, and AGENTS.md requires docs to update in the same change that alters behavior.

Failure scenario: A PR touches a file whose name carries an escape git C-quotes (e.g. a carriage return in the name). The file's finding posts as a standalone issue comment with the rejected-path note and never appears inline, while a user following the README expects changed-file findings to be anchored to the diff or to be described as beyond-diff.

Suggested fix

Extend the "What it does" bullet and "How it works" step 9 to name the route, e.g. "Findings in a changed file whose diff path cannot take an inline comment also post as standalone comments" — in the same shape as the existing beyond-diff clause.

---

umm-actually · deepseek/deepseek-v4.1-flash

[umm-actually]

Escape backticks in the finding location line
Low severity · subtle bugs · medium confidence

src/review/comment-mapping.ts:396 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: the location line wraps path:line in single backticks, so a filename containing a backtick closes the code span early and the comment renders a truncated location with unbalanced backticks. The decoder added by this PR keeps such paths (git never quotes a backtick), so they now flow reliably into this renderer, including the new renderRejectedPathFinding.

Failure scenario: A PR renames a file to src/ab.ts. Every rendered finding comment on that file has a broken location line: markdown renders src/a` as code and leaves the remainder (including the rest of the note) as loose prose.

Suggested fix

Size the code fence to the longest backtick run in the path the way suggestionBlock already sizes its diff fence (or render the location as a fenced block), so a backtick in the filename cannot close the span.

---

umm-actually · deepseek/deepseek-v4.1-flash

[aliasunder]

Dispositions for the two umm-actually findings posted as PR-level comments on 44f6ab5:

• 5922147789, document the standalone route for paths that cannot take an inline comment: valid, fixed in 25408be. The README's feature list and step 9 of How it works now name findings in a changed file whose diff path cannot take an inline comment among the findings that post as standalone comments. No other doc lists the standalone routes.
• 5922147913, escape backticks in the finding location line: valid, fixed in the same push. A new renderCodeSpan helper (src/review/markdown.ts) sizes the code-span delimiter one backtick past the path's longest backtick run, and it pads a path that starts or ends with a backtick or a space. It replaces the single-backtick span at all five places that wrapped a path in one: the finding location line, the max-findings cap note, and three context-note renderers (priority docs and related files, diff exclusions, the conventions file). Tests cover the helper's edge cases, a rejected-path body, and a diff-exclusion note with a backtick in the path. Fixing the delimiter at one backtick fails 6 of them.

---

🔍 ship-check · pr-monitor · claude-opus-5-5

[umm-actually]

Neutralize --> in a finding path before it enters the dedup anchor
Low severity · security · medium confidence

src/review/comment-mapping.ts:407 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: the anchor HTML comment interpolates the finding's path raw, so a file whose name contains --> closes the comment and everything after it in the path renders as markdown in the comment the bot posts. This PR added renderCodeSpan to the location line directly above the anchor but left the anchor (and the identical construction in renderCommentBody) unescaped.

Failure scenario: A PR adds a file named src/x --> ![a](https://evil.example/p.png) <!-- y.ts containing a bug; the model reports a finding on it and the posted body contains <!-- umm-actually:src/x --> ![a](https://evil.example/p.png) <!-- y.ts:correctness:1 -->, so the bot renders an attacker-chosen image/link inside its own review comment.

Suggested fix

Encode the `--` sequence when building the anchor key and reverse it in parseAnchorKey, or treat a path containing `-->` as uncommentable the way this PR treats rejected diff paths, keeping the raw path only in the escaped display surfaces.

---

umm-actually · deepseek/deepseek-v4.1-flash

[umm-actually]

Escape the PR's head and base refs in the job summary
Low severity · subtle bugs · medium confidence

src/review/review-summary.ts:109 — beyond the diff's line ranges, in code the changes touch or depend on.

Pre-existing: renderReviewSummary now escapes line breaks, backslashes, and pipes for every path list (renderCommaList) but the PR line still interpolates headRef and baseRef raw inside code spans, so a branch name containing a backtick closes the span and lets the remaining text render as loose markdown in the workflow job summary.

Failure scenario: A PR from a branch named chore/fixdocproduces the summary linePR #7 · chore/fixdoc→main·abc123d— the code span closes at the branch's backtick and `doc` plus the following text renders as markdown; a branch carrying ](https://evil.example/p.png) `` injects an image into the job summary.

Suggested fix

Run headRef and baseRef through the same escaping renderCommaList applies to paths (line breaks, backslashes, backticks/span delimiters) before interpolating them into the summary line.

---

umm-actually · deepseek/deepseek-v4.1-flash

[aliasunder]

Dispositions for the two umm-actually findings posted as PR-level comments on 7a1be0e:

• 5922355118, neutralize --> in a finding path before it enters the dedup anchor: this can't happen. The finding schema already rejects such a path before any rendering: findingSchema in src/review/finding.ts refines file with isSafeFilePath, which fails any path containing --> or a line break, with the message file must be a single-line path without "-->". A finding on src/x --> ![a](...) <!-- y.ts therefore never reaches computeAnchorKey, so neither anchor (renderCommentBody or renderIssueCommentFinding) can carry it. The suggested fix, encoding -- in the key and decoding it in parseAnchorKey, guards a value that validation has already excluded.
• 5922355226, escape the PR's head and base refs in the job summary: valid, fixed in b14ba0c. A branch name can contain a backtick, so the PR line now renders both refs with renderCodeSpan. A new test renders chore/fixdocand asserts the full line, and putting back the single-backtick span fails it. The other dynamic code spans hold only internal values (phase ids, the short SHA, configured model ids). The prompt'sBranch:` line is plain text, not markdown.

---

🔍 ship-check · pr-monitor · claude-opus-5-5

[umm-actually]

Update the diff-header spelling claim in the unknown-file filter docs
Low severity · correctness · medium confidence

README.md:151 — beyond the diff's line ranges, in code the changes touch or depend on.

The Unknown-file filter section states that a diff header prints a path as the diff spells it, but quoted diff paths are now decoded before annotateDiff builds the header, so the header prints the decoded spelling (a diff that spells docs/a escaped-quote b.md now appears as docs/a b.md). A reader is told the C-escaped spelling is a known header spelling when the filter now drops a finding reported that way as an unknown file.

Failure scenario: A PR changes a file whose quoted diff path decodes to docs/a quoted b.md; the model reports the finding using the escaped spelling docs/a backslash-quote b.md that the README says the header carries. The filter finds no given path matching it and drops the finding with dropping finding: file not in prompt context, contrary to what the documented filter behavior leads the operator to expect.

Suggested fix

Reword the sentence to name the decoded spelling, e.g. "A diff header prints the path in the spelling the filter matches (decoded, without the `"` escaping the prompt tags add), and the filter matches that spelling as written."

---

umm-actually · deepseek/deepseek-v4.1-flash

[aliasunder]

Disposition for umm-actually finding 5922423239 (the diff-header spelling claim in the Unknown-file filter section), posted on b14ba0c:

Valid, fixed in 1200638. Quoted diff paths are now decoded before the annotated diff is built, so the header shows each " as written. The README sentence now reads: "A diff header prints the decoded path, so each " appears as written, and the filter matches that spelling as written. A path whose decoding was rejected keeps the diff's escaped spelling in its header." The " sentences that follow are unchanged and still accurate. A search of the README, action.yml, AGENTS.md, and the source comments found no other claim that the header keeps the escaped spelling. The one remaining "as the diff spelled it" (the rejectedPaths doc in src/diff/quoted-paths.ts) describes rejected paths and stays.

---

🔍 ship-check · pr-monitor · claude-opus-5-5