From 490fe1be0eb1a4c27c267652f4f4b41f0d9f949f Mon Sep 17 00:00:00 2001 From: Cameron Beeley Date: Tue, 16 Jun 2026 21:58:34 +0100 Subject: [PATCH 1/5] feat(local-review): add fixed-claude CLI engine via --engine Generalizes runCodexProcess with an optional command param so the same bounded runner drives a fixed `claude` CLI; adds --engine codex|claude (codex default). claude engine: fixed binary (no operator override), codexEnv() scrub + read-only tools, subscription auth via CLAUDE_CODE_OAUTH_TOKEN. Provider-neutral, zero new deps. Inherits #298's committed-range/clean-checkout/unique-dir/reject-unsupported. Addresses the #274 close: fixed boundary, shared runner, scrubbed env, identical schema, no SDK deps, no configurable binary. --- docs/commit-sweeper.md | 7 ++++ src/codex-process.ts | 5 ++- src/commit-sweeper.ts | 85 ++++++++++++++++++++++++++++++++++++--- test/local-review.test.ts | 22 ++++++++++ 4 files changed, 113 insertions(+), 6 deletions(-) diff --git a/docs/commit-sweeper.md b/docs/commit-sweeper.md index 95a8ff3d1e..3bdc08702b 100644 --- a/docs/commit-sweeper.md +++ b/docs/commit-sweeper.md @@ -273,6 +273,13 @@ disables Codex web search, and explicitly forbids network lookups. Repositories without a configured profile are rejected (no foreign-profile fallback). Unlike the hosted lane it never writes to GitHub — the local Markdown report is the only output. +By default `local-review` uses the **codex** engine. Pass `--engine claude` to drive a +**fixed `claude` CLI** through the same bounded runner instead — provider-neutral, with +zero extra dependencies. The claude engine uses your existing Claude auth; for a +subscription set `CLAUDE_CODE_OAUTH_TOKEN` (from `claude setup-token`) and leave +`ANTHROPIC_API_KEY` unset. The binary is fixed (`codex`/`claude`), never +operator-configurable. + ## Enable / Disable Target repositories can disable hook-based dispatch with: diff --git a/src/codex-process.ts b/src/codex-process.ts index e69090223f..83cd21c50c 100644 --- a/src/codex-process.ts +++ b/src/codex-process.ts @@ -79,6 +79,9 @@ export function runCodexProcess(options: { stdoutPath?: string; stderrPath?: string; appServer?: CodexAppServerProcessOptions; + /** Override the spawned binary (defaults to codex) so the same bounded runner + * can drive another fixed review CLI, e.g. `claude`. */ + command?: string; }): CodexProcessResult { const workDir = mkdtempSync(join(tmpdir(), "clawsweeper-codex-process-")); const optionsPath = join(workDir, "options.json"); @@ -90,7 +93,7 @@ export function runCodexProcess(options: { optionsPath, JSON.stringify({ args: [...options.args], - command: codexProcessCommand(options.env), + command: options.command ?? codexProcessCommand(options.env), timeoutMs: options.timeoutMs, resultPath, stdoutPath, diff --git a/src/commit-sweeper.ts b/src/commit-sweeper.ts index 495a6b30e8..30edc906bf 100644 --- a/src/commit-sweeper.ts +++ b/src/commit-sweeper.ts @@ -353,6 +353,64 @@ function runCodex(options: { return stripMarkdownFence(readFileSync(outputPath, "utf8")); } +// Local-review Claude engine: drives a FIXED `claude` CLI through the same bounded +// runner as codex (no operator-configurable binary), credentials scrubbed via +// codexEnv, read-only tools. Auth is the caller's existing Claude auth — for a +// subscription set CLAUDE_CODE_OAUTH_TOKEN (and leave ANTHROPIC_API_KEY unset). +function runClaudeReview(options: { + targetDir: string; + targetRepo: string; + sha: string; + baseSha: string; + metadata: CommitMetadata; + timeoutMs: number; + additionalPrompt: string; +}): string { + const prompt = promptForCommit({ + targetDir: options.targetDir, + targetRepo: options.targetRepo, + sha: options.sha, + baseSha: options.baseSha, + metadata: options.metadata, + additionalPrompt: options.additionalPrompt, + }); + const result = runCodexProcess({ + command: "claude", + args: ["-p", "--output-format", "text", "--allowedTools", "Read Grep Glob"], + cwd: options.targetDir, + env: codexEnv(), + input: prompt, + timeoutMs: options.timeoutMs, + }); + if (result.error || result.status !== 0) { + const timeout = codexProcessErrorCode(result.error) === "ETIMEDOUT"; + const detail = + result.error instanceof Error + ? `${result.error.message}\n${safeOutputTail(result.stderr)}` + : `exit ${result.status ?? "unknown"}\n${safeOutputTail(result.stderr) || "No output."}`; + return failureReport({ + targetRepo: options.targetRepo, + sha: options.sha, + baseSha: options.baseSha, + metadata: options.metadata, + detail: detail.trim(), + timeout, + }); + } + const markdown = stripMarkdownFence(result.stdout); + if (!markdown.trim()) { + return failureReport({ + targetRepo: options.targetRepo, + sha: options.sha, + baseSha: options.baseSha, + metadata: options.metadata, + detail: "claude produced no output", + timeout: false, + }); + } + return markdown; +} + function reviewCommand(args: Args): void { const targetRepo = argString(args, "target_repo", DEFAULT_TARGET_REPO); const targetDir = resolve( @@ -483,8 +541,22 @@ function localReviewCommand(args: Args): void { `[local-review] repo=${targetRepo} profile=${profileSlug} base=${baseBranch} range=${baseSha.slice(0, 8)}..${headSha.slice(0, 8)}`, ); - const markdown = ensureCommitReportTimestamps( - runCodex({ + // codex is the built-in default engine; --engine claude drives a fixed `claude` + // CLI through the same bounded runner (provider-neutral, zero new deps). + const engine = argString(args, "engine", "codex"); + let reviewMarkdown: string; + if (engine === "claude") { + reviewMarkdown = runClaudeReview({ + targetDir, + targetRepo, + sha: headSha, + baseSha, + metadata, + timeoutMs: argNumber(args, "claude_timeout_ms", 1_800_000), + additionalPrompt, + }); + } else if (engine === "codex") { + reviewMarkdown = runCodex({ targetDir, targetRepo, sha: headSha, @@ -498,9 +570,12 @@ function localReviewCommand(args: Args): void { workDir: runDir, additionalPrompt, extraCodexConfig: [LOCAL_REVIEW_WEB_SEARCH_CONFIG], - }), - metadata, - ); + }); + } else { + console.error(`[local-review] --engine must be "codex" or "claude", got "${engine}"`); + process.exit(1); + } + const markdown = ensureCommitReportTimestamps(reviewMarkdown, metadata); const outputPath = join(runDir, "local-review.md"); writeFileSync(outputPath, markdown.endsWith("\n") ? markdown : `${markdown}\n`, "utf8"); diff --git a/test/local-review.test.ts b/test/local-review.test.ts index 647b729da5..e2cf4460aa 100644 --- a/test/local-review.test.ts +++ b/test/local-review.test.ts @@ -111,6 +111,28 @@ test("local-review rejects repositories covered only by a generic owner fallback } }); +test("local-review rejects an unknown --engine", () => { + const dir = initRepo(); + try { + const base = git(dir, "rev-parse", "HEAD"); + writeFileSync(join(dir, "b.txt"), "2\n"); + git(dir, "add", "b.txt"); + git(dir, "commit", "-q", "-m", "second"); + const { status, out } = runLocalReview(dir, [ + "--target-repo", + "openclaw/clawsweeper", + "--base", + base, + "--engine", + "bogus", + ]); + assert.equal(status, 1); + assert.match(out, /--engine must be "codex" or "claude"/); + } finally { + rmSync(dir, { recursive: true, force: true }); + } +}); + test("local-review reports nothing to review when HEAD has no commits beyond base", () => { const dir = initRepo(); try { From 97f579d53ae7904ecc41c9cb271b57b68042e9a7 Mon Sep 17 00:00:00 2001 From: Cameron Beeley Date: Tue, 16 Jun 2026 23:24:46 +0100 Subject: [PATCH 2/5] fix(local-review): validate --engine before creating run state Move the --engine codex|claude validation to the top of localReviewCommand so an invalid engine fails fast before any run state (run dir, GH_CONFIG_DIR) is created. Addresses the P3 review finding on #309. --- src/commit-sweeper.ts | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/commit-sweeper.ts b/src/commit-sweeper.ts index 30edc906bf..26f10977a7 100644 --- a/src/commit-sweeper.ts +++ b/src/commit-sweeper.ts @@ -480,6 +480,12 @@ export function localReviewAdditionalPrompt( function localReviewCommand(args: Args): void { const targetDir = resolve(argString(args, "target_dir", ".")); const baseBranch = argString(args, "base", "main"); + // Validate the engine up front, before creating any run state. + const engine = argString(args, "engine", "codex"); + if (engine !== "codex" && engine !== "claude") { + console.error(`[local-review] --engine must be "codex" or "claude", got "${engine}"`); + process.exit(1); + } const reportDir = resolve( argString(args, "report_dir", join(homedir(), ".clawsweeper-local-reviews")), ); @@ -541,9 +547,8 @@ function localReviewCommand(args: Args): void { `[local-review] repo=${targetRepo} profile=${profileSlug} base=${baseBranch} range=${baseSha.slice(0, 8)}..${headSha.slice(0, 8)}`, ); - // codex is the built-in default engine; --engine claude drives a fixed `claude` - // CLI through the same bounded runner (provider-neutral, zero new deps). - const engine = argString(args, "engine", "codex"); + // engine validated up front; codex is the default, --engine claude drives a fixed + // `claude` CLI through the same bounded runner (provider-neutral, zero new deps). let reviewMarkdown: string; if (engine === "claude") { reviewMarkdown = runClaudeReview({ @@ -555,7 +560,7 @@ function localReviewCommand(args: Args): void { timeoutMs: argNumber(args, "claude_timeout_ms", 1_800_000), additionalPrompt, }); - } else if (engine === "codex") { + } else { reviewMarkdown = runCodex({ targetDir, targetRepo, @@ -571,9 +576,6 @@ function localReviewCommand(args: Args): void { additionalPrompt, extraCodexConfig: [LOCAL_REVIEW_WEB_SEARCH_CONFIG], }); - } else { - console.error(`[local-review] --engine must be "codex" or "claude", got "${engine}"`); - process.exit(1); } const markdown = ensureCommitReportTimestamps(reviewMarkdown, metadata); From b4ea270165e2e69f7bc1a68e4c8d16c2f3e19cde Mon Sep 17 00:00:00 2001 From: Cameron Beeley Date: Wed, 17 Jun 2026 06:49:34 +0100 Subject: [PATCH 3/5] fix(local-review): claude engine parity with codex (diff visibility, full stdout, error fallback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found by local-review --engine claude reviewing its own PR. (1) Embed the full committed-range diff in the claude prompt — the read-only reviewer can't run git to see the change (codex's sandbox can). (2) Read the full captured stdout file instead of the 64 KiB tail, so a large report's YAML front matter isn't truncated. (3) Fall back to stdout in failure reports when stderr is empty (mirrors codex). --- src/commit-sweeper.ts | 38 +++++++++++++++++++++++++++++++++----- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/src/commit-sweeper.ts b/src/commit-sweeper.ts index 26f10977a7..fb4e99078e 100644 --- a/src/commit-sweeper.ts +++ b/src/commit-sweeper.ts @@ -364,16 +364,39 @@ function runClaudeReview(options: { baseSha: string; metadata: CommitMetadata; timeoutMs: number; + workDir: string; additionalPrompt: string; }): string { - const prompt = promptForCommit({ + // The claude reviewer is read-only (Read/Grep/Glob, no shell), so unlike the + // sandboxed codex lane it cannot run `git` to inspect the range. Embed the full + // committed-range diff in the prompt so it reviews the actual change, not just the + // current HEAD files plus a changed-file list. + const rawDiff = run("git", ["diff", `${options.baseSha}..${options.sha}`], { + cwd: options.targetDir, + }); + const maxDiffBytes = 256 * 1024; + const diff = + rawDiff.length > maxDiffBytes + ? `${rawDiff.slice(0, maxDiffBytes)}\n…(diff truncated at ${maxDiffBytes} bytes)…` + : rawDiff; + const prompt = `${promptForCommit({ targetDir: options.targetDir, targetRepo: options.targetRepo, sha: options.sha, baseSha: options.baseSha, metadata: options.metadata, additionalPrompt: options.additionalPrompt, - }); + })} + +## Full Range Diff (\`${options.baseSha}..${options.sha}\`) + +\`\`\`diff +${diff || "(empty diff)"} +\`\`\` +`; + // Capture the FULL stdout (not the runner's 64 KiB tail) so a large report's YAML + // front matter at the top is never truncated away. + const stdoutPath = join(options.workDir, "claude-stdout.log"); const result = runCodexProcess({ command: "claude", args: ["-p", "--output-format", "text", "--allowedTools", "Read Grep Glob"], @@ -381,13 +404,17 @@ function runClaudeReview(options: { env: codexEnv(), input: prompt, timeoutMs: options.timeoutMs, + stdoutPath, }); + const stdout = existsSync(stdoutPath) ? readFileSync(stdoutPath, "utf8") : result.stdout; if (result.error || result.status !== 0) { const timeout = codexProcessErrorCode(result.error) === "ETIMEDOUT"; const detail = result.error instanceof Error - ? `${result.error.message}\n${safeOutputTail(result.stderr)}` - : `exit ${result.status ?? "unknown"}\n${safeOutputTail(result.stderr) || "No output."}`; + ? `${result.error.message}\n${safeOutputTail(result.stderr) || safeOutputTail(stdout)}` + : `exit ${result.status ?? "unknown"}\n${ + safeOutputTail(result.stderr) || safeOutputTail(stdout) || "No output." + }`; return failureReport({ targetRepo: options.targetRepo, sha: options.sha, @@ -397,7 +424,7 @@ function runClaudeReview(options: { timeout, }); } - const markdown = stripMarkdownFence(result.stdout); + const markdown = stripMarkdownFence(stdout); if (!markdown.trim()) { return failureReport({ targetRepo: options.targetRepo, @@ -558,6 +585,7 @@ function localReviewCommand(args: Args): void { baseSha, metadata, timeoutMs: argNumber(args, "claude_timeout_ms", 1_800_000), + workDir: runDir, additionalPrompt, }); } else { From d0d627658e94345bf971fb8196874bac0e248fa6 Mon Sep 17 00:00:00 2001 From: Cameron Beeley Date: Wed, 17 Jun 2026 06:55:52 +0100 Subject: [PATCH 4/5] fix(local-review): comma-separate claude --allowedTools MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Read Grep Glob" was passed as a single space-joined token (one bogus rule → no tools enabled in -p mode). Comma-separate so Read/Grep/Glob parse as three rules and the reviewer can actually open repo files. Found by the claude meta self-review. --- src/commit-sweeper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/commit-sweeper.ts b/src/commit-sweeper.ts index fb4e99078e..e5af57f450 100644 --- a/src/commit-sweeper.ts +++ b/src/commit-sweeper.ts @@ -399,7 +399,7 @@ ${diff || "(empty diff)"} const stdoutPath = join(options.workDir, "claude-stdout.log"); const result = runCodexProcess({ command: "claude", - args: ["-p", "--output-format", "text", "--allowedTools", "Read Grep Glob"], + args: ["-p", "--output-format", "text", "--allowedTools", "Read,Grep,Glob"], cwd: options.targetDir, env: codexEnv(), input: prompt, From 36239522d6a1768986038b5aea4954b7ee7690f3 Mon Sep 17 00:00:00 2001 From: Cameron Beeley Date: Wed, 17 Jun 2026 07:02:48 +0100 Subject: [PATCH 5/5] fix(local-review): guard claude lane range-diff read against oversized branches run() buffers the whole diff host-side; a >maxBuffer whole-branch diff threw before the 256KiB cap applied, aborting the process with no report. Wrap in try/catch -> failureReport, matching the lane's graceful-degradation pattern. Found by the claude meta self-review. --- src/commit-sweeper.ts | 25 ++++++++++++++++++++++--- 1 file changed, 22 insertions(+), 3 deletions(-) diff --git a/src/commit-sweeper.ts b/src/commit-sweeper.ts index e5af57f450..52ba8cac21 100644 --- a/src/commit-sweeper.ts +++ b/src/commit-sweeper.ts @@ -371,9 +371,28 @@ function runClaudeReview(options: { // sandboxed codex lane it cannot run `git` to inspect the range. Embed the full // committed-range diff in the prompt so it reviews the actual change, not just the // current HEAD files plus a changed-file list. - const rawDiff = run("git", ["diff", `${options.baseSha}..${options.sha}`], { - cwd: options.targetDir, - }); + // `run` buffers the whole diff host-side (execFileSync). A huge whole-branch diff + // (vendored/generated/binary-ish blobs) can exceed the maxBuffer ceiling and throw + // before the cap below ever applies — degrade to a failure report, like the rest of + // the lane, instead of aborting the process with a stack trace. (codex avoids this by + // diffing inside its own sandbox.) + let rawDiff: string; + try { + rawDiff = run("git", ["diff", `${options.baseSha}..${options.sha}`], { + cwd: options.targetDir, + }); + } catch (error) { + return failureReport({ + targetRepo: options.targetRepo, + sha: options.sha, + baseSha: options.baseSha, + metadata: options.metadata, + detail: `failed to read range diff: ${ + error instanceof Error ? error.message : String(error) + }`, + timeout: false, + }); + } const maxDiffBytes = 256 * 1024; const diff = rawDiff.length > maxDiffBytes