fix(review): address fourth review pass — extra args error, guard tests, docs, fallback test

Author: soumojit-D48

README.md

@@ -56,6 +56,8 @@ nanocoder review main
 nanocoder review 42
 ```
 
+> **Note:** `nanocoder review` requires an interactive terminal (TTY). It cannot be used with pipes or redirection.
+
 ### Screen Modes
 
 Nanocoder supports two rendering modes, mirroring what Claude Code and Codex ship:

docs/getting-started/index.md

@@ -194,6 +194,8 @@ nanocoder review 42
 
 This fetches the diff against the default branch and runs an architect-level review identifying bugs, security issues, and style violations. You can also use `/review <target>` inside the interactive TUI.
 
+**Note:** `nanocoder review` requires an interactive terminal (TTY). It cannot be used with pipes or redirection (e.g. `nanocoder review main > review.md` will error). Use the `/review` slash command inside the interactive TUI for scripting workflows.
+
 **Non-interactive mode behavior:**
 
 - Automatically executes the given prompt

source/cli.spec.ts

@@ -1,4 +1,5 @@
 import test from 'ava';
+import {filterCliFlags} from '@/utils/cli-flags';
 
 // Test CLI argument parsing for non-interactive mode
 // These tests verify that the CLI correctly parses the 'run' command
@@ -13,47 +14,8 @@ function parsePrompt(args: string[]): string | undefined {
 	if (afterRunArgs.length === 0) {
 		return undefined;
 	}
-	// Filter out known flags after 'run' when constructing the prompt
-	const promptArgs: string[] = [];
-	for (let i = 0; i < afterRunArgs.length; i++) {
-		const arg = afterRunArgs[i];
-		if (arg === '--vscode') {
-			continue; // skip this flag
-		} else if (arg === '--vscode-port') {
-			i++; // skip this flag and its value
-			continue;
-		} else if (arg === '--provider') {
-			i++; // skip this flag and its value
-			continue;
-		} else if (arg === '--model') {
-			i++; // skip this flag and its value
-			continue;
-		} else if (arg === '--context-max') {
-			i++; // skip this flag and its value
-			continue;
-		} else if (arg === '--mode') {
-			i++; // skip this flag and its value
-			continue;
-		} else if (arg.startsWith('--mode=')) {
-			continue; // skip fused form
-		} else if (arg === '--json') {
-			continue; // skip this flag
-		} else if (arg === '--output-format') {
-			i++; // skip this flag and its value
-			continue;
-		} else if (arg.startsWith('--output-format=')) {
-			continue; // skip fused form
-		} else if (arg === '--trust-directory') {
-			continue; // skip this flag
-		} else if (arg === '--plain' || arg === '--no-plain') {
-			continue; // skip this flag
-		} else if (arg === '--no-alt-screen' || arg === '--alt-screen') {
-			continue; // skip this flag
-		} else {
-			promptArgs.push(arg);
-		}
-	}
-	return promptArgs.length > 0 ? promptArgs.join(' ') : undefined;
+	const positionals = filterCliFlags(afterRunArgs);
+	return positionals.length > 0 ? positionals.join(' ') : undefined;
 }
 
 test('CLI parsing: detects run command with single word prompt', t => {
@@ -593,3 +555,120 @@ test('CLI parsing: handles --mode before run command', t => {
 	const prompt = parsePrompt(args);
 	t.is(prompt, 'audit module');
 });
+
+// Review guard tests — mirrors the guards in cli.tsx
+function resolveReviewGuards(opts: {
+	args: string[];
+	stdoutIsTTY: boolean;
+	outputFormat: string;
+}): {ttyError: boolean; jsonError: boolean; collisionError: boolean} {
+	const {args, stdoutIsTTY, outputFormat} = opts;
+	const isRunCommand = args.findIndex(arg => arg === 'run') !== -1;
+	const isReviewCommand = args[0] === 'review';
+	const ttyError = isReviewCommand && !stdoutIsTTY;
+	const jsonError = isReviewCommand && outputFormat === 'json';
+	const collisionError = isRunCommand && isReviewCommand;
+	return {ttyError, jsonError, collisionError};
+}
+
+test('review guard: errors when stdout is not a TTY', t => {
+	const {ttyError} = resolveReviewGuards({
+		args: ['review', 'main'],
+		stdoutIsTTY: false,
+		outputFormat: 'text',
+	});
+	t.true(ttyError);
+});
+
+test('review guard: passes on a TTY', t => {
+	const {ttyError} = resolveReviewGuards({
+		args: ['review', 'main'],
+		stdoutIsTTY: true,
+		outputFormat: 'text',
+	});
+	t.false(ttyError);
+});
+
+test('review guard: --json is rejected with review', t => {
+	const {jsonError} = resolveReviewGuards({
+		args: ['review', 'main'],
+		stdoutIsTTY: true,
+		outputFormat: 'json',
+	});
+	t.true(jsonError);
+});
+
+test('review guard: --json is not rejected with run', t => {
+	const {jsonError} = resolveReviewGuards({
+		args: ['run', 'hello'],
+		stdoutIsTTY: true,
+		outputFormat: 'json',
+	});
+	t.false(jsonError);
+});
+
+test('review guard: run and review collision is detected', t => {
+	const {collisionError} = resolveReviewGuards({
+		args: ['review', 'run'],
+		stdoutIsTTY: true,
+		outputFormat: 'text',
+	});
+	t.true(collisionError);
+});
+
+test('review guard: review alone has no collision', t => {
+	const {collisionError} = resolveReviewGuards({
+		args: ['review', 'main'],
+		stdoutIsTTY: true,
+		outputFormat: 'text',
+	});
+	t.false(collisionError);
+});
+
+test('review guard: run alone has no collision', t => {
+	const {collisionError} = resolveReviewGuards({
+		args: ['run', 'hello'],
+		stdoutIsTTY: true,
+		outputFormat: 'text',
+	});
+	t.false(collisionError);
+});
+
+// filterCliFlags: shared flag filter produces single source of truth
+test('filterCliFlags: filters all known flags', t => {
+	const result = filterCliFlags([
+		'--vscode',
+		'--json',
+		'--trust-directory',
+		'--plain',
+		'--no-plain',
+		'--no-alt-screen',
+		'--alt-screen',
+		'--vscode-port',
+		'3000',
+		'--provider',
+		'ollama',
+		'--model',
+		'llama3',
+		'--context-max',
+		'128k',
+		'--output-format',
+		'json',
+		'--output-format=json',
+		'--mode',
+		'plan',
+		'--mode=plan',
+		'my-prompt',
+	]);
+	t.deepEqual(result, ['my-prompt']);
+});
+
+test('filterCliFlags: returns all args when no flags present', t => {
+	const result = filterCliFlags(['hello', 'world']);
+	t.deepEqual(result, ['hello', 'world']);
+});
+
+test('filterCliFlags: returns empty array for empty input', t => {
+	const result = filterCliFlags([]);
+	t.deepEqual(result, []);
+});

source/cli.tsx

@@ -214,8 +214,6 @@ async function main(): Promise<void> {
 	// Those packages pull ~thousand+ modules; --acp / --plain / auth must stay
 	// on the lightweight path. Ink + App load only in the final TUI branch.
 
-	const {parseReviewCliArgs} = await import('./commands/review-cli');
-
 	const vscodeMode = args.includes('--vscode');
 
 	// Extract VS Code port if specified
@@ -339,56 +337,34 @@ async function main(): Promise<void> {
 	const isRunCommand = runCommandIndex !== -1;
 	const afterRunArgs = isRunCommand ? args.slice(runCommandIndex + 1) : [];
 	if (isRunCommand && afterRunArgs.length > 0) {
-		// Filter out known flags when constructing the prompt
-		const promptArgs: string[] = [];
-		for (let i = 0; i < afterRunArgs.length; i++) {
-			const arg = afterRunArgs[i];
-			if (arg === '--vscode') {
-				continue; // skip this flag
-			} else if (arg === '--vscode-port') {
-				i++; // skip this flag and its value
-				continue;
-			} else if (arg === '--provider') {
-				i++; // skip this flag and its value
-				continue;
-			} else if (arg === '--model') {
-				i++; // skip this flag and its value
-				continue;
-			} else if (arg === '--context-max') {
-				i++; // skip this flag and its value
-				continue;
-			} else if (arg === '--mode') {
-				i++; // skip this flag and its value
-				continue;
-			} else if (arg.startsWith('--mode=')) {
-				continue; // skip fused form
-			} else if (arg === '--json') {
-				continue; // skip this flag
-			} else if (arg === '--output-format') {
-				i++; // skip this flag and its value
-				continue;
-			} else if (arg.startsWith('--output-format=')) {
-				continue; // skip fused form
-			} else if (arg === '--trust-directory') {
-				continue; // skip this flag
-			} else if (arg === '--plain' || arg === '--no-plain') {
-				continue; // skip this flag
-			} else if (arg === '--no-alt-screen' || arg === '--alt-screen') {
-				continue; // skip this flag
-			} else {
-				promptArgs.push(arg);
-			}
-		}
-		nonInteractivePrompt = promptArgs.join(' ');
+		const {filterCliFlags} = await import('@/utils/cli-flags');
+		nonInteractivePrompt = filterCliFlags(afterRunArgs).join(' ');
 	}
 
 	let nonInteractiveMode = isRunCommand;
 
 	// Check for `nanocoder review <target>` — syntactic sugar for
 	// `nanocoder run /review <target>`. The target is the branch or PR number
 	// to review. Flags between `review` and the target are filtered the same
-	// way as `run`.
-	const {isReviewCommand, prompt: reviewPrompt} = parseReviewCliArgs(args);
+	// way as `run`. Lazy-loaded to keep it off the lightweight path.
+	let isReviewCommand = false;
+	let reviewPrompt: string | undefined;
+	if (args[0] === 'review') {
+		const {parseReviewCliArgs} = await import('./commands/review-cli');
+		const result = parseReviewCliArgs(args);
+		isReviewCommand = result.isReviewCommand;
+		reviewPrompt = result.prompt;
+		if (result.error) {
+			console.error(`Error: ${result.error}`);
+			process.exit(1);
+		}
+	}
+
+	if (isRunCommand && isReviewCommand) {
+		console.error('Cannot use both `run` and `review` in the same invocation.');
+		process.exit(1);
+	}
+
 	if (isReviewCommand) {
 		nonInteractivePrompt = reviewPrompt;
 		nonInteractiveMode = true;
@@ -466,6 +442,13 @@ async function main(): Promise<void> {
 		process.exit(1);
 	}
 
+	if (outputFormat === 'json' && isReviewCommand) {
+		console.error(
+			'Error: --json cannot be used with `nanocoder review`. Review output is displayed in the interactive terminal.',
+		);
+		process.exit(1);
+	}
+
 	const ciDetected =
 		process.env.CI === 'true' ||
 		Boolean(