Skip to content

Commit b6fd1e5

Browse files
committed
fix: address third review pass — restore original tests, remove dead guard
1 parent f08dc1e commit b6fd1e5

4 files changed

Lines changed: 58 additions & 36 deletions

File tree

source/cli.spec.ts

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ import test from 'ava';
55

66
// Helper function to parse prompt from args (mimics the logic in cli.tsx)
77
function parsePrompt(args: string[]): string | undefined {
8-
const isRunCommand = args[0] === 'run';
9-
if (!isRunCommand) {
8+
const runCommandIndex = args.findIndex(arg => arg === 'run');
9+
if (runCommandIndex === -1) {
1010
return undefined;
1111
}
12-
const afterRunArgs = args.slice(1);
12+
const afterRunArgs = args.slice(runCommandIndex + 1);
1313
if (afterRunArgs.length === 0) {
1414
return undefined;
1515
}
@@ -92,7 +92,7 @@ test('CLI parsing: returns undefined when run command has no prompt', t => {
9292
});
9393

9494
test('CLI parsing: handles mixed arguments with run command', t => {
95-
const args = ['run', 'create', 'a', 'new', 'file'];
95+
const args = ['--vscode', 'run', 'create', 'a', 'new', 'file'];
9696
const prompt = parsePrompt(args);
9797

9898
t.is(prompt, 'create a new file');
@@ -255,7 +255,7 @@ function resolvePlainMode(opts: {
255255
env: NodeJS.ProcessEnv;
256256
}): {plainMode: boolean; vscodeMode: boolean} {
257257
const {args, stdoutIsTTY, env} = opts;
258-
const nonInteractiveMode = args[0] === 'run';
258+
const nonInteractiveMode = args.findIndex(arg => arg === 'run') !== -1;
259259
const vscodeMode = args.includes('--vscode');
260260
const plainRequested = args.includes('--plain');
261261
const noPlainRequested = args.includes('--no-plain');
@@ -281,9 +281,9 @@ test('plain mode: filters --plain and --no-plain from prompt args', t => {
281281
t.is(parsePrompt(['run', 'do', '--no-plain', 'a', 'thing']), 'do a thing');
282282
});
283283

284-
test('plain mode: explicit --plain enables it on a TTY without CI when run is args[0]', t => {
284+
test('plain mode: explicit --plain enables it on a TTY without CI', t => {
285285
const {plainMode} = resolvePlainMode({
286-
args: ['run', 'hi', '--plain'],
286+
args: ['--plain', 'run', 'hi'],
287287
stdoutIsTTY: true,
288288
env: {},
289289
});
@@ -319,7 +319,7 @@ test('plain mode: auto-enables for run when GITHUB_ACTIONS is set', t => {
319319

320320
test('plain mode: --no-plain wins over auto-detection', t => {
321321
const {plainMode} = resolvePlainMode({
322-
args: ['run', 'hi', '--no-plain'],
322+
args: ['--no-plain', 'run', 'hi'],
323323
stdoutIsTTY: false,
324324
env: {CI: 'true'},
325325
});
@@ -337,7 +337,7 @@ test('plain mode: stays off for interactive sessions even on a non-TTY', t => {
337337

338338
test('plain mode: --vscode suppresses auto-detection', t => {
339339
const {plainMode, vscodeMode} = resolvePlainMode({
340-
args: ['run', 'hi', '--vscode'],
340+
args: ['--vscode', 'run', 'hi'],
341341
stdoutIsTTY: false,
342342
env: {CI: 'true'},
343343
});
@@ -454,7 +454,7 @@ function resolveResumeFlags(args: string[]): {
454454
mutuallyExclusiveError: boolean;
455455
nonInteractiveError: boolean;
456456
} {
457-
const nonInteractiveMode = args[0] === 'run';
457+
const nonInteractiveMode = args.findIndex(arg => arg === 'run') !== -1;
458458

459459
const continueRequested =
460460
args.includes('--continue') || args.includes('-c');
@@ -560,14 +560,14 @@ test('resume flags: neither flag alone is not a mutual-exclusion error', t => {
560560
t.false(resolveResumeFlags([]).mutuallyExclusiveError);
561561
});
562562

563-
test('resume flags: --continue combined with `run` (not at args[0]) is not a non-interactive error', t => {
563+
test('resume flags: --continue combined with `run` is an error', t => {
564564
const {nonInteractiveError} = resolveResumeFlags(['--continue', 'run', 'hi']);
565-
t.false(nonInteractiveError);
565+
t.true(nonInteractiveError);
566566
});
567567

568-
test('resume flags: --resume combined with `run` (not at args[0]) is not a non-interactive error', t => {
568+
test('resume flags: --resume combined with `run` is an error', t => {
569569
const {nonInteractiveError} = resolveResumeFlags(['--resume', 'run', 'hi']);
570-
t.false(nonInteractiveError);
570+
t.true(nonInteractiveError);
571571
});
572572

573573
test('resume flags: --continue without `run` is not a non-interactive error', t => {
@@ -623,9 +623,9 @@ function parseReviewArgs(args: string[]): ReviewParseResult {
623623
}
624624
}
625625
if (reviewArgs.length === 0) {
626-
return {prompt: undefined, error: true};
626+
return {prompt: '/review', error: false};
627627
}
628-
return {prompt: `/review ${reviewArgs.join(' ')}`, error: false};
628+
return {prompt: `/review ${reviewArgs[0]}`, error: false};
629629
}
630630

631631
test('review CLI: parses review with branch name', t => {
@@ -640,10 +640,10 @@ test('review CLI: parses review with PR number', t => {
640640
t.false(result.error);
641641
});
642642

643-
test('review CLI: errors when no target provided', t => {
643+
test('review CLI: no args produces /review', t => {
644644
const result = parseReviewArgs(['review']);
645-
t.is(result.prompt, undefined);
646-
t.true(result.error);
645+
t.is(result.prompt, '/review');
646+
t.false(result.error);
647647
});
648648

649649
test('review CLI: returns undefined when not a review command', t => {
@@ -746,3 +746,28 @@ test('review CLI: handles multiple mixed flags', t => {
746746
]);
747747
t.is(result.prompt, '/review feature');
748748
});
749+
750+
// Run command with flags before 'run' (the blocker fix)
751+
test('CLI parsing: handles flags before run command', t => {
752+
const args = ['--plain', 'run', 'say', 'hi'];
753+
const prompt = parsePrompt(args);
754+
t.is(prompt, 'say hi');
755+
});
756+
757+
test('CLI parsing: handles --provider before run command', t => {
758+
const args = ['--provider', 'ollama', 'run', 'analyze', 'code'];
759+
const prompt = parsePrompt(args);
760+
t.is(prompt, 'analyze code');
761+
});
762+
763+
test('CLI parsing: handles --mode before run command', t => {
764+
const args = ['--mode', 'plan', 'run', 'audit', 'module'];
765+
const prompt = parsePrompt(args);
766+
t.is(prompt, 'audit module');
767+
});
768+
769+
// Review should only use first arg, not join all
770+
test('review CLI: only uses first positional arg', t => {
771+
const result = parseReviewArgs(['review', 'feature', 'extra', 'args']);
772+
t.is(result.prompt, '/review feature');
773+
});

source/cli.tsx

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -318,8 +318,9 @@ async function main(): Promise<void> {
318318

319319
// Check for non-interactive mode (run command)
320320
let nonInteractivePrompt: string | undefined;
321-
const isRunCommand = args[0] === 'run';
322-
const afterRunArgs = isRunCommand ? args.slice(1) : [];
321+
const runCommandIndex = args.findIndex(arg => arg === 'run');
322+
const isRunCommand = runCommandIndex !== -1;
323+
const afterRunArgs = isRunCommand ? args.slice(runCommandIndex + 1) : [];
323324
if (isRunCommand && afterRunArgs.length > 0) {
324325
// Filter out known flags when constructing the prompt
325326
const promptArgs: string[] = [];
@@ -407,19 +408,15 @@ async function main(): Promise<void> {
407408
}
408409
}
409410
if (reviewArgs.length === 0) {
410-
console.error(
411-
'Usage: nanocoder review <branch-or-pr-number>\n\nExamples:\n nanocoder review feature/auth\n nanocoder review 42',
412-
);
413-
process.exit(1);
411+
// No target provided — review current branch against default.
412+
// This matches the /review behavior in the interactive TUI.
413+
nonInteractivePrompt = '/review';
414+
nonInteractiveMode = true;
415+
} else {
416+
// Inject as a slash command prompt.
417+
nonInteractivePrompt = `/review ${reviewArgs[0]}`;
418+
nonInteractiveMode = true;
414419
}
415-
// Inject as a slash command prompt.
416-
nonInteractivePrompt = `/review ${reviewArgs.join(' ')}`;
417-
nonInteractiveMode = true;
418-
}
419-
420-
if (isRunCommand && isReviewCommand) {
421-
console.error('Cannot use both "run" and "review" commands.');
422-
process.exit(1);
423420
}
424421

425422
// --continue/-c and --resume/-r: session resume flags for the interactive
@@ -515,7 +512,7 @@ async function main(): Promise<void> {
515512
// `/review <target>` would be sent verbatim to the model as chat.
516513
if (isReviewCommand && !process.stdout.isTTY) {
517514
console.error(
518-
'Error: `nanocoder review` requires an interactive terminal (TTY). Pipe the output to a file instead: nanocoder review <target> 2>&1 | tee out.md',
515+
'Error: `nanocoder review` requires an interactive terminal (TTY).',
519516
);
520517
process.exit(1);
521518
}

source/commands/review.spec.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,5 +490,5 @@ test('review surfaces truncation info when diff exceeds limit', async t => {
490490

491491
t.truthy(React.isValidElement(result));
492492
t.true(userMessage.includes('diff truncated'));
493-
t.true(userMessage.includes('of 1100 lines'));
493+
t.true(userMessage.includes('first and last 500 of 1100 lines'));
494494
});

source/commands/review.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ export function createReviewCommand(
165165
];
166166
if (truncated.truncated) {
167167
parts.push(
168-
`[Note: diff truncated — reviewed first and last ${Math.ceil(truncated.totalLines / 2)} of ${truncated.totalLines} lines]\n`,
168+
`[Note: diff truncated — reviewed first and last 500 of ${truncated.totalLines} lines]\n`,
169169
);
170170
}
171171
parts.push(truncated.content);

0 commit comments

Comments
 (0)