fix(browser): default every daemon command to a background window - #328
Merged
Conversation
Adapter/plugin commands already defaulted to `--window background`, and `web fetch` never touches a browser, but raw `webcmd browser ...` commands sent no windowMode at all. The cloak session manager reads an unset windowMode as foreground: it skips the darwin background launch path and passes `focus: true` / `bringToFront()`, so those commands stole focus. Default windowMode to `background` in the local runtime's dispatch, the single entry point every browser action goes through. Session handoff bypasses dispatch and still foregrounds via foregroundSession, and an explicit `--window foreground` or `WEBCMD_WINDOW=foreground` still wins. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
🟠 Maintainer review suggested — low confidenceThe automated review could not reach a fully supported conclusion. Limitations
This review is advisory and does not block merging. |
`stepSort` compared every key through `localeCompare(..., { numeric: true })`
with no locale pinned. ICU's numeric collation only understands runs of
digits, so it splits a decimal at the separator and its ordering of "9.5"
against "10" is platform-dependent: Windows CI put NINE ahead of TEN in
plugins/binance/test/commands.test.js while macOS and Linux did not.
Compare keys numerically when both sides parse as finite numbers, and pin
the fallback collator to 'en' so non-numeric keys are ordered the same way
everywhere.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`browser-run.test.ts` still invoked the retired positional form `browser <session> run`, which the argv preprocessor now rejects with exit code 2 before anything runs, and used a hand-written session id that the daemon has no record of. The file runs in no CI workflow — ci.yml has no e2e job and e2e-headed.yml only runs browser-tabs and cloak-runtime — so nothing caught the drift. Create a real Session through `session create` and address it with the root `--session` selector, matching browser-tabs.test.ts and cloak-runtime.test.ts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`hoistBrowserWindowOption` moved a trailing `--window <mode>` from after a browser subcommand to just before it, so Commander would attach it to the `browser` command. That option was deleted from `browser` in #196 (feat/browser-run-playwright-sandbox) when the browser surface was collapsed, and the hoister was left behind: it still relocates the flag, Commander still rejects it as unknown, so the code path cannot succeed. Its test only inspected the returned array and never handed it to Commander, which is why the rot went unnoticed. Replace it with one that asserts a recognised subcommand and its own options pass through untouched. Behaviour is unchanged — `--window` on a raw browser command errored before this commit and errors after it. Adapter commands are unaffected: they register `--window` through command-surface and never enter this function. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
rajarshidattapy
added a commit
to rajarshidattapy/webcmd
that referenced
this pull request
Aug 18, 2026
Upstream's background-window default (agentrhq#328) adds a hidden anchor page per profile launch, so the first real tab in a fresh session now lands on pages[0], not pages[1] — matching the existing "creates, selects, and closes tabs by command op" test's convention. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
No command steals your focus anymore. Run
webcmd browser ...and Chromium stays behind whatever you're doing.Four commits, independent, reviewable in order:
provider.ts— the fix (~9 lines)transform.ts— unblocks Windows CI (unrelated, splittable)browser-run.test.ts— repairs a dead e2e test (unrelated, splittable)cli-argv-preprocess.ts— deletes dead code (root cause feat: browser run playwright sandbox #196)Related issue:
1. The fix — background by default
Focus behaviour was inconsistent across the three things webcmd does:
web fetchwebcmd browser ...Raw browser commands never set
windowMode. The Cloak runtime reads unset as foreground in three places:createWindowPage→focus: windowMode !== 'background'bringToFront()guards inselectPage/bindPageopen -gonly engaged on an explicit'background'Fix: default
windowModetobackgroundinLocalCloakRuntimeProvider.dispatch, the one entry point every browser action passes through.Why not in
session-manager.ts, where the mechanism lives? Tried it. Breaks 86 unit tests that construct the manager with onlylaunchPersistentContextmocked — they'd all need a second mock injected, which mutes the darwin launch-routing signal those tests exist to catch.dispatchis the policy layer; the manager stays the mechanism.Auth handoff still foregrounds.
startSessionHandoffbypassesdispatchand callsmanager.foregroundSessiondirectly. That's the one path that must keep raising the window.Two intentional changes:
browser tabs selectandbrowser bindno longer raise the window. Test names and assertions updated.How to force foreground
--window foregroundWEBCMD_WINDOW=foregroundwebcmd browser ...No new flag on raw browser commands: it would ripple through
command-catalog→ the generatedhosted-contract.json→ cloud contract → the in-flight cloud parity PR. The env var covers it. Worth its own PR later, after parity lands.2. Windows CI —
stepSortcompared numbers as stringsPlugin tests (windows-latest)failed onbinance gainers: expected[HUNDRED, TEN, NINE], got[HUNDRED, NINE, TEN].Cause:
stepSortran every key throughlocaleCompare(..., { numeric: true })with no locale pinned. ICU numeric collation only understands runs of digits — it splits"9.5"at the decimal point and compares the9. Ordering"9.5"against"10"therefore depends on the ICU build and the system locale. Windows disagreed with macOS/Linux on exactly that pair.Fix: compare numerically when both sides parse as finite numbers; pin the fallback collator to
'en'. Regression test pins the10.0 / 9.5 / 100.0trio.3. Dead e2e test
browser-run.test.tsstill used the retiredbrowser <session> rungrammar — the argv preprocessor rejects it with exit 2 before anything runs — and a hand-written session id the daemon has no record of.It survived because it runs in no CI workflow:
ci.ymlhas no e2e job, ande2e-headed.ymlruns onlybrowser-tabsandcloak-runtime.Fix: create a real Session via
session create, address it with the root--sessionselector, matching the sibling e2e files.4. Dead argv hoister
hoistBrowserWindowOptionrelocated a trailing--window <mode>to just before the browser subcommand, so Commander would attach it tobrowser.browserstopped declaring--windowin #196, when the browser surface was collapsed. The hoister stayed. It still moves the flag; Commander still rejects it as unknown. The path cannot succeed.Its test only inspected the returned array and never handed it to Commander — which is why it stayed green for six weeks while guarding nothing. It also asserted on
browser state, a subcommand that no longer exists.Same root event as commit 1: #196 dropped the option and left the fallout behind — dead plumbing here, and no
windowModeon the wire there. Behaviour is unchanged by this commit;--windowon a raw browser command errored before and errors after.Type of Change
Reviewer: start here
src/browser/runtime/local-cloak/provider.ts— the whole fix is one ternarystartSessionHandoffstill bypassesdispatch(it does; that's the only intentional focus-steal)webcmd-cloud: nothing to do
Hosted browsers are remote Browser Use instances — no local window, no focus to steal.
router.tsalready acceptswindowModeas optional passthrough with no foreground default. The hosted contract is untouched, so the open cloud parity PR needs nothing added.Deploy note
The default lives daemon-side, so a running daemon keeps the old behaviour until its version changes.
ensureBrowserBridgeReadyauto-restarts a version-mismatched daemon, so this lands automatically on a release bump. Testing locally on the same version? Runwebcmd daemon restartonce.Checklist
Screenshots / Output
npm run build && npm run test:all— everything, including e2e:npx tsc --noEmit— clean.New coverage:
provider.test.ts— parameterised: nowindowMode→background: true, focus: false; explicitforeground→background: false, focus: truetransform.test.ts— decimal ordering pinned against collation driftcli-argv-preprocess.test.ts— replaced the hoist test with one asserting a recognised subcommand and its options pass through untouched🤖 Generated with Claude Code