Skip to content

feat(browser): replace abandoned playwright crate with chromiumoxide (CDP) + Rust CI - #488

Merged
lacymorrow merged 14 commits into
mainfrom
feat/replace-playwright-with-cdp
Jul 24, 2026
Merged

feat(browser): replace abandoned playwright crate with chromiumoxide (CDP) + Rust CI#488
lacymorrow merged 14 commits into
mainfrom
feat/replace-playwright-with-cdp

Conversation

@lacymorrow

@lacymorrow lacymorrow commented Jul 24, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

Summary

Replaces the abandoned playwright crate with chromiumoxide (pure-Rust CDP), which is what finally makes clean and release builds of src-tauri possible again — then runs the browser automation against a real Chrome for the first time and fixes every defect that surfaced. Along the way, restores Rust CI and pays down the test/lint debt that was blocking it.

Paperclip: LAC-3057 (landing task) · fixes LAC-3055

Eight commits, each independently reviewable (rebased on main @ 9cc7244f):

Commit What
feat(browser) Drop playwright 0.0.20 → chromiumoxide 0.9
fix(browser) Fix 4 runtime bugs found by live QA + add the live-CDP suite
test Delete 18 tests that verify nothing
style (clippy) Clear all 46 clippy warnings
style (rustfmt) Apply rustfmt repo-wide (mechanical, isolated)
ci Gate fmt + clippy + test on macOS
feat(browser) browser_extract_content gains property option — read live DOM state (.value, .checked) not just attributes (fixes LAC-3055)
fix(scripts) Constants generator survives rustfmt line-wraps — the repo-wide rustfmt commit wrapped two long event consts, the generator silently dropped them, and frontend typecheck broke (this was the Frontend (typecheck + build) CI failure)

Why the dependency swap was mandatory

playwright 0.0.20 is abandoned (newest published version). Its build script downloaded a hardcoded 2021 browser driver from playwright.azureedge.net, a CDN Microsoft has decommissioned — so its build script panics on any clean checkout. The driver cache was gated behind cfg!(debug_assertions), so release builds never used it and always attempted the dead download: bun run tauri build had been impossible for everyone. chromiumoxide speaks CDP directly with no bundled Node and no 17.7MB driver embedded in every binary.

Bugs found by running it against a real Chrome

Pure cargo check could not see any of these; they only appeared once something drove the code:

  1. cleanup() terminated the user's entire Chrome. Over a CDP attach, Browser.close kills the whole application — every window and tab — because an agent task finished. Now detaches when attached.
  2. Any clone tore down the shared browser. BrowserController is Clone with all state behind Arc and has a Drop impl; the first clone dropped killed the browser every other clone was using. Drop now bails unless it holds the last handle.
  3. Tab leak. Fixing (1) naively leaked a tab per session; now tracks owns_page and closes only pages it opened. Regression test asserts the tab count is unchanged across a session — failing in both directions.
  4. Every CDP attach would have panicked in the shipped app. chromiumoxide pulls a reqwest built with rustls but no crypto provider; that client panics on construction. Installs a default provider in BrowserController::new().

New src-tauri/tests/browser_cdp_live.rs: #[ignore]d integration tests driving real Chrome on :9222 against a fixture served from an ephemeral localhost port. 7/7 pass; verified the user's 22 open tabs survived untouched.

Test & lint cleanup (unblocks CI)

  • Deleted 18 tests that assert nothing — the entire test_fix_verification.rs module (10 tests exercising local mocks and std-library behavior, never Juno) plus 8 whose only assertion was assert!(true, "..."). Replaced a 19th (test_config_validation, which set fields and asserted nothing) with a real test of validate_tool_choice_config.
  • Cleared all 46 clippy warnings. 26 auto-fixed; the rest hand-fixed. The auto-fix itself introduced a deny-by-default build failure (overly_complex_bool_expr) that a blind --fix && commit would have shipped.
  • Applied rustfmt repo-wide (223 files) in its own commit so git blame damage is contained.
  • Fixed a latent manifest bug enabling the gate surfaced: tauri-plugin-voice-transcription/Cargo.toml declared an [[example]] pointing at a file that doesn't exist, breaking every workspace-wide cargo command.

