diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index c401f0ca..f71a8e89 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -7,6 +7,7 @@ import { formatCommandFailure, runCommand, runCommandChecked } from "./process.m const MAX_UNTRACKED_BYTES = 24 * 1024; const DEFAULT_INLINE_DIFF_MAX_FILES = 2; const DEFAULT_INLINE_DIFF_MAX_BYTES = 256 * 1024; +const DEFAULT_UNTRACKED_TOTAL_MAX_BYTES = 256 * 1024; // Git is directly executable on Windows. Repository-derived arguments must never pass through a shell. function git(cwd, args, options = {}) { @@ -37,6 +38,14 @@ function normalizeMaxInlineDiffBytes(value) { return Math.floor(parsed); } +function normalizeMaxUntrackedTotalBytes(value) { + const parsed = Number(value); + if (!Number.isFinite(parsed) || parsed < 0) { + return DEFAULT_UNTRACKED_TOTAL_MAX_BYTES; + } + return Math.floor(parsed); +} + function measureGitOutputBytes(cwd, args, maxBytes) { const result = git(cwd, args, { maxBuffer: maxBytes + 1 }); if (result.error && /** @type {NodeJS.ErrnoException} */ (result.error).code === "ENOBUFS") { @@ -222,16 +231,37 @@ function formatUntrackedFile(cwd, relativePath) { return [`### ${relativePath}`, "```", buffer.toString("utf8").trimEnd(), "```"].join("\n"); } +function formatUntrackedSection(cwd, untracked, maxTotalBytes) { + const entries = []; + let totalBytes = 0; + for (let index = 0; index < untracked.length; index += 1) { + const entry = formatUntrackedFile(cwd, untracked[index]); + const entryBytes = Buffer.byteLength(entry, "utf8"); + if (totalBytes + entryBytes > maxTotalBytes) { + const omitted = untracked.length - index; + entries.push( + `(${omitted} untracked file(s) omitted: aggregate untracked content exceeds the ${maxTotalBytes} byte limit. ` + + "See the Git Status section for the full file list; inspect the omitted files directly, or commit or gitignore them to narrow the review.)" + ); + break; + } + totalBytes += entryBytes; + entries.push(entry); + } + return entries.join("\n\n"); +} + function collectWorkingTreeContext(cwd, state, options = {}) { const includeDiff = options.includeDiff !== false; + const maxUntrackedTotalBytes = normalizeMaxUntrackedTotalBytes(options.maxUntrackedTotalBytes); const status = gitChecked(cwd, ["status", "--short", "--untracked-files=all"]).stdout.trim(); const changedFiles = listUniqueFiles(state.staged, state.unstaged, state.untracked); + const untrackedBody = formatUntrackedSection(cwd, state.untracked, maxUntrackedTotalBytes); let parts; if (includeDiff) { const stagedDiff = gitChecked(cwd, ["diff", "--cached", "--binary", "--no-ext-diff", "--submodule=diff"]).stdout; const unstagedDiff = gitChecked(cwd, ["diff", "--binary", "--no-ext-diff", "--submodule=diff"]).stdout; - const untrackedBody = state.untracked.map((file) => formatUntrackedFile(cwd, file)).join("\n\n"); parts = [ formatSection("Git Status", status), formatSection("Staged Diff", stagedDiff), @@ -241,7 +271,6 @@ function collectWorkingTreeContext(cwd, state, options = {}) { } else { const stagedStat = gitChecked(cwd, ["diff", "--shortstat", "--cached"]).stdout.trim(); const unstagedStat = gitChecked(cwd, ["diff", "--shortstat"]).stdout.trim(); - const untrackedBody = state.untracked.map((file) => formatUntrackedFile(cwd, file)).join("\n\n"); parts = [ formatSection("Git Status", status), formatSection("Staged Diff Stat", stagedStat), @@ -302,6 +331,7 @@ export function collectReviewContext(cwd, target, options = {}) { const currentBranch = getCurrentBranch(repoRoot); const maxInlineFiles = normalizeMaxInlineFiles(options.maxInlineFiles); const maxInlineDiffBytes = normalizeMaxInlineDiffBytes(options.maxInlineDiffBytes); + const maxUntrackedTotalBytes = options.maxUntrackedTotalBytes; let details; let includeDiff; let diffBytes; @@ -320,7 +350,7 @@ export function collectReviewContext(cwd, target, options = {}) { options.includeDiff ?? (listUniqueFiles(state.staged, state.unstaged, state.untracked).length <= maxInlineFiles && diffBytes <= maxInlineDiffBytes); - details = collectWorkingTreeContext(repoRoot, state, { includeDiff }); + details = collectWorkingTreeContext(repoRoot, state, { includeDiff, maxUntrackedTotalBytes }); } else { const comparison = buildBranchComparison(repoRoot, target.baseRef); const fileCount = gitChecked(repoRoot, ["diff", "--name-only", comparison.commitRange]).stdout.trim().split("\n").filter(Boolean).length; diff --git a/tests/git.test.mjs b/tests/git.test.mjs index 5b5c266e..6df9d815 100644 --- a/tests/git.test.mjs +++ b/tests/git.test.mjs @@ -210,3 +210,41 @@ test("collectReviewContext keeps untracked file content in lightweight working t assert.match(context.content, /## Untracked Files/); assert.match(context.content, /UNTRACKED_RISK_MARKER/); }); + +test("collectReviewContext caps aggregate untracked content instead of growing unbounded", () => { + const cwd = makeTempDir(); + initGitRepo(cwd); + fs.writeFileSync(path.join(cwd, "app.js"), "console.log('v1');\n"); + run("git", ["add", "app.js"], { cwd }); + run("git", ["commit", "-m", "init"], { cwd }); + for (let index = 0; index < 5; index += 1) { + fs.writeFileSync(path.join(cwd, `untracked-${index}.txt`), `UNTRACKED_MARKER_${index} ${"x".repeat(400)}\n`); + } + + const target = resolveReviewTarget(cwd, { scope: "working-tree" }); + const context = collectReviewContext(cwd, target, { maxUntrackedTotalBytes: 1024 }); + + assert.match(context.content, /UNTRACKED_MARKER_0/); + assert.doesNotMatch(context.content, /UNTRACKED_MARKER_4/); + assert.match(context.content, /\(\d+ untracked file\(s\) omitted: aggregate untracked content exceeds the 1024 byte limit\./); + // The Git Status section still names every untracked file, so nothing is silently invisible. + assert.match(context.content, /\?\? untracked-4\.txt/); +}); + +test("collectReviewContext keeps all untracked content when under the aggregate cap", () => { + const cwd = makeTempDir(); + initGitRepo(cwd); + fs.writeFileSync(path.join(cwd, "app.js"), "console.log('v1');\n"); + run("git", ["add", "app.js"], { cwd }); + run("git", ["commit", "-m", "init"], { cwd }); + for (let index = 0; index < 5; index += 1) { + fs.writeFileSync(path.join(cwd, `untracked-${index}.txt`), `UNTRACKED_MARKER_${index}\n`); + } + + const target = resolveReviewTarget(cwd, { scope: "working-tree" }); + const context = collectReviewContext(cwd, target); + + assert.match(context.content, /UNTRACKED_MARKER_0/); + assert.match(context.content, /UNTRACKED_MARKER_4/); + assert.doesNotMatch(context.content, /untracked file\(s\) omitted/); +});