Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions src/browser/runtime/local-cloak/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ function makeProviderWithFakePage(initialViewport: { width: number; height: numb
const provider = new LocalCloakRuntimeProvider({
baseDir: '/tmp/webcmd-test',
launchPersistentContext: vi.fn().mockResolvedValue(context),
// Commands now default to background, which routes a darwin launch through
// the background launcher; the fake context stands in for both.
launchBackgroundPersistentContext: vi.fn().mockResolvedValue(context),
});
return { provider, browser, page: pages[0], pages, context, cdpSession, pageCdpSessions };
}
Expand Down Expand Up @@ -228,6 +231,30 @@ describe('LocalCloakRuntimeProvider', () => {
expect(page.goto).toHaveBeenCalledWith('https://example.com/', expect.objectContaining({ waitUntil: 'load' }));
});

it.each([
{ label: 'omits windowMode', windowMode: undefined, background: true, focus: false },
{ label: 'asks for foreground', windowMode: 'foreground' as const, background: false, focus: true },
])('opens a window tab per the command that $label', async ({ windowMode, background, focus }) => {
const { provider, cdpSession } = makeProviderWithFakePage();
for (const session of ['first', 'second']) {
await provider.dispatch({
id: `nav-${session}`,
action: 'navigate',
session,
surface: 'browser',
url: `https://${session}.example/`,
profileId: 'default',
...(windowMode ? { windowMode } : {}),
});
}

const windowTargets = cdpSession.send.mock.calls
.filter(([command, params]) => command === 'Target.createTarget' && !(params as { hidden?: boolean })?.hidden)
.map(([, params]) => params as { background?: boolean; focus?: boolean });
expect(windowTargets.length).toBeGreaterThan(0);
for (const params of windowTargets) expect(params).toMatchObject({ background, focus });
});

it("maps waitUntil 'none' to a commit-only navigation wait", async () => {
const { provider, page } = makeProviderWithFakePage();
const result = await provider.dispatch({
Expand Down Expand Up @@ -855,7 +882,7 @@ describe('LocalCloakRuntimeProvider', () => {

await expect(provider.dispatch({ id: 'select', action: 'tabs', op: 'select', session: 'work', surface: 'browser', page: created.page, profileId: 'default' }))
.resolves.toMatchObject({ id: 'select', ok: true, page: created.page, data: { selected: true } });
expect(pages[0].bringToFront).toHaveBeenCalled();
expect(pages[0].bringToFront).not.toHaveBeenCalled();

await expect(provider.dispatch({ id: 'close', action: 'tabs', op: 'close', session: 'work', surface: 'browser', page: created.page, profileId: 'default' }))
.resolves.toMatchObject({ id: 'close', ok: true, data: { closed: created.page } });
Expand Down Expand Up @@ -896,7 +923,7 @@ describe('LocalCloakRuntimeProvider', () => {
expect(pages[0].bringToFront).not.toHaveBeenCalled();
});

it('brings bound tabs to front by default', async () => {
it('does not bring bound tabs to front by default', async () => {
const { provider, pages } = makeProviderWithFakePage();
const created = await provider.dispatch({ id: 'new', action: 'tabs', op: 'new', session: 'source', surface: 'browser', url: 'https://second.example/', profileId: 'default' });

Expand Down
9 changes: 8 additions & 1 deletion src/browser/runtime/local-cloak/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
export interface LocalCloakRuntimeProviderOptions {
baseDir?: string;
launchPersistentContext?: LaunchPersistentContext;
launchBackgroundPersistentContext?: LaunchPersistentContext;
}

export class LocalCloakRuntimeProvider implements BrowserRuntimeProvider {
Expand Down Expand Up @@ -91,7 +92,13 @@ export class LocalCloakRuntimeProvider implements BrowserRuntimeProvider {
return { closed: closedCount > 0, alreadyIdle: closedCount === 0, session: record.id };
}

async dispatch(command: BrowserRuntimeCommand, signal?: AbortSignal): Promise<BrowserRuntimeResult> {
async dispatch(rawCommand: BrowserRuntimeCommand, signal?: AbortSignal): Promise<BrowserRuntimeResult> {
// Every dispatched command runs in the background unless the caller asked
// for a window explicitly, so no command steals focus by default. Session
// handoff bypasses dispatch and still foregrounds through foregroundSession.
const command: BrowserRuntimeCommand = rawCommand.windowMode
? rawCommand
: { ...rawCommand, windowMode: 'background' };
const key = this.commandQueueKey(command);
const previous = this.sessionQueues.get(key) ?? Promise.resolve();
let release!: () => void;
Expand Down
6 changes: 3 additions & 3 deletions src/cli-argv-preprocess.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ describe('rejectPositionalBrowserSessionArgv', () => {
});

describe('rejectPositionalBrowserSessionArgv details', () => {
it('keeps browser subcommands and hoists trailing --window', () => {
expect(rejectPositionalBrowserSessionArgv(['--session', 'session_a', 'browser', 'state', '--window', 'background'])).toEqual([
'--session', 'session_a', 'browser', '--window', 'background', 'state',
it('leaves a recognised browser subcommand and its options in place', () => {
expect(rejectPositionalBrowserSessionArgv(['--session', 'session_a', 'browser', 'snapshot', '--snapshot-mode', 'act'])).toEqual([
'--session', 'session_a', 'browser', 'snapshot', '--snapshot-mode', 'act',
]);
});

Expand Down
32 changes: 1 addition & 31 deletions src/cli-argv-preprocess.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,10 +75,7 @@ export function rejectPositionalBrowserSessionArgv(argv: readonly string[]): str
const commandIndex = findRootCommandIndex(result);
if (result[commandIndex] !== 'browser') return result;
const candidate = result[commandIndex + 1];
if (!candidate || candidate.startsWith('-') || BROWSER_SUBCOMMAND_NAMES.has(candidate)) {
hoistBrowserWindowOption(result, commandIndex + 1);
return result;
}
if (!candidate || candidate.startsWith('-') || BROWSER_SUBCOMMAND_NAMES.has(candidate)) return result;
const replacement = [
...result.slice(0, commandIndex),
'--session', candidate,
Expand Down Expand Up @@ -122,33 +119,6 @@ function findRootCommandIndex(argv: readonly string[]): number {
return index;
}

/**
* Move one trailing `--window <mode>` / `--window=<mode>` 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
Expand Down
36 changes: 33 additions & 3 deletions src/pipeline/steps/transform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,44 @@ export async function stepFilter(_page: IPage | null, params: unknown, data: unk
return data.filter((item, i) => evalExpr(String(params), { args, item, index: i }));
}

/** Parse a sort key as a number, or null when it is not one. */
function sortableNumber(value: unknown): number | null {
if (typeof value === 'number') return Number.isFinite(value) ? value : null;
if (typeof value !== 'string' || value.trim() === '') return null;
const parsed = Number(value);
return Number.isFinite(parsed) ? parsed : null;
}

/**
* Compare two sort keys.
*
* Columns whose non-empty values are all numbers are compared numerically.
* ICU's `numeric` collation only understands runs of digits, so it splits a decimal
* at the separator and its ordering of values like "9.5" against "10" varies by
* platform and locale — Windows CI disagreed with macOS/Linux on exactly that
* pair. Everything else falls back to a locale-pinned natural compare.
*/
function compareSortKeys(left: unknown, right: unknown, numericColumn: boolean): number {
if (numericColumn) {
const leftNumber = sortableNumber(left);
const rightNumber = sortableNumber(right);
if (leftNumber === null || rightNumber === null) {
return leftNumber === rightNumber ? 0 : leftNumber === null ? -1 : 1;
}
return leftNumber === rightNumber ? 0 : leftNumber < rightNumber ? -1 : 1;
}
return String(left ?? '').localeCompare(String(right ?? ''), 'en', { numeric: true });
}

export async function stepSort(_page: IPage | null, params: unknown, data: unknown, _args: Record<string, unknown>): Promise<unknown> {
if (!Array.isArray(data)) return data;
const key = isRecord(params) ? String(params.by ?? '') : String(params);
const reverse = isRecord(params) ? params.order === 'desc' : false;
const keys = data.map((item) => isRecord(item) ? item[key] : undefined);
const numericColumn = keys.some((value) => sortableNumber(value) !== null)
&& keys.every((value) => value == null || (typeof value === 'string' && value.trim() === '') || sortableNumber(value) !== null);
return [...data].sort((a, b) => {
const left = isRecord(a) ? a[key] : undefined;
const right = isRecord(b) ? b[key] : undefined;
const cmp = String(left ?? '').localeCompare(String(right ?? ''), undefined, { numeric: true });
const cmp = compareSortKeys(isRecord(a) ? a[key] : undefined, isRecord(b) ? b[key] : undefined, numericColumn);
return reverse ? -cmp : cmp;
});
}
Expand Down
24 changes: 24 additions & 0 deletions src/pipeline/transform.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,30 @@ describe('stepSort', () => {
expect((result as typeof data).map((r) => r.name)).toEqual(['B', 'C', 'A']);
});

it('orders decimals by value, not by collation', async () => {
// ICU numeric collation splits "9.5" at the separator, so the ordering of
// this trio against "10" used to differ between Windows and macOS/Linux.
const data = [
{ name: 'TEN', change: '10.0' },
{ name: 'NINE', change: '9.5' },
{ name: 'HUNDRED', change: '100.0' },
];
const result = await stepSort(null, { by: 'change', order: 'desc' }, data, {});
expect((result as typeof data).map((r) => r.name)).toEqual(['HUNDRED', 'TEN', 'NINE']);
});

it('uses one comparison mode for a mixed column', async () => {
const expected = ['0x10', '1a', '2'];
for (const values of [
['2', '0x10', '1a'],
['2', '1a', '0x10'],
['0x10', '2', '1a'],
]) {
const result = await stepSort(null, 'value', values.map((value) => ({ value })), {});
expect((result as Array<{ value: string }>).map(({ value }) => value)).toEqual(expected);
}
});

it('handles missing fields gracefully', async () => {
const data = [
{ name: 'A', value: '10' },
Expand Down
21 changes: 12 additions & 9 deletions tests/e2e/browser-run.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,13 @@ describe('browser run local lifecycle', () => {
servers.push(fixture.server);
const cacheDir = await fs.mkdtemp(path.join(os.tmpdir(), 'webcmd-browser-run-'));
tempDirs.push(cacheDir);
const env = { WEBCMD_CACHE_DIR: cacheDir };
const session = 'browser-run-lifecycle';
const env = { WEBCMD_CACHE_DIR: cacheDir, WEBCMD_CONFIG_DIR: cacheDir };

const first = await runCliWithStdin(['browser', session, 'run', '--stdin'], `
const created = await runCliWithStdin(['session', 'create', '-f', 'json'], '', env);
expect(created.code).toBe(0);
const session = parseJsonOutput(created.stdout).id as string;

const first = await runCliWithStdin(['--session', session, 'browser', 'run', '--stdin'], `
globalThis.onlyThisRun = 'gone';
await page.goto(${JSON.stringify(fixture.url)});
await page.locator('#name').fill('Ada');
Expand Down Expand Up @@ -128,11 +131,11 @@ describe('browser run local lifecycle', () => {
});
expect(firstData.snapshotDiff).toContain('Ada');

const snapshot = await runCliWithStdin(['browser', session, 'snapshot', '--snapshot-mode', 'act'], '', env);
const snapshot = await runCliWithStdin(['--session', session, 'browser', 'snapshot', '--snapshot-mode', 'act'], '', env);
expect(snapshot.code).toBe(0);
expect(snapshot.stdout).toContain('Ada');

const second = await runCliWithStdin(['browser', session, 'run', '--stdin', '--no-snapshot-diff'], `
const second = await runCliWithStdin(['--session', session, 'browser', 'run', '--stdin', '--no-snapshot-diff'], `
return {
saved: await page.locator('#status').innerText(),
variable: typeof globalThis.onlyThisRun,
Expand All @@ -145,20 +148,20 @@ describe('browser run local lifecycle', () => {
});
expect(secondData).not.toHaveProperty('snapshotDiff');

const tabs = await runCliWithStdin(['browser', session, 'tabs'], '', env);
const tabs = await runCliWithStdin(['--session', session, 'browser', 'tabs'], '', env);
expect(tabs.code).toBe(0);
const popup = parseJsonOutput(tabs.stdout).find((tab: { title: string }) => tab.title === 'Popup receipt');
expect(popup).toMatchObject({ id: expect.any(String) });

const bound = await runCliWithStdin(['browser', session, 'bind', '--page', popup.id], '', env);
const bound = await runCliWithStdin(['--session', session, 'browser', 'bind', '--page', popup.id], '', env);
expect(bound.code).toBe(0);
expect(parseJsonOutput(bound.stdout)).toMatchObject({ page: popup.id, title: 'Popup receipt' });

const closed = await runCliWithStdin(['browser', session, 'close'], '', env);
const closed = await runCliWithStdin(['--session', session, 'browser', 'close'], '', env);
expect(closed.code).toBe(0);
expect(parseJsonOutput(closed.stdout)).toMatchObject({ closed: true });

const tabsAfterClose = await runCliWithStdin(['browser', session, 'tabs'], '', env);
const tabsAfterClose = await runCliWithStdin(['--session', session, 'browser', 'tabs'], '', env);
expect(tabsAfterClose.code).toBe(0);
expect(parseJsonOutput(tabsAfterClose.stdout)).toEqual([]);
}, 120_000);
Expand Down