CI

Restores a Rust (fmt + clippy + test) job, impossible until the dead build-script download was removed. Runs on macos-latest (src-tauri links Apple frameworks; Linux fails at link time). All three gates verified locally with the exact commands, in CI order, before being enabled.

Verification

Re-run in full after the rebase onto main @ 9cc7244f (which took dep upgrades #487 and CI fixes #484/#486 — rebase was clean, no conflicts):

cargo fmt --check          clean
cargo clippy --all-targets clean (was 46)
cargo test                 296 passed, 0 failed
bun run build              tsc + vite clean
browser_cdp_live           7/7 pass against real Chrome 150

Behavior parity (Rule 18): old and new BrowserController expose the identical public surface (navigate, get_current_url, extract_content, interact, screenshot, cleanup) and the identical interact actions (click, type, scroll, select); the live suite exercises each of them.

Known caveats

  • The updater-signing step needs TAURI_SIGNING_PRIVATE_KEY (a CI secret in release-tauri.yml); unrelated to code.
  • LAC-3055 (agent can't read back live DOM properties) is fixed by this PR — the property option commit. LAC-3056 (rewrite race_condition_tests.rs to test real Juno types instead of std/tokio primitives) remains a follow-up.

🤖 Generated with Claude Code

@gemini-code-assist

gemini-code-assist Bot commented Jul 24, 2026

Copy link
Copy Markdown

Caution

The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased.

@lacymorrow
lacymorrow force-pushed the feat/replace-playwright-with-cdp branch from 5841157 to 51b1732 Compare July 24, 2026 07:58
lacymorrow
lacymorrow commented Jul 24, 2026
View reviewed changes

@lacymorrow lacymorrow left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CTO pre-merge review (LAC-3057)

Reviewed all 8 commits individually (the repo-wide rustfmt commit accounts for most of the 231-file diff; the substantive change is ~6 focused commits). Approving contingent on one fix: the Rust (fmt + clippy + test) job is red — do not merge until it's green.

The CI failure (must fix before merge)

cargo fmt --check fails on src-tauri/mcp-server-os-level/src/platforms/macos/attributes.rs — import ordering of __CFArray in the core_foundation::array use statement. Root cause is toolchain skew, not a bad commit: the branch was formatted with local rustfmt 1.8.0 (rustc 1.92, Dec 2025), while CI's dtolnay/rust-toolchain@stable resolves to current stable, whose rustfmt sorts leading-underscore identifiers first. Your local cargo fmt --check was genuinely clean — against the older rustfmt.

Fix: rustup update stable, re-run cargo fmt, commit the one-hunk reorder. Also add a rust-toolchain.toml pinning the channel (and point the CI action at it) so local and CI can never disagree on formatting again — without the pin this exact failure recurs on every future stable release that touches fmt rules.

What I verified (no changes requested)

  • Cleanup semantics (bd3d18ca): detach-not-close on CDP attach, owns_page tab ownership, last-handle guard in Drop via Arc::strong_count, crypto-provider install in new(). Logic is sound and fails in the safe direction (leak a tab rather than destroy the user's browser). The strong_count > 1 early-return has a theoretical concurrent-drop race (two clones dropping simultaneously both see count 2 and neither cleans up), but the failure mode is a leaked detach — acceptable; not blocking.
  • LAC-3055 (bde1e9e7): property vs attribute distinction is correct, mutual-exclusion error is right, and the live suite has a real regression test asserting both the live .value read and the untouched static attribute (Rule 27 satisfied).
  • Deleted tests (fe2f2784): spot-checked — assert!(true) and self-referential mocks. Correct deletion.
  • Constants parser fix (51b17329): \s* around :/= is the right minimal fix; generated output confirmed to match committed file.
  • CI job (f69b757d/bd3d18ca): macOS runner justified (Apple framework linking), cache keyed on Cargo.lock, cheap gates first, live suite correctly excluded. Good.
  • Rule 18 parity: public API and interact actions are 1:1 per your enumeration; live suite 7/7 against real Chrome exercises each surface.
  • Scope (Rule 28): the rustfmt/clippy/CI-gating commits are prerequisites for gating, not scope creep — coherent and documented.

Once Rust CI is green: merge (keep the branch — LAC-1913), then verify main post-merge per the issue checklist.

lacymorrow and others added 11 commits July 24, 2026 04:33
@lacymorrow @Paperclip-Paperclip
feat(browser): replace abandoned playwright crate with chromiumoxide …
1c6f1a1 
…(CDP)

playwright 0.0.20 is abandoned upstream and unbuildable: its build script
downloads a hardcoded Playwright 1.11.0 driver (May 2021) from
playwright.azureedge.net, which Microsoft decommissioned. Every URL variant
now 404s, and Microsoft's GitHub releases carry no driver assets.

Release builds were already impossible — the crate's /tmp driver cache is
gated behind cfg!(debug_assertions), so `tauri build` always attempted the
dead download and panicked on "file size is smaller than the driver". Only
debug builds worked, and only on machines holding a stale cached driver.

Replaces it with chromiumoxide 0.9 (maintained, pure Rust, ~3M downloads):

- No bundled Node runtime and no 17.7MB driver embedded via include_bytes!
  into every Juno binary
- Drops a 2021 Chromium/Firefox/WebKit stack out of the agent's browser path
- Screenshots return bytes directly, removing the tempfile round-trip

All three connection strategies are preserved (CDP attach to a running
Chrome, persistent user profile, temp profile, fresh instance), as is the
browser/profile detection and temp-profile cleanup logic. Page operations
already went through JavaScript evaluation, so they map 1:1 onto
chromiumoxide's evaluate_function.

Notes:
- chromiumoxide returns a Handler that must be polled for the connection to
  work; spawned via tauri::async_runtime::spawn per CLAUDE.md, and aborted
  on cleanup/Drop.
- Browser::close() needs &mut, so Browser lives behind a mutex. The browser
  lock is never held while acquiring the page lock (deadlock rules).
- AppState no longer needs a playwright_driver field or its lazy init, which
  removes one lock from the browser-controller init path.

cargo check passes. Browser automation itself is untested at runtime and
needs QA against a real Chrome.

Co-Authored-By: Paperclip <noreply@paperclip.ing>
@lacymorrow
fix(browser): stop CDP cleanup destroying the user's browser; add liv…
ff3183b 
…e QA

Runtime QA of the chromiumoxide port surfaced three defects that pure
`cargo check` could not see. Adds the integration suite that found them.

Destructive cleanup on an attached browser:
`cleanup()` and `Drop` called `browser.close()` unconditionally. Over a CDP
attach that sends `Browser.close`, terminating the user's entire Chrome —
every window and tab — because an agent task finished. Both paths now detach
instead when `connection_method` is an attach.

Shared browser torn down by any clone:
`BrowserController` derives `Clone` with all state behind `Arc` *and*
implements `Drop`, so the first clone dropped tore down the browser every
surviving clone was still using. `Drop` now returns early unless it holds the
last handle.

Page ownership:
Fixing the above naively leaked a tab per session. Track `owns_page` — set
when we call `new_page`, clear when we adopt `pages[0]` — and close only
pages we opened. Verified: tab count is unchanged across a full session.

Missing rustls crypto provider:
chromiumoxide pulls reqwest 0.13 built with rustls but no bundled provider;
such a client panics *on construction*, so every CDP attach would have
panicked in the shipped app. Installs a default provider in
`BrowserController::new()` (not just `run()` — the CLI, headless mode, and
tests never call it). This is what the existing
`test_browser_controller_lazy_initialization` was asserting all along.

CI:
Restores a Rust job, unblocked now that no build script fetches a dead
artifact. Runs on macOS (src-tauri links Apple frameworks). Gates
`cargo test` only; fmt and clippy stay ungated pending pre-existing debt
(3,589 rustfmt hunks over 222 files; 46 clippy warnings, incl. 11 tests whose
only assertion is `assert!(true, ...)`), documented inline.
@lacymorrow
test: delete tests that verify nothing
92b88b6 
Removes 18 tests that passed unconditionally or exercised only the standard
library, never Juno.

`test_fix_verification.rs` (whole module, 10 tests): every test built a local
mock and asserted against it — `MockVoiceSystem::new().is_ok()`,
`Err(x).is_err()`, that `Arc<Mutex<Vec>>` accepts a push, that three threads
incrementing a mutex reach three. `test_memory_safety_patterns` had no
assertion at all. None of it touched product code; the module was a wall of
`println!("✅ ...")`.

Eight more whose only assertion was `assert!(true, "...")`:
  lib.rs::test_compilation_safety
  window_management.rs::test_window_manager_safety
  startup.rs::test_startup_sequence_creation
  state_management.rs::test_state_management_module_compilation
  state_management.rs::test_state_summary_structure
  menu/tray_menu.rs::test_get_window_states_no_panic
  commands/ui_token_selection.rs::test_initialize_ui_token_selection
  commands/permissions.rs::test_no_admin_dependency_in_permission_checks

Kept two that do call real code — `test_environment_validation_safety` and
`test_quick_startup_safety` — with the tautological assert stripped and a
comment stating what reaching the end of the function actually proves.

309 -> 291 tests, all passing. The 18 that left contributed no coverage; they
only made the suite look larger than it was.
@lacymorrow
style: clear all clippy warnings so the lint can be gated
b24c5de 
`cargo clippy --all-targets -- -D warnings` now exits clean (was 46 warnings).

26 were mechanical and applied with `cargo clippy --fix`: redundant `vec!`,
manual range checks, a redundant import, an immediately-dereferenced
reference, `== true`/`== false` comparisons.

The auto-fix introduced a build failure that needed a real fix, not a
rewrite: it turned `assert!(result == true || result == false)` into
`assert!(result || !result)`, which trips the deny-by-default
`overly_complex_bool_expr`. Both forms are tautologies. The test's own
comment says what it meant to check — that the call returns instead of
calling `std::process::exit()` and killing the test runner — so it now says
that and drops the assertion.

Six `_ => assert!(false, "Expected X")` match fallbacks in agent/structs.rs
became `other => panic!("Expected X, got {:?}", other)`, which is idiomatic
and reports what actually arrived. Two `Some(true) => assert!(true)` arms in
cloud/connector.rs became `Some(true) => {}` with the sibling arm widened to
print the unexpected value.

Also replaces a 19th test that asserted nothing. `test_config_validation`
assigned config fields under a comment reading "In a real test, we'd call
validate_tool_choice_config" — that function is 60 lines above it and
perfectly testable. It now covers both ends of the confidence-threshold
range and asserts an invalid mode is named in its own error message.

291 tests pass.
@lacymorrow
style: apply rustfmt repo-wide
5b5ee17 
Mechanical only — no behaviour change. 291 tests pass before and after, and
this is deliberately its own commit so `git blame` damage is contained to one
revision and reviewers can skip it wholesale.

223 files, previously 3,589 unformatted hunks. `cargo fmt --check` now exits
clean, which is what lets the next commit gate it in CI.

Two spots needed hands. rustfmt aborts with an internal
"left behind trailing whitespace" error rather than fixing it, so
utils/resource_manager.rs:212 and integration.rs:125 kept the whole run from
succeeding until the whitespace was stripped.

Sorting the module list in lib.rs also orphaned trailing comments: the notes
belonging to `integration` and `scheduler` were concatenated onto
`window_management`, leaving one line carrying three unrelated comments. Each
is back on the item it describes.
@lacymorrow
ci: gate rustfmt and clippy now that both pass
0719dc8 
Adds `cargo fmt --check` and `cargo clippy --all-targets -- -D warnings` to
the Rust job. Both were verified locally with the exact commands CI runs,
in CI order, before being enabled — no gate lands here unwatched.

fmt runs first because it fails in seconds without compiling anything.

Turning the gate on immediately caught a manifest bug nothing else could see:
tauri-plugin-voice-transcription/Cargo.toml declared

    [[example]]
    name = "example"
    path = "examples/example.rs"

for a file that does not exist — `examples/` contains only `basic-app.tsx`
and `integration.md`. Clippy never noticed because it resolves only the
src-tauri package's targets, while `cargo fmt --all` walks the workspace and
died on the missing path. Any workspace-wide command (`cargo package`,
`cargo fmt --all`) has been broken by this; it simply went unnoticed because
nobody ran one. Removed, with a note against re-adding it without a real file.
@lacymorrow @claude
feat(browser): add 'property' option to browser_extract_content (LAC-…
51bcc49 
…3055)

'attribute' keeps reading static markup via getAttribute; the new
'property' option reads live DOM state (.value, .checked, ...) so an
agent can see the result of its own type/toggle actions. Passing both
is an error. Live test now reads the typed value straight off the
input instead of the oninput mirror div.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lacymorrow @claude
fix(scripts): make constants parser survive rustfmt line-wraps
fc52c02 
rustfmt (now repo-wide on this branch) wraps long const definitions across
two lines; the generator's rigid '= "' regexes silently dropped those
constants from constants.generated.ts, breaking frontend typecheck.
Use \s* around ':' and '=' in both parseEventConstants and
parseSimpleConstants.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@lacymorrow @claude
chore: pin Rust toolchain to 1.97 via rust-toolchain.toml
3aae6e4 
Prevents local/CI rustfmt disagreements when stable advances. CI's
dtolnay/rust-toolchain action honors this file; fmt output is now
identical between local and CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lacymorrow @claude
chore(fmt): reorder __CFArray import for rustfmt 1.97 compatibility
d4e804f 
rustfmt 1.97 sorts leading-underscore identifiers first within import
groups. Apply the expected ordering to pass CI fmt check.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lacymorrow
ci: trigger CI run on current branch HEAD
6609b1f
@lacymorrow
lacymorrow force-pushed the feat/replace-playwright-with-cdp branch from ca88a97 to 6609b1f Compare July 24, 2026 08:34
lacymorrow and others added 3 commits July 24, 2026 04:38
@lacymorrow @claude
chore: add rustfmt and clippy components to rust-toolchain.toml
57634ba 
Ensures cargo-fmt and cargo-clippy are installed for the pinned
1.97 toolchain in all environments including CI.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lacymorrow @claude
fix(clippy): resolve 5 clippy::collapsible_match/for_kv_map/sort_by w…
8e80d65 
…arnings

Rust 1.97 clippy enforces these pre-existing lints as errors:
- sort_by with Reverse pattern → sort_by_key (self_awareness_tools, memory_manager)
- for (_, v) in map.iter_mut() → map.values_mut() (mcp_integration)
- nested if inside match arm → match guard (tool_provider, lib)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lacymorrow @claude
style: rustfmt after clippy collapsible_match fix in lib.rs
1b351b3 
Removing one level of nesting changed line lengths; rustfmt collapses
two split expressions to single lines.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lacymorrow
lacymorrow merged commit 2378ac4 into main Jul 24, 2026
5 checks passed
@lacymorrow
lacymorrow deleted the feat/replace-playwright-with-cdp branch July 24, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@lacymorrow