Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion plugins/codex/commands/adversarial-review.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
description: Run a Codex review that challenges the implementation approach and design choices
argument-hint: '[--wait|--background] [--base <ref>] [--scope auto|working-tree|branch] [focus ...]'
argument-hint: '[--wait|--background] [--base <ref>] [--scope auto|working-tree|branch] [--max-inline-files <n>] [--max-inline-bytes <n>] [focus ...]'
disable-model-invocation: true
allowed-tools: Read, Glob, Grep, Bash(node:*), Bash(git:*), AskUserQuestion
---
Expand Down Expand Up @@ -43,6 +43,7 @@ Argument handling:
- It supports working-tree review, branch review, and `--base <ref>`.
- It does not support `--scope staged` or `--scope unstaged`.
- Unlike `/codex:review`, it can still take extra focus text after the flags.
- `--max-inline-files <n>` and `--max-inline-bytes <n>` override the inline diff limits when the diff is too large or too small for the defaults; pass `0` to force the lightweight self-collect prompt.

Foreground flow:
- Run:
Expand Down
24 changes: 21 additions & 3 deletions plugins/codex/scripts/codex-companion.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ function printUsage() {
"Usage:",
" node scripts/codex-companion.mjs setup [--enable-review-gate|--disable-review-gate] [--json]",
" node scripts/codex-companion.mjs review [--wait|--background] [--base <ref>] [--scope <auto|working-tree|branch>]",
" node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base <ref>] [--scope <auto|working-tree|branch>] [focus text]",
" node scripts/codex-companion.mjs adversarial-review [--wait|--background] [--base <ref>] [--scope <auto|working-tree|branch>] [--max-inline-files <n>] [--max-inline-bytes <n>] [focus text]",
" node scripts/codex-companion.mjs task [--background] [--write] [--resume-last|--resume|--fresh] [--model <model|spark>] [--effort <none|minimal|low|medium|high|xhigh>] [prompt]",
" node scripts/codex-companion.mjs status [job-id] [--all] [--json]",
" node scripts/codex-companion.mjs result [job-id] [--json]",
Expand Down Expand Up @@ -157,6 +157,17 @@ function sleep(ms) {
return new Promise((resolve) => setTimeout(resolve, ms));
}

function parseNonNegativeIntegerOption(rawValue, optionLabel) {
if (rawValue === undefined || rawValue === null || rawValue === "") {
return undefined;
}
const parsed = Number(rawValue);
if (!Number.isFinite(parsed) || parsed < 0 || !Number.isInteger(parsed)) {
throw new Error(`${optionLabel} must be a non-negative integer.`);
}
return parsed;
}

function shorten(text, limit = 96) {
const normalized = String(text ?? "").trim().replace(/\s+/g, " ");
if (!normalized) {
Expand Down Expand Up @@ -403,7 +414,10 @@ async function executeReviewRun(request) {
};
}

const context = collectReviewContext(request.cwd, target);
const context = collectReviewContext(request.cwd, target, {
maxInlineFiles: request.maxInlineFiles,
maxInlineDiffBytes: request.maxInlineDiffBytes
});
const prompt = buildAdversarialReviewPrompt(context, focusText);
const result = await runAppServerTurn(context.repoRoot, {
prompt,
Expand Down Expand Up @@ -681,7 +695,7 @@ function enqueueBackgroundTask(cwd, job, request) {

async function handleReviewCommand(argv, config) {
const { options, positionals } = parseCommandInput(argv, {
valueOptions: ["base", "scope", "model", "cwd"],
valueOptions: ["base", "scope", "model", "cwd", "max-inline-files", "max-inline-bytes"],
booleanOptions: ["json", "background", "wait"],
aliasMap: {
m: "model"
Expand All @@ -695,6 +709,8 @@ async function handleReviewCommand(argv, config) {
base: options.base,
scope: options.scope
});
const maxInlineFiles = parseNonNegativeIntegerOption(options["max-inline-files"], "--max-inline-files");
const maxInlineDiffBytes = parseNonNegativeIntegerOption(options["max-inline-bytes"], "--max-inline-bytes");

config.validateRequest?.(target, focusText);
const metadata = buildReviewJobMetadata(config.reviewName, target);
Expand All @@ -716,6 +732,8 @@ async function handleReviewCommand(argv, config) {
model: options.model,
focusText,
reviewName: config.reviewName,
maxInlineFiles,
maxInlineDiffBytes,
onProgress: progress
}),
{ json: options.json }
Expand Down
4 changes: 2 additions & 2 deletions plugins/codex/scripts/lib/git.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ import { isProbablyText } from "./fs.mjs";
import { formatCommandFailure, runCommand, runCommandChecked } from "./process.mjs";

const MAX_UNTRACKED_BYTES = 24 * 1024;
const DEFAULT_INLINE_DIFF_MAX_FILES = 2;
const DEFAULT_INLINE_DIFF_MAX_BYTES = 256 * 1024;
const DEFAULT_INLINE_DIFF_MAX_FILES = 50;
const DEFAULT_INLINE_DIFF_MAX_BYTES = 1024 * 1024;

function git(cwd, args, options = {}) {
return runCommand("git", args, { cwd, ...options });
Expand Down
4 changes: 3 additions & 1 deletion tests/commands.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,9 @@ test("adversarial review command uses AskUserQuestion and background Bash while
assert.match(source, /```bash/);
assert.match(source, /```typescript/);
assert.match(source, /adversarial-review "\$ARGUMENTS"/);
assert.match(source, /\[--scope auto\|working-tree\|branch\] \[focus \.\.\.\]/);
assert.match(source, /\[--scope auto\|working-tree\|branch\][^\n]*\[focus \.\.\.\]/);
assert.match(source, /\[--max-inline-files <n>\] \[--max-inline-bytes <n>\]/);
assert.match(source, /override the inline diff limits/i);
assert.match(source, /run_in_background:\s*true/);
assert.match(source, /command:\s*`node "\$\{CLAUDE_PLUGIN_ROOT\}\/scripts\/codex-companion\.mjs" adversarial-review "\$ARGUMENTS"`/);
assert.match(source, /description:\s*"Codex adversarial review"/);
Expand Down
44 changes: 42 additions & 2 deletions tests/git.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ test("collectReviewContext falls back to lightweight context for larger adversar
fs.writeFileSync(path.join(cwd, "c.js"), 'export const value = "SELF_COLLECT_MARKER_C";\n');

const target = resolveReviewTarget(cwd, {});
const context = collectReviewContext(cwd, target);
const context = collectReviewContext(cwd, target, { maxInlineFiles: 2 });

assert.equal(context.inputMode, "self-collect");
assert.equal(context.fileCount, 3);
Expand Down Expand Up @@ -173,11 +173,51 @@ test("collectReviewContext keeps untracked file content in lightweight working t
fs.writeFileSync(path.join(cwd, "new-risk.js"), 'export const value = "UNTRACKED_RISK_MARKER";\n');

const target = resolveReviewTarget(cwd, {});
const context = collectReviewContext(cwd, target);
const context = collectReviewContext(cwd, target, { maxInlineFiles: 2 });

assert.equal(context.inputMode, "self-collect");
assert.equal(context.fileCount, 3);
assert.doesNotMatch(context.content, /TRACKED_MARKER_[AB]/);
assert.match(context.content, /## Untracked Files/);
assert.match(context.content, /UNTRACKED_RISK_MARKER/);
});

test("collectReviewContext keeps inline diffs for medium-sized adversarial reviews under default limits", () => {
const cwd = makeTempDir();
initGitRepo(cwd);
const fileNames = Array.from({ length: 11 }, (_, i) => `mod-${i}.js`);
for (const name of fileNames) {
fs.writeFileSync(path.join(cwd, name), `export const value = "${name}-v1";\n`);
}
run("git", ["add", ...fileNames], { cwd });
run("git", ["commit", "-m", "init"], { cwd });
for (const name of fileNames) {
fs.writeFileSync(path.join(cwd, name), `export const value = "INLINE_DEFAULT_${name}";\n`);
}

const target = resolveReviewTarget(cwd, {});
const context = collectReviewContext(cwd, target);

assert.equal(context.inputMode, "inline-diff");
assert.equal(context.fileCount, 11);
assert.match(context.collectionGuidance, /primary evidence/i);
assert.match(context.content, /INLINE_DEFAULT_mod-0\.js/);
});

test("collectReviewContext respects maxInlineFiles override forcing self-collect", () => {
const cwd = makeTempDir();
initGitRepo(cwd);
fs.writeFileSync(path.join(cwd, "a.js"), "export const value = 'v1';\n");
fs.writeFileSync(path.join(cwd, "b.js"), "export const value = 'v1';\n");
run("git", ["add", "a.js", "b.js"], { cwd });
run("git", ["commit", "-m", "init"], { cwd });
fs.writeFileSync(path.join(cwd, "a.js"), "export const value = 'OVERRIDE_MARKER_A';\n");
fs.writeFileSync(path.join(cwd, "b.js"), "export const value = 'OVERRIDE_MARKER_B';\n");

const target = resolveReviewTarget(cwd, {});
const context = collectReviewContext(cwd, target, { maxInlineFiles: 1 });

assert.equal(context.inputMode, "self-collect");
assert.equal(context.fileCount, 2);
assert.doesNotMatch(context.content, /OVERRIDE_MARKER_[AB]/);
});
68 changes: 67 additions & 1 deletion tests/runtime.test.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -288,7 +288,7 @@ test("adversarial review asks Codex to inspect larger diffs itself", () => {
fs.writeFileSync(path.join(repo, "src", "b.js"), 'export const value = "PROMPT_SELF_COLLECT_B";\n');
fs.writeFileSync(path.join(repo, "src", "c.js"), 'export const value = "PROMPT_SELF_COLLECT_C";\n');

const result = run("node", [SCRIPT, "adversarial-review"], {
const result = run("node", [SCRIPT, "adversarial-review", "--max-inline-files", "2"], {
cwd: repo,
env: buildEnv(binDir)
});
Expand All @@ -300,6 +300,72 @@ test("adversarial review asks Codex to inspect larger diffs itself", () => {
assert.doesNotMatch(state.lastTurnStart.prompt, /PROMPT_SELF_COLLECT_[ABC]/);
});

test("adversarial review keeps medium-sized diffs inline under default limits", () => {
const repo = makeTempDir();
const binDir = makeTempDir();
installFakeCodex(binDir);
initGitRepo(repo);
fs.mkdirSync(path.join(repo, "src"));
const fileNames = Array.from({ length: 10 }, (_, i) => `mod-${i}.js`);
for (const name of fileNames) {
fs.writeFileSync(path.join(repo, "src", name), `export const value = "${name}-v1";\n`);
}
run("git", ["add", ...fileNames.map((name) => `src/${name}`)], { cwd: repo });
run("git", ["commit", "-m", "init"], { cwd: repo });
for (const name of fileNames) {
fs.writeFileSync(path.join(repo, "src", name), `export const value = "INLINE_DEFAULT_${name}";\n`);
}

const result = run("node", [SCRIPT, "adversarial-review"], {
cwd: repo,
env: buildEnv(binDir)
});

assert.equal(result.status, 0, result.stderr);
const state = JSON.parse(fs.readFileSync(path.join(binDir, "fake-codex-state.json"), "utf8"));
assert.match(state.lastTurnStart.prompt, /primary evidence/i);
assert.match(state.lastTurnStart.prompt, /INLINE_DEFAULT_mod-0\.js/);
});

test("adversarial review --max-inline-bytes forces self-collect on large diffs", () => {
const repo = makeTempDir();
const binDir = makeTempDir();
installFakeCodex(binDir);
initGitRepo(repo);
fs.writeFileSync(path.join(repo, "app.js"), "export const value = 'v1';\n");
run("git", ["add", "app.js"], { cwd: repo });
run("git", ["commit", "-m", "init"], { cwd: repo });
fs.writeFileSync(path.join(repo, "app.js"), `export const value = '${"x".repeat(512)}';\n`);

const result = run("node", [SCRIPT, "adversarial-review", "--max-inline-bytes", "128"], {
cwd: repo,
env: buildEnv(binDir)
});

assert.equal(result.status, 0, result.stderr);
const state = JSON.parse(fs.readFileSync(path.join(binDir, "fake-codex-state.json"), "utf8"));
assert.match(state.lastTurnStart.prompt, /lightweight summary/i);
});

test("adversarial review rejects negative --max-inline-files", () => {
const repo = makeTempDir();
const binDir = makeTempDir();
installFakeCodex(binDir);
initGitRepo(repo);
fs.writeFileSync(path.join(repo, "app.js"), "v1\n");
run("git", ["add", "app.js"], { cwd: repo });
run("git", ["commit", "-m", "init"], { cwd: repo });
fs.writeFileSync(path.join(repo, "app.js"), "v2\n");

const result = run("node", [SCRIPT, "adversarial-review", "--max-inline-files", "-1"], {
cwd: repo,
env: buildEnv(binDir)
});

assert.notEqual(result.status, 0);
assert.match(result.stderr, /--max-inline-files must be a non-negative integer/);
});

test("review includes reasoning output when the app server returns it", () => {
const repo = makeTempDir();
const binDir = makeTempDir();
Expand Down