diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 227b9f54f..0f971ce9c 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -5,13 +5,13 @@ }, "metadata": { "description": "Codex plugins to use in Claude Code for delegation and code review.", - "version": "1.0.44" + "version": "1.0.45" }, "plugins": [ { "name": "codex", "description": "Use Codex from Claude Code to review code or delegate tasks.", - "version": "1.0.44", + "version": "1.0.45", "author": { "name": "OpenAI" }, diff --git a/package-lock.json b/package-lock.json index 6ae452c42..2d9530d52 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@openai/codex-plugin-cc", - "version": "1.0.44", + "version": "1.0.45", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@openai/codex-plugin-cc", - "version": "1.0.44", + "version": "1.0.45", "license": "Apache-2.0", "devDependencies": { "@types/node": "^25.5.0", diff --git a/package.json b/package.json index 2d0a9d2e1..288f29824 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@openai/codex-plugin-cc", - "version": "1.0.44", + "version": "1.0.45", "private": true, "type": "module", "description": "Use Codex from Claude Code to review code or delegate tasks.", diff --git a/plugins/codex/.claude-plugin/plugin.json b/plugins/codex/.claude-plugin/plugin.json index 19c02acba..b95841a4a 100644 --- a/plugins/codex/.claude-plugin/plugin.json +++ b/plugins/codex/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "codex", - "version": "1.0.44", + "version": "1.0.45", "description": "Use Codex from Claude Code to review code or delegate tasks.", "author": { "name": "OpenAI" diff --git a/plugins/codex/scripts/lib/git.mjs b/plugins/codex/scripts/lib/git.mjs index bb4e7a291..3893fd9b2 100644 --- a/plugins/codex/scripts/lib/git.mjs +++ b/plugins/codex/scripts/lib/git.mjs @@ -8,6 +8,9 @@ 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 HASH_OBJECT_MAX_BATCH_PATHS = 256; +// Keep argv well below Windows' command-line limit, including fixed arguments and quoting. +const HASH_OBJECT_MAX_BATCH_ARG_BYTES = 8 * 1024; // Git is directly executable on Windows. Repository-derived arguments must never pass through a shell. function git(cwd, args, options = {}) { @@ -58,26 +61,32 @@ function hashNestedGitRepository(absolutePath) { return `submodule:${head.stdout.trim()}:${statusDigest}:${stagedDiffDigest}:${unstagedDiffDigest}`; } -function hashWorkingTreePath(cwd, relativePath) { +function classifyPath(cwd, relativePath, missingToken, nonRegularToken) { const absolutePath = path.join(cwd, relativePath); let stat; try { stat = fs.lstatSync(absolutePath); } catch { - return "missing"; + return { type: "token", token: missingToken }; } - if (stat.isSymbolicLink()) { - return `symlink:${createHash("sha256").update(fs.readlinkSync(absolutePath)).digest("hex")}`; - } - if (stat.isDirectory()) { - return hashNestedGitRepository(absolutePath) ?? `other:${stat.mode}:${stat.size}`; - } - if (!stat.isFile()) { - return `other:${stat.mode}:${stat.size}`; + if (stat.isFile()) { + return { type: "regular" }; } - return `file:${gitChecked(cwd, ["hash-object", "--no-filters", "--", relativePath]).stdout.trim()}`; + return { type: "token", token: nonRegularToken(stat, absolutePath) }; +} + +function classifyWorkingTreePath(cwd, relativePath) { + return classifyPath(cwd, relativePath, "missing", (stat, absolutePath) => { + if (stat.isSymbolicLink()) { + return `symlink:${createHash("sha256").update(fs.readlinkSync(absolutePath)).digest("hex")}`; + } + if (stat.isDirectory()) { + return hashNestedGitRepository(absolutePath) ?? `other:${stat.mode}:${stat.size}`; + } + return `other:${stat.mode}:${stat.size}`; + }); } function inspectUntrackedFile(cwd, relativePath) { @@ -109,30 +118,64 @@ function inspectUntrackedFile(cwd, relativePath) { return { content: buffer.toString("utf8").trimEnd() }; } -function hashUntrackedPath(cwd, relativePath) { - const absolutePath = path.join(cwd, relativePath); - let stat; - try { - stat = fs.lstatSync(absolutePath); - } catch { - return "skipped:(skipped: broken symlink or unreadable file)"; +function classifyUntrackedPath(cwd, relativePath) { + return classifyPath( + cwd, + relativePath, + "skipped:(skipped: broken symlink or unreadable file)", + () => { + // Preserve the existing handling for symlinks, directories, and other + // non-regular paths. Display limits must not affect regular-file identity. + const inspected = inspectUntrackedFile(cwd, relativePath); + if (inspected.skip) { + return `skipped:${inspected.skip}`; + } + return `file:${createHash("sha256").update(inspected.content).digest("hex")}`; + } + ); +} + +function hashRegularFile(cwd, relativePath, failureResults) { + const result = git(cwd, ["hash-object", "--no-filters", "--", relativePath]); + if (result.error || result.status !== 0) { + failureResults.set(relativePath, result); + return null; } + return result.stdout.trim(); +} - if (stat.isFile()) { - const result = git(cwd, ["hash-object", "--no-filters", "--", relativePath]); - if (!result.error && result.status === 0) { - return `file:${result.stdout.trim()}`; +function hashRegularFilesBatched(cwd, relativePaths, failureResults) { + const hashes = new Map(); + + for (let offset = 0; offset < relativePaths.length;) { + const chunk = []; + let chunkBytes = 0; + while (offset < relativePaths.length && chunk.length < HASH_OBJECT_MAX_BATCH_PATHS) { + const relativePath = relativePaths[offset]; + const pathBytes = Buffer.byteLength(relativePath, "utf8"); + if (chunk.length > 0 && chunkBytes + pathBytes > HASH_OBJECT_MAX_BATCH_ARG_BYTES) { + break; + } + chunk.push(relativePath); + chunkBytes += pathBytes; + offset += 1; } - return "skipped:(skipped: broken symlink or unreadable file)"; - } - // Preserve the existing handling for symlinks, directories, and other - // non-regular paths. Display limits must not affect regular-file identity. - const inspected = inspectUntrackedFile(cwd, relativePath); - if (inspected.skip) { - return `skipped:${inspected.skip}`; + const result = git(cwd, ["hash-object", "--no-filters", "--", ...chunk]); + const oids = !result.error && result.status === 0 + ? result.stdout.split(/\r?\n/).map((line) => line.trim()).filter(Boolean) + : []; + if (!result.error && result.status === 0 && oids.length === chunk.length) { + chunk.forEach((relativePath, index) => hashes.set(relativePath, oids[index])); + continue; + } + + for (const relativePath of chunk) { + hashes.set(relativePath, hashRegularFile(cwd, relativePath, failureResults)); + } } - return `file:${createHash("sha256").update(inspected.content).digest("hex")}`; + + return hashes; } function captureWorkingTreeDigest(cwd) { @@ -144,13 +187,48 @@ function captureWorkingTreeDigest(cwd) { // working-tree content. Fold dirty tracked paths in so repeated edits cannot // resolve to the same identity. const trackedPaths = listUniqueFiles(gitNullTerminatedPaths(cwd, ["diff", "--name-only"])); - for (const relativePath of trackedPaths) { - digest.update(`\0tracked\0${relativePath}\0${hashWorkingTreePath(cwd, relativePath)}`); - } + const trackedClassifications = trackedPaths.map((relativePath) => ({ + relativePath, + classification: classifyWorkingTreePath(cwd, relativePath) + })); const untrackedPaths = gitNullTerminatedPaths(cwd, ["ls-files", "--others", "--exclude-standard"]).sort(); - for (const relativePath of untrackedPaths) { - digest.update(`\0untracked\0${relativePath}\0${hashUntrackedPath(cwd, relativePath)}`); + const untrackedClassifications = untrackedPaths.map((relativePath) => ({ + relativePath, + classification: classifyUntrackedPath(cwd, relativePath) + })); + + const regularPaths = [...trackedClassifications, ...untrackedClassifications] + .filter(({ classification }) => classification.type === "regular") + .map(({ relativePath }) => relativePath); + const hashFailures = new Map(); + const hashes = hashRegularFilesBatched(cwd, regularPaths, hashFailures); + + for (const { relativePath, classification } of trackedClassifications) { + let token = classification.token; + if (classification.type === "regular") { + const oid = hashes.get(relativePath); + if (oid === null) { + const failure = hashFailures.get(relativePath); + if (failure.error) { + throw failure.error; + } + throw new Error(formatCommandFailure(failure)); + } + token = `file:${oid}`; + } + digest.update(`\0tracked\0${relativePath}\0${token}`); + } + + for (const { relativePath, classification } of untrackedClassifications) { + let token = classification.token; + if (classification.type === "regular") { + const oid = hashes.get(relativePath); + token = oid === null + ? "skipped:(skipped: broken symlink or unreadable file)" + : `file:${oid}`; + } + digest.update(`\0untracked\0${relativePath}\0${token}`); } return digest.digest("hex"); diff --git a/tests/git.test.mjs b/tests/git.test.mjs index 0596fd11d..419219077 100644 --- a/tests/git.test.mjs +++ b/tests/git.test.mjs @@ -10,7 +10,7 @@ import { resolveReviewTarget, resolveWorktreeWritableRoots } from "../plugins/codex/scripts/lib/git.mjs"; -import { initGitRepo, makeTempDir, run } from "./helpers.mjs"; +import { initGitRepo, makeTempDir, run, writeExecutable } from "./helpers.mjs"; test("resolveWorktreeWritableRoots returns the common git dir for linked worktrees only", () => { const mainRepo = makeTempDir(); @@ -254,6 +254,192 @@ test("repo state identity detects trailing-newline-only changes to an untracked assert.match(describeRepoStateDrift(cwd, target, identity), /working tree moved/i); }); +test("working-tree fingerprint batches hash-object processes", (t) => { + if (process.platform === "win32") { + t.skip("uses a POSIX sh git shim"); + return; + } + + const cwd = makeTempDir(); + const binDir = makeTempDir(); + const logPath = path.join(binDir, "git-args.log"); + const trackedCount = 300; + const untrackedCount = 300; + const trackedPaths = Array.from( + { length: trackedCount }, + (_, index) => `tracked-${String(index).padStart(3, "0")}.txt` + ); + const untrackedPaths = Array.from( + { length: untrackedCount }, + (_, index) => `untracked-${String(index).padStart(3, "0")}.txt` + ); + + initGitRepo(cwd); + for (const relativePath of trackedPaths) { + fs.writeFileSync(path.join(cwd, relativePath), "initial\n"); + } + run("git", ["add", "--", ...trackedPaths], { cwd }); + run("git", ["commit", "-m", "init"], { cwd }); + for (const relativePath of trackedPaths) { + fs.writeFileSync(path.join(cwd, relativePath), "dirty\n"); + } + for (const relativePath of untrackedPaths) { + fs.writeFileSync(path.join(cwd, relativePath), "untracked\n"); + } + + const target = resolveReviewTarget(cwd, { scope: "working-tree" }); + const realGitResult = run("which", ["git"], { cwd }); + assert.equal(realGitResult.status, 0, realGitResult.stderr); + const realGitPath = fs.realpathSync(realGitResult.stdout.trim()); + const shimPath = path.join(binDir, "git"); + writeExecutable( + shimPath, + [ + "#!/bin/sh", + `printf '%s\\n' \"$1\" >> '${logPath.replaceAll("'", "'\\''")}'`, + `exec '${realGitPath.replaceAll("'", "'\\''")}' \"$@\"`, + "" + ].join("\n") + ); + + const previousPath = process.env.PATH; + try { + process.env.PATH = `${binDir}${path.delimiter}${previousPath ?? ""}`; + captureRepoStateIdentity(cwd, target); + + const loggedCommands = fs.readFileSync(logPath, "utf8").trim().split(/\r?\n/).filter(Boolean); + const hashObjectCalls = loggedCommands.filter((command) => command === "hash-object").length; + assert.ok(hashObjectCalls <= 8, `expected at most 8 hash-object calls, got ${hashObjectCalls}`); + assert.ok( + hashObjectCalls < (trackedCount + untrackedCount) / 10, + `expected hash-object calls far below ${trackedCount + untrackedCount} paths, got ${hashObjectCalls}` + ); + } finally { + if (previousPath === undefined) { + delete process.env.PATH; + } else { + process.env.PATH = previousPath; + } + } +}); + +test("working-tree fingerprint preserves chunk-to-OID alignment", () => { + const cwd = makeTempDir(); + const fileCount = 300; + const relativePaths = Array.from( + { length: fileCount }, + (_, index) => `chunk-${String(index).padStart(3, "0")}.txt` + ); + + initGitRepo(cwd); + fs.writeFileSync(path.join(cwd, "tracked.txt"), "tracked\n"); + run("git", ["add", "tracked.txt"], { cwd }); + run("git", ["commit", "-m", "init"], { cwd }); + for (const relativePath of relativePaths) { + fs.writeFileSync(path.join(cwd, relativePath), `${relativePath}\n`); + } + + const target = resolveReviewTarget(cwd, { scope: "working-tree" }); + const identity = captureRepoStateIdentity(cwd, target); + assert.equal(describeRepoStateDrift(cwd, target, identity), null); + + const secondChunkPath = relativePaths.at(-1); + fs.writeFileSync(path.join(cwd, secondChunkPath), "changed\n"); + assert.match(describeRepoStateDrift(cwd, target, identity), /working tree moved/i); +}); + +test("working-tree fingerprint handles awkward regular-file paths", () => { + const cwd = makeTempDir(); + const trackedPaths = ["tracked space.txt", "tracked-é.txt", "-tracked-file.txt"]; + const untrackedPaths = ["untracked space.txt", "untracked-é.txt", "-untracked-file.txt"]; + + initGitRepo(cwd); + for (const relativePath of trackedPaths) { + fs.writeFileSync(path.join(cwd, relativePath), "initial\n"); + } + run("git", ["add", "--", ...trackedPaths], { cwd }); + run("git", ["commit", "-m", "init"], { cwd }); + for (const relativePath of trackedPaths) { + fs.writeFileSync(path.join(cwd, relativePath), "dirty\n"); + } + for (const relativePath of untrackedPaths) { + fs.writeFileSync(path.join(cwd, relativePath), "untracked\n"); + } + + const target = resolveReviewTarget(cwd, { scope: "working-tree" }); + const identity = captureRepoStateIdentity(cwd, target); + assert.equal(describeRepoStateDrift(cwd, target, identity), null); + + fs.writeFileSync(path.join(cwd, "-untracked-file.txt"), "changed\n"); + assert.match(describeRepoStateDrift(cwd, target, identity), /working tree moved/i); +}); + +test("working-tree fingerprint preserves mixed-tree non-regular branches", () => { + const cwd = makeTempDir(); + const outside = makeTempDir(); + const trackedFile = path.join(cwd, "tracked.txt"); + const trackedLink = path.join(cwd, "tracked-link"); + const outsideTarget = path.join(outside, "outside.txt"); + const outsideLink = path.join(cwd, "outside-link"); + const binaryPath = path.join(cwd, "artifact.bin"); + const largePath = path.join(cwd, "large.dat"); + const originalLarge = Buffer.alloc(25 * 1024, 0x61); + const originalOutside = "outside v1\n"; + const originalBinary = Buffer.from([0, 1, 2, 3]); + + initGitRepo(cwd); + fs.writeFileSync(trackedFile, "tracked v1\n"); + fs.symlinkSync("tracked-target.txt", trackedLink); + run("git", ["add", "--", "tracked.txt", "tracked-link"], { cwd }); + run("git", ["commit", "-m", "init"], { cwd }); + fs.writeFileSync(trackedFile, "tracked v2\n"); + fs.writeFileSync(outsideTarget, originalOutside); + fs.symlinkSync(outsideTarget, outsideLink); + fs.writeFileSync(binaryPath, originalBinary); + fs.writeFileSync(largePath, originalLarge); + + const target = resolveReviewTarget(cwd, { scope: "working-tree" }); + const identity = captureRepoStateIdentity(cwd, target); + assert.equal(describeRepoStateDrift(cwd, target, identity), null); + + const assertMutationDetected = (mutate, restore) => { + const before = captureRepoStateIdentity(cwd, target); + assert.equal(describeRepoStateDrift(cwd, target, before), null); + mutate(); + assert.match(describeRepoStateDrift(cwd, target, before), /working tree moved/i); + restore(); + }; + + assertMutationDetected( + () => fs.writeFileSync(trackedFile, "tracked v3\n"), + () => fs.writeFileSync(trackedFile, "tracked v2\n") + ); + assertMutationDetected( + () => { + fs.unlinkSync(trackedLink); + fs.symlinkSync("tracked-target-changed.txt", trackedLink); + }, + () => { + fs.unlinkSync(trackedLink); + fs.symlinkSync("tracked-target.txt", trackedLink); + } + ); + assertMutationDetected( + () => fs.writeFileSync(outsideTarget, "outside v2\n"), + () => fs.writeFileSync(outsideTarget, originalOutside) + ); + assertMutationDetected( + () => fs.writeFileSync(binaryPath, Buffer.from([0, 1, 2, 4])), + () => fs.writeFileSync(binaryPath, originalBinary) + ); + assertMutationDetected( + () => fs.writeFileSync(largePath, Buffer.alloc(25 * 1024, 0x62)), + () => fs.writeFileSync(largePath, originalLarge) + ); + + assert.equal(describeRepoStateDrift(cwd, target, identity), null); +}); + test("repo state identity detects further changes inside a dirty submodule", () => { const cwd = makeTempDir(); const nestedRepo = path.join(cwd, "vendor", "dependency");