mirror of
https://github.com/jackwener/OpenCLI.git
synced 2026-09-14 18:25:42 +08:00
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
325 lines
12 KiB
TypeScript
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);
|
|
});
|