fix: ask gh for an uncolored PR diff

gh colors `gh pr diff` even into a pipe when CLICOLOR_FORCE or GH_FORCE_TTY
is set. The escape codes before each `diff --git` made the whole PR parse as
no files, so a CI review reported "No reviewable changes". Pass
`--color never` in both places that read the PR diff.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>

src/__tests__/github-pr-diff.test.ts

@@ -0,0 +1,65 @@
+import { test, expect, afterEach, beforeEach } from "bun:test";
+import { chmod, mkdtemp, rm, writeFile } from "node:fs/promises";
+import { tmpdir } from "node:os";
+import path, { delimiter } from "node:path";
+
+import { resetTrustedToolCache } from "../core/exec.js";
+import type { LinkContext } from "../core/render.js";
+import { GitHubReporter } from "../reporters/github.js";
+import { GitHubPRSource } from "../sources/github-pr.js";
+
+// A fake `gh` that answers `pr diff` the way gh does when the environment forces
+// color (CLICOLOR_FORCE or GH_FORCE_TTY): escape codes around every line, even into
+// a pipe, unless the call passes `--color never`.
+const FAKE_GH = `#!/bin/sh
+case "$*" in
+  "pr diff"*)
+    case " $* " in
+      *" --color never "*) on=""; off="" ;;
+      *) on=$(printf '\\033[1;37m'); off=$(printf '\\033[m') ;;
+    esac
+    printf '%sdiff --git a/a.ts b/a.ts%s\\n' "$on" "$off"
+    printf '%sindex 1111111..2222222 100644%s\\n' "$on" "$off"
+    printf '%s--- a/a.ts%s\\n' "$on" "$off"
+    printf '%s+++ b/a.ts%s\\n' "$on" "$off"
+    printf '%s@@ -1 +1,2 @@%s\\n' "$on" "$off"
+    printf ' one\\n'
+    printf '%s+two%s\\n' "$on" "$off" ;;
+  "pr view"*) printf '{"baseRefOid": "0123456789abcdef0123456789abcdef01234567"}' ;;
+  *) echo "unexpected gh call: $*" >&2; exit 1 ;;
+esac
+`;
+
+let dir = "";
+let savedPath: string | undefined;
+
+beforeEach(async () => {
+  dir = await mkdtemp(path.join(tmpdir(), "ecr-gh-pr-diff-"));
+  await writeFile(path.join(dir, "gh"), FAKE_GH, "utf8");
+  await chmod(path.join(dir, "gh"), 0o755);
+  savedPath = process.env.PATH;
+  process.env.PATH = `${dir}${delimiter}${savedPath ?? ""}`;
+  resetTrustedToolCache();
+});
+
+afterEach(async () => {
+  process.env.PATH = savedPath;
+  resetTrustedToolCache();
+  await rm(dir, { recursive: true, force: true });
+});
+
+test("GitHubPRSource: the PR diff parses when the environment forces color", async () => {
+  const files = await new GitHubPRSource({ prNumber: 7, repo: "o/r" }).getChangedFiles();
+  expect(files.map((file) => [file.status, file.path])).toEqual([["M", "a.ts"]]);
+  expect(files[0]!.patch).toContain("+two");
+});
+
+test("GitHubReporter: the link context's diff lines parse when the environment forces color", async () => {
+  const reporter = new GitHubReporter({ prNumber: 7, repo: "o/r", commentTag: "t" });
+  // linkContextAsync is private; the test drives it directly so only the two gh
+  // calls it makes are involved.
+  const link = await (
+    reporter as unknown as { linkContextAsync: () => Promise<LinkContext> }
+  ).linkContextAsync();
+  expect([...(link.diffLines?.get("a.ts") ?? [])]).toEqual([1, 2]);
+});

src/reporters/github.ts

@@ -1140,7 +1140,10 @@ export class GitHubReporter implements Reporter {
       (async () => {
         try {
           const gh = await resolveTrustedTool("gh");
-          const { stdout } = await run(gh, ["pr", "diff", ...prArgs], { cwd });
+          // Never colored, even when the environment forces color (see GitHubPRSource).
+          const { stdout } = await run(gh, ["pr", "diff", ...prArgs, "--color", "never"], {
+            cwd,
+          });
           link.diffLines = buildDiffLineIndex(parseUnifiedDiff(stdout));
         } catch {
           // leave diffLines unset → in-diff findings degrade to plain text

src/sources/github-pr.ts

@@ -225,9 +225,12 @@ export class GitHubPRSource implements ReviewSource {
 
   async getChangedFiles(): Promise<DiffEntry[]> {
     const gh = await resolveTrustedTool("gh");
+    // gh colors the diff even into a pipe when the environment forces it
+    // (CLICOLOR_FORCE, GH_FORCE_TTY), and the escape codes before `diff --git` would
+    // make the whole PR parse as no files.
     const { stdout } = await run(
       gh,
-      ["pr", "diff", String(this.options.prNumber), ...this.repoArgs()],
+      ["pr", "diff", String(this.options.prNumber), ...this.repoArgs(), "--color", "never"],
       { cwd: this.options.cwd },
     );
     return parseUnifiedDiff(stdout);