fix(browser): accept --window after the leaf subcommand (#1850)

src/cli.test.ts

@@ -1100,6 +1100,41 @@ describe('browser tab targeting commands', () => {
     expect(browserState.page?.snapshot).toHaveBeenCalled();
   });
 
+  it('accepts browser --window in the trailing position after the leaf subcommand (#1850)', async () => {
+    const program = createProgram('', '');
+
+    // The natural agent-facing form: flag after the subcommand. Previously this
+    // failed with `error: unknown option '--window'` because --window was only
+    // declared on the parent `browser` command.
+    await program.parseAsync(['node', 'opencli', 'browser', '--session', 'test', 'state', '--window', 'background']);
+
+    expect(mockBrowserConnect).toHaveBeenCalledWith({ timeout: 30, session: 'test', surface: 'browser', windowMode: 'background' });
+    expect(browserState.page?.snapshot).toHaveBeenCalled();
+  });
+
+  it('registers --window on page leaves but not on session/scaffold leaves (#1850)', () => {
+    const program = createProgram('', '');
+    const browser = program.commands.find((c) => c.name() === 'browser');
+    const hasWindow = (c: ReturnType<typeof createProgram> | undefined) =>
+      !!c?.options.some((o) => o.long === '--window');
+
+    // Page-interaction leaves (route through browserAction → getBrowserWindowMode):
+    // representative across the different registration helpers.
+    expect(hasWindow(browser?.commands.find((c) => c.name() === 'click'))).toBe(true); // addSemanticLocatorOptions
+    expect(hasWindow(browser?.commands.find((c) => c.name() === 'open'))).toBe(true);  // addBrowserTabOption
+    expect(hasWindow(browser?.commands.find((c) => c.name() === 'eval'))).toBe(true);  // neither helper
+    const get = browser?.commands.find((c) => c.name() === 'get');                     // group → nested leaf
+    expect(hasWindow(get?.commands.find((c) => c.name() === 'url'))).toBe(true);
+
+    // Session/scaffold leaves never open a page, so --window would be inert and
+    // misleading in --help — it must NOT be registered there.
+    expect(hasWindow(browser?.commands.find((c) => c.name() === 'init'))).toBe(false);
+    expect(hasWindow(browser?.commands.find((c) => c.name() === 'verify'))).toBe(false);
+
+    // Group nodes themselves carry no --window (only their leaves do).
+    expect(hasWindow(get)).toBe(false);
+  });
+
   it('passes the opt-in AX source to browser state', async () => {
     const program = createProgram('', '');
 

src/cli.ts

@@ -41,6 +41,7 @@ import type { BrowserWindowMode } from './runtime.js';
 
 const CLI_FILE = fileURLToPath(import.meta.url);
 const BROWSER_TAB_OPTION_DESCRIPTION = 'Target tab/page identity returned by "browser open", "browser tab new", or "browser tab list"';
+const BROWSER_WINDOW_OPTION_DESCRIPTION = 'Browser window mode: foreground or background';
 const FOLLOW_POLL_MS = 1_000;
 
 type BrowserNetworkItem = {
@@ -562,6 +563,47 @@ function addBrowserTabOption(command: Command): Command {
   return command.option('--tab <targetId>', BROWSER_TAB_OPTION_DESCRIPTION);
 }
 
+// Browser session commands that never open or attach a page: they manage the
+// session lease or scaffold/verify adapters and so do not route through
+// browserAction()/getBrowserWindowMode(). Registering `--window` on them would be
+// inert and misleading in `--help`. Only skipped at the browser root — `tab close`
+// is a real page leaf under the `tab` group and shares the name `close`.
+const BROWSER_SESSION_COMMANDS = new Set(['bind', 'unbind', 'init', 'verify', 'close']);
+
+/**
+ * Register `--window <mode>` on the browser's page-interaction leaf commands.
+ *
+ * `--window` is declared on the parent `browser` command, but enablePositionalOptions()
+ * means a parent option is only accepted *before* the leaf subcommand token. Agents
+ * naturally write the flag in the trailing position
+ * (`browser <session> open <url> --window background`), which previously failed with
+ * `error: unknown option '--window'` (#1850). Declaring the same option on each page
+ * leaf makes both placements work — getBrowserWindowMode()/getCommandOption() already
+ * walk the parent chain, so wherever the flag binds, the value is read. It also makes
+ * `--window` show up in those leaves' `--help`, matching the parent help text.
+ *
+ * Only true page leaves get the option: group nodes (`get`/`tab`/`dialog`) are recursed
+ * into but never carry `--window` themselves, and the session commands
+ * ({bind,unbind,init,verify,close}) are skipped at the root because they never route
+ * through browserAction()/getBrowserWindowMode() — declaring it there would be inert.
+ */
+function addBrowserWindowOptionRecursively(command: Command, isBrowserRoot: boolean): void {
+  for (const sub of command.commands) {
+    // Skip session commands, but only at the browser root: a leaf named `close`
+    // under the `tab` group (`browser tab close`) is a real page leaf.
+    if (isBrowserRoot && BROWSER_SESSION_COMMANDS.has(sub.name())) continue;
+    if (sub.commands.length > 0) {
+      // Group node (get/tab/dialog): recurse into its leaves but do not add
+      // --window to the group itself.
+      addBrowserWindowOptionRecursively(sub, false);
+      continue;
+    }
+    if (!sub.options.some((opt) => opt.long === '--window')) {
+      sub.option('--window <mode>', BROWSER_WINDOW_OPTION_DESCRIPTION);
+    }
+  }
+}
+
 function getBrowserTargetId(command?: Command): string | undefined {
   if (!command) return undefined;
   const opts = command.optsWithGlobals ? command.optsWithGlobals() : command.opts();
@@ -833,7 +875,7 @@ export function createProgram(BUILTIN_CLIS: string, USER_CLIS: string): Command
     // program.parseAsync callers (tests). User-facing surface is the <session>
     // positional; main.ts argv preprocessor rewrites positional -> --session.
     .addOption(new Option('--session <name>', 'Internal — set automatically from the <session> positional').hideHelp())
-    .option('--window <mode>', 'Browser window mode: foreground or background')
+    .option('--window <mode>', BROWSER_WINDOW_OPTION_DESCRIPTION)
     .description('Browser control — navigate, click, type, extract, wait (no LLM needed)')
     .usage('<session> <command> [options]')
     .addHelpText('after', `
@@ -2913,6 +2955,12 @@ cli({
       console.log('Browser session tab lease released');
     }));
 
+  // All browser subcommands are registered above; mirror the parent `--window`
+  // option onto every page-interaction leaf so it is accepted in the trailing
+  // position too (#1850). Session commands (bind/unbind/init/verify/close) and the
+  // group nodes themselves are skipped — see addBrowserWindowOptionRecursively.
+  addBrowserWindowOptionRecursively(browser, true);
+
   // ── Built-in: doctor / completion ──────────────────────────────────────────
 
   program

src/help.test.ts

@@ -1,5 +1,17 @@
 import { describe, it, expect } from 'vitest';
-import { classifyAdapter, formatRootAdapterHelpText } from './help.js';
+import type { Command } from 'commander';
+import { classifyAdapter, commanderCommandHelpData, formatRootAdapterHelpText } from './help.js';
+import { createProgram } from './cli.js';
+
+function findChild(parent: Command, name: string): Command {
+  const child = parent.commands.find((c) => c.name() === name);
+  if (!child) throw new Error(`No subcommand "${name}" under "${parent.name()}"`);
+  return child;
+}
+
+function optionNames(spec: Record<string, unknown>): string[] {
+  return (spec.command_options as Array<{ name: string }>).map((o) => o.name);
+}
 
 describe('classifyAdapter', () => {
   it('classifies DNS-style domains as site', () => {
@@ -64,3 +76,44 @@ describe('formatRootAdapterHelpText', () => {
     expect(text).toContain("'opencli <site> --help -f yaml'");
   });
 });
+
+describe('commanderCommandHelpData namespace-option dedup (#1850)', () => {
+  it('drops namespace-inherited options (--window/--session) from a browser leaf, keeps its own', () => {
+    const program = createProgram('', '');
+    const browser = findChild(program, 'browser');
+    const leaf = findChild(browser, 'eval');
+
+    // Sanity: --window IS declared on the leaf (so it parses in the trailing
+    // position) and on the namespace root, so the dedup has something to remove.
+    expect(leaf.options.some((o) => o.long === '--window')).toBe(true);
+    expect(browser.options.some((o) => o.long === '--window')).toBe(true);
+
+    const data = commanderCommandHelpData(browser, leaf, { globalCommand: program });
+    const names = optionNames(data);
+
+    // Namespace-inherited options must not be repeated in the leaf's own list.
+    expect(names).not.toContain('window');
+    expect(names).not.toContain('session');
+    // The leaf's own option survives.
+    expect(names).toContain('frame');
+    // It is still surfaced once at the namespace level.
+    const namespaceOptionNames = (data.namespace_options as Array<{ name: string }>).map((o) => o.name);
+    expect(namespaceOptionNames).toContain('window');
+  });
+
+  it('leaves a non-browser namespace leaf unchanged (dedup only removes inherited opts)', () => {
+    const program = createProgram('', '');
+    const auth = findChild(program, 'auth');
+    const leaf = findChild(auth, 'status');
+
+    // The auth root declares no options of its own, so nothing is deduped: the
+    // leaf's full own-option set is preserved (modulo hidden options, which the
+    // help compactor always omits).
+    const data = commanderCommandHelpData(auth, leaf, { globalCommand: program });
+    const names = optionNames(data);
+    const visibleOwnNames = leaf.options.filter((o) => !o.hidden).map((o) => o.attributeName());
+    expect(names).toEqual(visibleOwnNames);
+    expect(names).toContain('site');
+    expect(names).toContain('format');
+  });
+});

src/help.ts

@@ -323,14 +323,23 @@ function compactCommanderCommand(
   opts: { globalCommand?: Command } = {},
 ): Record<string, unknown> {
   const relativePath = commandPathFromRoot(namespaceRoot, command);
+  // Dedup: remove options inherited from the namespace root (e.g. browser's
+  // `--window`) from this leaf's own `command_options`. They are surfaced once
+  // under `namespace_options`, but are also redeclared on each page leaf so they
+  // parse in the trailing position (#1850); without this filter the same flag
+  // would appear twice — once at the namespace level and once per leaf.
+  const namespaceOptionLongs = new Set(
+    namespaceRoot.options.map(opt => opt.long).filter((long): long is string => Boolean(long)),
+  );
+  const ownOptions = command.options.filter(opt => !opt.long || !namespaceOptionLongs.has(opt.long));
   return {
     name: relativePath.join(' '),
     command: commanderPath(command).join(' '),
     usage: formatCommanderUsage(command, { namespaceRoot, globalCommand: opts.globalCommand }),
     description: command.description(),
     ...(command.aliases().length ? { aliases: command.aliases() } : {}),
     positionals: command.registeredArguments.map(compactCommanderArgument),
-    command_options: compactCommanderOptions(command.options),
+    command_options: compactCommanderOptions(ownOptions),
   };
 }