diff --git a/README.md b/README.md index e47b098..c3c489e 100644 --- a/README.md +++ b/README.md @@ -63,7 +63,7 @@ npx cc-plugin-codex install On Windows, prefer the Sendbird marketplace path or the `npx` helper. The shell-script helper below is POSIX-only. Codex CLI's official guidance still treats Windows support as experimental and recommends a WSL workspace for the best Codex experience. Claude Code supports both native Windows and WSL. -> **Prerequisites:** Node.js 18+, Codex with hook support, and `claude` CLI installed and authenticated. +> **Prerequisites:** Node.js 18+, Codex with hook support, and `claude` CLI installed and authenticated. Branch reviews need Claude Code 2.1.281 or later. > If you don't have the Claude CLI yet: > ```bash > npm install -g @anthropic-ai/claude-code && claude auth login diff --git a/scripts/claude-companion.mjs b/scripts/claude-companion.mjs index 2a16c9e..f4099e4 100644 --- a/scripts/claude-companion.mjs +++ b/scripts/claude-companion.mjs @@ -805,6 +805,7 @@ async function executeReviewRun(request) { onSpawn: request.onSpawn, permissionMode: "dontAsk", settingsFile: sandboxSettingsFile, + settingSources: isolation.settingSources, mcpConfigFile, strictMcpConfig: true, }); @@ -878,6 +879,7 @@ async function executeReviewRun(request) { onSpawn: request.onSpawn, permissionMode: "dontAsk", settingsFile: sandboxSettingsFile, + settingSources: isolation.settingSources, mcpConfigFile, strictMcpConfig: true, } diff --git a/scripts/lib/claude-cli.mjs b/scripts/lib/claude-cli.mjs index 68ffdbb..7668cbf 100644 --- a/scripts/lib/claude-cli.mjs +++ b/scripts/lib/claude-cli.mjs @@ -148,6 +148,36 @@ export function getClaudeAvailability(cwd) { } } +// The latest Claude Code release that fixed `--setting-sources` letting +// excluded settings through. Older releases accept the flag but still load +// nested `.claude/rules/*.md` files (fixed in 2.1.211), apply the command +// sandbox's filesystem configuration from excluded sources (2.1.246), and +// start spawned sessions without the restriction (2.1.281), per the Claude +// Code changelog. +const SETTING_SOURCES_MIN_VERSION = [2, 1, 281]; + +/** + * Throw unless `claude --version` output starts with a release version, plain + * `major.minor.patch`, at or above SETTING_SOURCES_MIN_VERSION. + */ +/** @visibleForTesting */ +export function assertSettingSourcesSupported(versionOutput) { + const match = /^(\d+)\.(\d+)\.(\d+)(?=\s|$)/.exec(String(versionOutput ?? "").trim()); + if (!match) { + throw new Error( + `Cannot read a Claude Code release version from \`claude --version\` output ${JSON.stringify(versionOutput)}.` + ); + } + const [major, minor, patch] = match.slice(1, 4).map(Number); + const [minMajor, minMinor, minPatch] = SETTING_SOURCES_MIN_VERSION; + if ((major - minMajor || minor - minMinor || patch - minPatch) < 0) { + throw new Error( + `Claude Code ${match[0]} can load settings from the code under review even with --setting-sources. ` + + `Update Claude Code to ${SETTING_SOURCES_MIN_VERSION.join(".")} or later to review a branch.` + ); + } +} + export function getClaudeAuthStatus(cwd) { if (process.env.ANTHROPIC_API_KEY) { return { available: true, loggedIn: true, detail: "API key configured" }; @@ -746,6 +776,9 @@ export function buildArgs(prompt, options = {}) { if (options.settingsFile) { args.push("--settings", options.settingsFile); } + if (options.settingSources) { + args.push("--setting-sources", options.settingSources); + } if (options.mcpConfigFile) { args.push("--mcp-config", options.mcpConfigFile); } @@ -761,6 +794,13 @@ export function buildArgs(prompt, options = {}) { * Returns { status, sessionId, finalMessage, toolUses, touchedFiles, stderr, pid, pidIdentity } */ export async function runClaudeTurn(cwd, prompt, options = {}) { + if (options.settingSources) { + const claude = getClaudeAvailability(cwd); + if (!claude.available) { + throw new Error(claude.detail); + } + assertSettingSourcesSupported(claude.detail); + } const args = buildArgs(prompt, { outputFormat: "stream-json", ...options, diff --git a/scripts/lib/review-worktree.mjs b/scripts/lib/review-worktree.mjs index b82792e..6d82490 100644 --- a/scripts/lib/review-worktree.mjs +++ b/scripts/lib/review-worktree.mjs @@ -135,11 +135,22 @@ function cleanupWorktreeDir(worktreePath) { * that the reviewer is supposed to inspect — `git status` would report clean, * `git diff` would show nothing, and the MCP server pointed at the worktree * would mislead Claude into thinking the repo is unchanged. Instead we run in - * the original repo and rely on the Bash-free allowlist for containment. + * the original repo with the Bash-free allowlist. That allowlist limits the + * model's tools; it does not stop the repo's project settings from loading. * - * Returns `{ cwd, gitRoot, cleanup }`. `gitRoot` is the path the MCP git server - * should be rooted at; `cwd` is what the Claude CLI should treat as the - * working directory. + * Returns `{ cwd, gitRoot, cleanup, isolated }`, plus `settingSources` for a + * worktree. `gitRoot` is the path the MCP git server should be rooted at; + * `cwd` is what the Claude CLI should treat as the working directory. + * + * `settingSources` is the Claude `--setting-sources` value. The worktree checks + * out the commit under review, which may come from anyone, and a print-mode + * child does not ask for trust. Its project settings would otherwise load: + * hooks would run, `env` would reach the processes the child starts, and + * `CLAUDE.md` and rules would reach the model. With `user`, project and local + * settings do not load; user settings, managed settings and the `--settings` + * file still do. A working-tree review runs in the user's own repository and + * loads its project settings without a trust prompt: the plugin treats that + * repository as trusted. */ export function createReviewIsolation(repoRoot, target, { label = "review" } = {}) { if (target?.mode === "working-tree") { @@ -156,6 +167,7 @@ export function createReviewIsolation(repoRoot, target, { label = "review" } = { gitRoot: worktree.path, cleanup: () => worktree.cleanup(), isolated: true, + settingSources: "user", }; } diff --git a/tests/claude-cli.test.mjs b/tests/claude-cli.test.mjs index 5c37212..a53c6cd 100644 --- a/tests/claude-cli.test.mjs +++ b/tests/claude-cli.test.mjs @@ -14,6 +14,7 @@ import { resolveDefaultModel, resolveClaudeBin, buildArgs, + assertSettingSourcesSupported, EFFORT_ALIASES, VALID_EFFORTS, DEFAULT_MODEL, @@ -798,6 +799,44 @@ describe("buildArgs", () => { assert.ok(idx >= 0); assert.equal(args[idx + 1], "/tmp/s.json"); }); + + it("includes --setting-sources only when settingSources is provided", () => { + const args = buildArgs("p", { settingSources: "user" }); + assert.equal(args[args.indexOf("--setting-sources") + 1], "user"); + assert.equal(buildArgs("p", {}).includes("--setting-sources"), false); + }); +}); + +// =========================================================================== +// assertSettingSourcesSupported +// =========================================================================== + +describe("assertSettingSourcesSupported", () => { + it("accepts Claude Code 2.1.281 and later", () => { + for (const output of ["2.1.281 (Claude Code)", "2.1.283 (Claude Code)\r\n", "2.2.0", "3.0.0 (Claude Code)"]) { + assert.doesNotThrow(() => assertSettingSourcesSupported(output), output); + } + }); + + it("rejects releases with a known --setting-sources gap", () => { + for (const output of ["2.1.280 (Claude Code)", "2.1.246 (Claude Code)", "2.1.210 (Claude Code)", "2.0.999", "1.9.300"]) { + assert.throws(() => assertSettingSourcesSupported(output), /2\.1\.281 or later/, output); + } + }); + + it("rejects output that does not start with a release version", () => { + for (const output of [ + "", + "Claude Code", + "claude CLI not found in PATH", + "v2.1.283", + "2.1.283-rc.1 (Claude Code)", + "2.1.283.0", + undefined, + ]) { + assert.throws(() => assertSettingSourcesSupported(output), /Cannot read a Claude Code release version/, String(output)); + } + }); }); // =========================================================================== diff --git a/tests/hooks.test.mjs b/tests/hooks.test.mjs index a03c819..28b75a5 100644 --- a/tests/hooks.test.mjs +++ b/tests/hooks.test.mjs @@ -382,6 +382,8 @@ describe("hooks", () => { assert.ok(permissionModeIndex >= 0); assert.equal(claudeArgs[permissionModeIndex + 1], "dontAsk"); assert.ok(claudeArgs.includes("--settings")); + // The gate runs in the user's own repository and loads its settings. + assert.equal(claudeArgs.includes("--setting-sources"), false); assert.ok(claudeArgs.includes("--mcp-config")); assert.ok(claudeArgs.includes("--strict-mcp-config")); diff --git a/tests/integration/claude-companion.test.mjs b/tests/integration/claude-companion.test.mjs index 5e808ba..6430146 100644 --- a/tests/integration/claude-companion.test.mjs +++ b/tests/integration/claude-companion.test.mjs @@ -54,7 +54,7 @@ function sanitize(value) { async function main() { if (args[0] === "--version") { - process.stdout.write("2.1.90 (Claude Code)\\n"); + process.stdout.write((process.env.CLAUDE_VERSION_OUTPUT || "2.1.283 (Claude Code)") + "\\n"); return; } @@ -1059,6 +1059,96 @@ describe("claude-companion integration", () => { } }); + it("excludes project settings only for reviews in a review worktree", () => { + const testEnv = createTestEnvironment(); + + try { + const taskArgsFile = path.join(testEnv.rootDir, "setting-sources-task-args.json"); + runCompanion( + ["task", "--cwd", testEnv.workspaceDir, "--quiet-progress", "setting sources task delay=20"], + { env: { ...testEnv.env, CLAUDE_ARGS_FILE: taskArgsFile } } + ); + const taskArgs = JSON.parse(fs.readFileSync(taskArgsFile, "utf8")); + assert.equal(taskArgs.includes("--setting-sources"), false); + + setupGitWorkspace(testEnv.workspaceDir); + + const settingSourcesFor = (command, target, focusText) => { + const invocationFile = path.join(testEnv.rootDir, "setting-sources-invocation.json"); + fs.rmSync(invocationFile, { force: true }); + runCompanion( + [command, "--cwd", testEnv.workspaceDir, ...target, ...focusText], + { env: { ...testEnv.env, CLAUDE_INVOCATION_FILE: invocationFile } } + ); + const { args } = JSON.parse(fs.readFileSync(invocationFile, "utf8")); + const index = args.indexOf("--setting-sources"); + return index === -1 ? undefined : args[index + 1]; + }; + const commands = [ + ["review", []], + ["adversarial-review", ["focus on settings"]], + ]; + + // Auto scope on a clean tree reviews the branch in a worktree. + for (const [command, focusText] of commands) { + assert.equal(settingSourcesFor(command, [], focusText), "user", `${command} auto, clean tree`); + } + + seedWorkingTreeDiff(testEnv.workspaceDir); + for (const [command, focusText] of commands) { + for (const [target, expected] of [ + [[], undefined], + [["--scope", "working-tree"], undefined], + [["--scope", "branch"], "user"], + [["--base", "main"], "user"], + ]) { + assert.equal( + settingSourcesFor(command, target, focusText), + expected, + `${command} ${target.join(" ") || "auto, dirty tree"}` + ); + } + } + } finally { + cleanupTestEnvironment(testEnv); + } + }); + + it("refuses worktree reviews on a Claude CLI older than 2.1.281", () => { + const testEnv = createTestEnvironment(); + + try { + setupGitWorkspace(testEnv.workspaceDir); + seedWorkingTreeDiff(testEnv.workspaceDir); + const invocationFile = path.join(testEnv.rootDir, "old-cli-invocation.json"); + const env = { + ...testEnv.env, + CLAUDE_VERSION_OUTPUT: "2.1.280 (Claude Code)", + CLAUDE_INVOCATION_FILE: invocationFile, + }; + + for (const command of ["review", "adversarial-review"]) { + const result = runCompanionExpectFailure( + [command, "--cwd", testEnv.workspaceDir, "--base", "main"], + { env } + ); + assert.match(result.stderr, /Claude Code 2\.1\.280 .*2\.1\.281 or later/, command); + assert.equal(fs.existsSync(invocationFile), false, `${command} started Claude`); + assert.equal( + runGit(testEnv.workspaceDir, ["worktree", "list", "--porcelain"]).split("\n").filter((line) => line.startsWith("worktree ")).length, + 1, + `${command} left a review worktree behind` + ); + } + + // Working-tree reviews do not pass the flag, so the version does not matter. + runCompanion(["review", "--cwd", testEnv.workspaceDir, "--scope", "working-tree"], { env }); + assert.equal(fs.existsSync(invocationFile), true); + } finally { + cleanupTestEnvironment(testEnv); + } + }); + it("uses --resume to continue the latest session and keeps --fresh from injecting a resume id", async () => { const testEnv = createTestEnvironment(); const sessionEnv = { diff --git a/tests/review-worktree.test.mjs b/tests/review-worktree.test.mjs index 19a4fec..4c08407 100644 --- a/tests/review-worktree.test.mjs +++ b/tests/review-worktree.test.mjs @@ -155,6 +155,7 @@ describe("createReviewIsolation", () => { assert.equal(iso.cwd, repoRoot); assert.equal(iso.gitRoot, repoRoot); assert.equal(iso.isolated, false); + assert.equal(iso.settingSources, undefined); } finally { iso.cleanup(); } @@ -185,6 +186,7 @@ describe("createReviewIsolation", () => { assert.notEqual(iso.cwd, repoRoot); assert.equal(iso.gitRoot, iso.cwd); assert.equal(iso.isolated, true); + assert.equal(iso.settingSources, "user"); assert.ok(fs.existsSync(iso.cwd)); } finally { iso.cleanup();