mirror of
https://github.com/catlog22/Claude-Code-Workflow.git
synced 2026-02-12 02:37:45 +08:00
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
This commit is contained in:
@@ -309,3 +309,47 @@ git log --oneline -10
|
|||||||
- Model passed via prompt (codex uses `-c model=` internally)
|
- Model passed via prompt (codex uses `-c model=` internally)
|
||||||
- Target flags (`--uncommitted`, `--base`, `--commit`) passed through to codex
|
- Target flags (`--uncommitted`, `--base`, `--commit`) passed through to codex
|
||||||
- Prompt follows standard ccw cli template format for consistency
|
- 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 <BRANCH>' cannot be used with '[PROMPT]'
|
||||||
|
error: the argument '--commit <SHA>' 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
|
||||||
|
```
|
||||||
|
|||||||
@@ -299,10 +299,13 @@ ccw cli -p "..." --tool gemini --mode analysis --rule analysis-review-architectu
|
|||||||
- **`review`**
|
- **`review`**
|
||||||
- Permission: Read-only (code review output)
|
- Permission: Read-only (code review output)
|
||||||
- Use For: Git-aware code review of uncommitted changes, branch diffs, specific commits
|
- 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:
|
- 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)
|
- 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
|
### Command Options
|
||||||
|
|
||||||
@@ -445,14 +448,16 @@ CONSTRAINTS: Preserve all existing behavior | Tests must pass
|
|||||||
|
|
||||||
**Code Review Task** (codex review mode):
|
**Code Review Task** (codex review mode):
|
||||||
```bash
|
```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
|
ccw cli -p "Focus on security vulnerabilities and error handling" --tool codex --mode review
|
||||||
|
|
||||||
# Review with custom instructions
|
# Option 2: Target flag only (no prompt allowed with target flags)
|
||||||
ccw cli -p "Check for breaking changes in API contracts and backward compatibility" --tool codex --mode review
|
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.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|||||||
@@ -578,36 +578,49 @@ async function execAction(positionalPrompt: string | undefined, options: CliExec
|
|||||||
finalPrompt = positionalPrompt;
|
finalPrompt = positionalPrompt;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Prompt is required unless resuming
|
// Prompt is required unless resuming OR using review mode with target flags
|
||||||
if (!finalPrompt && !resume) {
|
// 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.red('Error: Prompt is required'));
|
||||||
console.error(chalk.gray('Usage: ccw cli -p "<prompt>" --tool gemini'));
|
console.error(chalk.gray('Usage: ccw cli -p "<prompt>" --tool gemini'));
|
||||||
console.error(chalk.gray(' or: ccw cli -f prompt.txt --tool codex'));
|
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 --resume --tool gemini'));
|
||||||
|
console.error(chalk.gray(' or: ccw cli --tool codex --mode review --uncommitted'));
|
||||||
process.exit(1);
|
process.exit(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
const prompt_to_use = finalPrompt || '';
|
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
|
// Default to universal-rigorous-style if --rule not specified
|
||||||
|
const skipTemplates = mode === 'review' && (uncommitted || base || commit);
|
||||||
const effectiveRule = rule || 'universal-rigorous-style';
|
const effectiveRule = rule || 'universal-rigorous-style';
|
||||||
let systemRules = ''; // Protocol content
|
let systemRules = ''; // Protocol content
|
||||||
let roles = ''; // Template content
|
let roles = ''; // Template content
|
||||||
try {
|
|
||||||
const { loadProtocol, loadTemplate } = await import('../tools/template-discovery.js');
|
if (skipTemplates) {
|
||||||
const proto = loadProtocol(mode);
|
|
||||||
const tmpl = loadTemplate(effectiveRule);
|
|
||||||
if (proto) systemRules = proto;
|
|
||||||
if (tmpl) roles = tmpl;
|
|
||||||
if (debug) {
|
if (debug) {
|
||||||
console.log(chalk.gray(` Rule loaded: ${effectiveRule}${!rule ? ' (default)' : ''}`));
|
console.log(chalk.gray(` Skipping templates: --commit with review mode doesn't support prompt`));
|
||||||
console.log(chalk.gray(` systemRules(${systemRules.length} chars) + roles(${roles.length} chars)`));
|
}
|
||||||
console.log(chalk.gray(` Rules will be prepended to prompt automatically`));
|
} 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
|
// 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)
|
// Concatenate systemRules and roles to the end of prompt (if loaded)
|
||||||
// Format: [USER_PROMPT]\n[SYSTEM_RULES]\n[ROLES]
|
// 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];
|
const parts: string[] = [actualPrompt];
|
||||||
if (systemRules) {
|
if (systemRules) {
|
||||||
parts.push(`=== SYSTEM RULES ===\n${systemRules}`);
|
parts.push(`=== SYSTEM RULES ===\n${systemRules}`);
|
||||||
@@ -1242,13 +1256,16 @@ export async function cliCommand(
|
|||||||
|
|
||||||
default: {
|
default: {
|
||||||
const execOptions = options as CliExecOptions;
|
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 hasPromptOption = !!execOptions.prompt;
|
||||||
const hasFileOption = !!execOptions.file;
|
const hasFileOption = !!execOptions.file;
|
||||||
const hasResume = execOptions.resume !== undefined;
|
const hasResume = execOptions.resume !== undefined;
|
||||||
const subcommandIsPrompt = subcommand && !subcommand.startsWith('-');
|
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
|
// Treat as exec: use subcommand as positional prompt if no -p/-f option
|
||||||
let positionalPrompt = subcommandIsPrompt ? subcommand : undefined;
|
let positionalPrompt = subcommandIsPrompt ? subcommand : undefined;
|
||||||
|
|
||||||
|
|||||||
@@ -350,7 +350,7 @@ type BuiltinCliTool = typeof BUILTIN_CLI_TOOLS[number];
|
|||||||
// tool accepts built-in tools or custom endpoint IDs (CLI封装)
|
// tool accepts built-in tools or custom endpoint IDs (CLI封装)
|
||||||
const ParamsSchema = z.object({
|
const ParamsSchema = z.object({
|
||||||
tool: z.string().min(1, 'Tool is required'), // Accept any tool ID (built-in or custom endpoint)
|
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'),
|
mode: z.enum(['analysis', 'write', 'auto', 'review']).default('analysis'),
|
||||||
format: z.enum(['plain', 'yaml', 'json']).default('plain'), // Multi-turn prompt concatenation format
|
format: z.enum(['plain', 'yaml', 'json']).default('plain'), // Multi-turn prompt concatenation format
|
||||||
model: z.string().optional(),
|
model: z.string().optional(),
|
||||||
|
|||||||
Reference in New Issue
Block a user