mirror of
https://github.com/jackwener/OpenCLI.git
synced 2026-09-14 18:25:42 +08:00
jakevin
0e168d570e
* 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