4 Commits

Author	SHA1	Message	Date
jakevin	0e168d570e	refactor(browser): replace --session flag with <sessionname> positional (#1505) ... * refactor(browser): replace --session flag with <sessionname> positional The `--session <name>` flag was semantically required but syntactically optional, which is an anti-pattern. Required + flag is a contradiction: flag form implies "optional", required is a runtime patch on top. Session is OpenCLI's "operation target" identifier — the natural form for that is a positional argument, like `docker exec <container> <cmd>` or `git checkout <branch>`. New surface: opencli browser <sessionname> open https://x.com opencli browser <sessionname> click 12 opencli browser <sessionname> bind opencli browser <sessionname> unbind Commander 14 cannot natively combine a parent positional with subcommand dispatch — the parent's positional is shadowed by subcommand matching. To bridge that, main.ts now pre-processes argv: when the token after `browser` is non-flag and not a known subcommand name, it is treated as the sessionname and rewritten to the internal `--session <name>` flag form before commander parses it. Help text on the `browser` command is overridden via `.usage('<sessionname> <command> [options]')` so users see the positional form. Reserved subcommand names (33) are listed in cli-argv-preprocess.ts and tested for parity with cli.ts subcommand registrations. If a future subcommand is added, the test fails loudly. Synced surfaces: - README.md / README.zh-CN.md — all examples - docs/guide/browser-bridge.md (+ zh) - skills/opencli-browser/SKILL.md (bind/unbind, examples, table) - skills/opencli-usage/SKILL.md - tests/e2e/browser-tabs.test.ts - CHANGELOG.md (Unreleased BREAKING) The internal `--session` flag and the unit tests calling `program.parseAsync(['...', 'browser', '--session', 'foo', ...])` are preserved as a stable internal API: tests bypass main.ts pre-processing and exercise commander directly. The pre-processor has its own targeted test file (cli-argv-preprocess.test.ts, 10 tests, all green). Verification: - npx tsc --noEmit — pass - npx vitest run --project unit — 1073/1074 pass (1 unrelated skip) - npx vitest run --project extension — 61/61 pass - npm run check:typed-error-lint — baseline 189 - npm run check:silent-column-drop — baseline 103 * fix(cli-argv): only rewrite when `browser` is the root command The preprocessor was looping through every argv slot and would mis-rewrite occurrences of the literal word `browser` deeper in argv (e.g. `opencli adapter init browser/x` or arg values containing `browser`). Now the preprocessor walks past leading root flags + their values to identify the root command token, and only acts when that token is `browser`. The full set of root value-consuming flags (`ROOT_VALUE_FLAGS`) is documented inline and kept in sync with the `program.option()` calls in cli.ts. Adds regression tests: - `opencli adapter init browser x` not rewritten - URL/path values containing `browser` not rewritten - `list browser state` (different root command) not rewritten - `--profile work browser foo state` correctly identifies `foo` as sessionname (not as --profile's value) - `--profile=work` long-form-with-equals consumes one slot only - boolean flags (`-v`) don't consume the next value 12/12 preprocessor tests pass. * fix(cli-argv): hide --session flag, fail-fast on retired form, rename to <session> Three blockers in #1505 review: 1. `--session` flag was still visible in `opencli browser --help` and could be used as a public entrance, contradicting "positional only" UX. Fix: switch from `.requiredOption()` to `.addOption(new Option(...).hideHelp())`. The flag is preserved as an internal API for the daemon protocol and direct `program.parseAsync` callers (tests), but is no longer documented or surfaced in structured help. 2. `opencli browser --session foo state` still succeeded. Now the argv preprocessor throws `BrowserSessionArgvError` when root `browser` is followed by `--session`, and main.ts catches it and exits with a user-facing usage error pointing to the positional form. 3. Missing-session error message exposed the internal flag: `required option '--session <name>' not specified`. Now `getBrowserSession()` in the action body throws `<session> is a required positional argument: opencli browser <session> <command>`, and commander no longer guards the hidden option. Also (per @WAWQAQ) rename placeholder `<sessionname>` -> `<session>` everywhere user-facing — shorter, matches CLI convention. The help text "<session> is a required positional: pass the name of the browser session..." carries the "name" semantics in description, not in the placeholder itself. Sync surfaces: - src/cli.ts — usage line, addOption with hideHelp, descriptions - src/cli-argv-preprocess.ts — throw on --session form - src/cli-argv-preprocess.test.ts — refusal test for old form - src/cli.test.ts — assertions updated for hidden option + new error path - src/help.ts — read `_usage` private field to respect `.usage()` override (commander's `.usage()` getter returns auto-generated form if not set, which would otherwise pollute every namespace's usage string) - src/main.ts — catch BrowserSessionArgvError, stderr + exit - README.md / README.zh-CN.md - docs/guide/browser-bridge.md / docs/zh/guide/browser-bridge.md - skills/opencli-browser/SKILL.md / skills/opencli-usage/SKILL.md - CHANGELOG.md Manual smoke tests (against built dist): - `opencli browser --help` shows `Usage: opencli browser <session> <command> [options]` - `opencli browser --help` Options block does NOT show `--session` - `opencli browser --session foo state` → friendly error, no commander stacktrace - `opencli browser state` → `<session> is a required positional argument: opencli browser <session> <command>` - `opencli browser foo state` → parses correctly * fix: inject <session> into subcommand help paths and drop stale sessions ref Two follow-up blockers from #1505 review: 1. Subcommand help and structured help still rendered the command path without the parent's positional. `opencli browser foo state --help` showed `Usage: opencli browser state [options]`, which would lead users (and agents reading structured help) to think `opencli browser state` was a valid invocation. Now: - `commanderPath()` injects an ancestor's leading-positional placeholder (extracted from its `.usage()` override) between the ancestor's name and the next path segment when building paths upward. - `commandPathFromRoot()` strips placeholder segments (e.g. `<session>`) from the relative `name` field so agents can still address subcommands by their leaf name; placeholders remain in the `command` / `usage` display paths. - `program.configureHelp({ commandUsage: ... })` is applied recursively to every descendant of `browser`, because commander does NOT inherit `configureHelp` into subcommands. Result: opencli browser <session> click --help -> Usage: opencli browser <session> click [target] [options] Daemon, plugin, adapter, profile namespaces (no `.usage()` override) are unaffected. 2. `skills/opencli-browser/SKILL.md` still referenced `opencli browser sessions`, which was removed in #1470. Replaced the sentence with the underlying invariant ("Bound sessions have no OpenCLI idle-close timer; the binding lasts until `unbind`, tab close, window close, or daemon restart") without mentioning the deleted command. Tests: - cli.test.ts: structured help expectations updated to include `<session>` in command/usage paths (3 tests) - cli-argv-preprocess.test.ts: 12 tests still green - 1136/1137 unit+extension green (1 unrelated skip) - typed-error-lint baseline 189 - silent-column-drop baseline 103	2026-05-12 20:44:29 +08:00
jakevin	9c06e84c89	refactor(browser): replace workspaces with sessions (#1461)	2026-05-11 04:26:51 +08:00
jakevin	19130ab1af	fix(e2e): match fake daemon version to running CLI (#1432) ... PR #1399 added auto-restart of stale daemons in BrowserBridge (daemonVersion ≠ PKG_VERSION → restart). The browser-tabs e2e fake daemon hard-coded `daemonVersion: 'test'`, so every test reported as stale and the bridge tried to /shutdown the fake daemon — which has no shutdown endpoint — causing all 4 tests in the file to exit with code 1. This has been the failing signal in `e2e-headed (ubuntu-latest)` and `e2e-headed (macos-latest)` on every main push since #1399. Read PKG_VERSION from package.json once at module load and feed that to the fake /status response. The fake daemon now matches the running CLI so the stale-daemon path is not triggered. Verification: - npx tsc --noEmit clean - npm run build clean - npx vitest run --project e2e tests/e2e/browser-tabs.test.ts → 4/4 pass	2026-05-10 15:26:52 +08:00
zhengyu	fbdb1b24dc	fix(browser): harden multi-tab routing and target isolation (#1072) ... * fix(browser): harden multi-tab routing and target isolation - make daemon command ids collision-resistant and retry duplicate pending ids\n- add validated tab list/new/select/close flows with persisted default targets\n- keep untargeted browser commands on the default tab unless tab select changes it\n- document tab targeting and add unit, extension, and e2e coverage for concurrent multi-tab execution * fix(browser): keep default tab stable after tab new * fix(browser): close remaining tab routing gates * docs(browser): align target id wording * docs(browser): refine target id examples --------- Co-authored-by: jackwener <jakevingoo@gmail.com>	2026-04-19 18:48:45 +08:00