From 680c2a05974708c64901471af5784c5a36e4fb4c Mon Sep 17 00:00:00 2001 From: catlog22 Date: Sat, 17 Jan 2026 22:07:26 +0800 Subject: [PATCH] fix(cli): allow codex review with target flags without prompt - Skip template concatenation when using --uncommitted/--base/--commit - Allow empty prompt for review mode with target flags - Add hasReviewTarget check in command routing - Update documentation with validation constraints codex review constraint: target flags and prompt are mutually exclusive --- .claude/commands/cli/codex-review.md | 44 +++++++++++++++++++++++ .claude/workflows/cli-tools-usage.md | 17 +++++---- ccw/src/commands/cli.ts | 53 ++++++++++++++++++---------- ccw/src/tools/cli-executor-core.ts | 2 +- 4 files changed, 91 insertions(+), 25 deletions(-) diff --git a/.claude/commands/cli/codex-review.md b/.claude/commands/cli/codex-review.md index 6c31ef7c..330bb482 100644 --- a/.claude/commands/cli/codex-review.md +++ b/.claude/commands/cli/codex-review.md @@ -309,3 +309,47 @@ git log --oneline -10 - Model passed via prompt (codex uses `-c model=` internally) - Target flags (`--uncommitted`, `--base`, `--commit`) passed through to codex - Prompt follows standard ccw cli template format for consistency + +## Validation Constraints + +**IMPORTANT: Target flags and prompt are mutually exclusive** + +The codex CLI has a constraint where target flags (`--uncommitted`, `--base`, `--commit`) cannot be used with a positional `[PROMPT]` argument: + +``` +error: the argument '--uncommitted' cannot be used with '[PROMPT]' +error: the argument '--base ' cannot be used with '[PROMPT]' +error: the argument '--commit ' cannot be used with '[PROMPT]' +``` + +**Behavior:** +- When ANY target flag is specified, ccw cli automatically skips template concatenation (systemRules/roles) +- The review uses codex's default review behavior for the specified target +- Custom prompts are only supported WITHOUT target flags (reviews uncommitted changes by default) + +**Valid combinations:** +| Command | Result | +|---------|--------| +| `codex review "Focus on security"` | ✓ Custom prompt, reviews uncommitted (default) | +| `codex review --uncommitted` | ✓ No prompt, uses default review | +| `codex review --base main` | ✓ No prompt, uses default review | +| `codex review --commit abc123` | ✓ No prompt, uses default review | +| `codex review --uncommitted "prompt"` | ✗ Invalid - mutually exclusive | +| `codex review --base main "prompt"` | ✗ Invalid - mutually exclusive | +| `codex review --commit abc123 "prompt"` | ✗ Invalid - mutually exclusive | + +**Examples:** +```bash +# ✓ Valid: prompt only (reviews uncommitted by default) +ccw cli -p "Focus on security" --tool codex --mode review + +# ✓ Valid: target flag only (no prompt) +ccw cli --tool codex --mode review --uncommitted +ccw cli --tool codex --mode review --base main +ccw cli --tool codex --mode review --commit abc123 + +# ✗ Invalid: target flag with prompt (will fail) +ccw cli -p "Review this" --tool codex --mode review --uncommitted +ccw cli -p "Review this" --tool codex --mode review --base main +ccw cli -p "Review this" --tool codex --mode review --commit abc123 +``` diff --git a/.claude/workflows/cli-tools-usage.md b/.claude/workflows/cli-tools-usage.md index db9fac05..4b15a54a 100644 --- a/.claude/workflows/cli-tools-usage.md +++ b/.claude/workflows/cli-tools-usage.md @@ -299,10 +299,13 @@ ccw cli -p "..." --tool gemini --mode analysis --rule analysis-review-architectu - **`review`** - Permission: Read-only (code review output) - Use For: Git-aware code review of uncommitted changes, branch diffs, specific commits - - Specification: **codex only** - uses `codex review` subcommand with `--uncommitted` by default + - Specification: **codex only** - uses `codex review` subcommand - Tool Behavior: - - `codex`: Executes `codex review --uncommitted [prompt]` for structured code review + - `codex`: Executes `codex review` for structured code review - Other tools (gemini/qwen/claude): Accept mode but no operation change (treated as analysis) + - **Constraint**: Target flags (`--uncommitted`, `--base`, `--commit`) and prompt are mutually exclusive + - With prompt only: `ccw cli -p "Focus on security" --tool codex --mode review` (reviews uncommitted by default) + - With target flag only: `ccw cli --tool codex --mode review --commit abc123` (no prompt allowed) ### Command Options @@ -445,14 +448,16 @@ CONSTRAINTS: Preserve all existing behavior | Tests must pass **Code Review Task** (codex review mode): ```bash -# Review uncommitted changes (default) +# Option 1: Custom prompt (reviews uncommitted changes by default) ccw cli -p "Focus on security vulnerabilities and error handling" --tool codex --mode review -# Review with custom instructions -ccw cli -p "Check for breaking changes in API contracts and backward compatibility" --tool codex --mode review +# Option 2: Target flag only (no prompt allowed with target flags) +ccw cli --tool codex --mode review --uncommitted +ccw cli --tool codex --mode review --base main +ccw cli --tool codex --mode review --commit abc123 ``` -> **Note**: `--mode review` only triggers special behavior for `codex` tool (uses `codex review --uncommitted`). Other tools accept the mode but execute as standard analysis. +> **Note**: `--mode review` only triggers special behavior for `codex` tool. Target flags (`--uncommitted`, `--base`, `--commit`) and prompt are **mutually exclusive** - use one or the other, not both. --- diff --git a/ccw/src/commands/cli.ts b/ccw/src/commands/cli.ts index 2dd3b6f8..491ea249 100644 --- a/ccw/src/commands/cli.ts +++ b/ccw/src/commands/cli.ts @@ -578,36 +578,49 @@ async function execAction(positionalPrompt: string | undefined, options: CliExec finalPrompt = positionalPrompt; } - // Prompt is required unless resuming - if (!finalPrompt && !resume) { + // Prompt is required unless resuming OR using review mode with target flags + // codex review: --uncommitted, --base, --commit don't require a prompt + const isReviewWithTarget = mode === 'review' && (uncommitted || base || commit); + if (!finalPrompt && !resume && !isReviewWithTarget) { console.error(chalk.red('Error: Prompt is required')); console.error(chalk.gray('Usage: ccw cli -p "" --tool gemini')); console.error(chalk.gray(' or: ccw cli -f prompt.txt --tool codex')); console.error(chalk.gray(' or: ccw cli --resume --tool gemini')); + console.error(chalk.gray(' or: ccw cli --tool codex --mode review --uncommitted')); process.exit(1); } const prompt_to_use = finalPrompt || ''; - // Load rules templates (concatenation mode - directly prepend to prompt) + // Load rules templates (concatenation mode - directly append to prompt) + // Skip template loading when using target flags with codex review + // codex review: --uncommitted, --base, --commit are all mutually exclusive with [PROMPT] // Default to universal-rigorous-style if --rule not specified + const skipTemplates = mode === 'review' && (uncommitted || base || commit); const effectiveRule = rule || 'universal-rigorous-style'; let systemRules = ''; // Protocol content let roles = ''; // Template content - try { - const { loadProtocol, loadTemplate } = await import('../tools/template-discovery.js'); - const proto = loadProtocol(mode); - const tmpl = loadTemplate(effectiveRule); - if (proto) systemRules = proto; - if (tmpl) roles = tmpl; + + if (skipTemplates) { if (debug) { - console.log(chalk.gray(` Rule loaded: ${effectiveRule}${!rule ? ' (default)' : ''}`)); - console.log(chalk.gray(` systemRules(${systemRules.length} chars) + roles(${roles.length} chars)`)); - console.log(chalk.gray(` Rules will be prepended to prompt automatically`)); + console.log(chalk.gray(` Skipping templates: --commit with review mode doesn't support prompt`)); + } + } else { + try { + const { loadProtocol, loadTemplate } = await import('../tools/template-discovery.js'); + const proto = loadProtocol(mode); + const tmpl = loadTemplate(effectiveRule); + if (proto) systemRules = proto; + if (tmpl) roles = tmpl; + if (debug) { + console.log(chalk.gray(` Rule loaded: ${effectiveRule}${!rule ? ' (default)' : ''}`)); + console.log(chalk.gray(` systemRules(${systemRules.length} chars) + roles(${roles.length} chars)`)); + console.log(chalk.gray(` Rules will be appended to prompt automatically`)); + } + } catch (error) { + console.error(chalk.red(`Error loading rule template: ${error instanceof Error ? error.message : error}`)); + process.exit(1); } - } catch (error) { - console.error(chalk.red(`Error loading rule template: ${error instanceof Error ? error.message : error}`)); - process.exit(1); } // Handle cache option: pack @patterns and/or content @@ -734,7 +747,8 @@ async function execAction(positionalPrompt: string | undefined, options: CliExec // Concatenate systemRules and roles to the end of prompt (if loaded) // Format: [USER_PROMPT]\n[SYSTEM_RULES]\n[ROLES] - if (systemRules || roles) { + // Skip concatenation when using --commit with review mode (prompt not allowed) + if (!skipTemplates && (systemRules || roles)) { const parts: string[] = [actualPrompt]; if (systemRules) { parts.push(`=== SYSTEM RULES ===\n${systemRules}`); @@ -1242,13 +1256,16 @@ export async function cliCommand( default: { const execOptions = options as CliExecOptions; - // Auto-exec if: has -p/--prompt, has -f/--file, has --resume, or subcommand looks like a prompt + // Auto-exec if: has -p/--prompt, has -f/--file, has --resume, subcommand looks like a prompt, + // or review mode with target flags (--uncommitted, --base, --commit) const hasPromptOption = !!execOptions.prompt; const hasFileOption = !!execOptions.file; const hasResume = execOptions.resume !== undefined; const subcommandIsPrompt = subcommand && !subcommand.startsWith('-'); + const hasReviewTarget = execOptions.mode === 'review' && + (execOptions.uncommitted || execOptions.base || execOptions.commit); - if (hasPromptOption || hasFileOption || hasResume || subcommandIsPrompt) { + if (hasPromptOption || hasFileOption || hasResume || subcommandIsPrompt || hasReviewTarget) { // Treat as exec: use subcommand as positional prompt if no -p/-f option let positionalPrompt = subcommandIsPrompt ? subcommand : undefined; diff --git a/ccw/src/tools/cli-executor-core.ts b/ccw/src/tools/cli-executor-core.ts index 12f5d767..f8e3ff20 100644 --- a/ccw/src/tools/cli-executor-core.ts +++ b/ccw/src/tools/cli-executor-core.ts @@ -350,7 +350,7 @@ type BuiltinCliTool = typeof BUILTIN_CLI_TOOLS[number]; // tool accepts built-in tools or custom endpoint IDs (CLI封装) const ParamsSchema = z.object({ tool: z.string().min(1, 'Tool is required'), // Accept any tool ID (built-in or custom endpoint) - prompt: z.string().min(1, 'Prompt is required'), + prompt: z.string(), // Prompt can be empty for review mode with target flags mode: z.enum(['analysis', 'write', 'auto', 'review']).default('analysis'), format: z.enum(['plain', 'yaml', 'json']).default('plain'), // Multi-turn prompt concatenation format model: z.string().optional(),