Files
jackwener__opencli/tests/e2e/browser-tabs.test.ts
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

325 lines
12 KiB
TypeScript

import { afterEach, describe, expect, it } from 'vitest';
import { createServer, type IncomingMessage, type ServerResponse } from 'node:http';
import * as fs from 'node:fs';
import * as os from 'node:os';
import * as path from 'node:path';
import { fileURLToPath } from 'node:url';
import { parseJsonOutput, runCli } from './helpers.js';
// Match the running CLI's package version so BrowserBridge does not classify
// this fake daemon as stale (PR #1399 auto-restarts daemons whose
// daemonVersion does not match PKG_VERSION; the fake daemon does not implement
// /shutdown, so a mismatch makes every test exit with code 1).
const PKG_VERSION: string = (() => {
const here = path.dirname(fileURLToPath(import.meta.url));
// tests/e2e -> repo root: ../..
const pkgPath = path.resolve(here, '..', '..', 'package.json');
try {
return JSON.parse(fs.readFileSync(pkgPath, 'utf-8')).version;
} catch {
return '0.0.0';
}
})();
type FakeTab = {
page: string;
url: string;
title: string;
active: boolean;
};
type FakeDaemon = {
port: number;
close: () => Promise<void>;
maxInFlightExec: () => number;
};
async function readBody(req: IncomingMessage): Promise<string> {
return await new Promise((resolve, reject) => {
const chunks: Buffer[] = [];
req.on('data', (chunk: Buffer) => chunks.push(chunk));
req.on('end', () => resolve(Buffer.concat(chunks).toString('utf-8')));
req.on('error', reject);
});
}
function json(res: ServerResponse, status: number, payload: unknown): void {
res.writeHead(status, { 'Content-Type': 'application/json' });
res.end(JSON.stringify(payload));
}
async function startFakeDaemon(): Promise<FakeDaemon> {
const tabs = new Map<string, FakeTab>([
['tab-1', { page: 'tab-1', url: 'https://one.example/', title: 'tab-one', active: true }],
['tab-2', { page: 'tab-2', url: 'https://two.example/', title: 'tab-two', active: false }],
]);
let nextId = 3;
let inFlightExec = 0;
let maxInFlightExec = 0;
const server = createServer(async (req, res) => {
const pathname = req.url?.split('?')[0] ?? '/';
if (req.method === 'GET' && pathname === '/status') {
const port = typeof server.address() === 'object' && server.address() ? server.address().port : 0;
json(res, 200, {
ok: true,
pid: process.pid,
uptime: 1,
daemonVersion: PKG_VERSION,
extensionConnected: true,
extensionVersion: 'test',
pending: 0,
memoryMB: 1,
port,
});
return;
}
if (req.method !== 'POST' || pathname !== '/command') {
json(res, 404, { ok: false, error: 'Not found' });
return;
}
const body = JSON.parse(await readBody(req)) as {
id: string;
action: string;
op?: string;
page?: string;
index?: number;
url?: string;
code?: string;
};
const listTabs = () => [...tabs.values()].map((tab, index) => ({ index, ...tab }));
const tabByIndex = (index?: number) => index === undefined ? undefined : listTabs()[index];
switch (body.action) {
case 'tabs': {
switch (body.op) {
case 'list':
json(res, 200, { id: body.id, ok: true, data: listTabs() });
return;
case 'new': {
const page = `tab-${nextId++}`;
const url = body.url ?? 'about:blank';
tabs.set(page, {
page,
url,
title: page,
active: true,
});
json(res, 200, { id: body.id, ok: true, page, data: { url } });
return;
}
case 'close': {
const targetPage = typeof body.page === 'string' ? body.page : tabByIndex(body.index)?.page;
if (!targetPage || !tabs.has(targetPage)) {
json(res, 200, { id: body.id, ok: false, error: 'Tab not found' });
return;
}
tabs.delete(targetPage);
json(res, 200, { id: body.id, ok: true, data: { closed: targetPage } });
return;
}
case 'select': {
const targetPage = typeof body.page === 'string' ? body.page : tabByIndex(body.index)?.page;
if (!targetPage || !tabs.has(targetPage)) {
json(res, 200, { id: body.id, ok: false, error: 'Tab not found' });
return;
}
json(res, 200, { id: body.id, ok: true, page: targetPage, data: { selected: true } });
return;
}
default:
json(res, 200, { id: body.id, ok: false, error: `Unknown tabs op: ${body.op}` });
return;
}
}
case 'navigate': {
const targetPage = typeof body.page === 'string' && tabs.has(body.page) ? body.page : 'tab-1';
const target = tabs.get(targetPage)!;
const url = body.url ?? target.url;
target.url = url;
target.title = url;
json(res, 200, {
id: body.id,
ok: true,
page: targetPage,
data: { title: target.title, url: target.url, timedOut: false },
});
return;
}
case 'exec': {
const targetPage = typeof body.page === 'string' ? body.page : 'tab-1';
const target = tabs.get(targetPage);
if (!target) {
json(res, 200, { id: body.id, ok: false, error: `Unknown page: ${targetPage}` });
return;
}
inFlightExec++;
maxInFlightExec = Math.max(maxInFlightExec, inFlightExec);
try {
if ((body.code ?? '').includes('__delay')) {
await new Promise(resolve => setTimeout(resolve, 200));
}
json(res, 200, {
id: body.id,
ok: true,
page: targetPage,
data: {
page: targetPage,
title: target.title,
url: target.url,
},
});
} finally {
inFlightExec--;
}
return;
}
default:
json(res, 200, { id: body.id, ok: false, error: `Unknown action: ${body.action}` });
}
});
await new Promise<void>((resolve) => {
server.listen(0, '127.0.0.1', () => resolve());
});
const address = server.address();
if (!address || typeof address !== 'object') {
throw new Error('Failed to bind fake daemon port');
}
return {
port: address.port,
close: async () => {
await new Promise<void>((resolve, reject) => {
server.close((err) => err ? reject(err) : resolve());
});
},
maxInFlightExec: () => maxInFlightExec,
};
}
describe('browser tab CLI e2e', () => {
const daemons: FakeDaemon[] = [];
const cacheDirs: string[] = [];
const browserArgs = (session: string, ...args: string[]) => ['browser', session, ...args];
afterEach(async () => {
while (daemons.length > 0) {
await daemons.pop()!.close();
}
while (cacheDirs.length > 0) {
fs.rmSync(cacheDirs.pop()!, { recursive: true, force: true });
}
});
it('lists, creates, and closes tabs through the built CLI', async () => {
const daemon = await startFakeDaemon();
daemons.push(daemon);
const env = { OPENCLI_DAEMON_PORT: String(daemon.port) };
const session = 'tabs-basic';
const listed = await runCli(browserArgs(session, 'tab', 'list'), { env });
expect(listed.code).toBe(0);
const listData = parseJsonOutput(listed.stdout);
expect(listData).toEqual(expect.arrayContaining([
expect.objectContaining({ page: 'tab-1', title: 'tab-one' }),
expect.objectContaining({ page: 'tab-2', title: 'tab-two' }),
]));
const created = await runCli(browserArgs(session, 'tab', 'new', 'https://three.example/'), { env });
expect(created.code).toBe(0);
const createdData = parseJsonOutput(created.stdout);
expect(createdData).toEqual(expect.objectContaining({
page: 'tab-3',
url: 'https://three.example/',
}));
const closed = await runCli(browserArgs(session, 'tab', 'close', 'tab-3'), { env });
expect(closed.code).toBe(0);
const closedData = parseJsonOutput(closed.stdout);
expect(closedData).toEqual({ closed: 'tab-3' });
const relisted = await runCli(browserArgs(session, 'tab', 'list'), { env });
expect(relisted.code).toBe(0);
const relistedData = parseJsonOutput(relisted.stdout);
expect(relistedData).toHaveLength(2);
expect(relistedData.some((tab: { page: string }) => tab.page === 'tab-3')).toBe(false);
}, 30_000);
it('routes concurrent browser commands to their requested tabs', async () => {
const daemon = await startFakeDaemon();
daemons.push(daemon);
const env = { OPENCLI_DAEMON_PORT: String(daemon.port) };
const session = 'tabs-concurrent';
const [left, right] = await Promise.all([
runCli(browserArgs(session, 'eval', '--tab', 'tab-1', 'window.__delay = "left"'), { env, timeout: 30_000 }),
runCli(browserArgs(session, 'eval', '--tab', 'tab-2', 'window.__delay = "right"'), { env, timeout: 30_000 }),
]);
expect(left.code).toBe(0);
expect(right.code).toBe(0);
const leftData = parseJsonOutput(left.stdout);
const rightData = parseJsonOutput(right.stdout);
expect(leftData).toEqual(expect.objectContaining({ page: 'tab-1', title: 'tab-one' }));
expect(rightData).toEqual(expect.objectContaining({ page: 'tab-2', title: 'tab-two' }));
expect(daemon.maxInFlightExec()).toBe(2);
}, 30_000);
it('keeps untargeted browser commands on the default tab after creating a new tab', async () => {
const daemon = await startFakeDaemon();
daemons.push(daemon);
const cacheDir = fs.mkdtempSync(path.join(os.tmpdir(), 'opencli-browser-tabs-'));
cacheDirs.push(cacheDir);
const env = {
OPENCLI_DAEMON_PORT: String(daemon.port),
OPENCLI_CACHE_DIR: cacheDir,
};
const session = 'tabs-default-new';
const created = await runCli(browserArgs(session, 'tab', 'new', 'https://three.example/'), { env });
expect(created.code).toBe(0);
expect(parseJsonOutput(created.stdout)).toEqual(expect.objectContaining({ page: 'tab-3' }));
const untargeted = await runCli(browserArgs(session, 'eval', 'document.title'), { env });
expect(untargeted.code).toBe(0);
expect(parseJsonOutput(untargeted.stdout)).toEqual(expect.objectContaining({ page: 'tab-1', title: 'tab-one' }));
}, 30_000);
it('uses an explicitly selected tab as the default target for later untargeted commands', async () => {
const daemon = await startFakeDaemon();
daemons.push(daemon);
const cacheDir = fs.mkdtempSync(path.join(os.tmpdir(), 'opencli-browser-tabs-'));
cacheDirs.push(cacheDir);
const env = {
OPENCLI_DAEMON_PORT: String(daemon.port),
OPENCLI_CACHE_DIR: cacheDir,
};
const session = 'tabs-selected-default';
const selected = await runCli(browserArgs(session, 'tab', 'select', 'tab-2'), { env });
expect(selected.code).toBe(0);
expect(parseJsonOutput(selected.stdout)).toEqual({ selected: 'tab-2' });
const untargeted = await runCli(browserArgs(session, 'eval', 'document.title'), { env });
expect(untargeted.code).toBe(0);
expect(parseJsonOutput(untargeted.stdout)).toEqual(expect.objectContaining({ page: 'tab-2', title: 'tab-two' }));
const closed = await runCli(browserArgs(session, 'tab', 'close', 'tab-2'), { env });
expect(closed.code).toBe(0);
expect(parseJsonOutput(closed.stdout)).toEqual({ closed: 'tab-2' });
const fallback = await runCli(browserArgs(session, 'eval', 'document.title'), { env });
expect(fallback.code).toBe(0);
expect(parseJsonOutput(fallback.stdout)).toEqual(expect.objectContaining({ page: 'tab-1', title: 'tab-one' }));
}, 30_000);
});