diff --git a/README.md b/README.md index 63947638..d626d812 100644 --- a/README.md +++ b/README.md @@ -172,11 +172,15 @@ CCS supports structural Droid command passthrough after profile selection: ```bash ccsd codex exec --skip-permissions-unsafe "fix failing tests" ccsd codex --skip-permissions-unsafe "fix failing tests" # auto-routed to: droid exec ... +ccsd codex -m custom:gpt-5.3-codex "fix failing tests" # short exec flags auto-routed too ``` If you pass exec-only flags without a prompt (for example `--skip-permissions-unsafe`), Droid `exec` will return its native "No prompt provided" usage guidance. +If multiple reasoning flags are provided in Droid exec mode, CCS keeps the first +flag and warns about duplicates. + Dashboard parity: `ccs config` -> `Factory Droid` ### Per-Profile Target Defaults diff --git a/src/ccs.ts b/src/ccs.ts index 654c5267..0d735973 100644 --- a/src/ccs.ts +++ b/src/ccs.ts @@ -62,7 +62,7 @@ import { DroidReasoningFlagError, resolveDroidReasoningRuntime, } from './targets/droid-reasoning-runtime'; -import { routeDroidCommandArgs } from './targets/droid-command-router'; +import { DroidCommandRouterError, routeDroidCommandArgs } from './targets/droid-command-router'; // Version and Update check utilities import { getVersion } from './utils/version'; @@ -719,11 +719,11 @@ async function main(): Promise { let targetRemainingArgs = remainingArgs; let droidReasoningOverride: string | number | undefined; if (resolvedTarget === 'droid') { - const droidRoute = routeDroidCommandArgs(remainingArgs); - targetRemainingArgs = droidRoute.argsForDroid; + try { + const droidRoute = routeDroidCommandArgs(remainingArgs); + targetRemainingArgs = droidRoute.argsForDroid; - if (droidRoute.mode === 'interactive') { - try { + if (droidRoute.mode === 'interactive') { const runtime = resolveDroidReasoningRuntime(remainingArgs, process.env.CCS_THINKING); targetRemainingArgs = runtime.argsWithoutReasoningFlags; droidReasoningOverride = runtime.reasoningOverride; @@ -735,19 +735,29 @@ async function main(): Promise { ) ); } - } catch (error) { - if (error instanceof DroidReasoningFlagError) { - console.error(fail(error.message)); - console.error(' Examples: --thinking low, --thinking 8192, --thinking off'); - console.error(' Codex alias: --effort medium|high|xhigh'); - process.exit(1); + } else { + if (droidRoute.duplicateReasoningDisplays.length > 0) { + console.error( + warn( + `[!] Multiple reasoning flags detected. Using first occurrence: ${droidRoute.reasoningSourceDisplay || ''}` + ) + ); + } + if (droidRoute.autoPrependedExec && process.stdout.isTTY) { + console.error( + info('Detected Droid exec-only flags. Routing as: droid exec [prompt]') + ); } - throw error; } - } else if (droidRoute.autoPrependedExec && process.stdout.isTTY) { - console.error( - info('Detected Droid exec-only flags. Routing as: droid exec [prompt]') - ); + } catch (error) { + if (error instanceof DroidReasoningFlagError || error instanceof DroidCommandRouterError) { + console.error(fail(error.message)); + console.error(' Examples: --thinking low, --thinking 8192, --thinking off'); + console.error(' Codex alias: --effort medium|high|xhigh'); + console.error(' Droid exec: --reasoning-effort high'); + process.exit(1); + } + throw error; } } diff --git a/src/commands/help-command.ts b/src/commands/help-command.ts index 919525a5..b7b8ed0e 100644 --- a/src/commands/help-command.ts +++ b/src/commands/help-command.ts @@ -342,6 +342,7 @@ Run ${color('ccs config', 'command')} for web dashboard`.trim(); 'ccsd codex exec --skip-permissions-unsafe "fix failing tests"', 'Pass through Droid exec mode', ], + ['ccsd codex -m custom:gpt-5.3-codex "fix failing tests"', 'Auto-routes short exec flags'], [ 'ccsd codex --skip-permissions-unsafe "fix failing tests"', 'Auto-routes to Droid exec when exec-only flags are detected', @@ -404,6 +405,7 @@ Run ${color('ccs config', 'command')} for web dashboard`.trim(); ['Droid exec:', 'Use native Droid flag: --reasoning-effort '], ['', 'CCS auto-maps --thinking/--effort to --reasoning-effort in droid exec mode.'], ['', 'For interactive droid sessions, CCS applies reasoning via Droid BYOK model config.'], + ['', 'When multiple reasoning flags are provided, the first flag wins.'], ['', ''], ['Note:', 'Extended thinking allocates compute for step-by-step reasoning'], ['', 'before responding.'], diff --git a/src/targets/droid-command-router.ts b/src/targets/droid-command-router.ts index 74d6962c..b5c6d7fd 100644 --- a/src/targets/droid-command-router.ts +++ b/src/targets/droid-command-router.ts @@ -15,6 +15,20 @@ export interface DroidCommandRoute { argsForDroid: string[]; command?: string; autoPrependedExec: boolean; + reasoningSourceDisplay?: string; + duplicateReasoningDisplays: string[]; +} + +type DroidReasoningFlag = '--reasoning-effort' | '-r' | '--effort' | '--thinking'; + +export class DroidCommandRouterError extends Error { + constructor( + message: string, + public readonly flag: DroidReasoningFlag + ) { + super(message); + this.name = 'DroidCommandRouterError'; + } } const DROID_SUBCOMMANDS = new Set([ @@ -48,6 +62,19 @@ const DROID_EXEC_ONLY_LONG_FLAGS = new Set([ '--list-tools', ]); +const DROID_EXEC_ONLY_SHORT_FLAGS = new Set(['-o', '-f', '-s', '-m']); +const DROID_REASONING_EFFORT_VALUES = new Set([ + 'none', + 'off', + 'minimal', + 'low', + 'medium', + 'high', + 'max', + 'xhigh', + 'auto', +]); + function getLongFlagToken(arg: string): string { const eqIndex = arg.indexOf('='); return eqIndex >= 0 ? arg.slice(0, eqIndex) : arg; @@ -57,6 +84,11 @@ function isExplicitSubcommand(arg: string | undefined): boolean { return !!arg && DROID_SUBCOMMANDS.has(arg); } +function isLikelyReasoningEffortValue(value: string | undefined): boolean { + if (!value || value.startsWith('-')) return false; + return DROID_REASONING_EFFORT_VALUES.has(value.toLowerCase()); +} + function hasExecOnlyFlagsAtFront(args: string[]): boolean { for (let i = 0; i < args.length; i++) { const arg = args[i]; @@ -75,7 +107,19 @@ function hasExecOnlyFlagsAtFront(args: string[]): boolean { } if (!arg.startsWith('-')) return false; - if (!arg.startsWith('--')) continue; // short flags are ambiguous at root (`-r` is resume) + if (!arg.startsWith('--')) { + // Short flags: + // - `-r` is ambiguous (root resume vs exec reasoning-effort), so only route + // when value looks like a reasoning effort level. + if (DROID_EXEC_ONLY_SHORT_FLAGS.has(arg)) { + return true; + } + if (arg === '-r') { + const value = args[i + 1]; + return isLikelyReasoningEffortValue(value); + } + continue; + } const flagToken = getLongFlagToken(arg); if (DROID_EXEC_ONLY_LONG_FLAGS.has(flagToken)) { @@ -86,38 +130,112 @@ function hasExecOnlyFlagsAtFront(args: string[]): boolean { return false; } -function normalizeLegacyReasoningAliasesForExec(args: string[]): string[] { - const normalized: string[] = []; +interface ExecReasoningNormalizationResult { + args: string[]; + sourceDisplay?: string; + duplicateDisplays: string[]; +} - for (let i = 0; i < args.length; i++) { +function normalizeExecReasoningFlags(args: string[]): ExecReasoningNormalizationResult { + const normalized: string[] = []; + const duplicateDisplays: string[] = []; + let sourceDisplay: string | undefined; + let hasReasoning = false; + + const applyReasoning = (value: string, display: string): void => { + if (!hasReasoning) { + normalized.push('--reasoning-effort', value); + hasReasoning = true; + sourceDisplay = display; + return; + } + + duplicateDisplays.push(display); + }; + + const handleMissingValue = ( + flag: DroidReasoningFlag, + missingDisplay: string + ): never | undefined => { + if (!hasReasoning) { + throw new DroidCommandRouterError(`${flag} requires a value`, flag); + } + + duplicateDisplays.push(missingDisplay); + return undefined; + }; + + // Preserve leading command token for explicit auto-prepended command mode. + const startsWithExec = args[0] === 'exec'; + let startIndex = 0; + if (startsWithExec) { + normalized.push('exec'); + startIndex = 1; + } + + for (let i = startIndex; i < args.length; i++) { const arg = args[i]; - if (arg === '--effort' || arg === '--thinking') { + if (arg === '--') { + normalized.push(...args.slice(i)); + break; + } + + if ( + arg === '--reasoning-effort' || + arg === '--effort' || + arg === '--thinking' || + arg === '-r' + ) { const value = args[i + 1]; - if (value && !value.startsWith('-')) { - normalized.push('--reasoning-effort', value); - i += 1; - } else { - // Keep invalid/missing-value form so Droid can surface native validation. - normalized.push(arg); + if (!value || value.startsWith('-')) { + handleMissingValue(arg as DroidReasoningFlag, `${arg} `); + continue; } + + applyReasoning(value, `${arg} ${value}`); + i += 1; + continue; + } + + if (arg.startsWith('--reasoning-effort=')) { + const value = arg.slice('--reasoning-effort='.length); + if (!value) { + handleMissingValue('--reasoning-effort', '--reasoning-effort='); + continue; + } + applyReasoning(value, `--reasoning-effort=${value}`); continue; } if (arg.startsWith('--effort=')) { - normalized.push(`--reasoning-effort=${arg.slice('--effort='.length)}`); + const value = arg.slice('--effort='.length); + if (!value) { + handleMissingValue('--effort', '--effort='); + continue; + } + applyReasoning(value, `--effort=${value}`); continue; } if (arg.startsWith('--thinking=')) { - normalized.push(`--reasoning-effort=${arg.slice('--thinking='.length)}`); + const value = arg.slice('--thinking='.length); + if (!value) { + handleMissingValue('--thinking', '--thinking='); + continue; + } + applyReasoning(value, `--thinking=${value}`); continue; } normalized.push(arg); } - return normalized; + return { + args: normalized, + sourceDisplay, + duplicateDisplays, + }; } export function routeDroidCommandArgs(args: string[]): DroidCommandRoute { @@ -126,28 +244,39 @@ export function routeDroidCommandArgs(args: string[]): DroidCommandRoute { mode: 'interactive', argsForDroid: [], autoPrependedExec: false, + duplicateReasoningDisplays: [], }; } if (isExplicitSubcommand(args[0])) { const command = args[0]; - const argsForDroid = - command === 'exec' ? normalizeLegacyReasoningAliasesForExec(args) : [...args]; + const normalized = + command === 'exec' + ? normalizeExecReasoningFlags(args) + : { + args: [...args], + duplicateDisplays: [], + }; return { mode: 'command', command, - argsForDroid, + argsForDroid: normalized.args, autoPrependedExec: false, + reasoningSourceDisplay: normalized.sourceDisplay, + duplicateReasoningDisplays: normalized.duplicateDisplays, }; } if (hasExecOnlyFlagsAtFront(args)) { const argsWithExec = ['exec', ...args]; + const normalized = normalizeExecReasoningFlags(argsWithExec); return { mode: 'command', command: 'exec', - argsForDroid: normalizeLegacyReasoningAliasesForExec(argsWithExec), + argsForDroid: normalized.args, autoPrependedExec: true, + reasoningSourceDisplay: normalized.sourceDisplay, + duplicateReasoningDisplays: normalized.duplicateDisplays, }; } @@ -155,5 +284,6 @@ export function routeDroidCommandArgs(args: string[]): DroidCommandRoute { mode: 'interactive', argsForDroid: [...args], autoPrependedExec: false, + duplicateReasoningDisplays: [], }; } diff --git a/tests/unit/targets/droid-command-router.test.ts b/tests/unit/targets/droid-command-router.test.ts index 0910e102..890e8a01 100644 --- a/tests/unit/targets/droid-command-router.test.ts +++ b/tests/unit/targets/droid-command-router.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'bun:test'; -import { routeDroidCommandArgs } from '../../../src/targets/droid-command-router'; +import { + DroidCommandRouterError, + routeDroidCommandArgs, +} from '../../../src/targets/droid-command-router'; describe('droid-command-router', () => { it('keeps interactive mode for plain profile launches', () => { @@ -8,6 +11,7 @@ describe('droid-command-router', () => { expect(route.mode).toBe('interactive'); expect(route.argsForDroid).toEqual([]); expect(route.autoPrependedExec).toBe(false); + expect(route.duplicateReasoningDisplays).toEqual([]); }); it('keeps explicit droid subcommands untouched', () => { @@ -17,6 +21,7 @@ describe('droid-command-router', () => { expect(route.command).toBe('mcp'); expect(route.argsForDroid).toEqual(['mcp', '--help']); expect(route.autoPrependedExec).toBe(false); + expect(route.duplicateReasoningDisplays).toEqual([]); }); it('auto-prepends exec for exec-only flags provided after profile', () => { @@ -26,6 +31,7 @@ describe('droid-command-router', () => { expect(route.command).toBe('exec'); expect(route.argsForDroid).toEqual(['exec', '--skip-permissions-unsafe']); expect(route.autoPrependedExec).toBe(true); + expect(route.duplicateReasoningDisplays).toEqual([]); }); it('does not auto-prepend exec for root help flag', () => { @@ -34,6 +40,7 @@ describe('droid-command-router', () => { expect(route.mode).toBe('interactive'); expect(route.argsForDroid).toEqual(['--help']); expect(route.autoPrependedExec).toBe(false); + expect(route.duplicateReasoningDisplays).toEqual([]); }); it('normalizes --effort alias to --reasoning-effort for explicit exec', () => { @@ -41,12 +48,8 @@ describe('droid-command-router', () => { expect(route.mode).toBe('command'); expect(route.command).toBe('exec'); - expect(route.argsForDroid).toEqual([ - 'exec', - '--reasoning-effort', - 'xhigh', - 'fix test flake', - ]); + expect(route.argsForDroid).toEqual(['exec', '--reasoning-effort', 'xhigh', 'fix test flake']); + expect(route.reasoningSourceDisplay).toBe('--effort xhigh'); }); it('normalizes --thinking alias when exec is auto-prepended', () => { @@ -58,10 +61,12 @@ describe('droid-command-router', () => { 'exec', '--auto', 'high', - '--reasoning-effort=medium', + '--reasoning-effort', + 'medium', 'summarize logs', ]); expect(route.autoPrependedExec).toBe(true); + expect(route.reasoningSourceDisplay).toBe('--thinking=medium'); }); it('still auto-prepends exec when --effort appears before exec-only flags', () => { @@ -82,5 +87,80 @@ describe('droid-command-router', () => { 'fix flaky test', ]); expect(route.autoPrependedExec).toBe(true); + expect(route.reasoningSourceDisplay).toBe('--effort xhigh'); + }); + + it('auto-prepends exec for non-ambiguous short exec flags', () => { + const route = routeDroidCommandArgs(['-m', 'custom:gpt-5.3-codex', 'fix flaky test']); + + expect(route.mode).toBe('command'); + expect(route.command).toBe('exec'); + expect(route.argsForDroid).toEqual(['exec', '-m', 'custom:gpt-5.3-codex', 'fix flaky test']); + expect(route.autoPrependedExec).toBe(true); + }); + + it('routes -r to exec when value matches reasoning effort level', () => { + const route = routeDroidCommandArgs(['-r', 'high', 'summarize logs']); + + expect(route.mode).toBe('command'); + expect(route.command).toBe('exec'); + expect(route.argsForDroid).toEqual(['exec', '--reasoning-effort', 'high', 'summarize logs']); + expect(route.autoPrependedExec).toBe(true); + }); + + it('keeps interactive mode for ambiguous -r resume-style usage', () => { + const route = routeDroidCommandArgs(['-r', 'session-1234']); + + expect(route.mode).toBe('interactive'); + expect(route.argsForDroid).toEqual(['-r', 'session-1234']); + expect(route.autoPrependedExec).toBe(false); + }); + + it('dedupes mixed reasoning flags with first occurrence precedence', () => { + const route = routeDroidCommandArgs([ + 'exec', + '--reasoning-effort', + 'high', + '--thinking', + 'low', + '--effort=xhigh', + 'summarize logs', + ]); + + expect(route.mode).toBe('command'); + expect(route.command).toBe('exec'); + expect(route.argsForDroid).toEqual(['exec', '--reasoning-effort', 'high', 'summarize logs']); + expect(route.reasoningSourceDisplay).toBe('--reasoning-effort high'); + expect(route.duplicateReasoningDisplays).toEqual(['--thinking low', '--effort=xhigh']); + }); + + it('throws for missing reasoning value in command mode (alias)', () => { + expect(() => routeDroidCommandArgs(['exec', '--effort'])).toThrow(DroidCommandRouterError); + }); + + it('throws for missing reasoning value in command mode (native)', () => { + expect(() => routeDroidCommandArgs(['exec', '--reasoning-effort'])).toThrow( + DroidCommandRouterError + ); + }); + + it('records malformed duplicate reasoning flags when first value is already selected', () => { + const route = routeDroidCommandArgs([ + 'exec', + '--thinking', + 'medium', + '--reasoning-effort', + '--skip-permissions-unsafe', + 'summarize logs', + ]); + + expect(route.argsForDroid).toEqual([ + 'exec', + '--reasoning-effort', + 'medium', + '--skip-permissions-unsafe', + 'summarize logs', + ]); + expect(route.duplicateReasoningDisplays).toEqual(['--reasoning-effort ']); }); }); diff --git a/tests/unit/targets/droid-command-routing-integration.test.ts b/tests/unit/targets/droid-command-routing-integration.test.ts new file mode 100644 index 00000000..a0844d22 --- /dev/null +++ b/tests/unit/targets/droid-command-routing-integration.test.ts @@ -0,0 +1,173 @@ +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { spawnSync } from 'child_process'; +import * as fs from 'fs'; +import * as os from 'os'; +import * as path from 'path'; + +interface RunResult { + status: number | null; + stdout: string; + stderr: string; +} + +function runCcs(args: string[], env: NodeJS.ProcessEnv): RunResult { + const ccsEntry = path.join(process.cwd(), 'src', 'ccs.ts'); + const result = spawnSync(process.execPath, [ccsEntry, ...args], { + encoding: 'utf8', + env, + timeout: 20000, + }); + + return { + status: result.status, + stdout: result.stdout || '', + stderr: result.stderr || '', + }; +} + +describe('droid command routing integration', () => { + let tmpHome: string; + let ccsDir: string; + let settingsPath: string; + let configPath: string; + let fakeDroidPath: string; + let droidArgsLogPath: string; + let baseEnv: NodeJS.ProcessEnv; + + beforeEach(() => { + if (process.platform === 'win32') { + return; + } + + tmpHome = fs.mkdtempSync(path.join(os.tmpdir(), 'ccs-droid-route-it-')); + ccsDir = path.join(tmpHome, '.ccs'); + fs.mkdirSync(ccsDir, { recursive: true }); + + settingsPath = path.join(ccsDir, 'myglm.settings.json'); + configPath = path.join(ccsDir, 'config.json'); + fakeDroidPath = path.join(tmpHome, 'fake-droid.js'); + droidArgsLogPath = path.join(tmpHome, 'droid-args.json'); + + fs.writeFileSync( + settingsPath, + JSON.stringify( + { + env: { + ANTHROPIC_BASE_URL: 'https://example.invalid/anthropic', + ANTHROPIC_AUTH_TOKEN: 'test-token', + ANTHROPIC_MODEL: 'gpt-5.3-codex', + CCS_DROID_PROVIDER: 'openai', + }, + }, + null, + 2 + ) + ); + + fs.writeFileSync( + configPath, + JSON.stringify( + { + profiles: { + myglm: settingsPath, + }, + }, + null, + 2 + ) + ); + + fs.writeFileSync( + fakeDroidPath, + `#!/usr/bin/env node +const fs = require('fs'); +const out = process.env.CCS_TEST_DROID_ARGS_OUT; +if (!out) process.exit(2); +fs.writeFileSync(out, JSON.stringify(process.argv.slice(2))); +process.exit(0); +`, + { encoding: 'utf8', mode: 0o755 } + ); + fs.chmodSync(fakeDroidPath, 0o755); + + baseEnv = { + ...process.env, + CI: '1', + NO_COLOR: '1', + CCS_HOME: tmpHome, + CCS_DROID_PATH: fakeDroidPath, + CCS_TEST_DROID_ARGS_OUT: droidArgsLogPath, + }; + }); + + afterEach(() => { + if (process.platform === 'win32') { + return; + } + + fs.rmSync(tmpHome, { recursive: true, force: true }); + }); + + it('auto-routes exec-only long flags to droid exec from main ccs flow', () => { + if (process.platform === 'win32') return; + + const result = runCcs( + ['myglm', '--target', 'droid', '--skip-permissions-unsafe', 'fix failing tests'], + baseEnv + ); + + expect(result.status).toBe(0); + const routedArgs = JSON.parse(fs.readFileSync(droidArgsLogPath, 'utf8')) as string[]; + expect(routedArgs).toEqual(['exec', '--skip-permissions-unsafe', 'fix failing tests']); + }); + + it('auto-routes non-ambiguous short exec flags', () => { + if (process.platform === 'win32') return; + + const result = runCcs( + ['myglm', '--target', 'droid', '-m', 'custom:gpt-5.3-codex', 'fix failing tests'], + baseEnv + ); + + expect(result.status).toBe(0); + const routedArgs = JSON.parse(fs.readFileSync(droidArgsLogPath, 'utf8')) as string[]; + expect(routedArgs).toEqual(['exec', '-m', 'custom:gpt-5.3-codex', 'fix failing tests']); + }); + + it('dedupes reasoning flags with first occurrence precedence in exec mode', () => { + if (process.platform === 'win32') return; + + const result = runCcs( + [ + 'myglm', + '--target', + 'droid', + 'exec', + '--reasoning-effort', + 'high', + '--thinking', + 'low', + 'summarize logs', + ], + baseEnv + ); + + expect(result.status).toBe(0); + expect(result.stderr).toContain('Multiple reasoning flags detected'); + const routedArgs = JSON.parse(fs.readFileSync(droidArgsLogPath, 'utf8')) as string[]; + expect(routedArgs).toEqual(['exec', '--reasoning-effort', 'high', 'summarize logs']); + }); + + it('fails fast for malformed reasoning alias in command mode', () => { + if (process.platform === 'win32') return; + + const result = runCcs(['myglm', '--target', 'droid', 'exec', '--effort'], baseEnv); + + expect(result.status).toBe(1); + expect(result.stderr).toContain('--effort requires a value'); + expect(fs.existsSync(droidArgsLogPath)).toBe(true); + const probeArgs = JSON.parse(fs.readFileSync(droidArgsLogPath, 'utf8')) as string[]; + // Droid binary is still invoked once for version preflight (`--version`) before routing. + expect(probeArgs).toEqual(['--version']); + }); +});