From cce877d3c453cee1a0a61b3c39b0c6b1421ebfb9 Mon Sep 17 00:00:00 2001 From: Ben Limmer <630449+blimmer@users.noreply.github.com> Date: Tue, 31 Mar 2026 12:08:44 -0600 Subject: [PATCH] fix(review): resolve repo root for untracked file discovery (#450) git ls-files scopes to the CWD subtree and returns CWD-relative paths, unlike git diff HEAD which covers the full repo with root-relative paths. When the hook process inherits a subdirectory CWD, untracked files outside that subtree are silently dropped and paths for files inside it are wrong. Resolve the repo root via rev-parse --show-toplevel and use it as the CWD for both git ls-files and git diff --no-index in getUntrackedFileDiffs. Fixes #449 --- packages/shared/review-core.test.ts | 27 ++++++++++++++++++++++++++- packages/shared/review-core.ts | 15 +++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/shared/review-core.test.ts b/packages/shared/review-core.test.ts index 65032fd6..f08927e7 100644 --- a/packages/shared/review-core.test.ts +++ b/packages/shared/review-core.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, test } from "bun:test"; import { spawnSync } from "node:child_process"; -import { mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join, resolve as resolvePath } from "node:path"; import { @@ -88,6 +88,31 @@ describe("review-core", () => { expect(result.patch).toContain("+++ b/untracked.txt"); }); + test("uncommitted diff includes untracked files when CWD is a subdirectory", async () => { + const repoDir = initRepo(); + + mkdirSync(join(repoDir, "packages", "infra", "lib"), { recursive: true }); + writeFileSync(join(repoDir, "packages", "infra", "lib", "Stack.ts"), "new stack\n", "utf-8"); + + writeFileSync(join(repoDir, "root-new.txt"), "root untracked\n", "utf-8"); + mkdirSync(join(repoDir, ".github", "workflows"), { recursive: true }); + writeFileSync(join(repoDir, ".github", "workflows", "ci.yml"), "name: CI\n", "utf-8"); + + writeFileSync(join(repoDir, "tracked.txt"), "after\n", "utf-8"); + + // Runtime whose default CWD is a subdirectory (simulates a hook process + // that inherits an agent CWD inside a monorepo package) + const subCwd = join(repoDir, "packages", "infra"); + const runtime = makeRuntime(subCwd); + + const result = await runGitDiff(runtime, "uncommitted", "main"); + + expect(result.patch).toContain("diff --git a/tracked.txt b/tracked.txt"); + expect(result.patch).toContain("diff --git a/packages/infra/lib/Stack.ts b/packages/infra/lib/Stack.ts"); + expect(result.patch).toContain("diff --git a/root-new.txt b/root-new.txt"); + expect(result.patch).toContain("diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml"); + }); + test("unstaged diff includes untracked files", async () => { const repoDir = initRepo(); const runtime = makeRuntime(repoDir); diff --git a/packages/shared/review-core.ts b/packages/shared/review-core.ts index 4d95a33d..bef3bb64 100644 --- a/packages/shared/review-core.ts +++ b/packages/shared/review-core.ts @@ -174,9 +174,20 @@ async function getUntrackedFileDiffs( dstPrefix = "b/", cwd?: string, ): Promise { + // git ls-files scopes to the CWD subtree and returns CWD-relative paths, + // unlike git diff HEAD which always covers the full repo with root-relative + // paths. Resolve the repo root so untracked files from the entire repo are + // included and their paths match the tracked-diff output. + const toplevelResult = await runtime.runGit( + ["rev-parse", "--show-toplevel"], + { cwd }, + ); + const rootCwd = + toplevelResult.exitCode === 0 ? toplevelResult.stdout.trim() : cwd; + const lsResult = await runtime.runGit( ["ls-files", "--others", "--exclude-standard"], - { cwd }, + { cwd: rootCwd }, ); if (lsResult.exitCode !== 0) return ""; @@ -199,7 +210,7 @@ async function getUntrackedFileDiffs( "/dev/null", file, ], - { cwd }, + { cwd: rootCwd }, ); return diffResult.stdout; }),