From 3113483ab3679d98ae4e82e22c14a12936acfdd6 Mon Sep 17 00:00:00 2001 From: jackwener Date: Wed, 17 Jun 2026 00:46:58 +0800 Subject: [PATCH] fix(cli): accept trailing browser window option Hoist user-facing browser --window placements after a leaf command into the browser namespace option slot before Commander parses argv. This keeps --window as a namespace option instead of mirroring it onto every browser leaf, while accepting the natural command shape agents tend to produce.\n\nAlso document the trailing form in browser help and cover nested browser groups plus literal -- separators.\n\nCo-authored-by: Zhongyue Lin \nCo-authored-by: Chen17-sq <109075336+Chen17-sq@users.noreply.github.com> --- src/cli-argv-preprocess.test.ts | 102 ++++++++++++++++++++++++++++++++ src/cli-argv-preprocess.ts | 33 +++++++++++ src/cli.ts | 1 + 3 files changed, 136 insertions(+) diff --git a/src/cli-argv-preprocess.test.ts b/src/cli-argv-preprocess.test.ts index 538c775c2..19b0d686a 100644 --- a/src/cli-argv-preprocess.test.ts +++ b/src/cli-argv-preprocess.test.ts @@ -116,6 +116,108 @@ describe('rewriteBrowserArgv', () => { ]); }); + it('hoists a trailing browser --window option to the namespace slot', () => { + expect(rewriteBrowserArgv(['browser', 'work', 'open', 'https://x.com', '--window', 'background'])).toEqual([ + 'browser', + '--session', + 'work', + '--window', + 'background', + 'open', + 'https://x.com', + ]); + expect(rewriteBrowserArgv(['browser', 'work', 'state', '--window', 'foreground'])).toEqual([ + 'browser', + '--session', + 'work', + '--window', + 'foreground', + 'state', + ]); + }); + + it('hoists trailing browser --window after leading root options', () => { + expect(rewriteBrowserArgv(['--profile', 'sandbox', 'browser', 'work', 'state', '--window', 'background'])).toEqual([ + '--profile', + 'sandbox', + 'browser', + '--session', + 'work', + '--window', + 'background', + 'state', + ]); + }); + + it('hoists a trailing browser --window= option', () => { + expect(rewriteBrowserArgv(['browser', 'work', 'open', 'https://x.com', '--window=background'])).toEqual([ + 'browser', + '--session', + 'work', + '--window=background', + 'open', + 'https://x.com', + ]); + }); + + it('hoists browser --window after nested browser leaf commands', () => { + expect(rewriteBrowserArgv(['browser', 'work', 'get', 'url', '--window', 'background'])).toEqual([ + 'browser', + '--session', + 'work', + '--window', + 'background', + 'get', + 'url', + ]); + expect(rewriteBrowserArgv(['browser', 'work', 'tab', 'close', 'abc123', '--window', 'background'])).toEqual([ + 'browser', + '--session', + 'work', + '--window', + 'background', + 'tab', + 'close', + 'abc123', + ]); + }); + + it('leaves an already parent-slot browser --window option untouched', () => { + expect(rewriteBrowserArgv(['browser', 'work', '--window', 'background', 'open', 'https://x.com'])).toEqual([ + 'browser', + '--session', + 'work', + '--window', + 'background', + 'open', + 'https://x.com', + ]); + }); + + it('does not hoist browser --window after a literal -- separator', () => { + expect(rewriteBrowserArgv(['browser', 'work', 'eval', 'console.log(1)', '--', '--window', 'background'])).toEqual([ + 'browser', + '--session', + 'work', + 'eval', + 'console.log(1)', + '--', + '--window', + 'background', + ]); + }); + + it('does not hoist a bare trailing browser --window without a value', () => { + expect(rewriteBrowserArgv(['browser', 'work', 'open', 'https://x.com', '--window'])).toEqual([ + 'browser', + '--session', + 'work', + 'open', + 'https://x.com', + '--window', + ]); + }); + it('leaves argv alone when the root command is not `browser`, even if `browser` appears later', () => { // The first browser keyword does NOT win — it must be at the root. expect(rewriteBrowserArgv(['twitter', 'browser', 'work', 'state'])).toEqual([ diff --git a/src/cli-argv-preprocess.ts b/src/cli-argv-preprocess.ts index fdd23f8b8..a964b6ddb 100644 --- a/src/cli-argv-preprocess.ts +++ b/src/cli-argv-preprocess.ts @@ -116,9 +116,42 @@ export function rewriteBrowserArgv(argv: readonly string[]): string[] { if (BROWSER_SUBCOMMAND_NAMES.has(next)) return result; // Splice in --session in place of the positional. result.splice(sessionIdx, 1, '--session', next); + // `--window` is a browser namespace option, so commander accepts it before the + // leaf command. Users naturally put it at the end: + // `browser work open https://x.com --window background`. Hoist that public + // form into the namespace-option slot instead of mirroring the option onto + // every browser leaf command. + hoistBrowserWindowOption(result, sessionIdx + 2); return result; } +/** + * Move one trailing `--window ` / `--window=` from after the browser + * subcommand to just before it. Stops at `--` so literal browser arguments are + * untouched. Mutates `argv` in place. + */ +function hoistBrowserWindowOption(argv: string[], fromIndex: number): void { + const subcommandIdx = argv.findIndex((tok, idx) => idx >= fromIndex && BROWSER_SUBCOMMAND_NAMES.has(tok)); + if (subcommandIdx === -1) return; + + for (let i = subcommandIdx + 1; i < argv.length; i += 1) { + const tok = argv[i]; + if (tok === '--') return; + if (tok.startsWith('--window=')) { + const removed = argv.splice(i, 1); + argv.splice(subcommandIdx, 0, ...removed); + return; + } + if (tok === '--window') { + const value = argv[i + 1]; + if (value === undefined || value === '--') return; + const removed = argv.splice(i, 2); + argv.splice(subcommandIdx, 0, ...removed); + return; + } + } +} + /** * Thrown by the preprocessor when user argv uses a retired/old form that we * intentionally refuse to accept. main.ts catches this and exits with a diff --git a/src/cli.ts b/src/cli.ts index 92ea3ee14..c9d98a220 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -885,6 +885,7 @@ export function createProgram(BUILTIN_CLIS: string, USER_CLIS: string): Command Examples: $ opencli browser work open https://x.com + $ opencli browser work open https://x.com --window background $ opencli browser work click 12 $ opencli browser work state $ opencli browser work bind