From 6548e5cffca07e1d069debb7f859072f9bc1cf5a Mon Sep 17 00:00:00 2001 From: Garry Tan Date: Wed, 22 Jul 2026 12:05:46 -0700 Subject: [PATCH] fix(autopilot): add --target to value-flag set so install targets survive positional translation Review finding on #3103: --target is installDaemon's value flag (macos | linux-systemd | ephemeral-container | linux-cron). The translator only knew --repo/--interval, so `gbrain autopilot --install --target linux-cron` misread the target value as an unknown positional subcommand and exited 2 before installDaemon ran. Co-Authored-By: Claude Fable 5 --- src/commands/autopilot.ts | 5 ++++- test/autopilot-positional-subcommands.test.ts | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/commands/autopilot.ts b/src/commands/autopilot.ts index 118f97165..58e742d05 100644 --- a/src/commands/autopilot.ts +++ b/src/commands/autopilot.ts @@ -172,7 +172,10 @@ export function shouldSpawnAutopilotWorker(args: string[]): boolean { * the unknown-positional error with the canonical alternatives. * - At most one positional allowed; multiple positionals fail loud. */ -const AUTOPILOT_VALUE_FLAGS = new Set(['--repo', '--interval']); +// Every flag that consumes the NEXT argv token. Missing one here makes the +// translator misread the flag's value as a positional subcommand and exit 2 +// (e.g. `--install --target linux-cron`). Keep in sync with parseArg call sites. +const AUTOPILOT_VALUE_FLAGS = new Set(['--repo', '--interval', '--target']); const AUTOPILOT_POSITIONAL_ALIASES: Record = { status: '--status', install: '--install', diff --git a/test/autopilot-positional-subcommands.test.ts b/test/autopilot-positional-subcommands.test.ts index 9da19dbf7..f8ea8a223 100644 --- a/test/autopilot-positional-subcommands.test.ts +++ b/test/autopilot-positional-subcommands.test.ts @@ -67,6 +67,20 @@ describe('translatePositionalSubcommands — flag/positional interleaving', () = if (r.ok) expect(r.args).toEqual(['--interval', '300', '--install']); }); + test('`--install --target linux-cron` does not mis-classify the target as positional', () => { + // --target is installDaemon's value flag; its value must never be read + // as a positional subcommand (regression guard for the review fix). + const r = translatePositionalSubcommands(['--install', '--target', 'linux-cron']); + expect(r.ok).toBe(true); + if (r.ok) expect(r.args).toEqual(['--install', '--target', 'linux-cron']); + }); + + test('`install --target macos` keeps the alias translation and the target value', () => { + const r = translatePositionalSubcommands(['install', '--target', 'macos']); + expect(r.ok).toBe(true); + if (r.ok) expect(r.args).toEqual(['--install', '--target', 'macos']); + }); + test('value-flag at end of argv with missing value passes through (so parseArg can report it)', () => { const r = translatePositionalSubcommands(['--repo']); expect(r.ok).toBe(true);