Skip to content

``waitUntil: 'load'is still hardcoded innewPage— the #106 fix reachednavigateonly, sotab new --url still hangs on never-idle sites #210

Description

@rajarshidattapy

Description

#106 reported navigate hard-coding waitUntil: 'load'. That fix landed, but the same root cause has a second call site that was not covered.

On main today:

// src/browser/runtime/local-cloak/actions.ts:116  — navigate, fixed by #106/#107
await lease.page.goto(command.url, { waitUntil: command.waitUntil === 'none' ? 'commit' : 'load' });

// src/browser/runtime/local-cloak/session-manager.ts:249  — newPage, still hardcoded
await acquired.page.goto(input.url, { waitUntil: 'load' });

So webcmd browser navigate --wait-until none now works, while webcmd browser tab new --url <url> (and the hosted tab-open path) still blocks until load fires. On a site that never goes idle — a streaming dashboard, a long-poll app shell, an ad-heavy page with a hanging subresource — opening a tab still hangs, which is the exact failure #106 was filed about.

Why it was missed

The gap is structural rather than a missed edit. newPage has no waitUntil in its signature at all:

// src/browser/runtime/local-cloak/session-manager.ts:237
async newPage(input: SessionKeyInput & { url?: string }): Promise<CloakPageLease>

And its only caller drops the field, even though it is available on the command object it is reading from:

// src/browser/runtime/local-cloak/actions.ts:156-163  — 'tabs' / 'new'
const lease = await manager.newPage({
  profileId: resolveCloakCommandProfileId(manager, command),
  session: command.session,
  surface: command.surface,
  siteSession: command.siteSession,
  idleTimeout: command.idleTimeout,
  url: command.url,
  windowMode: command.windowMode,
  // command.waitUntil is never passed
});

Every other layer is already plumbed for this:

Layer Status
src/browser/protocol.ts:36 waitUntil?: 'load' | 'none' already on the command type
src/browser/base-page.ts:167 abstract goto already accepts waitUntil
src/browser/cdp.ts:210 honours it (:215, :221)
src/browser/page.ts:103 honours it (:107, :134)
session-manager.ts:237 the only type in the chain missing it

Suggested fix

Three small edits, all in the local-cloak runtime:

  1. Widen the newPage input to SessionKeyInput & { url?: string; waitUntil?: 'load' | 'none' } (session-manager.ts:237).
  2. Pass waitUntil: command.waitUntil from the tabs/new branch (actions.ts:156-163).
  3. Use it at session-manager.ts:249 instead of the hardcoded literal.

Related inconsistency worth folding in

actions.ts:116 translates 'none' into Playwright's 'commit', while cdp.ts:215 and page.ts:134 branch on the string 'none' directly. Two vocabularies for one concept across three files is part of why the second call site was easy to overlook. A single shared helper — toGotoWaitUntil(waitUntil) — would make the next such change land everywhere at once.

Verification

Checked against main (53310bf) and upstream/main; both are identical for every file named here. A full sweep of waitUntil across src/ (excluding tests) shows session-manager.ts:249 as the only remaining hardcoded 'load' on a goto call.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions