From 64f58b05253ba5d4cac08e6904388c5eca60c935 Mon Sep 17 00:00:00 2001 From: rajashidattapy Date: Wed, 5 Aug 2026 00:30:58 +0530 Subject: [PATCH 1/2] The fix, routed through one helper so a third call site can't reintroduce the bug: MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - session-manager.ts:24-33 — new exported toGotoWaitUntil() carrying the 'none' → 'commit' mapping and the explanatory comment that previously lived inline at the navigate site. - session-manager.ts:248 — newPage input widened with waitUntil?: 'load' | 'none'. - session-manager.ts:260 — the hardcoded 'load' replaced with toGotoWaitUntil(input.waitUntil). - actions.ts:161 — tabs/new now passes waitUntil: command.waitUntil through, which it was silently dropping. - actions.ts:113 — navigate switched to the same helper, so both paths share one implementation. I used a shared helper rather than copying the ternary into newPage. Duplicating it would have been a two-line diff, but a duplicated mapping in two files is precisely what let this bug survive the #106 fix. Verification - npx vitest run --project unit src/browser/runtime/local-cloak/provider.test.ts — 27 passed. That includes a new test mirroring the existing navigate pair: tabs/new with waitUntil: 'none' now asserts goto receives 'commit'. The pre-existing test at line 318 still asserts the default is 'load', so both branches are covered. - npx tsc --noEmit reports one error, and it is not from this change: src/fetch/client.ts(2,23): Cannot find module 'impit'. impit@0.14.3 is in package.json dependencies but absent from node_modules here — a stale local install, not a code problem. Run npm install and it should clear; worth confirming on your side before you push, since I can't distinguish "not installed locally" from "genuinely broken on main" without it. --- src/browser/runtime/local-cloak/actions.ts | 8 +++----- src/browser/runtime/local-cloak/provider.test.ts | 16 ++++++++++++++++ .../runtime/local-cloak/session-manager.ts | 15 +++++++++++++-- 3 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/browser/runtime/local-cloak/actions.ts b/src/browser/runtime/local-cloak/actions.ts index db5b44a6..430433fb 100644 --- a/src/browser/runtime/local-cloak/actions.ts +++ b/src/browser/runtime/local-cloak/actions.ts @@ -1,6 +1,6 @@ import type { BrowserRuntimeCommand, BrowserRuntimeResult } from '../../protocol.js'; import { waitForDownload } from './downloads.js'; -import type { CloakSessionManager } from './session-manager.js'; +import { toGotoWaitUntil, type CloakSessionManager } from './session-manager.js'; import type { BrowserContext, Frame, Page as PlaywrightPage } from 'playwright-core'; class CloakActionError extends Error { @@ -110,10 +110,7 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command: case 'navigate': { if (!command.url) return invalidRequest(command, 'Missing url'); const lease = await resolveLease(manager, command); - // 'none' maps to Playwright's 'commit': sites that stream analytics forever - // never fire the load event, so adapters gating readiness on their own - // selector waits must be able to skip it. - await lease.page.goto(command.url, { waitUntil: command.waitUntil === 'none' ? 'commit' : 'load' }); + await lease.page.goto(command.url, { waitUntil: toGotoWaitUntil(command.waitUntil) }); return { id: command.id, ok: true, data: { title: await lease.page.title(), url: lease.page.url(), timedOut: false }, page: lease.pageId }; } case 'exec': { @@ -161,6 +158,7 @@ export async function dispatchCloakAction(manager: CloakSessionManager, command: siteSession: command.siteSession, idleTimeout: command.idleTimeout, url: command.url, + waitUntil: command.waitUntil, windowMode: command.windowMode, }); return { id: command.id, ok: true, data: { title: await lease.page.title(), url: lease.page.url() }, page: lease.pageId }; diff --git a/src/browser/runtime/local-cloak/provider.test.ts b/src/browser/runtime/local-cloak/provider.test.ts index cb0b2178..023f5a8d 100644 --- a/src/browser/runtime/local-cloak/provider.test.ts +++ b/src/browser/runtime/local-cloak/provider.test.ts @@ -88,6 +88,22 @@ describe('LocalCloakRuntimeProvider', () => { expect(page.goto).toHaveBeenCalledWith('https://example.com/', expect.objectContaining({ waitUntil: 'commit' })); }); + it("maps waitUntil 'none' to a commit-only wait when opening a tab", async () => { + const { provider, pages } = makeProviderWithFakePage(); + const result = await provider.dispatch({ + id: 'new', + action: 'tabs', + op: 'new', + session: 'work', + surface: 'browser', + url: 'https://second.example/', + waitUntil: 'none', + profileId: 'default', + }); + expect(result).toMatchObject({ id: 'new', ok: true, page: expect.any(String) }); + expect(pages[1].goto).toHaveBeenCalledWith('https://second.example/', expect.objectContaining({ waitUntil: 'commit' })); + }); + it('evaluates JavaScript in the resolved page', async () => { const { provider } = makeProviderWithFakePage(); const nav = await provider.dispatch({ id: 'nav', action: 'navigate', session: 'work', surface: 'browser', url: 'https://example.com/', profileId: 'default' }); diff --git a/src/browser/runtime/local-cloak/session-manager.ts b/src/browser/runtime/local-cloak/session-manager.ts index 1b8c3974..28594854 100644 --- a/src/browser/runtime/local-cloak/session-manager.ts +++ b/src/browser/runtime/local-cloak/session-manager.ts @@ -21,6 +21,17 @@ export function resolveCloakBrowserVersion(): string | undefined { } } +/** + * Map the protocol's navigation wait condition onto Playwright's `goto` option. + * 'none' becomes 'commit': sites that stream analytics forever never fire the + * load event, so callers gating readiness on their own selector waits must be + * able to skip it. Every `goto` in this runtime routes through here so a new + * call site cannot quietly reintroduce a hardcoded 'load'. + */ +export function toGotoWaitUntil(waitUntil?: 'load' | 'none'): 'load' | 'commit' { + return waitUntil === 'none' ? 'commit' : 'load'; +} + export type LaunchPersistentContext = typeof cloakLaunchPersistentContext; export type RecoverLockedProfile = (userDataDir: string) => Promise; @@ -234,7 +245,7 @@ export class CloakSessionManager { }))); } - async newPage(input: SessionKeyInput & { url?: string }): Promise { + async newPage(input: SessionKeyInput & { url?: string; waitUntil?: 'load' | 'none' }): Promise { const profileId = normalizeProfileId(input.profileId); const session = requireSession(input.session); const surface = normalizeSurface(input.surface); @@ -246,7 +257,7 @@ export class CloakSessionManager { ); if (input.url) { try { - await acquired.page.goto(input.url, { waitUntil: 'load' }); + await acquired.page.goto(input.url, { waitUntil: toGotoWaitUntil(input.waitUntil) }); } catch (error) { if (!pageIsClosed(acquired.page)) await acquired.page.close().catch(() => {}); throw error; From f7a618820f9bedbc0630c0db5e99ba9e2bdd9cd5 Mon Sep 17 00:00:00 2001 From: rajashidattapy Date: Wed, 5 Aug 2026 02:47:16 +0530 Subject: [PATCH 2/2] feat: add --wait-until option for new browser tab command --- docs/cli-reference.mdx | 14 ++++++++++++++ skills/webcmd-browser/SKILL.md | 2 +- src/browser/command-catalog.ts | 2 ++ src/browser/page.test.ts | 13 +++++++++++++ src/browser/page.ts | 3 ++- src/cli.test.ts | 20 ++++++++++++++++++-- src/cli.ts | 14 ++++++++++++-- src/types.ts | 2 +- 8 files changed, 63 insertions(+), 7 deletions(-) diff --git a/docs/cli-reference.mdx b/docs/cli-reference.mdx index 4073a61d..a9b7ee28 100644 --- a/docs/cli-reference.mdx +++ b/docs/cli-reference.mdx @@ -55,6 +55,20 @@ webcmd web fetch-browser --url https://example.com/app-shell The old `web read` command has been renamed to `web fetch-browser`. +## Browser Runtime + +`webcmd browser ` drives a real browser tab. Run `webcmd browser --help` for the full surface; the bundled `webcmd-browser` skill is the agent-facing reference. + +```bash +webcmd browser work open https://example.com +webcmd browser work tab new https://example.com +webcmd browser work tab new https://example.com --wait-until none +``` + +| Option | Purpose | +| --- | --- | +| `tab new --wait-until ` | `load` (default) waits for the load event; `none` returns as soon as navigation commits, for pages that stream and never finish loading. | + ## Top-Level Commands | Command | Purpose | diff --git a/skills/webcmd-browser/SKILL.md b/skills/webcmd-browser/SKILL.md index 321f9973..94c8ee3d 100644 --- a/skills/webcmd-browser/SKILL.md +++ b/skills/webcmd-browser/SKILL.md @@ -228,7 +228,7 @@ Default output keeps JSON/XML/plain-text and JS-like API responses, then drops o | command | purpose | |---------|---------| | `browser tab list` | JSON array of `{index, page, url, title, active}`. The `page` string is the tab identity you pass as `` to `tab select` / `tab close`, or to `--tab ` on any subcommand. (`--tab`'s placeholder is historical — the value is always `page`.) | -| `browser tab new [url]` | Open a new tab. Prints the new `page` string. | +| `browser tab new [url]` | Open a new tab. Prints the new `page` string. `--wait-until load` (default) waits for the load event; `--wait-until none` returns as soon as navigation commits — use it for pages that never finish loading (streaming/SPA shells). | | `browser tab select [targetId]` | Make a tab the default. All subcommands accept `--tab ` to target one without changing the default. | | `browser tab close [targetId]` | Close by `page`. | | `browser back` | History back on the active tab. | diff --git a/src/browser/command-catalog.ts b/src/browser/command-catalog.ts index 3a88a809..a7bb139c 100644 --- a/src/browser/command-catalog.ts +++ b/src/browser/command-catalog.ts @@ -103,6 +103,7 @@ const BROWSER_OPTION_VALUE_NAMES: Readonly> = { trace: 'mode', ttl: 'ms', until: 'duration', + waitUntil: 'mode', width: 'n', }; @@ -455,6 +456,7 @@ export const browserCommandCatalog: readonly HostedBrowserCommandContract[] = [ 'Create a new tab and print its target ID', 'tabs', [positional('url', 'Optional URL to open in the new tab')], + [option('waitUntil', 'Wait condition for [url]: load (default) waits for the load event, none returns as soon as navigation commits')], ), command( 'tab/select', diff --git a/src/browser/page.test.ts b/src/browser/page.test.ts index 157af61d..42fe9ec3 100644 --- a/src/browser/page.test.ts +++ b/src/browser/page.test.ts @@ -432,6 +432,19 @@ describe('Page active target tracking', () => { })); }); + it('forwards waitUntil to the tabs/new command', async () => { + sendCommandFullMock.mockResolvedValueOnce({ data: {}, page: 'page-2' }); + + const page = new Page('default'); + await page.newTab?.('https://second.example', { waitUntil: 'none' }); + + expect(sendCommandFullMock).toHaveBeenCalledWith('tabs', expect.objectContaining({ + op: 'new', + url: 'https://second.example', + waitUntil: 'none', + })); + }); + it('closes a tab by explicit page identity', async () => { sendCommandMock.mockResolvedValueOnce({ closed: 'page-2' }); diff --git a/src/browser/page.ts b/src/browser/page.ts index 927de36d..e118612e 100644 --- a/src/browser/page.ts +++ b/src/browser/page.ts @@ -224,10 +224,11 @@ export class Page extends BasePage { return Array.isArray(result) ? result : []; } - async newTab(url?: string): Promise { + async newTab(url?: string, options?: { waitUntil?: 'load' | 'none' }): Promise { const result = await sendCommandFull('tabs', { op: 'new', ...(url !== undefined && { url }), + ...(options?.waitUntil && { waitUntil: options.waitUntil }), ...this._sessionOpts(), }); this._lastUrl = null; diff --git a/src/cli.test.ts b/src/cli.test.ts index 6b98a601..51b4a469 100644 --- a/src/cli.test.ts +++ b/src/cli.test.ts @@ -1597,10 +1597,26 @@ describe('browser tab targeting commands', () => { await program.parseAsync(['node', 'webcmd', 'browser', '--session', 'test', 'tab', 'new', 'https://three.example']); - expect(browserState.page?.newTab).toHaveBeenCalledWith('https://three.example'); + expect(browserState.page?.newTab).toHaveBeenCalledWith('https://three.example', undefined); expect(consoleLogSpy.mock.calls.flat().join('\n')).toContain('"page": "tab-3"'); }); + it('passes --wait-until through to the new tab navigation', async () => { + const program = createProgram('', ''); + + await program.parseAsync(['node', 'webcmd', 'browser', '--session', 'test', 'tab', 'new', 'https://three.example', '--wait-until', 'none']); + + expect(browserState.page?.newTab).toHaveBeenCalledWith('https://three.example', { waitUntil: 'none' }); + }); + + it('rejects an unsupported --wait-until value on tab new', async () => { + const program = createProgram('', ''); + + await program.parseAsync(['node', 'webcmd', 'browser', '--session', 'test', 'tab', 'new', 'https://three.example', '--wait-until', 'networkidle']); + + expect(browserState.page?.newTab).not.toHaveBeenCalled(); + }); + it('prints the resolved target ID when browser open creates or navigates a tab', async () => { const program = createProgram('', ''); @@ -1637,7 +1653,7 @@ describe('browser tab targeting commands', () => { await program.parseAsync(['node', 'webcmd', 'browser', '--session', 'test', 'tab', 'new', 'https://three.example']); await program.parseAsync(['node', 'webcmd', 'browser', '--session', 'test', 'eval', 'document.title']); - expect(browserState.page?.newTab).toHaveBeenCalledWith('https://three.example'); + expect(browserState.page?.newTab).toHaveBeenCalledWith('https://three.example', undefined); expect(browserState.page?.setActivePage).not.toHaveBeenCalled(); expect(browserState.page?.evaluate).toHaveBeenCalledWith('document.title'); }); diff --git a/src/cli.ts b/src/cli.ts index 55237cf5..a5ae7712 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -50,6 +50,7 @@ import { configureRootCommandSurface } from './root-command-surface.js'; const CLI_FILE = fileURLToPath(import.meta.url); const BROWSER_TAB_OPTION_DESCRIPTION = 'Target tab/page identity returned by "browser open", "browser tab new", or "browser tab list"'; +const BROWSER_WAIT_UNTIL_DESCRIPTION = 'Wait condition for [url]: load (default) waits for the load event, none returns as soon as navigation commits'; const FOLLOW_POLL_MS = 1_000; type BrowserNetworkItem = { @@ -745,6 +746,13 @@ async function snapshotSourceMetrics(page: IPage, source: SnapshotSource): Promi } } +function resolveBrowserWaitUntil(opts?: { waitUntil?: string } | Command): 'load' | 'none' | undefined { + const raw = opts instanceof Command ? opts.opts().waitUntil : opts?.waitUntil; + if (raw === undefined || raw === '') return undefined; + if (raw === 'load' || raw === 'none') return raw; + throw new Error(`--wait-until must be one of: load, none. Received: "${String(raw)}"`); +} + function resolveBrowserTabTarget(targetId?: string, opts?: { tab?: string } | Command): string | undefined { if (typeof targetId === 'string' && targetId.trim()) return targetId.trim(); const tab = opts instanceof Command ? opts.opts().tab : opts?.tab; @@ -1172,12 +1180,14 @@ Examples: browserTab.command('new') .argument('[url]', 'Optional URL to open in the new tab') + .option('--wait-until ', BROWSER_WAIT_UNTIL_DESCRIPTION) .description('Create a new tab and print its target ID') - .action(browserAction(async (page, url?: string) => { + .action(browserAction(async (page, url?: string, opts?: { waitUntil?: string } | Command) => { if (!page.newTab) { throw new Error('This browser session does not support creating tabs'); } - const createdPage = await page.newTab(url); + const waitUntil = resolveBrowserWaitUntil(opts); + const createdPage = await page.newTab(url, waitUntil ? { waitUntil } : undefined); console.log(JSON.stringify({ page: createdPage, url: url ?? null, diff --git a/src/types.ts b/src/types.ts index 903379b0..1c23ba62 100644 --- a/src/types.ts +++ b/src/types.ts @@ -110,7 +110,7 @@ export interface IPage { waitForDownload?(pattern?: string, timeoutMs?: number): Promise; tabs(): Promise; closeTab?(target?: number | string): Promise; - newTab?(url?: string): Promise; + newTab?(url?: string, options?: { waitUntil?: 'load' | 'none' }): Promise; selectTab(target: number | string): Promise; networkRequests(includeStatic?: boolean): Promise; consoleMessages(level?: string): Promise;