From d70a5e7457c41743a29a9b328c4b8ccf334a0517 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 10:05:08 -0600 Subject: [PATCH 01/50] fix(cli): generate usable init configs from Tailwind projects --- Cargo.lock | 18 ++-- crates/plumb-cli/src/commands/init.rs | 44 ++++++++- crates/plumb-cli/tests/init_from.rs | 43 ++++++++- .../init_from__init_from_real_project.snap | 68 ++++---------- crates/plumb-codegen/src/lib.rs | 4 +- crates/plumb-codegen/src/render.rs | 92 ++++++++++++++++--- crates/plumb-config/src/css_props.rs | 37 +++++--- crates/plumb-config/src/tailwind/mod.rs | 46 ++++++---- .../plumb-config/tests/css_props_scraper.rs | 41 +++++++++ scripts/noise-scoreboard.sh | 27 +++++- 10 files changed, 308 insertions(+), 112 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b078976..769ff1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2116,7 +2116,7 @@ dependencies = [ [[package]] name = "plumb-cdp" -version = "0.0.12" +version = "0.0.13" dependencies = [ "chromiumoxide", "criterion", @@ -2136,7 +2136,7 @@ dependencies = [ [[package]] name = "plumb-cli" -version = "0.0.12" +version = "0.0.13" dependencies = [ "anyhow", "assert_cmd", @@ -2166,7 +2166,7 @@ dependencies = [ [[package]] name = "plumb-codegen" -version = "0.0.12" +version = "0.0.13" dependencies = [ "indexmap", "insta", @@ -2179,7 +2179,7 @@ dependencies = [ [[package]] name = "plumb-config" -version = "0.0.12" +version = "0.0.13" dependencies = [ "dunce", "figment", @@ -2200,7 +2200,7 @@ dependencies = [ [[package]] name = "plumb-core" -version = "0.0.12" +version = "0.0.13" dependencies = [ "indexmap", "insta", @@ -2214,7 +2214,7 @@ dependencies = [ [[package]] name = "plumb-e2e" -version = "0.0.12" +version = "0.0.13" dependencies = [ "anyhow", "assert_cmd", @@ -2231,7 +2231,7 @@ dependencies = [ [[package]] name = "plumb-format" -version = "0.0.12" +version = "0.0.13" dependencies = [ "indexmap", "insta", @@ -2243,7 +2243,7 @@ dependencies = [ [[package]] name = "plumb-mcp" -version = "0.0.12" +version = "0.0.13" dependencies = [ "axum", "indexmap", @@ -4430,7 +4430,7 @@ checksum = "1ffae5123b2d3fc086436f8834ae3ab053a283cfac8fe0a0b8eaae044768a4c4" [[package]] name = "xtask" -version = "0.0.12" +version = "0.0.13" dependencies = [ "anyhow", "clap", diff --git a/crates/plumb-cli/src/commands/init.rs b/crates/plumb-cli/src/commands/init.rs index dbd3c11..efe8f30 100644 --- a/crates/plumb-cli/src/commands/init.rs +++ b/crates/plumb-cli/src/commands/init.rs @@ -22,7 +22,8 @@ use std::path::Path; use std::process::ExitCode; use anyhow::{Context, Result, bail}; -use plumb_codegen::{InferredConfig, infer_config, render_toml}; +use plumb_codegen::{InferredConfig, TokenSourceKind, infer_config, render_toml}; +use plumb_config::{ConfigError, TailwindOptions, merge_tailwind}; const GENERIC_TEMPLATE: &str = include_str!("../../templates/plumb.toml"); const TAILWIND_TEMPLATE: &str = include_str!("../../templates/plumb-tailwind.toml"); @@ -93,14 +94,53 @@ pub fn run(force: bool, from: Option<&Path>) -> Result { /// Walk `source_dir` and render an inferred starter TOML. fn render_from_source(source_dir: &Path) -> Result<(String, String)> { - let inferred = + let mut inferred = infer_config(source_dir).with_context(|| format!("walk {}", source_dir.display()))?; + merge_tailwind_sources(&mut inferred, source_dir)?; let content = render_toml(&inferred) .with_context(|| format!("render TOML from {}", source_dir.display()))?; let summary = summary_for_inferred(&inferred, source_dir); Ok((content, summary)) } +fn merge_tailwind_sources(inferred: &mut InferredConfig, source_dir: &Path) -> Result<()> { + let options = TailwindOptions { + cwd_root: Some(source_dir.to_path_buf()), + ..TailwindOptions::default() + }; + let mut config = std::mem::take(&mut inferred.config); + + for source in &inferred.sources { + if source.kind != TokenSourceKind::TailwindConfig { + continue; + } + + let tailwind_path = source_dir.join(&source.relative_path); + let before = config.clone(); + match merge_tailwind(config, &tailwind_path, &options) { + Ok(merged) => config = merged, + Err(ConfigError::TailwindUnavailable { .. }) => { + config = before; + break; + } + Err(ConfigError::TailwindEval { reason, .. }) + if reason.contains("TS_LOADER_MISSING") => + { + config = before; + break; + } + Err(err) => { + inferred.config = before; + return Err(err) + .with_context(|| format!("merge Tailwind config {}", tailwind_path.display())); + } + } + } + + inferred.config = config; + Ok(()) +} + fn summary_for_inferred(inferred: &InferredConfig, source_dir: &Path) -> String { if inferred.sources.is_empty() { return format!( diff --git a/crates/plumb-cli/tests/init_from.rs b/crates/plumb-cli/tests/init_from.rs index 5026d74..44f44cf 100644 --- a/crates/plumb-cli/tests/init_from.rs +++ b/crates/plumb-cli/tests/init_from.rs @@ -16,12 +16,12 @@ fn init_from_infers_starter_config_from_real_project_tree() -> Result<(), Box Result<(), Box Result<(), Box Result<(), Box Result<(), Box") .replace(&project_path, ""); - insta::assert_snapshot!("init_from_real_project", redacted); + if node_available { + insta::assert_snapshot!("init_from_real_project", redacted); + } Ok(()) } +fn node_on_path() -> bool { + std::process::Command::new("node") + .arg("--version") + .output() + .is_ok_and(|out| out.status.success()) +} + #[test] fn init_from_missing_directory_errors() -> Result<(), Box> { let outdir = TempDir::new()?; diff --git a/crates/plumb-cli/tests/snapshots/init_from__init_from_real_project.snap b/crates/plumb-cli/tests/snapshots/init_from__init_from_real_project.snap index b2d6c01..cb49b1a 100644 --- a/crates/plumb-cli/tests/snapshots/init_from__init_from_real_project.snap +++ b/crates/plumb-cli/tests/snapshots/init_from__init_from_real_project.snap @@ -1,45 +1,23 @@ --- source: crates/plumb-cli/tests/init_from.rs -assertion_line: 97 +assertion_line: 124 expression: redacted --- # Plumb configuration — bootstrapped by `plumb init --from ` from the sources below. # -# - tailwind: tailwind.config.ts +# - tailwind: tailwind.config.js # - css: src/styles/tokens.css # -# Tailwind config detected. Plumb merges Tailwind theme tokens at lint time — -# run `plumb lint` from the same directory and the adapter will resolve Tailwind's theme. - -[viewports] - -[spacing] -base_unit = 4 -scale = [ - 4, - 8, - 16, -] - -[spacing.tokens] -space-xs = 4 -space-sm = 8 -space-md = 16 - -[type] -families = [] -weights = [] -scale = [] - -[type.tokens] +# Tailwind config detected. Resolved theme tokens are included when available — +# edit the inferred values below if your project overrides them elsewhere. [color] delta_e_tolerance = 2.0 [color.tokens] +color-accent = "#0b7285" color-bg = "#ffffff" color-fg = "#0b0b0b" -color-accent = "#0b7285" [radius] scale = [ @@ -47,25 +25,19 @@ scale = [ 8, ] -[alignment] -tolerance_px = 3 - -[shadow] -scale = [] - -[z_index] -scale = [] - -[opacity] -scale = [] - -[rhythm] -base_line_px = 0 -tolerance_px = 2 -cap_height_fallback_px = 0 - -[a11y.touch_target] -min_width_px = 24 -min_height_px = 24 +[spacing] +base_unit = 4 +scale = [ + 2, + 4, + 6, + 8, + 16, +] -[rules] +[spacing.tokens] +"0.5" = 2 +"1.5" = 6 +space-md = 16 +space-sm = 8 +space-xs = 4 diff --git a/crates/plumb-codegen/src/lib.rs b/crates/plumb-codegen/src/lib.rs index fc8521a..4789315 100644 --- a/crates/plumb-codegen/src/lib.rs +++ b/crates/plumb-codegen/src/lib.rs @@ -127,8 +127,8 @@ pub struct TokenSource { #[non_exhaustive] pub enum TokenSourceKind { /// `tailwind.config.{js,ts,mjs,cjs,mts,cts}` at the project root. - /// V0 records the presence in the header comment; full theme - /// resolution is on the linter side. + /// The codegen crate records the presence in the header comment; + /// CLI callers may resolve the theme before rendering. TailwindConfig, /// CSS file containing one or more `:root` blocks. CssCustomProperties, diff --git a/crates/plumb-codegen/src/render.rs b/crates/plumb-codegen/src/render.rs index e65bd91..4675ebf 100644 --- a/crates/plumb-codegen/src/render.rs +++ b/crates/plumb-codegen/src/render.rs @@ -1,9 +1,10 @@ //! Render an [`InferredConfig`] to a `plumb.toml` string. //! //! The output starts with a generated header comment that records, in -//! sorted order, every source the inference pass consumed. The body is -//! the [`plumb_core::Config`] serialized via `toml::to_string_pretty`, -//! preserving `IndexMap` insertion order for tokens. +//! sorted order, every source the inference pass consumed. The body +//! contains only sections whose inferred values differ from +//! [`plumb_core::Config::default`], preserving runtime defaults for +//! viewports and rules. use crate::{CodegenError, InferredConfig, TokenSource, TokenSourceKind}; @@ -13,17 +14,15 @@ use crate::{CodegenError, InferredConfig, TokenSource, TokenSourceKind}; const HEADER: &str = "# Plumb configuration — bootstrapped by `plumb init --from ` from the sources below."; -/// Note appended when a Tailwind config was discovered. Plumb's -/// `extends = "./tailwind.config.*"` directive is still in flight; this -/// wording is shared with `examples/plumb-tailwind.toml`. +/// Note appended when a Tailwind config was discovered. const TAILWIND_HINT: &str = - "# Tailwind config detected. Plumb merges Tailwind theme tokens at lint time —"; + "# Tailwind config detected. Resolved theme tokens are included when available —"; /// Render `inferred` to a TOML string. /// /// The output is byte-identical given the same [`InferredConfig`]. The -/// header lists discovered source files in stable order; the body is -/// `toml::to_string_pretty` over the [`plumb_core::Config`]. +/// header lists discovered source files in stable order; the body is a +/// minimal TOML table that omits default-empty sections. /// /// # Errors /// @@ -45,12 +44,14 @@ pub fn render_toml(inferred: &InferredConfig) -> Result { out.push_str("#\n"); out.push_str(TAILWIND_HINT); out.push('\n'); - out.push_str("# run `plumb lint` from the same directory and the adapter will resolve Tailwind's theme.\n"); + out.push_str( + "# edit the inferred values below if your project overrides them elsewhere.\n", + ); } out.push('\n'); - let body = toml::to_string_pretty(&inferred.config)?; + let body = render_config_body(&inferred.config)?; out.push_str(&body); // Always end on a single newline. `toml::to_string_pretty` emits @@ -62,6 +63,75 @@ pub fn render_toml(inferred: &InferredConfig) -> Result { Ok(out) } +fn render_config_body(config: &plumb_core::Config) -> Result { + let default = plumb_core::Config::default(); + let mut table = toml::Table::new(); + + if config.viewports != default.viewports { + table.insert( + "viewports".to_owned(), + toml::Value::try_from(&config.viewports)?, + ); + } + if config.spacing != default.spacing { + table.insert( + "spacing".to_owned(), + toml::Value::try_from(&config.spacing)?, + ); + } + if config.type_scale != default.type_scale { + table.insert( + "type".to_owned(), + toml::Value::try_from(&config.type_scale)?, + ); + } + if config.color != default.color { + table.insert("color".to_owned(), toml::Value::try_from(&config.color)?); + } + if config.radius != default.radius { + table.insert("radius".to_owned(), toml::Value::try_from(&config.radius)?); + } + if config.alignment != default.alignment { + table.insert( + "alignment".to_owned(), + toml::Value::try_from(&config.alignment)?, + ); + } + if config.shadow != default.shadow { + table.insert("shadow".to_owned(), toml::Value::try_from(&config.shadow)?); + } + if config.z_index != default.z_index { + table.insert( + "z_index".to_owned(), + toml::Value::try_from(&config.z_index)?, + ); + } + if config.opacity != default.opacity { + table.insert( + "opacity".to_owned(), + toml::Value::try_from(&config.opacity)?, + ); + } + if config.rhythm != default.rhythm { + table.insert("rhythm".to_owned(), toml::Value::try_from(&config.rhythm)?); + } + if config.a11y != default.a11y { + table.insert("a11y".to_owned(), toml::Value::try_from(&config.a11y)?); + } + if config.rules != default.rules { + table.insert("rules".to_owned(), toml::Value::try_from(&config.rules)?); + } + if config.ignore != default.ignore { + table.insert("ignore".to_owned(), toml::Value::try_from(&config.ignore)?); + } + + if table.is_empty() { + Ok(String::new()) + } else { + toml::to_string_pretty(&toml::Value::Table(table)) + } +} + fn write_source_list(out: &mut String, sources: &[TokenSource]) { use std::fmt::Write as _; let mut sorted: Vec<&TokenSource> = sources.iter().collect(); diff --git a/crates/plumb-config/src/css_props.rs b/crates/plumb-config/src/css_props.rs index c90b0d6..5224685 100644 --- a/crates/plumb-config/src/css_props.rs +++ b/crates/plumb-config/src/css_props.rs @@ -1,8 +1,8 @@ //! CSS custom-properties scraper for token discovery (e.g. `plumb init`). //! //! Scans each input file for `:root { ... }` blocks at the top level or -//! wrapped inside a single `@media` / `@supports` at-rule, then extracts -//! every `--foo: ;` declaration. +//! wrapped inside a single supported at-rule (`@media`, `@supports`, or +//! `@layer`), then extracts every `--foo: ;` declaration. //! //! Values are lightly typed: //! @@ -38,9 +38,10 @@ use crate::ConfigError; pub struct CssPropertyScrape { /// Source path the declaration came from. pub source: PathBuf, - /// `None` for top-level `:root`. `Some("@media (...)")` (or - /// `"@supports (...)"`) when the `:root` block was wrapped in a - /// single at-rule. Preserves the at-rule prelude verbatim. + /// `None` for top-level `:root`. `Some("@media (...)")`, + /// `Some("@supports (...)")`, or `Some("@layer ...")` when the + /// `:root` block was wrapped in a single supported at-rule. + /// Preserves the at-rule prelude verbatim. pub at_rule: Option, /// Custom-property name, e.g. `--bg-canvas`. pub name: String, @@ -127,6 +128,9 @@ fn scrape_one( } if !parser.consume_byte_eq(b'{') { + if parser.consume_byte_eq(b';') { + continue; + } return Err(parse_error( path, contents, @@ -138,10 +142,10 @@ fn scrape_one( if is_root_selector(prelude_trimmed) { collect_root_block(&mut parser, path, contents, None, out)?; } else if let Some(at_rule) = parse_at_rule_prelude(prelude_trimmed) { - // We allow a single level of @media / @supports wrapping a - // :root block. Any other at-rule (or nested rules inside - // this one beyond a single :root) gets skipped without - // erroring — that's tolerant by design. + // We allow a single supported at-rule wrapping a :root + // block. Any nested rules inside this one beyond a direct + // :root get skipped without erroring — that's tolerant by + // design. scan_at_rule_body(&mut parser, path, contents, &at_rule, out)?; } else { // Plain selector that isn't :root — skip its block. @@ -161,8 +165,8 @@ fn is_root_selector(prelude: &str) -> bool { prelude.split_whitespace().collect::() == ":root" } -/// Recognize `@media (...)` / `@supports (...)` and return the -/// trimmed prelude (e.g. `@media (prefers-color-scheme: dark)`). +/// Recognize supported wrapper at-rules and return the trimmed prelude +/// (e.g. `@media (prefers-color-scheme: dark)`). fn parse_at_rule_prelude(prelude: &str) -> Option { let trimmed = prelude.trim(); if !trimmed.starts_with('@') { @@ -171,7 +175,7 @@ fn parse_at_rule_prelude(prelude: &str) -> Option { let (kw, _) = trimmed .split_once(|c: char| c.is_ascii_whitespace() || c == '(') .unwrap_or((trimmed, "")); - if kw == "@media" || kw == "@supports" { + if kw == "@media" || kw == "@supports" || kw == "@layer" { Some(trimmed.to_owned()) } else { None @@ -223,9 +227,9 @@ fn collect_root_block( } } -/// We're sitting at the open brace of an `@media (...)` / -/// `@supports (...)` block. Look inside for a single `:root { ... }` -/// rule and collect from it. Anything else is tolerantly skipped. +/// We're sitting at the open brace of a supported wrapper at-rule. Look +/// inside for a direct `:root { ... }` rule and collect from it. +/// Anything else is tolerantly skipped. fn scan_at_rule_body( parser: &mut Parser<'_>, path: &Path, @@ -256,6 +260,9 @@ fn scan_at_rule_body( .map_err(|fault| fault.into_error(path, contents))?; let prelude_trimmed = prelude.trim(); if !parser.consume_byte_eq(b'{') { + if parser.consume_byte_eq(b';') { + continue; + } return Err(parse_error( path, contents, diff --git a/crates/plumb-config/src/tailwind/mod.rs b/crates/plumb-config/src/tailwind/mod.rs index 71859e9..2bae345 100644 --- a/crates/plumb-config/src/tailwind/mod.rs +++ b/crates/plumb-config/src/tailwind/mod.rs @@ -642,31 +642,36 @@ fn merge_font_weight(spec: &mut TypeScaleSpec, font_weight: &serde_json::Map) { let mut seen: IndexMap = IndexMap::new(); for family in &spec.families { seen.insert(family.clone(), ()); } for value in font_family.values() { - let primary = match value { - Value::String(s) => Some(s.trim().trim_matches(['\'', '"']).to_owned()), - Value::Array(arr) => arr - .first() - .and_then(Value::as_str) - .map(|s| s.trim().trim_matches(['\'', '"']).to_owned()), - _ => None, - }; - if let Some(family) = primary - && !family.is_empty() - { - seen.insert(family, ()); + match value { + Value::String(s) => insert_font_family(&mut seen, s), + Value::Array(arr) => { + for item in arr { + if let Some(s) = item.as_str() { + insert_font_family(&mut seen, s); + } + } + } + _ => {} } } spec.families = seen.into_keys().collect(); } +fn insert_font_family(seen: &mut IndexMap, raw: &str) { + let family = raw.trim().trim_matches(['\'', '"']).to_owned(); + if !family.is_empty() { + seen.insert(family, ()); + } +} + /// Merge `theme.borderRadius` into [`RadiusSpec`]. fn merge_radius(spec: &mut RadiusSpec, radius: &serde_json::Map) { let mut values: Vec = spec.scale.clone(); @@ -849,7 +854,7 @@ mod tests { } #[test] - fn merge_font_family_keeps_primary() { + fn merge_font_family_keeps_stack_entries() { let mut spec = TypeScaleSpec::default(); let theme = serde_json::json!({ "sans": ["Inter", "ui-sans-serif", "system-ui"], @@ -857,7 +862,16 @@ mod tests { }); let map = theme.as_object().expect("object"); merge_font_family(&mut spec, map); - assert_eq!(spec.families, vec!["Inter", "JetBrains Mono"]); + assert_eq!( + spec.families, + vec![ + "Inter", + "ui-sans-serif", + "system-ui", + "JetBrains Mono", + "monospace" + ] + ); } #[test] diff --git a/crates/plumb-config/tests/css_props_scraper.rs b/crates/plumb-config/tests/css_props_scraper.rs index 21a06c3..38406d9 100644 --- a/crates/plumb-config/tests/css_props_scraper.rs +++ b/crates/plumb-config/tests/css_props_scraper.rs @@ -79,6 +79,47 @@ fn scrapes_root_inside_supports_block() { assert!(matches!(grid_gap.value, ScrapedValue::Px(16))); } +#[test] +fn scrapes_root_inside_tailwind_layer_with_directives() { + let dir = tempfile::tempdir().expect("tempdir"); + let path = dir.path().join("tailwind.css"); + std::fs::write( + &path, + "@tailwind base;\n\ + @tailwind components;\n\ + @tailwind utilities;\n\ + \n\ + @layer base {\n\ + :root {\n\ + --bg-canvas: #ffffff;\n\ + --space-2: 8px;\n\ + }\n\ + }\n\ + \n\ + @layer utilities {\n\ + .focus-ring {\n\ + @apply focus-visible:outline-none focus-visible:ring-2;\n\ + }\n\ + }\n", + ) + .expect("write css"); + + let scrapes = scrape_css_properties(&[path]).expect("scrape"); + + let bg = scrapes + .iter() + .find(|s| s.name == "--bg-canvas") + .expect("--bg-canvas in @layer base"); + assert_eq!(bg.at_rule.as_deref(), Some("@layer base")); + assert!(matches!(&bg.value, ScrapedValue::Color(c) if c == "#ffffff")); + + let space = scrapes + .iter() + .find(|s| s.name == "--space-2") + .expect("--space-2 in @layer base"); + assert!(matches!(space.value, ScrapedValue::Px(8))); +} + #[test] fn handles_comments_and_quoted_strings_in_values() { let dir = tempfile::tempdir().expect("tempdir"); diff --git a/scripts/noise-scoreboard.sh b/scripts/noise-scoreboard.sh index 2b55077..5ac729c 100755 --- a/scripts/noise-scoreboard.sh +++ b/scripts/noise-scoreboard.sh @@ -30,26 +30,43 @@ CHROME="${PLUMB_CHROME:-/Applications/Google Chrome.app/Contents/MacOS/Google Ch KITCHEN_SINK="file://$ROOT/e2e-sites/noise-kitchen-sink/dist/index.html" bucket() { # reads JSON on stdin, prints "total" + per-rule counts - python3 - <<'PY' + python3 -c ' import json,sys,collections try: d=json.load(sys.stdin) except Exception: - print(" (no JSON — lint failed)"); sys.exit(0) + print(" (no JSON — lint failed)"); sys.exit(1) s=d.get("summary",{}) -print(f" total={s.get('total','?')} (error={s.get('error',0)} warning={s.get('warning',0)} info={s.get('info',0)})") +print(" total={} (error={} warning={} info={})".format( + s.get("total", "?"), + s.get("error", 0), + s.get("warning", 0), + s.get("info", 0), +)) c=collections.Counter(v["rule_id"] for v in d.get("violations",[])) for k,v in sorted(c.items()): print(f" {v:5d} {k}") -PY +' } lint() { # $1=label $2=url [$3=config-flag...] local label="$1"; shift local url="$1"; shift + local status=0 + local output + output="$(mktemp "${TMPDIR:-/tmp}/plumb-scoreboard.XXXXXX")" rm -rf "${TMPDIR:-/tmp}/chromiumoxide-runner" 2>/dev/null || true echo "#### $label" - "$BIN" lint "$url" --executable-path "$CHROME" --format json "$@" 2>/dev/null | bucket || echo " (lint errored)" + if "$BIN" lint "$url" --executable-path "$CHROME" --format json "$@" >"$output" 2>/dev/null; then + status=0 + else + status=$? + fi + bucket <"$output" || true + rm -f "$output" + if [ "$status" -gt 1 ]; then + echo " (lint errored; exit $status)" + fi echo } From b0245449020db1ef10be7e33365d70bf9a7d5c2b Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 11:11:13 -0600 Subject: [PATCH 02/50] fix(e2e): bound lint child runs in harness --- crates/plumb-e2e/README.md | 3 + crates/plumb-e2e/src/main.rs | 6 + crates/plumb-e2e/src/runner.rs | 429 ++++++++++++++++++++++++++++++++- 3 files changed, 429 insertions(+), 9 deletions(-) diff --git a/crates/plumb-e2e/README.md b/crates/plumb-e2e/README.md index aed6d04..33ac67c 100644 --- a/crates/plumb-e2e/README.md +++ b/crates/plumb-e2e/README.md @@ -43,6 +43,9 @@ cargo run -p plumb-e2e -- --all --chrome-path /usr/bin/google-chrome-stable # Override the plumb binary path. cargo run -p plumb-e2e -- --all --plumb-bin /tmp/plumb + +# Fail a stuck child `plumb lint` run faster. +cargo run -p plumb-e2e -- --site html-css --lint-timeout-secs 30 ``` The simpler entry point is `just test-e2e`, which builds the binary diff --git a/crates/plumb-e2e/src/main.rs b/crates/plumb-e2e/src/main.rs index ef9cf15..6010e17 100644 --- a/crates/plumb-e2e/src/main.rs +++ b/crates/plumb-e2e/src/main.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; use std::process::ExitCode; +use std::time::Duration; use anyhow::Context as _; use clap::Parser; @@ -45,6 +46,10 @@ struct Cli { /// Number of lint runs to compare for byte-equality. Defaults to 3. #[arg(long, default_value_t = 3)] determinism_runs: usize, + + /// Timeout, in seconds, for each child `plumb lint` invocation. + #[arg(long, default_value_t = 120, value_parser = clap::value_parser!(u64).range(1..))] + lint_timeout_secs: u64, } fn main() -> ExitCode { @@ -74,6 +79,7 @@ fn real_main() -> Result<(), anyhow::Error> { config.chrome_path = cli.chrome_path; config.build_first = !cli.no_build; config.determinism_runs = cli.determinism_runs; + config.lint_timeout = Duration::from_secs(cli.lint_timeout_secs); let sites: Vec = if cli.all || cli.site.is_empty() { SITES.iter().map(|s| s.name.to_owned()).collect() diff --git a/crates/plumb-e2e/src/runner.rs b/crates/plumb-e2e/src/runner.rs index 7aa9401..e302dc4 100644 --- a/crates/plumb-e2e/src/runner.rs +++ b/crates/plumb-e2e/src/runner.rs @@ -1,7 +1,9 @@ //! Per-site execution. Build → serve → lint × 3 → assert. use std::path::{Path, PathBuf}; -use std::process::Command; +use std::process::{Command, ExitStatus, Stdio}; +use std::thread; +use std::time::Duration; use indexmap::IndexMap; @@ -9,6 +11,11 @@ use crate::HarnessError; use crate::expected::{Expected, WaitFor}; use crate::server::StaticServer; +const DEFAULT_LINT_TIMEOUT_SECS: u64 = 120; +const POLL_INTERVAL: Duration = Duration::from_millis(50); +const CLEANUP_RETRIES: usize = 100; +const DIAGNOSTIC_LIMIT: usize = 4096; + /// Runtime configuration for a harness invocation. #[derive(Debug, Clone)] pub struct HarnessConfig { @@ -26,6 +33,8 @@ pub struct HarnessConfig { /// How many lint runs to compare for byte-equality. The default /// is 3, matching `just determinism-check`. pub determinism_runs: usize, + /// Timeout for each child `plumb lint` invocation. + pub lint_timeout: Duration, } impl HarnessConfig { @@ -40,6 +49,7 @@ impl HarnessConfig { chrome_path: None, build_first: true, determinism_runs: 3, + lint_timeout: Duration::from_secs(DEFAULT_LINT_TIMEOUT_SECS), } } } @@ -95,7 +105,7 @@ pub fn run_site(name: &str, config: &HarnessConfig) -> Result, + run_idx: usize, ) -> Result, HarnessError> { let plumb_config = config.workspace_root.join("e2e-sites").join("plumb.toml"); let mut cmd = Command::new(&config.plumb_bin); @@ -187,27 +198,307 @@ fn run_lint( cmd.arg("--wait-for").arg(&gate.selector); cmd.arg("--wait-ms").arg(gate.timeout_ms.to_string()); } - let output = cmd.output().map_err(|err| HarnessError::Lint { + let tmp_dir = isolated_tmp_dir(name, run_idx, std::process::id()); + std::fs::create_dir_all(&tmp_dir).map_err(|err| HarnessError::Lint { site: name.to_owned(), - reason: format!("spawn plumb binary `{}`: {err}", config.plumb_bin.display()), + reason: format!("create isolated TMPDIR `{}`: {err}", tmp_dir.display()), + })?; + set_child_temp_env(&mut cmd, &tmp_dir); + let command_preview = format_command(&cmd); + tracing::info!( + site = %name, + run = run_idx, + timeout_secs = config.lint_timeout.as_secs(), + command = %command_preview, + tmp_dir = %tmp_dir.display(), + "harness — running plumb lint", + ); + + let output = run_command_with_timeout(&mut cmd, config.lint_timeout).map_err(|err| { + cleanup_tmp_dir(&tmp_dir); + HarnessError::Lint { + site: name.to_owned(), + reason: format!( + "spawn or wait for plumb binary `{}`: {err}; command={command_preview}", + config.plumb_bin.display() + ), + } })?; + cleanup_tmp_dir(&tmp_dir); + + let (status, stdout, stderr, pid) = match output { + ChildOutput::Exited { + status, + stdout, + stderr, + pid, + } => (status, stdout, stderr, pid), + ChildOutput::TimedOut { + stdout, + stderr, + pid, + } => { + return Err(HarnessError::Lint { + site: name.to_owned(), + reason: format!( + "plumb lint timed out after {:?}; pid={pid}; run={run_idx}; command={command_preview}; stdout_bytes={}; stderr=\n{}", + config.lint_timeout, + stdout.len(), + diagnostic_text(&stderr), + ), + }); + } + }; + // PRD §13.3: 0 = clean, 1 = one or more violations at/above the // default `--min-severity warn` threshold, 2 = CLI / infrastructure // failure. The fixtures intentionally produce warnings, so exit 1 is // the steady state; anything outside {0, 1} is an infrastructure // failure. - let code = output.status.code(); + let code = status.code(); let allowed = matches!(code, Some(0 | 1)); if !allowed { return Err(HarnessError::Lint { site: name.to_owned(), reason: format!( - "plumb exited with code {code:?}; stderr=\n{}", - String::from_utf8_lossy(&output.stderr), + "plumb exited with code {code:?}; pid={pid}; run={run_idx}; command={command_preview}; stdout_bytes={}; stderr=\n{}", + stdout.len(), + diagnostic_text(&stderr), ), }); } - Ok(output.stdout) + Ok(stdout) +} + +enum ChildOutput { + Exited { + status: ExitStatus, + stdout: Vec, + stderr: Vec, + pid: u32, + }, + TimedOut { + stdout: Vec, + stderr: Vec, + pid: u32, + }, +} + +fn run_command_with_timeout( + cmd: &mut Command, + timeout: Duration, +) -> Result { + configure_child_process(cmd); + let mut child = cmd + .stdin(Stdio::null()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn()?; + let pid = child.id(); + + // Reader threads must start before waiting so large JSON/stdout or + // stderr output cannot fill an OS pipe and deadlock the child. + let stdout_handle = child.stdout.take().map(spawn_reader); + let stderr_handle = child.stderr.take().map(spawn_reader); + + match wait_with_timeout(&mut child, timeout) { + WaitOutcome::Exited(status) => Ok(ChildOutput::Exited { + status, + stdout: drain_reader(stdout_handle), + stderr: drain_reader(stderr_handle), + pid, + }), + WaitOutcome::TimedOut => { + kill_child_process_tree(&mut child, pid); + Ok(ChildOutput::TimedOut { + stdout: drain_reader(stdout_handle), + stderr: drain_reader(stderr_handle), + pid, + }) + } + WaitOutcome::Errored(err) => { + kill_child_process_tree(&mut child, pid); + let _ = drain_reader(stdout_handle); + let _ = drain_reader(stderr_handle); + Err(err) + } + } +} + +#[cfg(unix)] +fn configure_child_process(cmd: &mut Command) { + use std::os::unix::process::CommandExt as _; + + cmd.process_group(0); +} + +#[cfg(not(unix))] +fn configure_child_process(_cmd: &mut Command) {} + +#[cfg(unix)] +fn kill_child_process_tree(child: &mut std::process::Child, pid: u32) { + signal_process_group(pid, "TERM"); + wait_for_child_exit(child, Duration::from_millis(500)); + signal_process_group(pid, "KILL"); + let _ = child.kill(); + let _ = child.wait(); +} + +#[cfg(not(unix))] +fn kill_child_process_tree(child: &mut std::process::Child, _pid: u32) { + let _ = child.kill(); + let _ = child.wait(); +} + +#[cfg(unix)] +fn signal_process_group(pid: u32, signal: &str) { + let group = format!("-{pid}"); + let _ = Command::new("kill") + .arg(format!("-{signal}")) + .arg(group) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) + .status(); +} + +#[cfg(unix)] +fn wait_for_child_exit(child: &mut std::process::Child, timeout: Duration) { + let max_ticks = (timeout.as_millis() / POLL_INTERVAL.as_millis()).max(1); + for _ in 0..max_ticks { + match child.try_wait() { + Ok(None) => thread::sleep(POLL_INTERVAL), + Ok(Some(_)) | Err(_) => return, + } + } +} + +fn spawn_reader(mut reader: R) -> thread::JoinHandle>> +where + R: std::io::Read + Send + 'static, +{ + thread::spawn(move || { + let mut buf = Vec::new(); + reader.read_to_end(&mut buf)?; + Ok(buf) + }) +} + +fn drain_reader(handle: Option>>>) -> Vec { + match handle { + None => Vec::new(), + Some(h) => match h.join() { + Ok(Ok(buf)) => buf, + Ok(Err(_)) | Err(_) => Vec::new(), + }, + } +} + +enum WaitOutcome { + Exited(ExitStatus), + Errored(std::io::Error), + TimedOut, +} + +fn wait_with_timeout(child: &mut std::process::Child, timeout: Duration) -> WaitOutcome { + let max_ticks = (timeout.as_millis() / POLL_INTERVAL.as_millis()).max(1); + for _ in 0..max_ticks { + match child.try_wait() { + Ok(Some(status)) => return WaitOutcome::Exited(status), + Ok(None) => thread::sleep(POLL_INTERVAL), + Err(err) => return WaitOutcome::Errored(err), + } + } + WaitOutcome::TimedOut +} + +fn isolated_tmp_dir(site: &str, run_idx: usize, process_id: u32) -> PathBuf { + let safe_site = site + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '-' || ch == '_' { + ch + } else { + '-' + } + }) + .collect::(); + short_temp_root().join(format!("pe2e-{safe_site}-{process_id}-{run_idx}")) +} + +fn short_temp_root() -> PathBuf { + #[cfg(unix)] + { + PathBuf::from("/tmp") + } + #[cfg(windows)] + { + for key in ["TMP", "TEMP"] { + if let Some(path) = std::env::var_os(key) { + return PathBuf::from(path); + } + } + PathBuf::from(r"C:\Temp") + } + #[cfg(not(any(unix, windows)))] + { + PathBuf::from("tmp") + } +} + +fn set_child_temp_env(cmd: &mut Command, tmp_dir: &Path) { + cmd.env("TMPDIR", tmp_dir); + cmd.env("TMP", tmp_dir); + cmd.env("TEMP", tmp_dir); +} + +fn cleanup_tmp_dir(path: &Path) { + for _ in 0..CLEANUP_RETRIES { + match std::fs::remove_dir_all(path) { + Ok(()) => return, + Err(err) if err.kind() == std::io::ErrorKind::NotFound => return, + Err(_) => thread::sleep(POLL_INTERVAL), + } + } + let _ = std::fs::remove_dir_all(path); +} + +fn format_command(cmd: &Command) -> String { + std::iter::once(cmd.get_program()) + .chain(cmd.get_args()) + .map(shell_quote) + .collect::>() + .join(" ") +} + +fn shell_quote(arg: &std::ffi::OsStr) -> String { + let s = arg.to_string_lossy(); + if s.is_empty() { + return String::from("''"); + } + if s.chars().all(|ch| { + ch.is_ascii_alphanumeric() || matches!(ch, '/' | '.' | '_' | '-' | ':' | '=' | ',' | '@') + }) { + return s.into_owned(); + } + format!("'{}'", s.replace('\'', "'\\''")) +} + +fn diagnostic_text(bytes: &[u8]) -> String { + let prefix = if bytes.len() > DIAGNOSTIC_LIMIT { + &bytes[..DIAGNOSTIC_LIMIT] + } else { + bytes + }; + let text = String::from_utf8_lossy(prefix); + if bytes.len() > DIAGNOSTIC_LIMIT { + format!( + "{text}\n", + bytes.len() + ) + } else { + text.into_owned() + } } #[derive(Debug)] @@ -249,7 +540,12 @@ fn parse_counts(stdout: &[u8], target_rules: &[String]) -> Result", DIAGNOSTIC_LIMIT + 2))); + } + + #[test] + fn set_child_temp_env_sets_unix_and_windows_vars() { + let mut cmd = Command::new("plumb"); + let tmp = std::path::Path::new("/workspace/target/plumb-e2e-tmp/run"); + let expected = tmp.as_os_str().to_owned(); + + set_child_temp_env(&mut cmd, tmp); + + let envs = cmd + .get_envs() + .filter_map(|(key, value)| value.map(|v| (key.to_owned(), v.to_owned()))) + .collect::>(); + assert_eq!(envs.get(std::ffi::OsStr::new("TMPDIR")), Some(&expected)); + assert_eq!(envs.get(std::ffi::OsStr::new("TMP")), Some(&expected)); + assert_eq!(envs.get(std::ffi::OsStr::new("TEMP")), Some(&expected)); + } + + #[test] + #[cfg(unix)] + fn command_timeout_kills_stuck_child() { + use std::time::Duration; + + use super::{ChildOutput, run_command_with_timeout}; + + let mut cmd = Command::new("sh"); + cmd.arg("-c").arg("printf ready; sleep 5; printf never >&2"); + + let output = + run_command_with_timeout(&mut cmd, Duration::from_millis(50)).expect("run command"); + + let ChildOutput::TimedOut { stdout, stderr, .. } = output else { + panic!("child should time out"); + }; + assert_eq!(stdout, b"ready"); + assert!(stderr.is_empty()); + } + + #[test] + #[cfg(unix)] + fn command_timeout_kills_process_group() { + use std::time::Duration; + + use super::{ChildOutput, run_command_with_timeout}; + + let mut cmd = Command::new("sh"); + cmd.arg("-c") + .arg("(trap '' TERM; printf child-ready; sleep 1; printf late) & wait"); + + let output = + run_command_with_timeout(&mut cmd, Duration::from_millis(50)).expect("run command"); + + let ChildOutput::TimedOut { stdout, .. } = output else { + panic!("child should time out"); + }; + assert_eq!(stdout, b"child-ready"); + } + + #[test] + #[cfg(unix)] + fn command_output_drains_large_stdout() { + use std::time::Duration; + + use super::{ChildOutput, run_command_with_timeout}; + + let mut cmd = Command::new("sh"); + cmd.arg("-c").arg("yes x | head -c 131072"); + + let output = + run_command_with_timeout(&mut cmd, Duration::from_secs(5)).expect("run command"); + + let ChildOutput::Exited { status, stdout, .. } = output else { + panic!("child should exit"); + }; + assert!(status.success()); + assert_eq!(stdout.len(), 131_072); + } } From 4cc2556cfeb4ef5891a62b7d78cb988a1a8910d9 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 11:47:19 -0600 Subject: [PATCH 03/50] fix(cdp): launch Chrome with new headless mode --- crates/plumb-cdp/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 0fd4ce9..af641e5 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -837,6 +837,7 @@ impl ChromiumDriver { // (scrollbar overlay differences across DPRs, OS-level scaling). let scale_factor_arg = format!("--force-device-scale-factor={}", target.device_pixel_ratio); let builder = BrowserConfig::builder() + .new_headless_mode() .chrome_detection(DetectionOptions { msedge: false, unstable: false, @@ -1173,6 +1174,7 @@ fn persistent_browser_config( // snapshot calls `Emulation.setDeviceMetricsOverride` to drive // both viewport and DPR per-call. let builder = BrowserConfig::builder() + .new_headless_mode() .chrome_detection(DetectionOptions { msedge: false, unstable: false, From 191af7131c5b5b61525d75abc67fe1b093a77dbf Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 11:55:09 -0600 Subject: [PATCH 04/50] fix(e2e): avoid process-group signals in timeout cleanup --- crates/plumb-e2e/src/runner.rs | 73 +++++++++++++++++++++++++--------- 1 file changed, 55 insertions(+), 18 deletions(-) diff --git a/crates/plumb-e2e/src/runner.rs b/crates/plumb-e2e/src/runner.rs index e302dc4..02cc389 100644 --- a/crates/plumb-e2e/src/runner.rs +++ b/crates/plumb-e2e/src/runner.rs @@ -288,7 +288,6 @@ fn run_command_with_timeout( cmd: &mut Command, timeout: Duration, ) -> Result { - configure_child_process(cmd); let mut child = cmd .stdin(Stdio::null()) .stdout(Stdio::piped()) @@ -325,21 +324,17 @@ fn run_command_with_timeout( } } -#[cfg(unix)] -fn configure_child_process(cmd: &mut Command) { - use std::os::unix::process::CommandExt as _; - - cmd.process_group(0); -} - -#[cfg(not(unix))] -fn configure_child_process(_cmd: &mut Command) {} - #[cfg(unix)] fn kill_child_process_tree(child: &mut std::process::Child, pid: u32) { - signal_process_group(pid, "TERM"); + let mut pids = child_process_tree(pid); + pids.push(pid); + signal_pids(&pids, "TERM"); wait_for_child_exit(child, Duration::from_millis(500)); - signal_process_group(pid, "KILL"); + + pids.extend(child_process_tree(pid)); + pids.sort_unstable(); + pids.dedup(); + signal_pids(&pids, "KILL"); let _ = child.kill(); let _ = child.wait(); } @@ -351,11 +346,53 @@ fn kill_child_process_tree(child: &mut std::process::Child, _pid: u32) { } #[cfg(unix)] -fn signal_process_group(pid: u32, signal: &str) { - let group = format!("-{pid}"); +fn child_process_tree(root: u32) -> Vec { + let mut seen = std::collections::BTreeSet::new(); + collect_child_processes(root, &mut seen); + seen.into_iter().collect() +} + +#[cfg(unix)] +fn collect_child_processes(parent: u32, seen: &mut std::collections::BTreeSet) { + for child in direct_child_pids(parent) { + if seen.insert(child) { + collect_child_processes(child, seen); + } + } +} + +#[cfg(unix)] +fn direct_child_pids(parent: u32) -> Vec { + let Ok(output) = Command::new("pgrep") + .arg("-P") + .arg(parent.to_string()) + .stdin(Stdio::null()) + .stderr(Stdio::null()) + .output() + else { + return Vec::new(); + }; + if !output.status.success() { + return Vec::new(); + } + String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(|line| line.trim().parse::().ok()) + .collect() +} + +#[cfg(unix)] +fn signal_pids(pids: &[u32], signal: &str) { + for pid in pids.iter().rev() { + signal_pid(*pid, signal); + } +} + +#[cfg(unix)] +fn signal_pid(pid: u32, signal: &str) { let _ = Command::new("kill") .arg(format!("-{signal}")) - .arg(group) + .arg(pid.to_string()) .stdin(Stdio::null()) .stdout(Stdio::null()) .stderr(Stdio::null()) @@ -655,14 +692,14 @@ mod tests { #[test] #[cfg(unix)] - fn command_timeout_kills_process_group() { + fn command_timeout_kills_descendant_processes() { use std::time::Duration; use super::{ChildOutput, run_command_with_timeout}; let mut cmd = Command::new("sh"); cmd.arg("-c") - .arg("(trap '' TERM; printf child-ready; sleep 1; printf late) & wait"); + .arg("(trap '' TERM; printf child-ready; while :; do sleep 10; done) & wait"); let output = run_command_with_timeout(&mut cmd, Duration::from_millis(50)).expect("run command"); From 9d15b1ea8a017e0583d7f6c89d17a3877af25459 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 12:14:01 -0600 Subject: [PATCH 05/50] fix(cdp): avoid double-waiting after page navigation --- crates/plumb-cdp/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index af641e5..fc41db2 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -962,7 +962,6 @@ async fn capture_on_page( let storage_state = pre_navigate(page, target, options).await?; page.goto(target.url.as_str()).await.map_err(driver_error)?; - page.wait_for_navigation().await.map_err(driver_error)?; apply_post_navigate_waits(page, target).await?; apply_storage_state_local_storage(page, target, storage_state.as_ref()).await?; From f2c363f697039d0d165ce0a4abf6aef9b4997b87 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 13:36:29 -0600 Subject: [PATCH 06/50] fix(cdp): avoid chromiumoxide navigation hangs --- crates/plumb-cdp/Cargo.toml | 2 +- crates/plumb-cdp/src/lib.rs | 364 +++++++++++++++++++++++++++++++++--- 2 files changed, 338 insertions(+), 28 deletions(-) diff --git a/crates/plumb-cdp/Cargo.toml b/crates/plumb-cdp/Cargo.toml index 109948f..dcde80d 100644 --- a/crates/plumb-cdp/Cargo.toml +++ b/crates/plumb-cdp/Cargo.toml @@ -52,12 +52,12 @@ serde_json = { workspace = true } url = { workspace = true } # SHA-256 verification of auto-fetched Chromium binaries (issue #78). sha2 = { workspace = true } +tempfile = { workspace = true } [dev-dependencies] plumb-core = { workspace = true, features = ["test-fake"] } tokio = { workspace = true, features = ["test-util"] } serde_json = { workspace = true } -tempfile = { workspace = true } tracing-subscriber = { workspace = true } criterion = { workspace = true } diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index fc41db2..dfd0d4d 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -60,21 +60,25 @@ use plumb_core::{PlumbSnapshot, ViewportKey}; use std::io; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; +use tempfile::TempDir; use chromiumoxide::Page; +use chromiumoxide::browser::BrowserConfigBuilder; use chromiumoxide::cdp::browser_protocol::browser::CloseParams as BrowserCloseParams; use chromiumoxide::cdp::browser_protocol::dom_snapshot::{ CaptureSnapshotParams, CaptureSnapshotReturns, DocumentSnapshot, }; use chromiumoxide::cdp::browser_protocol::emulation::SetDeviceMetricsOverrideParams; use chromiumoxide::cdp::browser_protocol::network::{ - CookieParam, Headers, SetCookiesParams, SetExtraHttpHeadersParams, + CookieParam, EnableParams as NetworkEnableParams, EventLoadingFailed, Headers, ResourceType, + SetCookiesParams, SetExtraHttpHeadersParams, }; use chromiumoxide::cdp::browser_protocol::page::AddScriptToEvaluateOnNewDocumentParams; use chromiumoxide::cdp::browser_protocol::target::{ CreateBrowserContextParams, CreateTargetParams, }; use chromiumoxide::detection::DetectionOptions; +use chromiumoxide::listeners::EventStream; use chromiumoxide::{Browser, BrowserConfig, Handler}; use futures_util::StreamExt; use serde::Deserialize; @@ -777,11 +781,9 @@ pub struct ChromiumOptions { /// Explicit Chrome or Chromium executable path. When unset, Plumb asks /// `chromiumoxide` to detect stable Chrome/Chromium installations. pub executable_path: Option, - /// Override the Chromium profile directory. When unset, `chromiumoxide` - /// reuses a single temp directory across all launches — which is fine - /// for sequential CLI invocations but causes profile-singleton lock - /// contention when multiple drivers run concurrently (e.g. the e2e - /// test suite). Tests pass per-thread tempdirs here. + /// Override the Chromium profile directory. When unset, Plumb creates an + /// isolated temporary profile per browser launch so concurrent or + /// back-to-back drivers never contend on Chromium's SingletonLock. /// /// Profile contents do not flow into [`PlumbSnapshot`] output, so /// varying this path does not violate the determinism invariant. @@ -821,6 +823,11 @@ pub struct ChromiumDriver { options: ChromiumOptions, } +struct ChromiumLaunch { + config: BrowserConfig, + profile_dir: Option, +} + impl ChromiumDriver { /// Build a driver with explicit options. #[must_use] @@ -832,7 +839,7 @@ impl ChromiumDriver { &self, target: &Target, resolved_executable: Option<&Path>, - ) -> Result { + ) -> Result { // PRD §16: pinning launch args removes a class of nondeterminism // (scrollbar overlay differences across DPRs, OS-level scaling). let scale_factor_arg = format!("--force-device-scale-factor={}", target.device_pixel_ratio); @@ -865,13 +872,14 @@ impl ChromiumDriver { builder }; - let builder = if let Some(profile) = &self.options.user_data_dir { - builder.user_data_dir(profile) - } else { - builder - }; + let (builder, profile_dir) = + apply_user_data_dir(builder, self.options.user_data_dir.as_deref())?; - builder.build().map_err(|_| chromium_not_found()) + let config = builder.build().map_err(|_| chromium_not_found())?; + Ok(ChromiumLaunch { + config, + profile_dir, + }) } } @@ -901,8 +909,8 @@ impl BrowserDriver for ChromiumDriver { // factor for every page after the first. let first = &targets[0]; let resolved_executable = resolve_auto_fetch(&self.options).await?; - let config = self.browser_config(first, resolved_executable.as_deref())?; - let mut session = ChromiumSession::launch(config).await?; + let launch = self.browser_config(first, resolved_executable.as_deref())?; + let mut session = ChromiumSession::launch(launch).await?; let result: Result, CdpError> = async { validate_browser_version(&session.browser).await?; @@ -961,7 +969,7 @@ async fn capture_on_page( // cookie installation and localStorage replay. let storage_state = pre_navigate(page, target, options).await?; - page.goto(target.url.as_str()).await.map_err(driver_error)?; + navigate_page(page, target.url.as_str()).await?; apply_post_navigate_waits(page, target).await?; apply_storage_state_local_storage(page, target, storage_state.as_ref()).await?; @@ -1002,6 +1010,7 @@ pub struct PersistentBrowser { struct PersistentBrowserInner { browser: Browser, handler_task: Mutex>>, + _profile_dir: Option, options: ChromiumOptions, } @@ -1021,8 +1030,10 @@ impl PersistentBrowser { /// range, or [`CdpError::Driver`] for any other launch failure. pub async fn launch(options: ChromiumOptions) -> Result { let resolved_executable = resolve_auto_fetch(&options).await?; - let config = persistent_browser_config(&options, resolved_executable.as_deref())?; - let (browser, handler) = Browser::launch(config).await.map_err(map_launch_error)?; + let launch = persistent_browser_config(&options, resolved_executable.as_deref())?; + let (browser, handler) = Browser::launch(launch.config) + .await + .map_err(map_launch_error)?; let handler_task = poll_handler(handler); // Validate the version before stashing the browser in `Arc` — @@ -1038,6 +1049,7 @@ impl PersistentBrowser { inner: Arc::new(PersistentBrowserInner { browser, handler_task: Mutex::new(Some(handler_task)), + _profile_dir: launch.profile_dir, options, }), }) @@ -1163,10 +1175,30 @@ impl BrowserDriver for PersistentBrowser { } } +fn apply_user_data_dir( + builder: BrowserConfigBuilder, + user_data_dir: Option<&Path>, +) -> Result<(BrowserConfigBuilder, Option), CdpError> { + if let Some(profile) = user_data_dir { + return Ok((builder.user_data_dir(profile), None)); + } + + let profile = tempfile::Builder::new() + .prefix("plumb-chromium-") + .tempdir() + .map_err(|err| { + CdpError::Driver(Box::new(io::Error::other(format!( + "create isolated Chromium profile: {err}" + )))) + })?; + let builder = builder.user_data_dir(profile.path()); + Ok((builder, Some(profile))) +} + fn persistent_browser_config( options: &ChromiumOptions, resolved_executable: Option<&Path>, -) -> Result { +) -> Result { // PRD §16: pinning launch args removes a class of nondeterminism // (scrollbar overlay differences across DPRs, OS-level scaling). // `PersistentBrowser` does not fix a launch-time DPR — every @@ -1194,13 +1226,13 @@ fn persistent_browser_config( builder }; - let builder = if let Some(profile) = &options.user_data_dir { - builder.user_data_dir(profile) - } else { - builder - }; + let (builder, profile_dir) = apply_user_data_dir(builder, options.user_data_dir.as_deref())?; - builder.build().map_err(|_| chromium_not_found()) + let config = builder.build().map_err(|_| chromium_not_found())?; + Ok(ChromiumLaunch { + config, + profile_dir, + }) } /// When auto-fetch is enabled and the user didn't pin an @@ -1295,6 +1327,139 @@ async fn pre_navigate( Ok(storage_state) } +#[derive(Debug, Deserialize)] +struct NavigationState { + href: String, + #[serde(rename = "readyState")] + ready_state: String, + #[serde(rename = "isChromeErrorPage", default)] + is_chrome_error_page: bool, +} + +async fn navigate_page(page: &Page, url: &str) -> Result<(), CdpError> { + if uses_chromiumoxide_goto(url) { + page.goto(url).await.map_err(driver_error)?; + return Ok(()); + } + + let mut navigation_failures = page + .event_listener::() + .await + .map_err(driver_error)?; + page.execute(NetworkEnableParams::default()) + .await + .map_err(driver_error)?; + + let script = navigation_assignment_script(url)?; + let initial_result = page.evaluate(script.as_str()).await.map_err(driver_error); + + wait_for_document_ready(page, url, initial_result.err(), &mut navigation_failures).await +} + +fn uses_chromiumoxide_goto(url: &str) -> bool { + url.starts_with("file://") +} + +fn navigation_assignment_script(url: &str) -> Result { + let quoted_url = serde_json::to_string(url).map_err(|err| { + CdpError::Driver(Box::new(io::Error::other(format!( + "serialize navigation URL: {err}" + )))) + })?; + Ok(format!("window.location.assign({quoted_url});")) +} + +async fn wait_for_document_ready( + page: &Page, + display_url: &str, + initial_error: Option, + navigation_failures: &mut EventStream, +) -> Result<(), CdpError> { + let attempt = async { + loop { + tokio::select! { + biased; + + failure = navigation_failures.next() => { + if let Some(failure) = failure + && document_navigation_failed(&failure) + { + return Err(navigation_failed_error(display_url, &failure.error_text)); + } + } + () = tokio::time::sleep(std::time::Duration::from_millis(50)) => { + match read_navigation_state(page).await { + Ok(state) if state.is_chrome_error_page => { + return Err(chrome_error_page_error(display_url, &state.href)); + } + Ok(state) if document_is_loaded(&state) => return Ok(()), + Ok(_) | Err(_) => {} + } + } + } + } + }; + + if let Ok(result) = tokio::time::timeout(std::time::Duration::from_secs(10), attempt).await { + return result; + } + + let mut reason = format!("navigation to `{display_url}` exhausted 10s ready-state budget"); + if let Some(err) = initial_error { + reason.push_str(" after initial location assignment failed: "); + reason.push_str(&err.to_string()); + } + Err(CdpError::Driver(Box::new(io::Error::other(reason)))) +} + +fn document_navigation_failed(failure: &EventLoadingFailed) -> bool { + failure.r#type == ResourceType::Document && failure.canceled != Some(true) +} + +fn navigation_failed_error(display_url: &str, error_text: &str) -> CdpError { + CdpError::Driver(Box::new(io::Error::other(format!( + "navigation to `{display_url}` failed: {error_text}" + )))) +} + +fn chrome_error_page_error(display_url: &str, error_href: &str) -> CdpError { + CdpError::Driver(Box::new(io::Error::other(format!( + "navigation to `{display_url}` failed: Chrome rendered error page `{error_href}`" + )))) +} + +fn document_is_loaded(state: &NavigationState) -> bool { + state.href != "about:blank" && state.ready_state == "complete" && !state.is_chrome_error_page +} + +async fn read_navigation_state(page: &Page) -> Result { + let result = page + .evaluate( + "JSON.stringify({ + href: window.location.href, + readyState: document.readyState, + isChromeErrorPage: window.location.protocol === 'chrome-error:' + || document.getElementById('main-frame-error') !== null + })", + ) + .await + .map_err(driver_error)?; + let raw: String = result.into_value().map_err(|err| { + CdpError::Driver(Box::new(io::Error::other(format!( + "read navigation state: {err}" + )))) + })?; + parse_navigation_state(&raw) +} + +fn parse_navigation_state(raw: &str) -> Result { + serde_json::from_str(raw).map_err(|err| { + CdpError::Driver(Box::new(io::Error::other(format!( + "parse navigation state `{raw}`: {err}" + )))) + }) +} + /// Wait stages that must run *after* navigation. PRD §15 — `--wait-for` /// and `--wait-ms`. /// @@ -1646,15 +1811,19 @@ fn chromium_install_hint() -> String { struct ChromiumSession { browser: Browser, handler_task: JoinHandle<()>, + profile_dir: Option, } impl ChromiumSession { - async fn launch(config: BrowserConfig) -> Result { - let (browser, handler) = Browser::launch(config).await.map_err(map_launch_error)?; + async fn launch(launch: ChromiumLaunch) -> Result { + let (browser, handler) = Browser::launch(launch.config) + .await + .map_err(map_launch_error)?; let handler_task = poll_handler(handler); Ok(Self { browser, handler_task, + profile_dir: launch.profile_dir, }) } @@ -1665,6 +1834,7 @@ impl ChromiumSession { tracing::debug!(error = %kill_err, "failed to kill Chromium after close error"); } self.abort_handler().await; + let _profile_dir = self.profile_dir.take(); return Err(close_err); } @@ -1674,10 +1844,12 @@ impl ChromiumSession { tracing::debug!(error = %kill_err, "failed to kill Chromium after wait error"); } self.abort_handler().await; + let _profile_dir = self.profile_dir.take(); return Err(cleanup_err); } self.abort_handler().await; + let _profile_dir = self.profile_dir.take(); Ok(()) } @@ -2360,6 +2532,8 @@ fn rect_from_bounds(inner: &[f64]) -> Rect { #[cfg(test)] mod tests { + use std::path::PathBuf; + use super::{ COMPUTED_STYLE_WHITELIST, CdpError, MAX_SUPPORTED_CHROMIUM_MAJOR, MIN_SUPPORTED_CHROMIUM_MAJOR, @@ -2764,6 +2938,142 @@ mod tests { assert!((t.effective_dpr() - 3.0).abs() < f64::EPSILON); } + #[test] + fn browser_config_creates_isolated_profile_by_default() { + let driver = super::ChromiumDriver::new(super::ChromiumOptions { + executable_path: Some(PathBuf::from("/bin/echo")), + ..super::ChromiumOptions::default() + }); + let launch = match driver.browser_config(&super::Target::default(), None) { + Ok(launch) => launch, + Err(err) => panic!("browser config failed: {err}"), + }; + + assert!(launch.profile_dir.is_some()); + let Some(configured) = launch.config.user_data_dir.as_deref() else { + panic!("expected generated user data dir"); + }; + assert!(configured.exists()); + } + + #[test] + fn browser_config_preserves_explicit_profile() { + let profile = match tempfile::tempdir() { + Ok(profile) => profile, + Err(err) => panic!("tempdir failed: {err}"), + }; + let driver = super::ChromiumDriver::new(super::ChromiumOptions { + executable_path: Some(PathBuf::from("/bin/echo")), + user_data_dir: Some(profile.path().to_path_buf()), + ..super::ChromiumOptions::default() + }); + let launch = match driver.browser_config(&super::Target::default(), None) { + Ok(launch) => launch, + Err(err) => panic!("browser config failed: {err}"), + }; + + assert!(launch.profile_dir.is_none()); + assert_eq!(launch.config.user_data_dir.as_deref(), Some(profile.path())); + } + + #[test] + fn navigation_assignment_script_json_escapes_url() { + let script = match super::navigation_assignment_script("https://example.com/a\"b\nc") { + Ok(script) => script, + Err(err) => panic!("script generation failed: {err}"), + }; + assert_eq!( + script, + "window.location.assign(\"https://example.com/a\\\"b\\nc\");" + ); + } + + #[test] + fn file_urls_keep_chromiumoxide_goto_path() { + assert!(super::uses_chromiumoxide_goto("file:///tmp/static.html")); + assert!(!super::uses_chromiumoxide_goto("http://127.0.0.1:49197/")); + assert!(!super::uses_chromiumoxide_goto("https://example.com/")); + } + + #[test] + fn document_load_wait_accepts_redirected_complete_document_only() { + assert!(super::document_is_loaded(&super::NavigationState { + href: "https://example.com/login".to_string(), + ready_state: "complete".to_string(), + is_chrome_error_page: false, + })); + assert!(!super::document_is_loaded(&super::NavigationState { + href: "https://example.com/login".to_string(), + ready_state: "interactive".to_string(), + is_chrome_error_page: false, + })); + assert!(!super::document_is_loaded(&super::NavigationState { + href: "about:blank".to_string(), + ready_state: "complete".to_string(), + is_chrome_error_page: false, + })); + assert!(!super::document_is_loaded(&super::NavigationState { + href: "chrome-error://chromewebdata/".to_string(), + ready_state: "complete".to_string(), + is_chrome_error_page: true, + })); + } + + #[test] + fn document_navigation_failed_rejects_only_uncanceled_documents() { + let document_failure = parse_loading_failed( + r#"{"requestId":"1","timestamp":1.0,"type":"Document","errorText":"net::ERR_CONNECTION_REFUSED"}"#, + ); + assert!(super::document_navigation_failed(&document_failure)); + + let canceled_document = parse_loading_failed( + r#"{"requestId":"1","timestamp":1.0,"type":"Document","errorText":"net::ERR_ABORTED","canceled":true}"#, + ); + assert!(!super::document_navigation_failed(&canceled_document)); + + let stylesheet_failure = parse_loading_failed( + r#"{"requestId":"1","timestamp":1.0,"type":"Stylesheet","errorText":"net::ERR_CONNECTION_REFUSED"}"#, + ); + assert!(!super::document_navigation_failed(&stylesheet_failure)); + } + + #[test] + fn parse_navigation_state_reads_href_and_ready_state() { + let state = match super::parse_navigation_state( + r#"{"href":"http://127.0.0.1:49197/","readyState":"complete"}"#, + ) { + Ok(state) => state, + Err(err) => panic!("navigation state parse failed: {err}"), + }; + assert_eq!(state.href, "http://127.0.0.1:49197/"); + assert_eq!(state.ready_state, "complete"); + assert!(!state.is_chrome_error_page); + } + + #[test] + fn parse_navigation_state_reads_chrome_error_page_marker() { + let state = match super::parse_navigation_state( + r#"{"href":"chrome-error://chromewebdata/","readyState":"complete","isChromeErrorPage":true}"#, + ) { + Ok(state) => state, + Err(err) => panic!("navigation state parse failed: {err}"), + }; + assert!(state.is_chrome_error_page); + } + + #[test] + fn parse_navigation_state_rejects_malformed_json() { + let err = super::parse_navigation_state("not json"); + assert!(matches!(err, Err(CdpError::Driver(_)))); + } + + fn parse_loading_failed(raw: &str) -> super::EventLoadingFailed { + match serde_json::from_str(raw) { + Ok(event) => event, + Err(err) => panic!("loading failed event parse failed: {err}"), + } + } + #[test] fn origin_of_handles_https_url() { assert_eq!( From 69999b6b403c29204ab9a083e8159b20a8777323 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 13:44:52 -0600 Subject: [PATCH 07/50] test(cdp): avoid unix-only executable fixture --- crates/plumb-cdp/src/lib.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index dfd0d4d..9c0cb8b 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -2941,7 +2941,7 @@ mod tests { #[test] fn browser_config_creates_isolated_profile_by_default() { let driver = super::ChromiumDriver::new(super::ChromiumOptions { - executable_path: Some(PathBuf::from("/bin/echo")), + executable_path: Some(test_executable_path()), ..super::ChromiumOptions::default() }); let launch = match driver.browser_config(&super::Target::default(), None) { @@ -2963,7 +2963,7 @@ mod tests { Err(err) => panic!("tempdir failed: {err}"), }; let driver = super::ChromiumDriver::new(super::ChromiumOptions { - executable_path: Some(PathBuf::from("/bin/echo")), + executable_path: Some(test_executable_path()), user_data_dir: Some(profile.path().to_path_buf()), ..super::ChromiumOptions::default() }); @@ -3074,6 +3074,13 @@ mod tests { } } + fn test_executable_path() -> PathBuf { + match std::env::current_exe() { + Ok(path) => path, + Err(err) => panic!("current executable path unavailable: {err}"), + } + } + #[test] fn origin_of_handles_https_url() { assert_eq!( From 914c184241dae833b7a5ac9a42e5865398ee3996 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 15:05:14 -0600 Subject: [PATCH 08/50] fix(cdp): clean up failed persistent launches --- crates/plumb-cdp/src/lib.rs | 49 +++++++++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 8 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 9c0cb8b..3e12909 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1031,17 +1031,26 @@ impl PersistentBrowser { pub async fn launch(options: ChromiumOptions) -> Result { let resolved_executable = resolve_auto_fetch(&options).await?; let launch = persistent_browser_config(&options, resolved_executable.as_deref())?; - let (browser, handler) = Browser::launch(launch.config) - .await - .map_err(map_launch_error)?; + let ChromiumLaunch { + config, + profile_dir, + } = launch; + let (mut browser, handler) = Browser::launch(config).await.map_err(map_launch_error)?; let handler_task = poll_handler(handler); // Validate the version before stashing the browser in `Arc` — - // on failure, dropping the browser here causes - // `Browser::drop` to reap the child synchronously. + // on failure, explicitly close/wait before the isolated + // profile is dropped so Windows does not retain locked files. if let Err(err) = validate_browser_version(&browser).await { - handler_task.abort(); - drop(browser); + if let Err(cleanup_err) = + cleanup_failed_persistent_launch(&mut browser, handler_task).await + { + tracing::debug!( + error = %cleanup_err, + "failed to clean up Chromium after version validation failure" + ); + } + let _profile_dir = profile_dir; return Err(err); } @@ -1049,7 +1058,7 @@ impl PersistentBrowser { inner: Arc::new(PersistentBrowserInner { browser, handler_task: Mutex::new(Some(handler_task)), - _profile_dir: launch.profile_dir, + _profile_dir: profile_dir, options, }), }) @@ -1873,6 +1882,30 @@ fn poll_handler(mut handler: Handler) -> JoinHandle<()> { }) } +async fn cleanup_failed_persistent_launch( + browser: &mut Browser, + handler_task: JoinHandle<()>, +) -> Result<(), CdpError> { + let close_result = browser.close().await.map_err(driver_error); + if close_result.is_err() + && let Err(kill_err) = kill_browser(browser).await + { + tracing::debug!(error = %kill_err, "failed to kill Chromium after close error"); + } + + let wait_result = browser.wait().await.map_err(io_error); + handler_task.abort(); + if let Err(join_err) = handler_task.await + && !join_err.is_cancelled() + { + tracing::debug!(error = %join_err, "Chromium handler task failed"); + } + + close_result?; + wait_result?; + Ok(()) +} + async fn kill_browser(browser: &mut Browser) -> Result<(), CdpError> { if let Some(result) = browser.kill().await { result.map_err(io_error)?; From 55af8848a4594d4dce6bc66ccfcbe325c2623082 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 15:41:02 -0600 Subject: [PATCH 09/50] fix(cdp): bound browser waits in capture flow --- crates/plumb-cdp/src/lib.rs | 296 ++++++++++++++++++++++++++---------- crates/plumb-mcp/src/lib.rs | 4 +- 2 files changed, 218 insertions(+), 82 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 3e12909..c8cda5f 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -57,9 +57,11 @@ use indexmap::IndexMap; use plumb_core::report::Rect; use plumb_core::snapshot::{SnapshotNode, TextBox}; use plumb_core::{PlumbSnapshot, ViewportKey}; +use std::future::Future; use std::io; use std::path::{Path, PathBuf}; use std::sync::{Arc, Mutex}; +use std::time::Duration; use tempfile::TempDir; use chromiumoxide::Page; @@ -93,6 +95,16 @@ pub const MIN_SUPPORTED_CHROMIUM_MAJOR: u32 = 131; /// constant after running the e2e suite against the new major. pub const MAX_SUPPORTED_CHROMIUM_MAJOR: u32 = 150; +const BROWSER_LAUNCH_TIMEOUT: Duration = Duration::from_secs(30); +const BROWSER_CLOSE_TIMEOUT: Duration = Duration::from_secs(5); +const BROWSER_WAIT_TIMEOUT: Duration = Duration::from_secs(5); +const BROWSER_KILL_TIMEOUT: Duration = Duration::from_secs(5); +const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); +const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); +const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); +const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); +const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(30); + /// CSS property whitelist passed to `DOMSnapshot.captureSnapshot` as the /// `computedStyles` argument. /// @@ -939,10 +951,10 @@ async fn capture_target( target: &Target, options: &ChromiumOptions, ) -> Result { - let page = browser - .new_page("about:blank") - .await - .map_err(driver_error)?; + let page = with_timeout("Target.createTarget", CDP_CONTROL_TIMEOUT, async { + browser.new_page("about:blank").await.map_err(driver_error) + }) + .await?; capture_on_page(&page, target, options).await } @@ -985,7 +997,12 @@ async fn capture_on_page( include_text_color_opacities: None, }; - let response = page.execute(params).await.map_err(driver_error)?; + let response = with_timeout( + "DOMSnapshot.captureSnapshot", + SNAPSHOT_CAPTURE_TIMEOUT, + async { page.execute(params).await.map_err(driver_error) }, + ) + .await?; flatten_snapshot(target, &response.result) } @@ -1035,7 +1052,11 @@ impl PersistentBrowser { config, profile_dir, } = launch; - let (mut browser, handler) = Browser::launch(config).await.map_err(map_launch_error)?; + let (mut browser, handler) = + with_timeout("Chromium launch", BROWSER_LAUNCH_TIMEOUT, async { + Browser::launch(config).await.map_err(map_launch_error) + }) + .await?; let handler_task = poll_handler(handler); // Validate the version before stashing the browser in `Arc` — @@ -1073,12 +1094,14 @@ impl PersistentBrowser { /// [`CdpError::MalformedSnapshot`] when the response cannot be /// flattened. pub async fn snapshot(&self, target: Target) -> Result { - let ctx_id = self - .inner - .browser - .create_browser_context(CreateBrowserContextParams::default()) - .await - .map_err(driver_error)?; + let ctx_id = with_timeout("Target.createBrowserContext", CDP_CONTROL_TIMEOUT, async { + self.inner + .browser + .create_browser_context(CreateBrowserContextParams::default()) + .await + .map_err(driver_error) + }) + .await?; let result: Result = async { let create_params = CreateTargetParams { @@ -1095,12 +1118,14 @@ impl PersistentBrowser { for_tab: None, hidden: None, }; - let page = self - .inner - .browser - .new_page(create_params) - .await - .map_err(driver_error)?; + let page = with_timeout("Target.createTarget", CDP_CONTROL_TIMEOUT, async { + self.inner + .browser + .new_page(create_params) + .await + .map_err(driver_error) + }) + .await?; capture_on_page(&page, &target, &self.inner.options).await } .await; @@ -1108,12 +1133,14 @@ impl PersistentBrowser { // Always dispose the incognito context, even on failure. Mirror // the swallow-and-log pattern from `ChromiumSession::shutdown` // so cleanup errors never mask the underlying snapshot result. - if let Err(err) = self - .inner - .browser - .dispose_browser_context(ctx_id) - .await - .map_err(driver_error) + if let Err(err) = with_timeout("Target.disposeBrowserContext", CDP_CONTROL_TIMEOUT, async { + self.inner + .browser + .dispose_browser_context(ctx_id) + .await + .map_err(driver_error) + }) + .await { tracing::debug!(error = %err, "failed to dispose incognito browser context"); } @@ -1347,20 +1374,43 @@ struct NavigationState { async fn navigate_page(page: &Page, url: &str) -> Result<(), CdpError> { if uses_chromiumoxide_goto(url) { - page.goto(url).await.map_err(driver_error)?; + with_timeout("Page.navigate", DOCUMENT_READY_TIMEOUT, async { + page.goto(url).await.map(|_| ()).map_err(driver_error) + }) + .await?; return Ok(()); } - let mut navigation_failures = page - .event_listener::() - .await - .map_err(driver_error)?; - page.execute(NetworkEnableParams::default()) - .await - .map_err(driver_error)?; + let mut navigation_failures = with_timeout( + "Network.loadingFailed listener", + CDP_CONTROL_TIMEOUT, + async { + page.event_listener::() + .await + .map_err(driver_error) + }, + ) + .await?; + with_timeout("Network.enable", CDP_CONTROL_TIMEOUT, async { + page.execute(NetworkEnableParams::default()) + .await + .map(|_| ()) + .map_err(driver_error) + }) + .await?; let script = navigation_assignment_script(url)?; - let initial_result = page.evaluate(script.as_str()).await.map_err(driver_error); + let initial_result = with_timeout( + "navigation location assignment", + NAVIGATION_ASSIGNMENT_TIMEOUT, + async { + page.evaluate(script.as_str()) + .await + .map(|_| ()) + .map_err(driver_error) + }, + ) + .await; wait_for_document_ready(page, url, initial_result.err(), &mut navigation_failures).await } @@ -1384,6 +1434,7 @@ async fn wait_for_document_ready( initial_error: Option, navigation_failures: &mut EventStream, ) -> Result<(), CdpError> { + let mut last_state_error = None; let attempt = async { loop { tokio::select! { @@ -1396,28 +1447,42 @@ async fn wait_for_document_ready( return Err(navigation_failed_error(display_url, &failure.error_text)); } } - () = tokio::time::sleep(std::time::Duration::from_millis(50)) => { - match read_navigation_state(page).await { - Ok(state) if state.is_chrome_error_page => { + () = tokio::time::sleep(Duration::from_millis(50)) => { + match tokio::time::timeout( + NAVIGATION_STATE_READ_TIMEOUT, + read_navigation_state(page), + ) + .await + { + Ok(Ok(state)) if state.is_chrome_error_page => { return Err(chrome_error_page_error(display_url, &state.href)); } - Ok(state) if document_is_loaded(&state) => return Ok(()), - Ok(_) | Err(_) => {} + Ok(Ok(state)) if document_is_loaded(&state) => return Ok(()), + Ok(Ok(_)) => {} + Ok(Err(err)) => { + last_state_error = Some(err.to_string()); + } + Err(_) => { + last_state_error = Some(timeout_reason( + "navigation state read", + NAVIGATION_STATE_READ_TIMEOUT, + )); + } } } } } }; - if let Ok(result) = tokio::time::timeout(std::time::Duration::from_secs(10), attempt).await { + if let Ok(result) = tokio::time::timeout(DOCUMENT_READY_TIMEOUT, attempt).await { return result; } - let mut reason = format!("navigation to `{display_url}` exhausted 10s ready-state budget"); - if let Some(err) = initial_error { - reason.push_str(" after initial location assignment failed: "); - reason.push_str(&err.to_string()); - } + let reason = navigation_ready_timeout_reason( + display_url, + initial_error.as_ref().map(ToString::to_string).as_deref(), + last_state_error.as_deref(), + ); Err(CdpError::Driver(Box::new(io::Error::other(reason)))) } @@ -1469,6 +1534,26 @@ fn parse_navigation_state(raw: &str) -> Result { }) } +fn navigation_ready_timeout_reason( + display_url: &str, + initial_error: Option<&str>, + last_state_error: Option<&str>, +) -> String { + let mut reason = format!( + "navigation to `{display_url}` exhausted {} ready-state budget", + timeout_budget_label(DOCUMENT_READY_TIMEOUT) + ); + if let Some(err) = initial_error { + reason.push_str(" after initial location assignment failed: "); + reason.push_str(err); + } + if let Some(err) = last_state_error { + reason.push_str("; last navigation state read failed: "); + reason.push_str(err); + } + reason +} + /// Wait stages that must run *after* navigation. PRD §15 — `--wait-for` /// and `--wait-ms`. /// @@ -1825,9 +1910,12 @@ struct ChromiumSession { impl ChromiumSession { async fn launch(launch: ChromiumLaunch) -> Result { - let (browser, handler) = Browser::launch(launch.config) - .await - .map_err(map_launch_error)?; + let (browser, handler) = with_timeout("Chromium launch", BROWSER_LAUNCH_TIMEOUT, async { + Browser::launch(launch.config) + .await + .map_err(map_launch_error) + }) + .await?; let handler_task = poll_handler(handler); Ok(Self { browser, @@ -1837,29 +1925,11 @@ impl ChromiumSession { } async fn shutdown(&mut self) -> Result<(), CdpError> { - let close_result = self.browser.close().await.map_err(driver_error); - if let Err(close_err) = close_result { - if let Err(kill_err) = kill_browser(&mut self.browser).await { - tracing::debug!(error = %kill_err, "failed to kill Chromium after close error"); - } - self.abort_handler().await; - let _profile_dir = self.profile_dir.take(); - return Err(close_err); - } - - if let Err(wait_err) = self.browser.wait().await { - let cleanup_err = io_error(wait_err); - if let Err(kill_err) = kill_browser(&mut self.browser).await { - tracing::debug!(error = %kill_err, "failed to kill Chromium after wait error"); - } - self.abort_handler().await; - let _profile_dir = self.profile_dir.take(); - return Err(cleanup_err); - } + let cleanup_result = close_browser_best_effort(&mut self.browser).await; self.abort_handler().await; let _profile_dir = self.profile_dir.take(); - Ok(()) + cleanup_result } async fn abort_handler(&mut self) { @@ -1882,18 +1952,43 @@ fn poll_handler(mut handler: Handler) -> JoinHandle<()> { }) } +async fn with_timeout(operation: &str, timeout: Duration, future: F) -> Result +where + F: Future>, +{ + match tokio::time::timeout(timeout, future).await { + Ok(result) => result, + Err(_) => Err(timeout_error(operation, timeout)), + } +} + +fn timeout_error(operation: &str, timeout: Duration) -> CdpError { + CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + timeout_reason(operation, timeout), + ))) +} + +fn timeout_reason(operation: &str, timeout: Duration) -> String { + format!( + "{operation} exceeded {} budget", + timeout_budget_label(timeout) + ) +} + +fn timeout_budget_label(timeout: Duration) -> String { + if timeout.as_millis().is_multiple_of(1_000) { + format!("{}s", timeout.as_secs()) + } else { + format!("{}ms", timeout.as_millis()) + } +} + async fn cleanup_failed_persistent_launch( browser: &mut Browser, handler_task: JoinHandle<()>, ) -> Result<(), CdpError> { - let close_result = browser.close().await.map_err(driver_error); - if close_result.is_err() - && let Err(kill_err) = kill_browser(browser).await - { - tracing::debug!(error = %kill_err, "failed to kill Chromium after close error"); - } - - let wait_result = browser.wait().await.map_err(io_error); + let close_result = close_browser_best_effort(browser).await; handler_task.abort(); if let Err(join_err) = handler_task.await && !join_err.is_cancelled() @@ -1901,20 +1996,50 @@ async fn cleanup_failed_persistent_launch( tracing::debug!(error = %join_err, "Chromium handler task failed"); } - close_result?; - wait_result?; + close_result +} + +async fn close_browser_best_effort(browser: &mut Browser) -> Result<(), CdpError> { + if let Err(err) = with_timeout("Browser.close", BROWSER_CLOSE_TIMEOUT, async { + browser.close().await.map_err(driver_error) + }) + .await + { + tracing::debug!(error = %err, "failed to close Chromium"); + } + + if let Err(wait_err) = with_timeout("Chromium process wait", BROWSER_WAIT_TIMEOUT, async { + browser.wait().await.map_err(io_error) + }) + .await + { + tracing::debug!(error = %wait_err, "failed to wait for Chromium process"); + kill_browser(browser).await?; + with_timeout( + "Chromium process wait after kill", + BROWSER_WAIT_TIMEOUT, + async { browser.wait().await.map_err(io_error) }, + ) + .await?; + } Ok(()) } async fn kill_browser(browser: &mut Browser) -> Result<(), CdpError> { - if let Some(result) = browser.kill().await { + if let Some(result) = tokio::time::timeout(BROWSER_KILL_TIMEOUT, browser.kill()) + .await + .map_err(|_| timeout_error("Chromium kill", BROWSER_KILL_TIMEOUT))? + { result.map_err(io_error)?; } Ok(()) } async fn validate_browser_version(browser: &Browser) -> Result<(), CdpError> { - let version = browser.version().await.map_err(driver_error)?; + let version = with_timeout("Browser.version", CDP_CONTROL_TIMEOUT, async { + browser.version().await.map_err(driver_error) + }) + .await?; validate_chromium_product_major(&version.product) } @@ -3100,6 +3225,19 @@ mod tests { assert!(matches!(err, Err(CdpError::Driver(_)))); } + #[test] + fn navigation_ready_timeout_reason_preserves_stage_errors() { + let reason = super::navigation_ready_timeout_reason( + "http://127.0.0.1:49197/", + Some("navigation location assignment exceeded 2s budget"), + Some("navigation state read exceeded 2s budget"), + ); + + assert!(reason.contains("exhausted 30s ready-state budget")); + assert!(reason.contains("after initial location assignment failed")); + assert!(reason.contains("last navigation state read failed")); + } + fn parse_loading_failed(raw: &str) -> super::EventLoadingFailed { match serde_json::from_str(raw) { Ok(event) => event, diff --git a/crates/plumb-mcp/src/lib.rs b/crates/plumb-mcp/src/lib.rs index 9b47f4a..d868525 100644 --- a/crates/plumb-mcp/src/lib.rs +++ b/crates/plumb-mcp/src/lib.rs @@ -1252,9 +1252,7 @@ async fn authenticate_http_request( /// on transport errors. pub async fn run_stdio(cwd: PathBuf) -> Result<(), McpError> { let handler = PlumbServer::new(cwd); - let service = handler - .clone() - .serve(stdio()) + let service = Box::pin(handler.clone().serve(stdio())) .await .map_err(|e| McpError::Service(e.to_string()))?; let service_result = service From 6a1214303b7494221f13b8d4686e9b90a4cee847 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 15:51:01 -0600 Subject: [PATCH 10/50] fix(cdp): allow slower target creation --- crates/plumb-cdp/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index c8cda5f..027d1dd 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -100,6 +100,7 @@ const BROWSER_CLOSE_TIMEOUT: Duration = Duration::from_secs(5); const BROWSER_WAIT_TIMEOUT: Duration = Duration::from_secs(5); const BROWSER_KILL_TIMEOUT: Duration = Duration::from_secs(5); const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); +const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(30); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); @@ -951,7 +952,7 @@ async fn capture_target( target: &Target, options: &ChromiumOptions, ) -> Result { - let page = with_timeout("Target.createTarget", CDP_CONTROL_TIMEOUT, async { + let page = with_timeout("Target.createTarget", TARGET_CREATE_TIMEOUT, async { browser.new_page("about:blank").await.map_err(driver_error) }) .await?; @@ -1118,7 +1119,7 @@ impl PersistentBrowser { for_tab: None, hidden: None, }; - let page = with_timeout("Target.createTarget", CDP_CONTROL_TIMEOUT, async { + let page = with_timeout("Target.createTarget", TARGET_CREATE_TIMEOUT, async { self.inner .browser .new_page(create_params) From d59c0860074fefd5103571743507c9a6798fdf5c Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 16:02:55 -0600 Subject: [PATCH 11/50] fix(cdp): avoid waiting on blank page load --- crates/plumb-cdp/src/lib.rs | 41 +++++++++++++++++++++++++++---------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 027d1dd..ca077da 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -101,6 +101,7 @@ const BROWSER_WAIT_TIMEOUT: Duration = Duration::from_secs(5); const BROWSER_KILL_TIMEOUT: Duration = Duration::from_secs(5); const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(30); +const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(30); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); @@ -952,12 +953,37 @@ async fn capture_target( target: &Target, options: &ChromiumOptions, ) -> Result { - let page = with_timeout("Target.createTarget", TARGET_CREATE_TIMEOUT, async { - browser.new_page("about:blank").await.map_err(driver_error) + let page = + create_page_without_load_wait(browser, CreateTargetParams::new("about:blank")).await?; + + capture_on_page(&page, target, options).await +} + +async fn create_page_without_load_wait( + browser: &Browser, + params: CreateTargetParams, +) -> Result { + let target_id = with_timeout("Target.createTarget", TARGET_CREATE_TIMEOUT, async { + browser + .execute(params) + .await + .map(|response| response.result.target_id) + .map_err(driver_error) }) .await?; - capture_on_page(&page, target, options).await + with_timeout("Target.attachToTarget", TARGET_ATTACH_TIMEOUT, async { + loop { + match browser.get_page(target_id.clone()).await { + Ok(page) => return Ok(page), + Err(chromiumoxide::error::CdpError::NotFound) => { + tokio::time::sleep(Duration::from_millis(50)).await; + } + Err(err) => return Err(driver_error(err)), + } + } + }) + .await } /// Apply viewport / animation hooks, install cookies and headers, @@ -1119,14 +1145,7 @@ impl PersistentBrowser { for_tab: None, hidden: None, }; - let page = with_timeout("Target.createTarget", TARGET_CREATE_TIMEOUT, async { - self.inner - .browser - .new_page(create_params) - .await - .map_err(driver_error) - }) - .await?; + let page = create_page_without_load_wait(&self.inner.browser, create_params).await?; capture_on_page(&page, &target, &self.inner.options).await } .await; From 77a3ca1f716bfe47f26d8601449396972cee1032 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 16:11:35 -0600 Subject: [PATCH 12/50] fix(cdp): extend chromiumoxide request budget --- crates/plumb-cdp/src/lib.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index ca077da..f67ae10 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -99,6 +99,7 @@ const BROWSER_LAUNCH_TIMEOUT: Duration = Duration::from_secs(30); const BROWSER_CLOSE_TIMEOUT: Duration = Duration::from_secs(5); const BROWSER_WAIT_TIMEOUT: Duration = Duration::from_secs(5); const BROWSER_KILL_TIMEOUT: Duration = Duration::from_secs(5); +const CHROMIUMOXIDE_REQUEST_TIMEOUT: Duration = Duration::from_mins(1); const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(30); const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(30); @@ -863,6 +864,7 @@ impl ChromiumDriver { msedge: false, unstable: false, }) + .request_timeout(CHROMIUMOXIDE_REQUEST_TIMEOUT) .window_size(target.width, target.height) .arg("--hide-scrollbars") .arg(scale_factor_arg); @@ -1266,6 +1268,7 @@ fn persistent_browser_config( msedge: false, unstable: false, }) + .request_timeout(CHROMIUMOXIDE_REQUEST_TIMEOUT) .window_size(1280, 800) .arg("--hide-scrollbars"); From a8485780d21934562ddc57253ffe0e1eba137c00 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 16:29:33 -0600 Subject: [PATCH 13/50] fix(cdp): allow slower target attachment --- crates/plumb-cdp/src/lib.rs | 37 +++++++++++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index f67ae10..ffe44ee 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -102,7 +102,7 @@ const BROWSER_KILL_TIMEOUT: Duration = Duration::from_secs(5); const CHROMIUMOXIDE_REQUEST_TIMEOUT: Duration = Duration::from_mins(1); const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(30); -const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(30); +const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(75); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); @@ -1980,11 +1980,32 @@ where F: Future>, { match tokio::time::timeout(timeout, future).await { - Ok(result) => result, + Ok(result) => result.map_err(|err| contextualize_request_timeout(operation, err)), Err(_) => Err(timeout_error(operation, timeout)), } } +fn contextualize_request_timeout(operation: &str, err: CdpError) -> CdpError { + let CdpError::Driver(source) = &err else { + return err; + }; + + if matches!( + source.downcast_ref::(), + Some(chromiumoxide::error::CdpError::Timeout) + ) { + return CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + format!( + "{operation} hit Chromiumoxide request budget ({})", + timeout_budget_label(CHROMIUMOXIDE_REQUEST_TIMEOUT) + ), + ))); + } + + err +} + fn timeout_error(operation: &str, timeout: Duration) -> CdpError { CdpError::Driver(Box::new(io::Error::new( io::ErrorKind::TimedOut, @@ -3261,6 +3282,18 @@ mod tests { assert!(reason.contains("last navigation state read failed")); } + #[test] + fn contextualize_request_timeout_labels_operation() { + let err = super::contextualize_request_timeout( + "Target.attachToTarget", + CdpError::Driver(Box::new(chromiumoxide::error::CdpError::Timeout)), + ); + + let message = err.to_string(); + assert!(message.contains("Target.attachToTarget")); + assert!(message.contains("Chromiumoxide request budget")); + } + fn parse_loading_failed(raw: &str) -> super::EventLoadingFailed { match serde_json::from_str(raw) { Ok(event) => event, From b529fce4d854d496cb5147c9d16d79178ead3b21 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 16:47:22 -0600 Subject: [PATCH 14/50] fix(cdp): retry transient capture timeouts --- crates/plumb-cdp/src/lib.rs | 144 +++++++++++++++++++++++++++++++++--- 1 file changed, 133 insertions(+), 11 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index ffe44ee..4ad00a2 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -103,10 +103,12 @@ const CHROMIUMOXIDE_REQUEST_TIMEOUT: Duration = Duration::from_mins(1); const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(30); const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(75); +const PAGE_COMMAND_TIMEOUT: Duration = Duration::from_secs(25); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); -const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(30); +const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(25); +const TRANSIENT_CAPTURE_RETRIES: usize = 1; /// CSS property whitelist passed to `DOMSnapshot.captureSnapshot` as the /// `computedStyles` argument. @@ -917,6 +919,28 @@ impl BrowserDriver for ChromiumDriver { return Ok(Vec::new()); } + let mut attempts = 0; + loop { + let result = self.snapshot_all_once(&targets).await; + if attempts < TRANSIENT_CAPTURE_RETRIES + && result + .as_ref() + .err() + .is_some_and(is_retryable_capture_timeout) + { + if let Err(err) = &result { + tracing::debug!(attempt = attempts + 1, error = %err, "retrying Chromium capture after transient timeout"); + } + attempts += 1; + continue; + } + return result; + } + } +} + +impl ChromiumDriver { + async fn snapshot_all_once(&self, targets: &[Target]) -> Result, CdpError> { // Use the first target's dimensions and DPR for the initial // launch (the `--force-device-scale-factor` arg is fixed at // launch time). Per-target viewport / DPR is then applied via @@ -931,7 +955,7 @@ impl BrowserDriver for ChromiumDriver { let result: Result, CdpError> = async { validate_browser_version(&session.browser).await?; let mut snapshots = Vec::with_capacity(targets.len()); - for target in &targets { + for target in targets { let snap = capture_target(&session.browser, target, &self.options).await?; snapshots.push(snap); } @@ -1123,6 +1147,26 @@ impl PersistentBrowser { /// [`CdpError::MalformedSnapshot`] when the response cannot be /// flattened. pub async fn snapshot(&self, target: Target) -> Result { + let mut attempts = 0; + loop { + let result = self.snapshot_once(&target).await; + if attempts < TRANSIENT_CAPTURE_RETRIES + && result + .as_ref() + .err() + .is_some_and(is_retryable_capture_timeout) + { + if let Err(err) = &result { + tracing::debug!(attempt = attempts + 1, error = %err, "retrying persistent Chromium capture after transient timeout"); + } + attempts += 1; + continue; + } + return result; + } + } + + async fn snapshot_once(&self, target: &Target) -> Result { let ctx_id = with_timeout("Target.createBrowserContext", CDP_CONTROL_TIMEOUT, async { self.inner .browser @@ -1148,7 +1192,7 @@ impl PersistentBrowser { hidden: None, }; let page = create_page_without_load_wait(&self.inner.browser, create_params).await?; - capture_on_page(&page, &target, &self.inner.options).await + capture_on_page(&page, target, &self.inner.options).await } .await; @@ -1334,7 +1378,12 @@ async fn apply_viewport(page: &Page, target: &Target) -> Result<(), CdpError> { screen_orientation: None, viewport: None, }; - page.execute(params).await.map_err(driver_error)?; + with_timeout( + "Emulation.setDeviceMetricsOverride", + PAGE_COMMAND_TIMEOUT, + async { page.execute(params).await.map(|_| ()).map_err(driver_error) }, + ) + .await?; Ok(()) } @@ -1634,7 +1683,17 @@ async fn apply_storage_state_local_storage( } })?; let script = format!("window.localStorage.setItem({key}, {value});"); - page.evaluate(script.as_str()).await.map_err(driver_error)?; + with_timeout( + "Runtime.evaluate localStorage", + PAGE_COMMAND_TIMEOUT, + async { + page.evaluate(script.as_str()) + .await + .map(|_| ()) + .map_err(driver_error) + }, + ) + .await?; } } Ok(()) @@ -1727,7 +1786,12 @@ async fn add_script_to_evaluate_on_new_document(page: &Page, source: &str) -> Re include_command_line_api: None, run_immediately: Some(true), }; - page.execute(params).await.map_err(driver_error)?; + with_timeout( + "Page.addScriptToEvaluateOnNewDocument", + PAGE_COMMAND_TIMEOUT, + async { page.execute(params).await.map(|_| ()).map_err(driver_error) }, + ) + .await?; Ok(()) } @@ -1749,7 +1813,10 @@ async fn install_extra_headers(page: &Page, headers: &[(String, String)]) -> Res object.insert(name, serde_json::Value::String(value)); } let params = SetExtraHttpHeadersParams::new(Headers::new(serde_json::Value::Object(object))); - page.execute(params).await.map_err(driver_error)?; + with_timeout("Network.setExtraHTTPHeaders", PAGE_COMMAND_TIMEOUT, async { + page.execute(params).await.map(|_| ()).map_err(driver_error) + }) + .await?; Ok(()) } @@ -1790,7 +1857,10 @@ async fn install_cookies( .map(|c| c.into_cdp_param(url_for_cookies)) .collect(), ); - page.execute(params).await.map_err(driver_error)?; + with_timeout("Network.setCookies", PAGE_COMMAND_TIMEOUT, async { + page.execute(params).await.map(|_| ()).map_err(driver_error) + }) + .await?; Ok(()) } @@ -1807,9 +1877,17 @@ async fn install_storage_state_cookies(page: &Page, state: &StorageState) -> Res p.http_only = Some(cookie.http_only); params.push(p); } - page.execute(SetCookiesParams::new(params)) - .await - .map_err(driver_error)?; + with_timeout( + "Network.setCookies storageState", + PAGE_COMMAND_TIMEOUT, + async { + page.execute(SetCookiesParams::new(params)) + .await + .map(|_| ()) + .map_err(driver_error) + }, + ) + .await?; Ok(()) } @@ -2006,6 +2084,23 @@ fn contextualize_request_timeout(operation: &str, err: CdpError) -> CdpError { err } +fn is_retryable_capture_timeout(err: &CdpError) -> bool { + let CdpError::Driver(source) = err else { + return false; + }; + + if matches!( + source.downcast_ref::(), + Some(chromiumoxide::error::CdpError::Timeout) + ) { + return true; + } + + source + .downcast_ref::() + .is_some_and(|err| err.kind() == io::ErrorKind::TimedOut) +} + fn timeout_error(operation: &str, timeout: Duration) -> CdpError { CdpError::Driver(Box::new(io::Error::new( io::ErrorKind::TimedOut, @@ -2734,6 +2829,7 @@ fn rect_from_bounds(inner: &[f64]) -> Rect { #[cfg(test)] mod tests { + use std::io; use std::path::PathBuf; use super::{ @@ -3294,6 +3390,32 @@ mod tests { assert!(message.contains("Chromiumoxide request budget")); } + #[test] + fn retryable_capture_timeout_accepts_chromiumoxide_timeout() { + let err = CdpError::Driver(Box::new(chromiumoxide::error::CdpError::Timeout)); + + assert!(super::is_retryable_capture_timeout(&err)); + } + + #[test] + fn retryable_capture_timeout_accepts_plumb_timed_out_io() { + let err = CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "Emulation.setDeviceMetricsOverride exceeded 25s budget", + ))); + + assert!(super::is_retryable_capture_timeout(&err)); + } + + #[test] + fn retryable_capture_timeout_rejects_non_timeout_errors() { + let err = CdpError::MalformedSnapshot { + reason: "missing document".to_owned(), + }; + + assert!(!super::is_retryable_capture_timeout(&err)); + } + fn parse_loading_failed(raw: &str) -> super::EventLoadingFailed { match serde_json::from_str(raw) { Ok(event) => event, From 0dd6825bb2e70ae0f9c66e1c64902d2061854255 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 17:15:13 -0600 Subject: [PATCH 15/50] fix(cdp): use launch viewport for first capture --- crates/plumb-cdp/src/lib.rs | 96 ++++++++++++++++++++++++++++++++----- 1 file changed, 83 insertions(+), 13 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 4ad00a2..cd1ae85 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -23,10 +23,11 @@ //! //! [`ChromiumDriver::snapshot_all`] launches Chromium exactly once, //! validates [`Browser::version`](chromiumoxide::Browser::version), -//! and then loops over the requested targets — for each it opens a -//! fresh page, applies the per-target viewport via CDP -//! `Emulation.setDeviceMetricsOverride`, navigates to the URL, and -//! calls `DOMSnapshot.captureSnapshot` with the +//! and then loops over the requested targets — the first target's +//! viewport is pinned at launch through Chromiumoxide's viewport +//! configuration, later targets and explicit DPR pins are applied via +//! CDP `Emulation.setDeviceMetricsOverride`, then Plumb navigates to +//! the URL and calls `DOMSnapshot.captureSnapshot` with the //! [`COMPUTED_STYLE_WHITELIST`] from PRD §10.3. Each CDP response is //! flattened into a [`PlumbSnapshot`] with deterministic ordering //! (nodes sorted by `dom_order`, computed styles inserted in @@ -80,6 +81,7 @@ use chromiumoxide::cdp::browser_protocol::target::{ CreateBrowserContextParams, CreateTargetParams, }; use chromiumoxide::detection::DetectionOptions; +use chromiumoxide::handler::viewport::Viewport as ChromiumViewport; use chromiumoxide::listeners::EventStream; use chromiumoxide::{Browser, BrowserConfig, Handler}; use futures_util::StreamExt; @@ -868,6 +870,7 @@ impl ChromiumDriver { }) .request_timeout(CHROMIUMOXIDE_REQUEST_TIMEOUT) .window_size(target.width, target.height) + .viewport(chromiumoxide_viewport(target)) .arg("--hide-scrollbars") .arg(scale_factor_arg); @@ -943,10 +946,9 @@ impl ChromiumDriver { async fn snapshot_all_once(&self, targets: &[Target]) -> Result, CdpError> { // Use the first target's dimensions and DPR for the initial // launch (the `--force-device-scale-factor` arg is fixed at - // launch time). Per-target viewport / DPR is then applied via - // CDP `Emulation.setDeviceMetricsOverride` inside - // `capture_target`, which overrides the launch-time scale - // factor for every page after the first. + // launch time). The first unpinned target can reuse that + // launch-pinned viewport; later targets, and explicit `--dpr` + // pins, still need CDP `Emulation.setDeviceMetricsOverride`. let first = &targets[0]; let resolved_executable = resolve_auto_fetch(&self.options).await?; let launch = self.browser_config(first, resolved_executable.as_deref())?; @@ -955,8 +957,14 @@ impl ChromiumDriver { let result: Result, CdpError> = async { validate_browser_version(&session.browser).await?; let mut snapshots = Vec::with_capacity(targets.len()); - for target in targets { - let snap = capture_target(&session.browser, target, &self.options).await?; + for (target_index, target) in targets.iter().enumerate() { + let snap = capture_target( + &session.browser, + target, + &self.options, + should_apply_viewport_override(target_index, target), + ) + .await?; snapshots.push(snap); } Ok(snapshots) @@ -978,11 +986,12 @@ async fn capture_target( browser: &Browser, target: &Target, options: &ChromiumOptions, + apply_viewport_override: bool, ) -> Result { let page = create_page_without_load_wait(browser, CreateTargetParams::new("about:blank")).await?; - capture_on_page(&page, target, options).await + capture_on_page(&page, target, options, apply_viewport_override).await } async fn create_page_without_load_wait( @@ -1025,8 +1034,11 @@ async fn capture_on_page( page: &Page, target: &Target, options: &ChromiumOptions, + apply_viewport_override: bool, ) -> Result { - apply_viewport(page, target).await?; + if apply_viewport_override { + apply_viewport(page, target).await?; + } // `pre_navigate` returns the parsed `StorageState` (when one is // configured) so the post-navigate localStorage step reuses the // same parsed value. Loading the file twice would open a @@ -1192,7 +1204,7 @@ impl PersistentBrowser { hidden: None, }; let page = create_page_without_load_wait(&self.inner.browser, create_params).await?; - capture_on_page(&page, target, &self.inner.options).await + capture_on_page(&page, target, &self.inner.options, true).await } .await; @@ -1360,6 +1372,21 @@ async fn resolve_auto_fetch(options: &ChromiumOptions) -> Result Ok(Some(path)) } +fn should_apply_viewport_override(target_index: usize, target: &Target) -> bool { + target_index != 0 || target.pin_dpr.is_some() +} + +fn chromiumoxide_viewport(target: &Target) -> ChromiumViewport { + ChromiumViewport { + width: target.width, + height: target.height, + device_scale_factor: Some(f64::from(target.device_pixel_ratio)), + emulating_mobile: false, + is_landscape: target.width >= target.height, + has_touch: false, + } +} + async fn apply_viewport(page: &Page, target: &Target) -> Result<(), CdpError> { // `pin_dpr` (PRD §15 — `--dpr`) wins over `device_pixel_ratio` so // that callers can stress determinism by pinning a hidpi factor @@ -3236,6 +3263,49 @@ mod tests { assert!((t.effective_dpr() - 3.0).abs() < f64::EPSILON); } + #[test] + fn viewport_override_skips_first_unpinned_target() { + let target = super::Target::default(); + + assert!(!super::should_apply_viewport_override(0, &target)); + } + + #[test] + fn viewport_override_applies_to_first_pinned_target() { + let target = super::Target { + pin_dpr: Some(2.0), + ..super::Target::default() + }; + + assert!(super::should_apply_viewport_override(0, &target)); + } + + #[test] + fn viewport_override_applies_to_later_targets() { + let target = super::Target::default(); + + assert!(super::should_apply_viewport_override(1, &target)); + } + + #[test] + fn chromiumoxide_viewport_uses_target_dimensions_and_dpr() { + let target = super::Target { + width: 390, + height: 844, + device_pixel_ratio: 3.0, + ..super::Target::default() + }; + + let viewport = super::chromiumoxide_viewport(&target); + + assert_eq!(viewport.width, 390); + assert_eq!(viewport.height, 844); + assert_eq!(viewport.device_scale_factor, Some(3.0)); + assert!(!viewport.is_landscape); + assert!(!viewport.emulating_mobile); + assert!(!viewport.has_touch); + } + #[test] fn browser_config_creates_isolated_profile_by_default() { let driver = super::ChromiumDriver::new(super::ChromiumOptions { From 4f141514e702f579669e40d691807fcc6b547201 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 17:30:34 -0600 Subject: [PATCH 16/50] fix(cdp): register init scripts for future documents --- crates/plumb-cdp/src/lib.rs | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index cd1ae85..b538bb0 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1807,12 +1807,7 @@ async fn inject_auth_script(page: &Page, path: &Path) -> Result<(), CdpError> { } async fn add_script_to_evaluate_on_new_document(page: &Page, source: &str) -> Result<(), CdpError> { - let params = AddScriptToEvaluateOnNewDocumentParams { - source: source.to_owned(), - world_name: None, - include_command_line_api: None, - run_immediately: Some(true), - }; + let params = add_script_to_evaluate_params(source); with_timeout( "Page.addScriptToEvaluateOnNewDocument", PAGE_COMMAND_TIMEOUT, @@ -1822,6 +1817,15 @@ async fn add_script_to_evaluate_on_new_document(page: &Page, source: &str) -> Re Ok(()) } +fn add_script_to_evaluate_params(source: &str) -> AddScriptToEvaluateOnNewDocumentParams { + AddScriptToEvaluateOnNewDocumentParams { + source: source.to_owned(), + world_name: None, + include_command_line_api: None, + run_immediately: None, + } +} + async fn install_extra_headers(page: &Page, headers: &[(String, String)]) -> Result<(), CdpError> { // Sort by name for deterministic CDP traffic. Plumb's invariant is // byte-identical *output*, but stable network-layer requests make @@ -3306,6 +3310,16 @@ mod tests { assert!(!viewport.has_touch); } + #[test] + fn add_script_params_registers_for_future_documents_only() { + let params = super::add_script_to_evaluate_params("window.__plumb = true;"); + + assert_eq!(params.source, "window.__plumb = true;"); + assert!(params.world_name.is_none()); + assert!(params.include_command_line_api.is_none()); + assert!(params.run_immediately.is_none()); + } + #[test] fn browser_config_creates_isolated_profile_by_default() { let driver = super::ChromiumDriver::new(super::ChromiumOptions { From 4554c306c62452ba91a9091911a2ecef99043a35 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 18:10:26 -0600 Subject: [PATCH 17/50] fix(cdp): disable chromiumoxide viewport emulation --- crates/plumb-cdp/src/lib.rs | 64 +++++++++++++------------------------ 1 file changed, 22 insertions(+), 42 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index b538bb0..cf1bfae 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -24,10 +24,10 @@ //! [`ChromiumDriver::snapshot_all`] launches Chromium exactly once, //! validates [`Browser::version`](chromiumoxide::Browser::version), //! and then loops over the requested targets — the first target's -//! viewport is pinned at launch through Chromiumoxide's viewport -//! configuration, later targets and explicit DPR pins are applied via -//! CDP `Emulation.setDeviceMetricsOverride`, then Plumb navigates to -//! the URL and calls `DOMSnapshot.captureSnapshot` with the +//! viewport is pinned at launch through Chromium's window size and DPR +//! flags, later targets and explicit DPR pins are applied via CDP +//! `Emulation.setDeviceMetricsOverride`, then Plumb navigates to the +//! URL and calls `DOMSnapshot.captureSnapshot` with the //! [`COMPUTED_STYLE_WHITELIST`] from PRD §10.3. Each CDP response is //! flattened into a [`PlumbSnapshot`] with deterministic ordering //! (nodes sorted by `dom_order`, computed styles inserted in @@ -81,7 +81,6 @@ use chromiumoxide::cdp::browser_protocol::target::{ CreateBrowserContextParams, CreateTargetParams, }; use chromiumoxide::detection::DetectionOptions; -use chromiumoxide::handler::viewport::Viewport as ChromiumViewport; use chromiumoxide::listeners::EventStream; use chromiumoxide::{Browser, BrowserConfig, Handler}; use futures_util::StreamExt; @@ -870,7 +869,7 @@ impl ChromiumDriver { }) .request_timeout(CHROMIUMOXIDE_REQUEST_TIMEOUT) .window_size(target.width, target.height) - .viewport(chromiumoxide_viewport(target)) + .viewport(None) .arg("--hide-scrollbars") .arg(scale_factor_arg); @@ -945,10 +944,12 @@ impl BrowserDriver for ChromiumDriver { impl ChromiumDriver { async fn snapshot_all_once(&self, targets: &[Target]) -> Result, CdpError> { // Use the first target's dimensions and DPR for the initial - // launch (the `--force-device-scale-factor` arg is fixed at - // launch time). The first unpinned target can reuse that - // launch-pinned viewport; later targets, and explicit `--dpr` - // pins, still need CDP `Emulation.setDeviceMetricsOverride`. + // launch. Chromiumoxide's built-in viewport emulation is + // disabled in `browser_config`; otherwise it sends its own + // unlabelled page-level Emulation commands during target + // initialization. The first unpinned target can reuse the + // launch-pinned window/DPR, while later targets and explicit + // `--dpr` pins still use Plumb's bounded/labeled override. let first = &targets[0]; let resolved_executable = resolve_auto_fetch(&self.options).await?; let launch = self.browser_config(first, resolved_executable.as_deref())?; @@ -988,8 +989,16 @@ async fn capture_target( options: &ChromiumOptions, apply_viewport_override: bool, ) -> Result { - let page = - create_page_without_load_wait(browser, CreateTargetParams::new("about:blank")).await?; + let page = create_page_without_load_wait( + browser, + CreateTargetParams { + width: Some(i64::from(target.width)), + height: Some(i64::from(target.height)), + new_window: Some(true), + ..CreateTargetParams::new("about:blank") + }, + ) + .await?; capture_on_page(&page, target, options, apply_viewport_override).await } @@ -1326,6 +1335,7 @@ fn persistent_browser_config( }) .request_timeout(CHROMIUMOXIDE_REQUEST_TIMEOUT) .window_size(1280, 800) + .viewport(None) .arg("--hide-scrollbars"); // Same precedence rule as `ChromiumDriver::browser_config`. @@ -1376,17 +1386,6 @@ fn should_apply_viewport_override(target_index: usize, target: &Target) -> bool target_index != 0 || target.pin_dpr.is_some() } -fn chromiumoxide_viewport(target: &Target) -> ChromiumViewport { - ChromiumViewport { - width: target.width, - height: target.height, - device_scale_factor: Some(f64::from(target.device_pixel_ratio)), - emulating_mobile: false, - is_landscape: target.width >= target.height, - has_touch: false, - } -} - async fn apply_viewport(page: &Page, target: &Target) -> Result<(), CdpError> { // `pin_dpr` (PRD §15 — `--dpr`) wins over `device_pixel_ratio` so // that callers can stress determinism by pinning a hidpi factor @@ -3291,25 +3290,6 @@ mod tests { assert!(super::should_apply_viewport_override(1, &target)); } - #[test] - fn chromiumoxide_viewport_uses_target_dimensions_and_dpr() { - let target = super::Target { - width: 390, - height: 844, - device_pixel_ratio: 3.0, - ..super::Target::default() - }; - - let viewport = super::chromiumoxide_viewport(&target); - - assert_eq!(viewport.width, 390); - assert_eq!(viewport.height, 844); - assert_eq!(viewport.device_scale_factor, Some(3.0)); - assert!(!viewport.is_landscape); - assert!(!viewport.emulating_mobile); - assert!(!viewport.has_touch); - } - #[test] fn add_script_params_registers_for_future_documents_only() { let params = super::add_script_to_evaluate_params("window.__plumb = true;"); From 2c4f030e99590353320bbf3afb2c404785093e5c Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 18:10:29 -0600 Subject: [PATCH 18/50] fix(core): ignore root body margins in grid rule --- .../src/rules/spacing/grid_conformance.rs | 7 +++++ .../plumb-core/tests/golden_spacing_grid.rs | 28 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/crates/plumb-core/src/rules/spacing/grid_conformance.rs b/crates/plumb-core/src/rules/spacing/grid_conformance.rs index 4844b50..c0893c3 100644 --- a/crates/plumb-core/src/rules/spacing/grid_conformance.rs +++ b/crates/plumb-core/src/rules/spacing/grid_conformance.rs @@ -64,6 +64,9 @@ impl Rule for GridConformance { for node in ctx.nodes() { for prop in SPACING_PROPERTIES { + if is_root_body_margin(&node.tag, &node.selector, prop) { + continue; + } let Some(raw) = node.computed_styles.get(*prop) else { continue; }; @@ -122,3 +125,7 @@ impl Rule for GridConformance { } } } + +fn is_root_body_margin(tag: &str, selector: &str, prop: &str) -> bool { + tag == "body" && selector == "html > body" && prop.starts_with("margin-") +} diff --git a/crates/plumb-core/tests/golden_spacing_grid.rs b/crates/plumb-core/tests/golden_spacing_grid.rs index 76baa0c..0bfd470 100644 --- a/crates/plumb-core/tests/golden_spacing_grid.rs +++ b/crates/plumb-core/tests/golden_spacing_grid.rs @@ -242,6 +242,34 @@ fn spacing_grid_conformance_tolerance_band() { ); } +#[test] +fn spacing_grid_conformance_skips_root_body_margins() { + let mut body = body_node(); + body.computed_styles + .insert("margin-top".to_owned(), "98.55px".to_owned()); + body.computed_styles + .insert("margin-bottom".to_owned(), "98.55px".to_owned()); + body.computed_styles + .insert("padding-top".to_owned(), "13px".to_owned()); + + let snapshot = PlumbSnapshot { + url: "plumb-fake://spacing-grid-root-body".into(), + viewport: ViewportKey::new("desktop"), + viewport_width: 1280, + viewport_height: 800, + nodes: vec![root_html_with_body(), body], + text_boxes: Vec::new(), + }; + let violations: Vec = run(&snapshot, &fixture_config()) + .into_iter() + .filter(|v| v.rule_id == "spacing/grid-conformance") + .collect(); + + assert_eq!(violations.len(), 1); + assert_eq!(violations[0].selector, "html > body"); + assert!(violations[0].message.contains("padding-top 13px")); +} + /// Build a one-node snapshot carrying a single spacing property and /// count `spacing/grid-conformance` violations under `config`. Lets the /// scale-deferral tests vary both the property value and the configured From 9f1e656ea7d9e6a21c0a864633a69119a5fbf0d9 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 18:38:56 -0600 Subject: [PATCH 19/50] fix(cdp): apply deterministic styles before snapshot --- crates/plumb-cdp/src/lib.rs | 130 ++++++++++++++++++++++++----------- crates/plumb-cli/src/main.rs | 8 +-- 2 files changed, 92 insertions(+), 46 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index cf1bfae..9cc882a 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -188,8 +188,8 @@ pub struct Target { /// Optional additional milliseconds to sleep before capturing the /// snapshot, after navigation (and after [`Self::wait_for_selector`]). pub wait_ms: Option, - /// Inject CSS that disables animations and transitions before the - /// page renders. Defaults to `true` — the historical Plumb behavior + /// Inject CSS that disables animations and transitions before + /// capture. Defaults to `true` — the historical Plumb behavior /// (PRD §16) — and the CLI exposes a flag that flips this value. pub disable_animations: bool, /// Inject CSS that hides page-level scrollbars. Defaults to `true` @@ -1030,15 +1030,16 @@ async fn create_page_without_load_wait( .await } -/// Apply viewport / animation hooks, install cookies and headers, -/// navigate, capture a DOM snapshot. +/// Apply viewport, install pre-navigation state, navigate, wait for +/// final page state, apply deterministic styling, then capture a DOM +/// snapshot. /// /// Shared between `ChromiumDriver::capture_target` and /// [`PersistentBrowser::snapshot`] so that the per-target work is /// expressed in exactly one place. The function is split into discrete /// stages — `apply_viewport` (DPR + dimensions), `pre_navigate` -/// (cookies, headers, auth-script, storage-state, animation killer, -/// scrollbar killer), `goto` + waits, then capture. +/// (cookies, headers, auth-script, storage-state cookies), `goto`, +/// waits, deterministic style injection, then capture. async fn capture_on_page( page: &Page, target: &Target, @@ -1059,6 +1060,7 @@ async fn capture_on_page( apply_post_navigate_waits(page, target).await?; apply_storage_state_local_storage(page, target, storage_state.as_ref()).await?; + apply_deterministic_styles(page, target).await?; let params = CaptureSnapshotParams { computed_styles: COMPUTED_STYLE_WHITELIST @@ -1416,12 +1418,11 @@ async fn apply_viewport(page: &Page, target: &Target) -> Result<(), CdpError> { /// All work that must happen on a fresh page before navigation. /// /// Runs in this fixed order so behavior matches what users expect: -/// 1. Animation/scrollbar CSS killers — PRD §16 determinism. -/// 2. Auth script — runs before any page script, so the page-side +/// 1. Auth script — runs before any page script, so the page-side /// bootstrap can set window globals before the SPA boots. -/// 3. Cookies and HTTP headers — set on the network layer before the +/// 2. Cookies and HTTP headers — set on the network layer before the /// very first request leaves Chromium. -/// 4. Storage-state cookies — same network layer; localStorage entries +/// 3. Storage-state cookies — same network layer; localStorage entries /// in the storage-state are deferred to [`apply_storage_state_local_storage`] /// after the origin loads, since localStorage is origin-scoped. /// @@ -1436,12 +1437,6 @@ async fn pre_navigate( target: &Target, options: &ChromiumOptions, ) -> Result, CdpError> { - if target.disable_animations { - inject_animation_killer(page).await?; - } - if target.hide_scrollbars { - inject_scrollbar_killer(page).await?; - } if let Some(script_path) = options.auth_script.as_deref() { inject_auth_script(page, script_path).await?; } @@ -1744,37 +1739,64 @@ fn origin_of(input: &str) -> Option { } } -async fn inject_animation_killer(page: &Page) -> Result<(), CdpError> { - // PRD §16 determinism mitigation: install a CSS-injection script that - // runs before any page script, so transitions/animations don't race - // with `captureSnapshot` and produce different bounds across runs. - let source = "(() => { \ - const style = document.createElement('style'); \ - style.textContent = '*, *::before, *::after { \ +async fn apply_deterministic_styles(page: &Page, target: &Target) -> Result<(), CdpError> { + let Some(source) = deterministic_style_source(target) else { + return Ok(()); + }; + + with_timeout( + "Runtime.evaluate deterministic styles", + PAGE_COMMAND_TIMEOUT, + async { + page.evaluate(source.as_str()) + .await + .map(|_| ()) + .map_err(driver_error) + }, + ) + .await?; + Ok(()) +} + +fn deterministic_style_source(target: &Target) -> Option { + if !target.disable_animations && !target.hide_scrollbars { + return None; + } + + let mut css = String::new(); + if target.disable_animations { + // PRD §16 determinism mitigation: transitions/animations should + // not race with `captureSnapshot` and produce different bounds + // across runs. + css.push_str( + "*, *::before, *::after { \ animation-duration: 0s !important; \ animation-delay: 0s !important; \ transition-duration: 0s !important; \ transition-delay: 0s !important; \ caret-color: transparent !important; \ - }'; \ - (document.head || document.documentElement).appendChild(style); \ - })();"; - add_script_to_evaluate_on_new_document(page, source).await -} - -async fn inject_scrollbar_killer(page: &Page) -> Result<(), CdpError> { - // PRD §16 determinism mitigation: scrollbar overlay differs across - // platforms / DPRs. The `--hide-scrollbars` Chromium launch arg is a - // first line of defense; this CSS injection covers the cases where - // the launch arg alone is not honored (Linux non-overlay scrollbars, - // CSS-painted scrollbars in some apps). - let source = "(() => { \ - const style = document.createElement('style'); \ - style.textContent = 'html { overflow: hidden !important; } \ - ::-webkit-scrollbar { display: none !important; }'; \ - (document.head || document.documentElement).appendChild(style); \ - })();"; - add_script_to_evaluate_on_new_document(page, source).await + }", + ); + } + if target.hide_scrollbars { + // The `--hide-scrollbars` Chromium launch arg is the first line + // of defense; this CSS covers cases where the launch arg alone + // is not honored or the page paints custom scrollbars. + css.push_str( + "html { overflow: hidden !important; } \ + ::-webkit-scrollbar { display: none !important; }", + ); + } + + let css_literal = serde_json::to_string(&css).ok()?; + Some(format!( + "(() => {{ \ + const style = document.createElement('style'); \ + style.setAttribute('data-plumb-deterministic-style', 'true'); \ + style.textContent = {css_literal}; \ + (document.head || document.documentElement).appendChild(style); \ + }})();" + )) } /// Read `path` (validated as a `.js` file under the CWD) and register @@ -3255,6 +3277,30 @@ mod tests { assert!(t.pin_dpr.is_none()); } + #[test] + fn deterministic_style_source_uses_default_capture_knobs() { + let Some(source) = super::deterministic_style_source(&super::Target::default()) else { + panic!("default target should inject deterministic CSS"); + }; + + assert!(source.contains("data-plumb-deterministic-style")); + assert!(source.contains("animation-duration")); + assert!(source.contains("transition-duration")); + assert!(source.contains("overflow: hidden")); + assert!(source.contains("::-webkit-scrollbar")); + } + + #[test] + fn deterministic_style_source_skips_when_knobs_disabled() { + let target = super::Target { + disable_animations: false, + hide_scrollbars: false, + ..super::Target::default() + }; + + assert!(super::deterministic_style_source(&target).is_none()); + } + #[test] fn target_effective_dpr_prefers_pin_over_default() { let mut t = super::Target { diff --git a/crates/plumb-cli/src/main.rs b/crates/plumb-cli/src/main.rs index 707afef..671138e 100644 --- a/crates/plumb-cli/src/main.rs +++ b/crates/plumb-cli/src/main.rs @@ -140,7 +140,7 @@ enum Command { /// navigation. #[arg(long, value_name = "PATH")] storage_state: Option, - /// Disable CSS animations and transitions before navigation + /// Disable CSS animations and transitions before capture /// for byte-stable snapshots. The driver already does this by /// default; pass `--disable-animations false` to opt out. #[arg( @@ -151,7 +151,7 @@ enum Command { default_missing_value = "true" )] disable_animations: bool, - /// Inject CSS that hides scrollbars before navigation. The + /// Inject CSS that hides scrollbars before capture. The /// driver already does this by default; pass /// `--hide-scrollbars false` to opt out. #[arg( @@ -273,7 +273,7 @@ enum Command { /// Path to a Playwright `storage-state.json`. #[arg(long, value_name = "PATH")] storage_state: Option, - /// Disable CSS animations and transitions before navigation. + /// Disable CSS animations and transitions before capture. #[arg( long = "disable-animations", default_value_t = true, @@ -282,7 +282,7 @@ enum Command { default_missing_value = "true" )] disable_animations: bool, - /// Inject CSS that hides scrollbars before navigation. + /// Inject CSS that hides scrollbars before capture. #[arg( long = "hide-scrollbars", default_value_t = true, From 89a6900b92295348f110fb5aa2b5922729b4f378 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Fri, 19 Jun 2026 19:00:44 -0600 Subject: [PATCH 20/50] fix(cdp,mcp): stabilize data-url page linting --- crates/plumb-cdp/src/lib.rs | 1455 +++++++++++++++++++++--- crates/plumb-mcp/src/lib.rs | 115 +- crates/plumb-mcp/tests/mcp_protocol.rs | 146 ++- 3 files changed, 1572 insertions(+), 144 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 9cc882a..c6e4a8b 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -67,22 +67,27 @@ use tempfile::TempDir; use chromiumoxide::Page; use chromiumoxide::browser::BrowserConfigBuilder; -use chromiumoxide::cdp::browser_protocol::browser::CloseParams as BrowserCloseParams; +use chromiumoxide::cdp::browser_protocol::browser::{ + CloseParams as BrowserCloseParams, GetVersionParams, +}; use chromiumoxide::cdp::browser_protocol::dom_snapshot::{ CaptureSnapshotParams, CaptureSnapshotReturns, DocumentSnapshot, }; use chromiumoxide::cdp::browser_protocol::emulation::SetDeviceMetricsOverrideParams; use chromiumoxide::cdp::browser_protocol::network::{ - CookieParam, EnableParams as NetworkEnableParams, EventLoadingFailed, Headers, ResourceType, - SetCookiesParams, SetExtraHttpHeadersParams, + CookieParam, Headers, SetCookiesParams, SetExtraHttpHeadersParams, +}; +use chromiumoxide::cdp::browser_protocol::page::{ + AddScriptToEvaluateOnNewDocumentParams, EnableParams as PageEnableParams, NavigateParams, }; -use chromiumoxide::cdp::browser_protocol::page::AddScriptToEvaluateOnNewDocumentParams; use chromiumoxide::cdp::browser_protocol::target::{ - CreateBrowserContextParams, CreateTargetParams, + AttachToTargetParams, CreateBrowserContextParams, CreateTargetParams, SessionId, }; +use chromiumoxide::cdp::js_protocol::runtime::EvaluateParams; +use chromiumoxide::cdp::{CdpEvent, CdpEventMessage}; use chromiumoxide::detection::DetectionOptions; -use chromiumoxide::listeners::EventStream; -use chromiumoxide::{Browser, BrowserConfig, Handler}; +use chromiumoxide::types::{CallId, Command, Message, MethodId, Response}; +use chromiumoxide::{Browser, BrowserConfig, Connection, Handler}; use futures_util::StreamExt; use serde::Deserialize; use tokio::task::JoinHandle; @@ -102,14 +107,16 @@ const BROWSER_WAIT_TIMEOUT: Duration = Duration::from_secs(5); const BROWSER_KILL_TIMEOUT: Duration = Duration::from_secs(5); const CHROMIUMOXIDE_REQUEST_TIMEOUT: Duration = Duration::from_mins(1); const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); -const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(30); +const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(75); const PAGE_COMMAND_TIMEOUT: Duration = Duration::from_secs(25); +const PAGE_ENABLE_TIMEOUT: Duration = Duration::from_secs(5); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(25); const TRANSIENT_CAPTURE_RETRIES: usize = 1; +const INITIAL_PAGE_URL: &str = "data:text/html,%3C!doctype%20html%3E%3Ctitle%3Eplumb%3C%2Ftitle%3E"; /// CSS property whitelist passed to `DOMSnapshot.captureSnapshot` as the /// `computedStyles` argument. @@ -868,6 +875,7 @@ impl ChromiumDriver { unstable: false, }) .request_timeout(CHROMIUMOXIDE_REQUEST_TIMEOUT) + .launch_timeout(BROWSER_LAUNCH_TIMEOUT) .window_size(target.width, target.height) .viewport(None) .arg("--hide-scrollbars") @@ -953,14 +961,15 @@ impl ChromiumDriver { let first = &targets[0]; let resolved_executable = resolve_auto_fetch(&self.options).await?; let launch = self.browser_config(first, resolved_executable.as_deref())?; - let mut session = ChromiumSession::launch(launch).await?; + let mut session = RawChromiumSession::launch(launch).await?; + let mut raw = RawCdpClient::connect(session.websocket_address()).await?; let result: Result, CdpError> = async { - validate_browser_version(&session.browser).await?; + validate_browser_version_raw(&mut raw).await?; let mut snapshots = Vec::with_capacity(targets.len()); for (target_index, target) in targets.iter().enumerate() { - let snap = capture_target( - &session.browser, + let snap = capture_target_raw( + &mut raw, target, &self.options, should_apply_viewport_override(target_index, target), @@ -972,7 +981,7 @@ impl ChromiumDriver { } .await; - if let Err(cleanup_err) = session.shutdown().await { + if let Err(cleanup_err) = session.shutdown(&mut raw).await { tracing::debug!(error = %cleanup_err, "failed to clean up Chromium session"); if result.is_ok() { return Err(cleanup_err); @@ -983,24 +992,370 @@ impl ChromiumDriver { } } -async fn capture_target( - browser: &Browser, +async fn capture_target_raw( + cdp: &mut RawCdpClient, target: &Target, options: &ChromiumOptions, apply_viewport_override: bool, ) -> Result { - let page = create_page_without_load_wait( - browser, + let page = RawPage::create( + cdp, CreateTargetParams { width: Some(i64::from(target.width)), height: Some(i64::from(target.height)), new_window: Some(true), - ..CreateTargetParams::new("about:blank") + ..CreateTargetParams::new(INITIAL_PAGE_URL) }, ) .await?; - capture_on_page(&page, target, options, apply_viewport_override).await + wait_for_initial_document_ready_raw(cdp, &page).await?; + capture_on_raw_page(cdp, &page, target, options, apply_viewport_override).await +} + +struct RawCdpClient { + conn: Connection, +} + +impl RawCdpClient { + async fn connect(websocket_address: &str) -> Result { + let conn = with_timeout("CDP websocket connect", CDP_CONTROL_TIMEOUT, async { + Connection::::connect(websocket_address) + .await + .map_err(driver_error) + }) + .await?; + Ok(Self { conn }) + } + + async fn execute( + &mut self, + session_id: Option<&SessionId>, + cmd: T, + ) -> Result { + let method = cmd.identifier(); + let params = serde_json::to_value(cmd).map_err(serde_driver_error)?; + let call_id = self + .conn + .submit_command(method.clone(), session_id.cloned(), params) + .map_err(serde_driver_error)?; + self.wait_for_response::(call_id, method).await + } + + async fn execute_collecting_page_events( + &mut self, + session_id: &SessionId, + cmd: T, + events: &mut RawNavigationEvents, + ) -> Result { + let method = cmd.identifier(); + let params = serde_json::to_value(cmd).map_err(serde_driver_error)?; + let call_id = self + .conn + .submit_command(method.clone(), Some(session_id.clone()), params) + .map_err(serde_driver_error)?; + self.wait_for_response_collecting_page_events::(call_id, method, session_id, events) + .await + } + + async fn wait_for_response( + &mut self, + call_id: CallId, + method: MethodId, + ) -> Result { + loop { + match self.conn.next().await { + Some(Ok(Message::Response(response))) if response.id == call_id => { + return raw_command_response::(response, &method); + } + Some(Ok(Message::Response(_) | Message::Event(_))) => {} + Some(Err(err)) => return Err(driver_error(err)), + None => { + return Err(CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::UnexpectedEof, + format!("{method} received no response from Chromium"), + )))); + } + } + } + } + + async fn wait_for_response_collecting_page_events( + &mut self, + call_id: CallId, + method: MethodId, + session_id: &SessionId, + events: &mut RawNavigationEvents, + ) -> Result { + loop { + match self.conn.next().await { + Some(Ok(Message::Response(response))) if response.id == call_id => { + return raw_command_response::(response, &method); + } + Some(Ok(Message::Event(event))) => { + events.observe_message(&event, session_id); + } + Some(Ok(Message::Response(_))) => {} + Some(Err(err)) => return Err(driver_error(err)), + None => { + return Err(CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::UnexpectedEof, + format!("{method} received no response from Chromium"), + )))); + } + } + } + } + + async fn collect_next_page_event( + &mut self, + session_id: &SessionId, + events: &mut RawNavigationEvents, + ) -> Result<(), CdpError> { + loop { + match self.conn.next().await { + Some(Ok(Message::Event(event))) => { + if events.observe_message(&event, session_id) { + return Ok(()); + } + } + Some(Ok(Message::Response(_))) => {} + Some(Err(err)) => return Err(driver_error(err)), + None => { + return Err(CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::UnexpectedEof, + "raw CDP event stream ended before navigation completed", + )))); + } + } + } + } +} + +fn raw_command_response( + response: Response, + method: &MethodId, +) -> Result { + if let Some(result) = response.result { + return T::response_from_value(result).map_err(serde_driver_error); + } + if let Some(err) = response.error { + return Err(CdpError::Driver(Box::new(io::Error::other(format!( + "{method} failed: {err}" + ))))); + } + Err(CdpError::Driver(Box::new(io::Error::other(format!( + "{method} returned neither result nor error" + ))))) +} + +struct RawPage { + session_id: SessionId, +} + +#[derive(Default)] +struct RawNavigationEvents { + main_frame_url: Option, + dom_content_event: bool, + load_event: bool, +} + +impl RawNavigationEvents { + fn observe_message(&mut self, event: &CdpEventMessage, session_id: &SessionId) -> bool { + if event.session_id.as_deref() != Some(session_id.as_ref()) { + return false; + } + match &event.params { + CdpEvent::PageFrameNavigated(frame) if frame.frame.parent_id.is_none() => { + self.observe_main_frame_url(&frame.frame.url); + true + } + CdpEvent::PageDomContentEventFired(_) => { + self.observe_dom_content_event(); + true + } + CdpEvent::PageLoadEventFired(_) => { + self.observe_load_event(); + true + } + _ => false, + } + } + + fn observe_main_frame_url(&mut self, url: &str) { + if url_has_navigated(url) { + self.main_frame_url = Some(url.to_owned()); + self.dom_content_event = false; + self.load_event = false; + } + } + + fn observe_dom_content_event(&mut self) { + self.dom_content_event = true; + } + + fn observe_load_event(&mut self) { + self.load_event = true; + } + + fn is_ready_for_capture(&self, allow_interactive: bool) -> bool { + self.main_frame_url.is_some() + && (self.load_event || (allow_interactive && self.dom_content_event)) + } + + fn is_chrome_error_page(&self) -> bool { + self.main_frame_url + .as_deref() + .is_some_and(|url| url.starts_with("chrome-error:")) + } + + fn main_frame_url(&self) -> Option<&str> { + self.main_frame_url.as_deref() + } +} + +impl RawPage { + async fn create(cdp: &mut RawCdpClient, params: CreateTargetParams) -> Result { + let target_id = with_timeout("Target.createTarget", TARGET_CREATE_TIMEOUT, async { + cdp.execute(None, params) + .await + .map(|response| response.target_id) + }) + .await + .map_err(|err| target_lifecycle_error("Target.createTarget", &err))?; + + let attach = AttachToTargetParams::builder() + .target_id(target_id) + .flatten(true) + .build() + .map_err(driver_message)?; + let session_id = with_timeout("Target.attachToTarget", TARGET_ATTACH_TIMEOUT, async { + cdp.execute(None, attach) + .await + .map(|response| response.session_id) + }) + .await + .map_err(|err| target_lifecycle_error("Target.attachToTarget", &err))?; + + Ok(Self { session_id }) + } + + async fn execute( + &self, + cdp: &mut RawCdpClient, + operation: &str, + timeout: Duration, + cmd: T, + ) -> Result { + with_timeout(operation, timeout, async { + cdp.execute(Some(&self.session_id), cmd).await + }) + .await + } + + async fn execute_collecting_page_events( + &self, + cdp: &mut RawCdpClient, + operation: &str, + timeout: Duration, + cmd: T, + events: &mut RawNavigationEvents, + ) -> Result { + with_timeout(operation, timeout, async { + cdp.execute_collecting_page_events(&self.session_id, cmd, events) + .await + }) + .await + } + + async fn evaluate_value( + &self, + cdp: &mut RawCdpClient, + operation: &str, + timeout: Duration, + expression: &str, + ) -> Result { + let params = EvaluateParams::builder() + .expression(expression) + .await_promise(true) + .return_by_value(true) + .build() + .map_err(driver_message)?; + let result = self.execute(cdp, operation, timeout, params).await?; + if let Some(exception) = result.exception_details { + return Err(driver_error( + chromiumoxide::error::CdpError::JavascriptException(Box::new(exception)), + )); + } + let value = result.result.value.ok_or_else(|| { + CdpError::Driver(Box::new(io::Error::other(format!( + "{operation} returned no value" + )))) + })?; + serde_json::from_value(value).map_err(serde_driver_error) + } + + async fn evaluate_unit( + &self, + cdp: &mut RawCdpClient, + operation: &str, + timeout: Duration, + expression: &str, + ) -> Result<(), CdpError> { + let params = EvaluateParams::builder() + .expression(expression) + .await_promise(true) + .return_by_value(true) + .build() + .map_err(driver_message)?; + let result = self.execute(cdp, operation, timeout, params).await?; + if let Some(exception) = result.exception_details { + return Err(driver_error( + chromiumoxide::error::CdpError::JavascriptException(Box::new(exception)), + )); + } + Ok(()) + } +} + +async fn capture_on_raw_page( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, + options: &ChromiumOptions, + apply_viewport_override: bool, +) -> Result { + if apply_viewport_override { + apply_viewport_raw(cdp, page, target).await?; + } + let storage_state = pre_navigate_raw(cdp, page, target, options).await?; + + navigate_raw(cdp, page, target).await?; + + apply_post_navigate_waits_raw(cdp, page, target).await?; + apply_storage_state_local_storage_raw(cdp, page, target, storage_state.as_ref()).await?; + apply_deterministic_styles_raw(cdp, page, target).await?; + + let params = CaptureSnapshotParams { + computed_styles: COMPUTED_STYLE_WHITELIST + .iter() + .map(|s| (*s).to_string()) + .collect(), + include_paint_order: Some(true), + include_dom_rects: Some(true), + include_blended_background_colors: Some(true), + include_text_color_opacities: None, + }; + + let response = page + .execute( + cdp, + "DOMSnapshot.captureSnapshot", + SNAPSHOT_CAPTURE_TIMEOUT, + params, + ) + .await?; + flatten_snapshot(target, &response) } async fn create_page_without_load_wait( @@ -1014,7 +1369,8 @@ async fn create_page_without_load_wait( .map(|response| response.result.target_id) .map_err(driver_error) }) - .await?; + .await + .map_err(|err| target_lifecycle_error("Target.createTarget", &err))?; with_timeout("Target.attachToTarget", TARGET_ATTACH_TIMEOUT, async { loop { @@ -1028,6 +1384,19 @@ async fn create_page_without_load_wait( } }) .await + .map_err(|err| target_lifecycle_error("Target.attachToTarget", &err)) +} + +fn target_lifecycle_error(stage: &str, err: &CdpError) -> CdpError { + let kind = if is_retryable_capture_timeout(err) { + io::ErrorKind::TimedOut + } else { + io::ErrorKind::Other + }; + CdpError::Driver(Box::new(io::Error::new( + kind, + format!("{stage} failed before navigation: {err}"), + ))) } /// Apply viewport, install pre-navigation state, navigate, wait for @@ -1201,7 +1570,7 @@ impl PersistentBrowser { let result: Result = async { let create_params = CreateTargetParams { - url: "about:blank".to_string(), + url: INITIAL_PAGE_URL.to_string(), left: None, top: None, width: None, @@ -1215,6 +1584,7 @@ impl PersistentBrowser { hidden: None, }; let page = create_page_without_load_wait(&self.inner.browser, create_params).await?; + wait_for_initial_document_ready(&page).await?; capture_on_page(&page, target, &self.inner.options, true).await } .await; @@ -1336,6 +1706,7 @@ fn persistent_browser_config( unstable: false, }) .request_timeout(CHROMIUMOXIDE_REQUEST_TIMEOUT) + .launch_timeout(BROWSER_LAUNCH_TIMEOUT) .window_size(1280, 800) .viewport(None) .arg("--hide-scrollbars"); @@ -1415,6 +1786,35 @@ async fn apply_viewport(page: &Page, target: &Target) -> Result<(), CdpError> { Ok(()) } +async fn apply_viewport_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, +) -> Result<(), CdpError> { + let params = SetDeviceMetricsOverrideParams { + width: i64::from(target.width), + height: i64::from(target.height), + device_scale_factor: target.effective_dpr(), + mobile: false, + scale: None, + screen_width: None, + screen_height: None, + position_x: None, + position_y: None, + dont_set_visible_size: None, + screen_orientation: None, + viewport: None, + }; + page.execute( + cdp, + "Emulation.setDeviceMetricsOverride", + PAGE_COMMAND_TIMEOUT, + params, + ) + .await?; + Ok(()) +} + /// All work that must happen on a fresh page before navigation. /// /// Runs in this fixed order so behavior matches what users expect: @@ -1456,6 +1856,31 @@ async fn pre_navigate( Ok(storage_state) } +async fn pre_navigate_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, + options: &ChromiumOptions, +) -> Result, CdpError> { + if let Some(script_path) = options.auth_script.as_deref() { + inject_auth_script_raw(cdp, page, script_path).await?; + } + if !options.headers.is_empty() { + install_extra_headers_raw(cdp, page, &options.headers).await?; + } + if !options.cookies.is_empty() { + install_cookies_raw(cdp, page, &options.cookies, target.url.as_str()).await?; + } + let storage_state = if let Some(state_path) = options.storage_state.as_deref() { + let state = StorageState::load_from_path(state_path)?; + install_storage_state_cookies_raw(cdp, page, &state).await?; + Some(state) + } else { + None + }; + Ok(storage_state) +} + #[derive(Debug, Deserialize)] struct NavigationState { href: String, @@ -1466,52 +1891,134 @@ struct NavigationState { } async fn navigate_page(page: &Page, url: &str) -> Result<(), CdpError> { - if uses_chromiumoxide_goto(url) { - with_timeout("Page.navigate", DOCUMENT_READY_TIMEOUT, async { - page.goto(url).await.map(|_| ()).map_err(driver_error) - }) - .await?; - return Ok(()); - } - - let mut navigation_failures = with_timeout( - "Network.loadingFailed listener", - CDP_CONTROL_TIMEOUT, - async { - page.event_listener::() - .await - .map_err(driver_error) - }, - ) - .await?; - with_timeout("Network.enable", CDP_CONTROL_TIMEOUT, async { - page.execute(NetworkEnableParams::default()) + let initial_result = match navigation_method_for_url(url) { + NavigationMethod::ChromiumoxideGoto => { + with_timeout("Page.navigate", DOCUMENT_READY_TIMEOUT, async { + page.goto(url).await.map(|_| ()).map_err(driver_error) + }) .await - .map(|_| ()) - .map_err(driver_error) - }) - .await?; + } + NavigationMethod::CdpNavigate => { + with_timeout("Page.navigate", PAGE_COMMAND_TIMEOUT, async { + page.execute(NavigateParams::new(url)) + .await + .map_err(driver_error) + .and_then(|response| { + if let Some(error_text) = &response.error_text { + Err(CdpError::Driver(Box::new(io::Error::other(format!( + "Page.navigate failed: {error_text}" + ))))) + } else { + Ok(()) + } + }) + }) + .await + } + NavigationMethod::LocationAssign => { + let script = navigation_assignment_script(url)?; + with_timeout( + "navigation location assignment", + NAVIGATION_ASSIGNMENT_TIMEOUT, + async { + page.evaluate(script.as_str()) + .await + .map(|_| ()) + .map_err(driver_error) + }, + ) + .await + } + }; - let script = navigation_assignment_script(url)?; - let initial_result = with_timeout( - "navigation location assignment", - NAVIGATION_ASSIGNMENT_TIMEOUT, - async { - page.evaluate(script.as_str()) - .await - .map(|_| ()) - .map_err(driver_error) - }, + wait_for_document_ready(page, navigation_display_url(url), initial_result.err()).await +} + +async fn navigate_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, +) -> Result<(), CdpError> { + let page_events_enabled = enable_raw_page_events(cdp, page).await; + let mut events = RawNavigationEvents::default(); + let initial_result = if page_events_enabled { + page.execute_collecting_page_events( + cdp, + "Page.navigate", + PAGE_COMMAND_TIMEOUT, + NavigateParams::new(target.url.as_str()), + &mut events, + ) + .await + } else { + page.execute( + cdp, + "Page.navigate", + PAGE_COMMAND_TIMEOUT, + NavigateParams::new(target.url.as_str()), + ) + .await + } + .and_then(|response| { + if let Some(error_text) = response.error_text { + Err(CdpError::Driver(Box::new(io::Error::other(format!( + "Page.navigate failed: {error_text}" + ))))) + } else { + Ok(()) + } + }); + + wait_for_document_ready_raw( + cdp, + page, + navigation_display_url(target.url.as_str()), + initial_result.err(), + target.wait_for_selector.is_some(), + page_events_enabled.then_some(events), ) - .await; + .await +} - wait_for_document_ready(page, url, initial_result.err(), &mut navigation_failures).await +async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { + match page + .execute( + cdp, + "Page.enable", + PAGE_ENABLE_TIMEOUT, + PageEnableParams::default(), + ) + .await + { + Ok(_) => true, + Err(err) => { + tracing::debug!(error = %err, "Page.enable failed; falling back to raw ready-state polling"); + false + } + } } fn uses_chromiumoxide_goto(url: &str) -> bool { url.starts_with("file://") } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NavigationMethod { + ChromiumoxideGoto, + CdpNavigate, + LocationAssign, +} + +fn navigation_method_for_url(url: &str) -> NavigationMethod { + if uses_chromiumoxide_goto(url) { + NavigationMethod::ChromiumoxideGoto + } else if url.starts_with("data:") { + NavigationMethod::CdpNavigate + } else { + NavigationMethod::LocationAssign + } +} + fn navigation_assignment_script(url: &str) -> Result { let quoted_url = serde_json::to_string(url).map_err(|err| { CdpError::Driver(Box::new(io::Error::other(format!( @@ -1525,43 +2032,81 @@ async fn wait_for_document_ready( page: &Page, display_url: &str, initial_error: Option, - navigation_failures: &mut EventStream, ) -> Result<(), CdpError> { let mut last_state_error = None; let attempt = async { loop { - tokio::select! { - biased; - - failure = navigation_failures.next() => { - if let Some(failure) = failure - && document_navigation_failed(&failure) - { - return Err(navigation_failed_error(display_url, &failure.error_text)); - } + tokio::time::sleep(Duration::from_millis(50)).await; + if poll_document_ready(page, display_url, &mut last_state_error).await? { + return Ok(()); + } + } + }; + + if let Ok(result) = tokio::time::timeout(DOCUMENT_READY_TIMEOUT, attempt).await { + return result; + } + + let reason = navigation_ready_timeout_reason( + display_url, + initial_error.as_ref().map(ToString::to_string).as_deref(), + last_state_error.as_deref(), + ); + Err(CdpError::Driver(Box::new(io::Error::other(reason)))) +} + +async fn wait_for_initial_document_ready(page: &Page) -> Result<(), CdpError> { + let mut last_state_error = None; + let attempt = async { + loop { + tokio::time::sleep(Duration::from_millis(50)).await; + match tokio::time::timeout(NAVIGATION_STATE_READ_TIMEOUT, read_navigation_state(page)) + .await + { + Ok(Ok(state)) if initial_document_is_ready(&state) => return Ok(()), + Ok(Ok(_)) => {} + Ok(Err(err)) => last_state_error = Some(err.to_string()), + Err(_) => { + last_state_error = Some(timeout_reason( + "initial navigation state read", + NAVIGATION_STATE_READ_TIMEOUT, + )); } - () = tokio::time::sleep(Duration::from_millis(50)) => { - match tokio::time::timeout( + } + } + }; + + if let Ok(result) = tokio::time::timeout(DOCUMENT_READY_TIMEOUT, attempt).await { + return result; + } + + Err(CdpError::Driver(Box::new(io::Error::other( + initial_document_ready_timeout_reason(last_state_error.as_deref()), + )))) +} + +async fn wait_for_initial_document_ready_raw( + cdp: &mut RawCdpClient, + page: &RawPage, +) -> Result<(), CdpError> { + let mut last_state_error = None; + let attempt = async { + loop { + tokio::time::sleep(Duration::from_millis(50)).await; + match tokio::time::timeout( + NAVIGATION_STATE_READ_TIMEOUT, + read_navigation_state_raw(cdp, page), + ) + .await + { + Ok(Ok(state)) if initial_document_is_ready(&state) => return Ok(()), + Ok(Ok(_)) => {} + Ok(Err(err)) => last_state_error = Some(err.to_string()), + Err(_) => { + last_state_error = Some(timeout_reason( + "initial navigation state read", NAVIGATION_STATE_READ_TIMEOUT, - read_navigation_state(page), - ) - .await - { - Ok(Ok(state)) if state.is_chrome_error_page => { - return Err(chrome_error_page_error(display_url, &state.href)); - } - Ok(Ok(state)) if document_is_loaded(&state) => return Ok(()), - Ok(Ok(_)) => {} - Ok(Err(err)) => { - last_state_error = Some(err.to_string()); - } - Err(_) => { - last_state_error = Some(timeout_reason( - "navigation state read", - NAVIGATION_STATE_READ_TIMEOUT, - )); - } - } + )); } } } @@ -1571,6 +2116,81 @@ async fn wait_for_document_ready( return result; } + Err(CdpError::Driver(Box::new(io::Error::other( + initial_document_ready_timeout_reason(last_state_error.as_deref()), + )))) +} + +async fn wait_for_document_ready_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + display_url: &str, + initial_error: Option, + allow_interactive: bool, + events: Option, +) -> Result<(), CdpError> { + let Some(mut events) = events else { + return wait_for_document_ready_raw_by_polling( + cdp, + page, + display_url, + initial_error, + allow_interactive, + ) + .await; + }; + + let mut last_state_error = None; + if !events.is_ready_for_capture(allow_interactive) { + match wait_for_raw_navigation_events(cdp, page, &mut events, allow_interactive).await { + Ok(()) => {} + Err(err) => last_state_error = Some(err.to_string()), + } + } + + if events.is_chrome_error_page() { + return Err(chrome_error_page_error( + display_url, + events + .main_frame_url() + .unwrap_or("chrome-error://chromewebdata/"), + )); + } + + match tokio::time::timeout( + NAVIGATION_STATE_READ_TIMEOUT, + read_navigation_state_raw(cdp, page), + ) + .await + { + Ok(Ok(state)) if state.is_chrome_error_page => { + return Err(chrome_error_page_error(display_url, &state.href)); + } + Ok(Ok(state)) if document_is_ready_for_capture(&state, allow_interactive) => return Ok(()), + Ok(Ok(state)) + if events.is_ready_for_capture(allow_interactive) && document_has_navigated(&state) => + { + return Ok(()); + } + Ok(Ok(_)) if events.is_ready_for_capture(allow_interactive) => return Ok(()), + Ok(Ok(_)) => {} + Ok(Err(err)) if events.is_ready_for_capture(allow_interactive) => { + tracing::debug!(error = %err, "raw navigation state check failed after page event readiness"); + return Ok(()); + } + Ok(Err(err)) => last_state_error = Some(err.to_string()), + Err(_) if events.is_ready_for_capture(allow_interactive) => { + tracing::debug!("raw navigation state check timed out after page event readiness"); + return Ok(()); + } + Err(_) => { + last_state_error = Some(timeout_reason( + "navigation state read", + NAVIGATION_STATE_READ_TIMEOUT, + )); + } + } + let reason = navigation_ready_timeout_reason( display_url, initial_error.as_ref().map(ToString::to_string).as_deref(), @@ -1579,14 +2199,122 @@ async fn wait_for_document_ready( Err(CdpError::Driver(Box::new(io::Error::other(reason)))) } -fn document_navigation_failed(failure: &EventLoadingFailed) -> bool { - failure.r#type == ResourceType::Document && failure.canceled != Some(true) +async fn wait_for_document_ready_raw_by_polling( + cdp: &mut RawCdpClient, + page: &RawPage, + display_url: &str, + initial_error: Option, + allow_interactive: bool, +) -> Result<(), CdpError> { + let mut last_state_error = None; + let attempt = async { + loop { + tokio::time::sleep(Duration::from_millis(50)).await; + if poll_document_ready_raw( + cdp, + page, + display_url, + &mut last_state_error, + allow_interactive, + ) + .await? + { + return Ok(()); + } + } + }; + + if let Ok(result) = tokio::time::timeout(DOCUMENT_READY_TIMEOUT, attempt).await { + return result; + } + + let reason = navigation_ready_timeout_reason( + display_url, + initial_error.as_ref().map(ToString::to_string).as_deref(), + last_state_error.as_deref(), + ); + Err(CdpError::Driver(Box::new(io::Error::other(reason)))) } -fn navigation_failed_error(display_url: &str, error_text: &str) -> CdpError { - CdpError::Driver(Box::new(io::Error::other(format!( - "navigation to `{display_url}` failed: {error_text}" - )))) +async fn wait_for_raw_navigation_events( + cdp: &mut RawCdpClient, + page: &RawPage, + events: &mut RawNavigationEvents, + allow_interactive: bool, +) -> Result<(), CdpError> { + let attempt = async { + loop { + if events.is_ready_for_capture(allow_interactive) { + return Ok(()); + } + cdp.collect_next_page_event(&page.session_id, events) + .await?; + } + }; + + match tokio::time::timeout(DOCUMENT_READY_TIMEOUT, attempt).await { + Ok(result) => result, + Err(_) => Err(CdpError::Driver(Box::new(io::Error::other( + timeout_reason("raw navigation page event", DOCUMENT_READY_TIMEOUT), + )))), + } +} + +async fn poll_document_ready_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + display_url: &str, + last_state_error: &mut Option, + allow_interactive: bool, +) -> Result { + match tokio::time::timeout( + NAVIGATION_STATE_READ_TIMEOUT, + read_navigation_state_raw(cdp, page), + ) + .await + { + Ok(Ok(state)) if state.is_chrome_error_page => { + Err(chrome_error_page_error(display_url, &state.href)) + } + Ok(Ok(state)) if document_is_ready_for_capture(&state, allow_interactive) => Ok(true), + Ok(Ok(_)) => Ok(false), + Ok(Err(err)) => { + *last_state_error = Some(err.to_string()); + Ok(false) + } + Err(_) => { + *last_state_error = Some(timeout_reason( + "navigation state read", + NAVIGATION_STATE_READ_TIMEOUT, + )); + Ok(false) + } + } +} + +async fn poll_document_ready( + page: &Page, + display_url: &str, + last_state_error: &mut Option, +) -> Result { + match tokio::time::timeout(NAVIGATION_STATE_READ_TIMEOUT, read_navigation_state(page)).await { + Ok(Ok(state)) if state.is_chrome_error_page => { + Err(chrome_error_page_error(display_url, &state.href)) + } + Ok(Ok(state)) if document_is_loaded(&state) => Ok(true), + Ok(Ok(_)) => Ok(false), + Ok(Err(err)) => { + *last_state_error = Some(err.to_string()); + Ok(false) + } + Err(_) => { + *last_state_error = Some(timeout_reason( + "navigation state read", + NAVIGATION_STATE_READ_TIMEOUT, + )); + Ok(false) + } + } } fn chrome_error_page_error(display_url: &str, error_href: &str) -> CdpError { @@ -1596,7 +2324,25 @@ fn chrome_error_page_error(display_url: &str, error_href: &str) -> CdpError { } fn document_is_loaded(state: &NavigationState) -> bool { - state.href != "about:blank" && state.ready_state == "complete" && !state.is_chrome_error_page + document_has_navigated(state) && state.ready_state == "complete" +} + +fn document_is_ready_for_capture(state: &NavigationState, allow_interactive: bool) -> bool { + document_has_navigated(state) + && (state.ready_state == "complete" + || (allow_interactive && state.ready_state == "interactive")) +} + +fn initial_document_is_ready(state: &NavigationState) -> bool { + !state.is_chrome_error_page && state.ready_state == "complete" +} + +fn document_has_navigated(state: &NavigationState) -> bool { + url_has_navigated(&state.href) && !state.is_chrome_error_page +} + +fn url_has_navigated(url: &str) -> bool { + url != "about:blank" && url != INITIAL_PAGE_URL } async fn read_navigation_state(page: &Page) -> Result { @@ -1619,6 +2365,26 @@ async fn read_navigation_state(page: &Page) -> Result parse_navigation_state(&raw) } +async fn read_navigation_state_raw( + cdp: &mut RawCdpClient, + page: &RawPage, +) -> Result { + let raw: String = page + .evaluate_value( + cdp, + "Runtime.evaluate navigation state", + PAGE_COMMAND_TIMEOUT, + "JSON.stringify({ + href: window.location.href, + readyState: document.readyState, + isChromeErrorPage: window.location.protocol === 'chrome-error:' + || document.getElementById('main-frame-error') !== null + })", + ) + .await?; + parse_navigation_state(&raw) +} + fn parse_navigation_state(raw: &str) -> Result { serde_json::from_str(raw).map_err(|err| { CdpError::Driver(Box::new(io::Error::other(format!( @@ -1647,6 +2413,26 @@ fn navigation_ready_timeout_reason( reason } +fn navigation_display_url(url: &str) -> &str { + if url.starts_with("data:") { + "data:" + } else { + url + } +} + +fn initial_document_ready_timeout_reason(last_state_error: Option<&str>) -> String { + let mut reason = format!( + "initial document exhausted {} ready-state budget", + timeout_budget_label(DOCUMENT_READY_TIMEOUT) + ); + if let Some(err) = last_state_error { + reason.push_str("; last initial navigation state read failed: "); + reason.push_str(err); + } + reason +} + /// Wait stages that must run *after* navigation. PRD §15 — `--wait-for` /// and `--wait-ms`. /// @@ -1664,6 +2450,20 @@ async fn apply_post_navigate_waits(page: &Page, target: &Target) -> Result<(), C Ok(()) } +async fn apply_post_navigate_waits_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, +) -> Result<(), CdpError> { + if let Some(selector) = target.wait_for_selector.as_deref() { + wait_for_selector_raw(cdp, page, selector).await?; + } + if let Some(ms) = target.wait_ms { + tokio::time::sleep(std::time::Duration::from_millis(ms)).await; + } + Ok(()) +} + /// Install localStorage entries from an already-parsed Playwright /// storage-state. /// @@ -1720,6 +2520,46 @@ async fn apply_storage_state_local_storage( Ok(()) } +async fn apply_storage_state_local_storage_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, + state: Option<&StorageState>, +) -> Result<(), CdpError> { + let Some(state) = state else { + return Ok(()); + }; + let target_origin = origin_of(target.url.as_str()).unwrap_or_default(); + for origin_entry in &state.origins { + if origin_entry.origin != target_origin { + continue; + } + for entry in &origin_entry.local_storage { + let key = serde_json::to_string(&entry.name).map_err(|err| { + CdpError::MalformedStorageState { + path: PathBuf::new(), + reason: format!("could not serialize key: {err}"), + } + })?; + let value = serde_json::to_string(&entry.value).map_err(|err| { + CdpError::MalformedStorageState { + path: PathBuf::new(), + reason: format!("could not serialize value: {err}"), + } + })?; + let script = format!("window.localStorage.setItem({key}, {value});"); + page.evaluate_unit( + cdp, + "Runtime.evaluate localStorage", + PAGE_COMMAND_TIMEOUT, + script.as_str(), + ) + .await?; + } + } + Ok(()) +} + fn origin_of(input: &str) -> Option { // WHATWG-compliant origin: `Url::origin().ascii_serialization()` // handles default-port elision (`:443` for `https`, `:80` for @@ -1758,6 +2598,25 @@ async fn apply_deterministic_styles(page: &Page, target: &Target) -> Result<(), Ok(()) } +async fn apply_deterministic_styles_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, +) -> Result<(), CdpError> { + let Some(source) = deterministic_style_source(target) else { + return Ok(()); + }; + + page.evaluate_unit( + cdp, + "Runtime.evaluate deterministic styles", + PAGE_COMMAND_TIMEOUT, + source.as_str(), + ) + .await?; + Ok(()) +} + fn deterministic_style_source(target: &Target) -> Option { if !target.disable_animations && !target.hide_scrollbars { return None; @@ -1827,6 +2686,25 @@ async fn inject_auth_script(page: &Page, path: &Path) -> Result<(), CdpError> { add_script_to_evaluate_on_new_document(page, &source).await } +async fn inject_auth_script_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + path: &Path, +) -> Result<(), CdpError> { + let canonical = canonicalize_safe_path(path)?; + if canonical.extension().and_then(|s| s.to_str()) != Some("js") { + return Err(CdpError::InvalidPath { + path: path.to_path_buf(), + reason: "auth script must have a `.js` extension".to_owned(), + }); + } + let source = std::fs::read_to_string(&canonical).map_err(|err| CdpError::InvalidPath { + path: canonical.clone(), + reason: format!("could not read: {err}"), + })?; + add_script_to_evaluate_on_new_document_raw(cdp, page, &source).await +} + async fn add_script_to_evaluate_on_new_document(page: &Page, source: &str) -> Result<(), CdpError> { let params = add_script_to_evaluate_params(source); with_timeout( @@ -1838,6 +2716,22 @@ async fn add_script_to_evaluate_on_new_document(page: &Page, source: &str) -> Re Ok(()) } +async fn add_script_to_evaluate_on_new_document_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + source: &str, +) -> Result<(), CdpError> { + let params = add_script_to_evaluate_params(source); + page.execute( + cdp, + "Page.addScriptToEvaluateOnNewDocument", + PAGE_COMMAND_TIMEOUT, + params, + ) + .await?; + Ok(()) +} + fn add_script_to_evaluate_params(source: &str) -> AddScriptToEvaluateOnNewDocumentParams { AddScriptToEvaluateOnNewDocumentParams { source: source.to_owned(), @@ -1872,6 +2766,30 @@ async fn install_extra_headers(page: &Page, headers: &[(String, String)]) -> Res Ok(()) } +async fn install_extra_headers_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + headers: &[(String, String)], +) -> Result<(), CdpError> { + let mut entries: Vec<(String, String)> = headers.to_vec(); + entries.sort_by(|a, b| a.0.cmp(&b.0)); + let mut object = serde_json::Map::with_capacity(entries.len()); + for (name, value) in entries { + validate_header_name(&name)?; + validate_no_ctl(&value, "value", "header")?; + object.insert(name, serde_json::Value::String(value)); + } + let params = SetExtraHttpHeadersParams::new(Headers::new(serde_json::Value::Object(object))); + page.execute( + cdp, + "Network.setExtraHTTPHeaders", + PAGE_COMMAND_TIMEOUT, + params, + ) + .await?; + Ok(()) +} + async fn install_cookies( page: &Page, cookies: &[Cookie], @@ -1916,6 +2834,42 @@ async fn install_cookies( Ok(()) } +async fn install_cookies_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + cookies: &[Cookie], + default_url: &str, +) -> Result<(), CdpError> { + let mut sorted: Vec = cookies.to_vec(); + sorted.sort_by(|a, b| { + (a.name.as_str(), a.value.as_str()).cmp(&(b.name.as_str(), b.value.as_str())) + }); + for cookie in &sorted { + validate_cookie_name(&cookie.name)?; + validate_cookie_value(&cookie.value)?; + if let Some(domain) = cookie.domain.as_deref() { + validate_no_ctl(domain, "domain", "cookie")?; + } + if let Some(path) = cookie.path.as_deref() { + validate_no_ctl(path, "path", "cookie")?; + } + } + let url_for_cookies = if default_url.starts_with("http") { + Some(default_url) + } else { + None + }; + let params = SetCookiesParams::new( + sorted + .into_iter() + .map(|c| c.into_cdp_param(url_for_cookies)) + .collect(), + ); + page.execute(cdp, "Network.setCookies", PAGE_COMMAND_TIMEOUT, params) + .await?; + Ok(()) +} + async fn install_storage_state_cookies(page: &Page, state: &StorageState) -> Result<(), CdpError> { if state.cookies.is_empty() { return Ok(()); @@ -1943,6 +2897,33 @@ async fn install_storage_state_cookies(page: &Page, state: &StorageState) -> Res Ok(()) } +async fn install_storage_state_cookies_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + state: &StorageState, +) -> Result<(), CdpError> { + if state.cookies.is_empty() { + return Ok(()); + } + let mut params: Vec = Vec::with_capacity(state.cookies.len()); + for cookie in &state.cookies { + let mut p = CookieParam::new(cookie.name.clone(), cookie.value.clone()); + p.domain = Some(cookie.domain.clone()); + p.path = Some(cookie.path.clone()); + p.secure = Some(cookie.secure); + p.http_only = Some(cookie.http_only); + params.push(p); + } + page.execute( + cdp, + "Network.setCookies storageState", + PAGE_COMMAND_TIMEOUT, + SetCookiesParams::new(params), + ) + .await?; + Ok(()) +} + async fn wait_for_selector(page: &Page, selector: &str) -> Result<(), CdpError> { // Poll `find_element` with a 50ms backoff up to 10 seconds total // (PRD §15 default). The selector is the users contract for "the @@ -1974,6 +2955,39 @@ async fn wait_for_selector(page: &Page, selector: &str) -> Result<(), CdpError> } } +async fn wait_for_selector_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + selector: &str, +) -> Result<(), CdpError> { + let selector = serde_json::to_string(selector).map_err(serde_driver_error)?; + let script = format!("document.querySelector({selector}) !== null"); + let attempt = async { + loop { + match page + .evaluate_value::( + cdp, + "Runtime.evaluate wait_for_selector", + PAGE_COMMAND_TIMEOUT, + script.as_str(), + ) + .await + { + Ok(true) => return Ok::<(), CdpError>(()), + Ok(false) | Err(_) => { + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + } + } + } + }; + match tokio::time::timeout(std::time::Duration::from_secs(10), attempt).await { + Ok(result) => result, + Err(_) => Err(CdpError::Driver(Box::new(io::Error::other(format!( + "wait_for_selector `{selector}` exhausted 10s budget" + ))))), + } +} + /// Deterministic fake driver. Recognizes `plumb-fake://hello` and returns /// [`PlumbSnapshot::canned`]. Used by the walking-skeleton CLI and by /// downstream tests. @@ -2055,44 +3069,34 @@ fn chromium_install_hint() -> String { ) } -struct ChromiumSession { +struct RawChromiumSession { browser: Browser, - handler_task: JoinHandle<()>, profile_dir: Option, } -impl ChromiumSession { +impl RawChromiumSession { async fn launch(launch: ChromiumLaunch) -> Result { - let (browser, handler) = with_timeout("Chromium launch", BROWSER_LAUNCH_TIMEOUT, async { + let (browser, _handler) = with_timeout("Chromium launch", BROWSER_LAUNCH_TIMEOUT, async { Browser::launch(launch.config) .await .map_err(map_launch_error) }) .await?; - let handler_task = poll_handler(handler); Ok(Self { browser, - handler_task, profile_dir: launch.profile_dir, }) } - async fn shutdown(&mut self) -> Result<(), CdpError> { - let cleanup_result = close_browser_best_effort(&mut self.browser).await; + fn websocket_address(&self) -> &str { + self.browser.websocket_address() + } - self.abort_handler().await; + async fn shutdown(&mut self, cdp: &mut RawCdpClient) -> Result<(), CdpError> { + let cleanup_result = close_raw_browser_best_effort(cdp, &mut self.browser).await; let _profile_dir = self.profile_dir.take(); cleanup_result } - - async fn abort_handler(&mut self) { - self.handler_task.abort(); - if let Err(join_err) = (&mut self.handler_task).await - && !join_err.is_cancelled() - { - tracing::debug!(error = %join_err, "Chromium handler task failed"); - } - } } fn poll_handler(mut handler: Handler) -> JoinHandle<()> { @@ -2216,6 +3220,37 @@ async fn close_browser_best_effort(browser: &mut Browser) -> Result<(), CdpError Ok(()) } +async fn close_raw_browser_best_effort( + cdp: &mut RawCdpClient, + browser: &mut Browser, +) -> Result<(), CdpError> { + if let Err(err) = with_timeout("Browser.close", BROWSER_CLOSE_TIMEOUT, async { + cdp.execute(None, BrowserCloseParams::default()) + .await + .map(|_: chromiumoxide::cdp::browser_protocol::browser::CloseReturns| ()) + }) + .await + { + tracing::debug!(error = %err, "failed to close Chromium over raw CDP"); + } + + if let Err(wait_err) = with_timeout("Chromium process wait", BROWSER_WAIT_TIMEOUT, async { + browser.wait().await.map_err(io_error) + }) + .await + { + tracing::debug!(error = %wait_err, "failed to wait for Chromium process"); + kill_browser(browser).await?; + with_timeout( + "Chromium process wait after kill", + BROWSER_WAIT_TIMEOUT, + async { browser.wait().await.map_err(io_error) }, + ) + .await?; + } + Ok(()) +} + async fn kill_browser(browser: &mut Browser) -> Result<(), CdpError> { if let Some(result) = tokio::time::timeout(BROWSER_KILL_TIMEOUT, browser.kill()) .await @@ -2234,6 +3269,14 @@ async fn validate_browser_version(browser: &Browser) -> Result<(), CdpError> { validate_chromium_product_major(&version.product) } +async fn validate_browser_version_raw(cdp: &mut RawCdpClient) -> Result<(), CdpError> { + let version = with_timeout("Browser.version", CDP_CONTROL_TIMEOUT, async { + cdp.execute(None, GetVersionParams::default()).await + }) + .await?; + validate_chromium_product_major(&version.product) +} + fn validate_chromium_product_major(product: &str) -> Result<(), CdpError> { let found = chromium_major_from_product(product).ok_or_else(|| { CdpError::Driver(Box::new(io::Error::new( @@ -2287,6 +3330,14 @@ fn driver_error(err: chromiumoxide::error::CdpError) -> CdpError { CdpError::Driver(Box::new(err)) } +fn serde_driver_error(err: serde_json::Error) -> CdpError { + driver_error(chromiumoxide::error::CdpError::Serde(err)) +} + +fn driver_message(message: impl Into) -> CdpError { + CdpError::Driver(Box::new(io::Error::other(message.into()))) +} + fn io_error(err: io::Error) -> CdpError { CdpError::Driver(Box::new(err)) } @@ -3336,6 +4387,38 @@ mod tests { assert!(super::should_apply_viewport_override(1, &target)); } + #[test] + fn initial_page_url_uses_loaded_non_blank_document() { + assert!(super::INITIAL_PAGE_URL.starts_with("data:text/html,")); + assert!(!super::INITIAL_PAGE_URL.contains("about:blank")); + } + + #[test] + fn initial_document_ready_accepts_loaded_initial_url() { + let state = super::NavigationState { + href: super::INITIAL_PAGE_URL.to_string(), + ready_state: "complete".to_string(), + is_chrome_error_page: false, + }; + + assert!(super::initial_document_is_ready(&state)); + assert!(!super::document_is_loaded(&state)); + } + + #[test] + fn initial_document_ready_rejects_partial_or_error_documents() { + assert!(!super::initial_document_is_ready(&super::NavigationState { + href: super::INITIAL_PAGE_URL.to_string(), + ready_state: "interactive".to_string(), + is_chrome_error_page: false, + })); + assert!(!super::initial_document_is_ready(&super::NavigationState { + href: "chrome-error://chromewebdata/".to_string(), + ready_state: "complete".to_string(), + is_chrome_error_page: true, + })); + } + #[test] fn add_script_params_registers_for_future_documents_only() { let params = super::add_script_to_evaluate_params("window.__plumb = true;"); @@ -3399,8 +4482,26 @@ mod tests { #[test] fn file_urls_keep_chromiumoxide_goto_path() { assert!(super::uses_chromiumoxide_goto("file:///tmp/static.html")); - assert!(!super::uses_chromiumoxide_goto("http://127.0.0.1:49197/")); - assert!(!super::uses_chromiumoxide_goto("https://example.com/")); + assert_eq!( + super::navigation_method_for_url("file:///tmp/static.html"), + super::NavigationMethod::ChromiumoxideGoto + ); + } + + #[test] + fn navigation_method_avoids_script_assignment_for_data_urls() { + assert_eq!( + super::navigation_method_for_url("data:text/html;base64,PHNjcmlwdD4="), + super::NavigationMethod::CdpNavigate + ); + assert_eq!( + super::navigation_method_for_url("http://127.0.0.1:49197/"), + super::NavigationMethod::LocationAssign + ); + assert_eq!( + super::navigation_method_for_url("https://example.com/"), + super::NavigationMethod::LocationAssign + ); } #[test] @@ -3420,6 +4521,11 @@ mod tests { ready_state: "complete".to_string(), is_chrome_error_page: false, })); + assert!(!super::document_is_loaded(&super::NavigationState { + href: super::INITIAL_PAGE_URL.to_string(), + ready_state: "complete".to_string(), + is_chrome_error_page: false, + })); assert!(!super::document_is_loaded(&super::NavigationState { href: "chrome-error://chromewebdata/".to_string(), ready_state: "complete".to_string(), @@ -3428,21 +4534,66 @@ mod tests { } #[test] - fn document_navigation_failed_rejects_only_uncanceled_documents() { - let document_failure = parse_loading_failed( - r#"{"requestId":"1","timestamp":1.0,"type":"Document","errorText":"net::ERR_CONNECTION_REFUSED"}"#, - ); - assert!(super::document_navigation_failed(&document_failure)); + fn selector_gated_raw_navigation_accepts_interactive_document() { + let state = super::NavigationState { + href: "https://example.com/app".to_string(), + ready_state: "interactive".to_string(), + is_chrome_error_page: false, + }; - let canceled_document = parse_loading_failed( - r#"{"requestId":"1","timestamp":1.0,"type":"Document","errorText":"net::ERR_ABORTED","canceled":true}"#, - ); - assert!(!super::document_navigation_failed(&canceled_document)); + assert!(super::document_is_ready_for_capture(&state, true)); + assert!(!super::document_is_ready_for_capture(&state, false)); + } - let stylesheet_failure = parse_loading_failed( - r#"{"requestId":"1","timestamp":1.0,"type":"Stylesheet","errorText":"net::ERR_CONNECTION_REFUSED"}"#, - ); - assert!(!super::document_navigation_failed(&stylesheet_failure)); + #[test] + fn selector_gated_raw_navigation_still_rejects_initial_documents() { + assert!(!super::document_is_ready_for_capture( + &super::NavigationState { + href: super::INITIAL_PAGE_URL.to_string(), + ready_state: "interactive".to_string(), + is_chrome_error_page: false, + }, + true, + )); + assert!(!super::document_is_ready_for_capture( + &super::NavigationState { + href: "chrome-error://chromewebdata/".to_string(), + ready_state: "interactive".to_string(), + is_chrome_error_page: true, + }, + true, + )); + } + + #[test] + fn raw_navigation_events_require_navigated_main_frame() { + let mut events = super::RawNavigationEvents::default(); + events.observe_load_event(); + + assert!(!events.is_ready_for_capture(false)); + + events.observe_main_frame_url(super::INITIAL_PAGE_URL); + events.observe_load_event(); + + assert!(!events.is_ready_for_capture(false)); + + events.observe_main_frame_url("https://example.com/app"); + + assert!(!events.is_ready_for_capture(false)); + + events.observe_load_event(); + + assert!(events.is_ready_for_capture(false)); + } + + #[test] + fn selector_gated_raw_events_accept_dom_content_after_navigation() { + let mut events = super::RawNavigationEvents::default(); + events.observe_main_frame_url("https://example.com/app"); + events.observe_dom_content_event(); + + assert!(events.is_ready_for_capture(true)); + assert!(!events.is_ready_for_capture(false)); } #[test] @@ -3488,6 +4639,18 @@ mod tests { assert!(reason.contains("last navigation state read failed")); } + #[test] + fn navigation_display_url_redacts_data_urls() { + assert_eq!( + super::navigation_display_url("data:text/html;base64,PHNjcmlwdD4="), + "data:" + ); + assert_eq!( + super::navigation_display_url("http://127.0.0.1:49197/"), + "http://127.0.0.1:49197/" + ); + } + #[test] fn contextualize_request_timeout_labels_operation() { let err = super::contextualize_request_timeout( @@ -3500,6 +4663,35 @@ mod tests { assert!(message.contains("Chromiumoxide request budget")); } + #[test] + fn target_lifecycle_error_labels_pre_navigation_stage() { + let err = super::target_lifecycle_error( + "Target.createTarget", + &CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "Target.createTarget exceeded 10s budget", + ))), + ); + + let message = err.to_string(); + assert!(message.contains("Target.createTarget failed before navigation")); + assert!(message.contains("Target.createTarget exceeded 10s budget")); + assert!(super::is_retryable_capture_timeout(&err)); + } + + #[test] + fn target_lifecycle_error_keeps_non_timeout_errors_non_retryable() { + let err = super::target_lifecycle_error( + "Target.attachToTarget", + &CdpError::Driver(Box::new(io::Error::other("target disappeared"))), + ); + + let message = err.to_string(); + assert!(message.contains("Target.attachToTarget failed before navigation")); + assert!(message.contains("target disappeared")); + assert!(!super::is_retryable_capture_timeout(&err)); + } + #[test] fn retryable_capture_timeout_accepts_chromiumoxide_timeout() { let err = CdpError::Driver(Box::new(chromiumoxide::error::CdpError::Timeout)); @@ -3526,13 +4718,6 @@ mod tests { assert!(!super::is_retryable_capture_timeout(&err)); } - fn parse_loading_failed(raw: &str) -> super::EventLoadingFailed { - match serde_json::from_str(raw) { - Ok(event) => event, - Err(err) => panic!("loading failed event parse failed: {err}"), - } - } - fn test_executable_path() -> PathBuf { match std::env::current_exe() { Ok(path) => path, diff --git a/crates/plumb-mcp/src/lib.rs b/crates/plumb-mcp/src/lib.rs index d868525..cac9f80 100644 --- a/crates/plumb-mcp/src/lib.rs +++ b/crates/plumb-mcp/src/lib.rs @@ -404,9 +404,10 @@ impl PlumbServer { let config = self.resolve_config_object(args.working_dir.as_deref())?; + let guarded_html = lint_page_html_guarded_document(&args.html); let data_url = format!( "data:text/html;base64,{}", - base64_encode(args.html.as_bytes()) + base64_encode(guarded_html.as_bytes()) ); let target = Target { url: data_url, @@ -822,6 +823,74 @@ const LINT_PAGE_HTML_INPUT_BYTE_CAP: usize = 1024 * 1024; /// (10 000), estimated cheaply from the raw HTML before rendering. const LINT_PAGE_HTML_ELEMENT_CAP: usize = 10_000; +const LINT_PAGE_HTML_CSP_META: &str = concat!( + r#""# +); + +fn lint_page_html_guarded_document(html: &str) -> String { + let mut guarded = String::with_capacity(LINT_PAGE_HTML_CSP_META.len() + html.len()); + if let Some(index) = lint_page_html_csp_insert_index(html) { + guarded.push_str(&html[..index]); + guarded.push_str(LINT_PAGE_HTML_CSP_META); + guarded.push_str(&html[index..]); + } else { + guarded.push_str(LINT_PAGE_HTML_CSP_META); + guarded.push_str(html); + } + guarded +} + +fn lint_page_html_csp_insert_index(html: &str) -> Option { + find_head_tag_end(html).or_else(|| find_doctype_end(html)) +} + +fn find_head_tag_end(html: &str) -> Option { + let bytes = html.as_bytes(); + let needle = b"').map(|offset| after + offset + 1); + } + None +} + +fn find_doctype_end(html: &str) -> Option { + let start = html + .char_indices() + .find(|(_, ch)| !ch.is_ascii_whitespace()) + .map_or(html.len(), |(index, _)| index); + let rest = &html[start..]; + let needle = b"').map(|offset| start + offset + 1) +} + +fn is_html_tag_boundary(byte: u8) -> bool { + matches!(byte, b'>' | b'/' | b' ' | b'\t' | b'\n' | b'\r' | 0x0c) +} + /// Conservative pre-render estimate of an HTML document's element count: /// the number of `<` bytes immediately followed by an ASCII letter (an /// opening tag). Closing tags (`

x

"#; + let guarded = lint_page_html_guarded_document(html); + + assert!( + guarded.starts_with(r#"x

"#)); + } + + #[test] + fn lint_page_html_guarded_document_inserts_csp_before_head_resources() { + let html = r#"x"#; + let guarded = lint_page_html_guarded_document(html); + + assert!(guarded.starts_with("")); + let csp_index = guarded + .find(r#"x

"#; + let guarded = lint_page_html_guarded_document(html); + + assert!( + guarded.starts_with(r#" PlumbServer { PlumbServer::new(PathBuf::from("/")) } +fn tool_text(result: &CallToolResult) -> Option { + result + .content + .iter() + .find_map(|content| content.as_text().map(|text| text.text.clone())) +} + +#[cfg(feature = "e2e-chromium")] +fn chromium_unavailable_result(result: &CallToolResult) -> bool { + if !result.is_error.unwrap_or(false) { + return false; + } + + tool_text(result).is_some_and(|text| { + text.contains("Chromium executable not found") + || text.contains("Chromium major version") + || text.contains("Chromium auto-fetch failed") + }) +} + +#[cfg(feature = "e2e-chromium")] +fn assert_rendered_or_skip_unavailable(result: &CallToolResult, context: &str) -> bool { + if chromium_unavailable_result(result) { + return false; + } + + assert!( + !result.is_error.unwrap_or(false), + "{context}: {}", + tool_text(result).unwrap_or_else(|| "".to_owned()) + ); + true +} + #[test] fn server_info_declares_plumb() { let server = server(); @@ -351,11 +395,7 @@ async fn lint_page_html_defect_html_never_returns_false_clean() { if result.is_error.unwrap_or(false) { // Chromium unavailable (or another driver failure): the response // must be a clear error, not a misleading clean. - let text = result - .content - .iter() - .find_map(|content| content.as_text().map(|text| text.text.clone())) - .expect("error response must include a text content block"); + let text = tool_text(&result).expect("error response must include a text content block"); assert!( text.contains("lint_page_html failed"), "driver failure must be explicit, got: {text}" @@ -395,8 +435,10 @@ async fn lint_page_html_renders_embedded_style_into_grid_finding() { .await .expect("lint_page_html must not raise a JSON-RPC error for valid input"); - if result.is_error.unwrap_or(false) { - // No usable Chromium on this host — skip, same as the cdp e2e suite. + if !assert_rendered_or_skip_unavailable( + &result, + "lint_page_html must render embedded styles when Chromium is available", + ) { return; } @@ -415,6 +457,94 @@ async fn lint_page_html_renders_embedded_style_into_grid_finding() { server.shutdown().await.expect("shutdown must succeed"); } +#[cfg(feature = "e2e-chromium")] +#[tokio::test] +async fn lint_page_html_blocks_external_resource_fetches() { + let listener = TcpListener::bind(("127.0.0.1", 0)).expect("bind local canary server"); + listener + .set_nonblocking(true) + .expect("listener must be nonblocking"); + let port = listener.local_addr().expect("local addr").port(); + let hits = Arc::new(AtomicUsize::new(0)); + let stop = Arc::new(AtomicBool::new(false)); + let thread_hits = Arc::clone(&hits); + let thread_stop = Arc::clone(&stop); + let handle = std::thread::spawn(move || { + while !thread_stop.load(Ordering::SeqCst) { + match listener.accept() { + Ok((mut stream, _addr)) => { + thread_hits.fetch_add(1, Ordering::SeqCst); + let mut buf = [0; 512]; + let _ = stream.read(&mut buf); + let _ = + stream.write_all(b"HTTP/1.1 204 No Content\r\nContent-Length: 0\r\n\r\n"); + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + std::thread::sleep(Duration::from_millis(20)); + } + Err(_) => break, + } + } + }); + + let external = format!("http://127.0.0.1:{port}/blocked"); + let html = format!( + r#" + + + + + + + + +
x
+ +"# + ); + + let server = server(); + let result = server + .lint_page_html(LintPageHtmlArgs { + html, + base_url: "https://example.com/".to_owned(), + working_dir: None, + }) + .await + .expect("lint_page_html must not raise a JSON-RPC error for valid input"); + + tokio::time::sleep(Duration::from_millis(250)).await; + stop.store(true, Ordering::SeqCst); + handle.join().expect("canary thread must join"); + + assert_eq!( + hits.load(Ordering::SeqCst), + 0, + "lint_page_html must not fetch external resources" + ); + + if !assert_rendered_or_skip_unavailable( + &result, + "lint_page_html must block external resources without breaking data-url rendering", + ) { + return; + } + + let structured = result + .structured_content + .expect("successful response must include structured_content"); + let by_rule = structured + .get("by_rule") + .and_then(serde_json::Value::as_object) + .expect("structuredContent.by_rule must be an object"); + assert!( + by_rule.contains_key("spacing/grid-conformance"), + "inline style must still render after external fetches are blocked: {structured}", + ); + + server.shutdown().await.expect("shutdown must succeed"); +} + /// `lint_page_html` rejects empty `html` with a JSON-RPC `invalid_params` /// error so the agent gets a clear contract failure rather than a /// silently-empty snapshot. Validated before any rendering, so it runs From d08bcdbe6b9431251b87f2012119bcc8d5b79240 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 07:15:51 -0600 Subject: [PATCH 21/50] fix(core): ignore hidden route announcer spacing --- .../src/rules/spacing/grid_conformance.rs | 5 ++- crates/plumb-core/src/rules/spacing/mod.rs | 6 ++++ .../src/rules/spacing/scale_conformance.rs | 5 ++- .../plumb-core/tests/golden_spacing_grid.rs | 34 +++++++++++++++++++ .../plumb-core/tests/golden_spacing_scale.rs | 34 +++++++++++++++++++ e2e-sites/nextjs/README.md | 13 ++++--- e2e-sites/nextjs/expected.json | 8 ++--- 7 files changed, 92 insertions(+), 13 deletions(-) diff --git a/crates/plumb-core/src/rules/spacing/grid_conformance.rs b/crates/plumb-core/src/rules/spacing/grid_conformance.rs index c0893c3..c379508 100644 --- a/crates/plumb-core/src/rules/spacing/grid_conformance.rs +++ b/crates/plumb-core/src/rules/spacing/grid_conformance.rs @@ -20,7 +20,7 @@ use indexmap::IndexMap; use crate::config::Config; use crate::report::{Confidence, Fix, FixKind, Severity, Violation, ViolationSink}; use crate::rules::Rule; -use crate::rules::spacing::SPACING_PROPERTIES; +use crate::rules::spacing::{SPACING_PROPERTIES, is_framework_hidden_spacing_node}; use crate::rules::util::{nearest_in_scale, nearest_multiple, parse_px}; use crate::snapshot::SnapshotCtx; @@ -63,6 +63,9 @@ impl Rule for GridConformance { } for node in ctx.nodes() { + if is_framework_hidden_spacing_node(node) { + continue; + } for prop in SPACING_PROPERTIES { if is_root_body_margin(&node.tag, &node.selector, prop) { continue; diff --git a/crates/plumb-core/src/rules/spacing/mod.rs b/crates/plumb-core/src/rules/spacing/mod.rs index ec8170e..222c6c3 100644 --- a/crates/plumb-core/src/rules/spacing/mod.rs +++ b/crates/plumb-core/src/rules/spacing/mod.rs @@ -12,6 +12,8 @@ pub mod grid_conformance; pub mod scale_conformance; +use crate::snapshot::SnapshotNode; + /// Physical-longhand spacing properties the rules in this category /// inspect. /// @@ -32,3 +34,7 @@ pub(crate) const SPACING_PROPERTIES: &[&str] = &[ "row-gap", "column-gap", ]; + +pub(crate) fn is_framework_hidden_spacing_node(node: &SnapshotNode) -> bool { + node.selector == "html > body > next-route-announcer > div" +} diff --git a/crates/plumb-core/src/rules/spacing/scale_conformance.rs b/crates/plumb-core/src/rules/spacing/scale_conformance.rs index 425b695..4e69d41 100644 --- a/crates/plumb-core/src/rules/spacing/scale_conformance.rs +++ b/crates/plumb-core/src/rules/spacing/scale_conformance.rs @@ -11,7 +11,7 @@ use indexmap::IndexMap; use crate::config::Config; use crate::report::{Confidence, Fix, FixKind, Severity, Violation, ViolationSink}; use crate::rules::Rule; -use crate::rules::spacing::SPACING_PROPERTIES; +use crate::rules::spacing::{SPACING_PROPERTIES, is_framework_hidden_spacing_node}; use crate::rules::util::{nearest_in_scale, parse_px}; use crate::snapshot::SnapshotCtx; @@ -47,6 +47,9 @@ impl Rule for ScaleConformance { } for node in ctx.nodes() { + if is_framework_hidden_spacing_node(node) { + continue; + } for prop in SPACING_PROPERTIES { let Some(raw) = node.computed_styles.get(*prop) else { continue; diff --git a/crates/plumb-core/tests/golden_spacing_grid.rs b/crates/plumb-core/tests/golden_spacing_grid.rs index 0bfd470..b6c02f9 100644 --- a/crates/plumb-core/tests/golden_spacing_grid.rs +++ b/crates/plumb-core/tests/golden_spacing_grid.rs @@ -270,6 +270,40 @@ fn spacing_grid_conformance_skips_root_body_margins() { assert!(violations[0].message.contains("padding-top 13px")); } +#[test] +fn spacing_grid_conformance_skips_next_route_announcer() { + let announcer = node( + 2, + "html > body > next-route-announcer > div", + &[ + ("margin-top", "-1px"), + ("margin-right", "-1px"), + ("margin-bottom", "-1px"), + ("margin-left", "-1px"), + ], + Some(Rect { + x: -1, + y: 333, + width: 1, + height: 1, + }), + ); + + let snapshot = PlumbSnapshot { + url: "plumb-fake://spacing-grid-next-route-announcer".into(), + viewport: ViewportKey::new("desktop"), + viewport_width: 1280, + viewport_height: 800, + nodes: vec![root_html_with_body(), body_node(), announcer], + text_boxes: Vec::new(), + }; + let has_violation = run(&snapshot, &fixture_config()) + .into_iter() + .any(|v| v.rule_id == "spacing/grid-conformance"); + + assert!(!has_violation); +} + /// Build a one-node snapshot carrying a single spacing property and /// count `spacing/grid-conformance` violations under `config`. Lets the /// scale-deferral tests vary both the property value and the configured diff --git a/crates/plumb-core/tests/golden_spacing_scale.rs b/crates/plumb-core/tests/golden_spacing_scale.rs index 8b482d0..7482010 100644 --- a/crates/plumb-core/tests/golden_spacing_scale.rs +++ b/crates/plumb-core/tests/golden_spacing_scale.rs @@ -178,6 +178,40 @@ fn spacing_scale_conformance_golden() -> Result<(), serde_json::Error> { Ok(()) } +#[test] +fn spacing_scale_conformance_skips_next_route_announcer() { + let announcer = node( + 2, + "html > body > next-route-announcer > div", + &[ + ("margin-top", "-1px"), + ("margin-right", "-1px"), + ("margin-bottom", "-1px"), + ("margin-left", "-1px"), + ], + Some(Rect { + x: -1, + y: 333, + width: 1, + height: 1, + }), + ); + + let snapshot = PlumbSnapshot { + url: "plumb-fake://spacing-scale-next-route-announcer".into(), + viewport: ViewportKey::new("desktop"), + viewport_width: 1280, + viewport_height: 800, + nodes: vec![root_html(), body_node(), announcer], + text_boxes: Vec::new(), + }; + let has_violation = run(&snapshot, &fixture_config()) + .into_iter() + .any(|v| v.rule_id == "spacing/scale-conformance"); + + assert!(!has_violation); +} + #[test] fn spacing_scale_conformance_run_is_deterministic() -> Result<(), serde_json::Error> { let snapshot = fixture_snapshot(); diff --git a/e2e-sites/nextjs/README.md b/e2e-sites/nextjs/README.md index 11362e0..c4ac37b 100644 --- a/e2e-sites/nextjs/README.md +++ b/e2e-sites/nextjs/README.md @@ -13,16 +13,15 @@ post-hydration DOM via Chromium DevTools. Next.js injects a hidden `` element with `margin: -1px` (the standard "visually hidden" trick used to stay accessible -without contributing visible layout). The four longhand margin values -are off-grid against `spacing.base_unit = 4` and off-scale against the -configured `spacing.scale`, so they emit 4 grid + 4 scale violations -on top of the hero's intended 4 + 4. `expected.json` documents the -total target counts as `8` + `8` accordingly. +without contributing visible layout). Plumb ignores that framework +infrastructure for spacing rules because the node is hidden, +nondeterministic across Next versions, and not actionable for an agent +fix loop. The target counts below cover the app-owned hero violation. | Rule | Source | Count | | --------------------------- | ------------------------------------------------------- | ----- | -| `spacing/grid-conformance` | `
` + route announcer | 8 | -| `spacing/scale-conformance` | `
` + route announcer | 8 | +| `spacing/grid-conformance` | `
` | 4 | +| `spacing/scale-conformance` | `
` | 4 | | `color/palette-conformance` | `

` | 1 | ## Build diff --git a/e2e-sites/nextjs/expected.json b/e2e-sites/nextjs/expected.json index 8f01738..1a64506 100644 --- a/e2e-sites/nextjs/expected.json +++ b/e2e-sites/nextjs/expected.json @@ -1,5 +1,5 @@ { - "$comment": "Next.js 14 statically exported. The framework injects a hidden `` that uses the visually-hidden `margin: -1px` trick — that contributes 4 grid + 4 scale violations on top of the intentional `p-[13px]` hero. Expected counts are doubled accordingly. The non-target rules `sibling/height-consistency` and `sibling/padding-consistency` also fire on the announcer; they're not asserted on. The `wait_for` block points at a `data-plumb-ready` sentinel server-rendered onto `` in `app/layout.tsx`, which makes the captured DOM byte-stable across Linux/macOS/Windows runs.", + "$comment": "Next.js 14 statically exported. The framework may inject a hidden `` that uses the visually-hidden `margin: -1px` trick; spacing rules ignore that framework infrastructure because it is nondeterministic and unactionable. The `wait_for` block points at a `data-plumb-ready` sentinel server-rendered onto `` in `app/layout.tsx`, which makes the app-owned DOM byte-stable across Linux/macOS/Windows runs.", "target_rules": [ "color/palette-conformance", "spacing/grid-conformance", @@ -7,10 +7,10 @@ ], "by_rule_id": { "color/palette-conformance": 1, - "spacing/grid-conformance": 8, - "spacing/scale-conformance": 8 + "spacing/grid-conformance": 4, + "spacing/scale-conformance": 4 }, - "total_target_violations": 17, + "total_target_violations": 9, "wait_for": { "selector": "html[data-plumb-ready=\"true\"]", "timeout_ms": 30000 From c8796150fcf3b33577be59c39056f6a4e8c9af34 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 07:44:33 -0600 Subject: [PATCH 22/50] fix(cdp): tolerate slow bootstrap document readiness --- crates/plumb-cdp/src/lib.rs | 55 ++++++++++++++++++++++++++++++------- 1 file changed, 45 insertions(+), 10 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index c6e4a8b..1f6ccbc 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -113,6 +113,8 @@ const PAGE_COMMAND_TIMEOUT: Duration = Duration::from_secs(25); const PAGE_ENABLE_TIMEOUT: Duration = Duration::from_secs(5); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); +const INITIAL_DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(2); +const INITIAL_NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_millis(500); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(25); const TRANSIENT_CAPTURE_RETRIES: usize = 1; @@ -1009,7 +1011,7 @@ async fn capture_target_raw( ) .await?; - wait_for_initial_document_ready_raw(cdp, &page).await?; + best_effort_wait_for_initial_document_ready_raw(cdp, &page).await; capture_on_raw_page(cdp, &page, target, options, apply_viewport_override).await } @@ -1584,7 +1586,7 @@ impl PersistentBrowser { hidden: None, }; let page = create_page_without_load_wait(&self.inner.browser, create_params).await?; - wait_for_initial_document_ready(&page).await?; + best_effort_wait_for_initial_document_ready(&page).await; capture_on_page(&page, target, &self.inner.options, true).await } .await; @@ -2055,13 +2057,25 @@ async fn wait_for_document_ready( Err(CdpError::Driver(Box::new(io::Error::other(reason)))) } +async fn best_effort_wait_for_initial_document_ready(page: &Page) { + if let Err(err) = wait_for_initial_document_ready(page).await { + tracing::debug!( + error = %err, + "initial document did not report ready before navigation; continuing" + ); + } +} + async fn wait_for_initial_document_ready(page: &Page) -> Result<(), CdpError> { let mut last_state_error = None; let attempt = async { loop { tokio::time::sleep(Duration::from_millis(50)).await; - match tokio::time::timeout(NAVIGATION_STATE_READ_TIMEOUT, read_navigation_state(page)) - .await + match tokio::time::timeout( + INITIAL_NAVIGATION_STATE_READ_TIMEOUT, + read_navigation_state(page), + ) + .await { Ok(Ok(state)) if initial_document_is_ready(&state) => return Ok(()), Ok(Ok(_)) => {} @@ -2069,14 +2083,14 @@ async fn wait_for_initial_document_ready(page: &Page) -> Result<(), CdpError> { Err(_) => { last_state_error = Some(timeout_reason( "initial navigation state read", - NAVIGATION_STATE_READ_TIMEOUT, + INITIAL_NAVIGATION_STATE_READ_TIMEOUT, )); } } } }; - if let Ok(result) = tokio::time::timeout(DOCUMENT_READY_TIMEOUT, attempt).await { + if let Ok(result) = tokio::time::timeout(INITIAL_DOCUMENT_READY_TIMEOUT, attempt).await { return result; } @@ -2085,6 +2099,15 @@ async fn wait_for_initial_document_ready(page: &Page) -> Result<(), CdpError> { )))) } +async fn best_effort_wait_for_initial_document_ready_raw(cdp: &mut RawCdpClient, page: &RawPage) { + if let Err(err) = wait_for_initial_document_ready_raw(cdp, page).await { + tracing::debug!( + error = %err, + "initial raw document did not report ready before navigation; continuing" + ); + } +} + async fn wait_for_initial_document_ready_raw( cdp: &mut RawCdpClient, page: &RawPage, @@ -2094,7 +2117,7 @@ async fn wait_for_initial_document_ready_raw( loop { tokio::time::sleep(Duration::from_millis(50)).await; match tokio::time::timeout( - NAVIGATION_STATE_READ_TIMEOUT, + INITIAL_NAVIGATION_STATE_READ_TIMEOUT, read_navigation_state_raw(cdp, page), ) .await @@ -2105,14 +2128,14 @@ async fn wait_for_initial_document_ready_raw( Err(_) => { last_state_error = Some(timeout_reason( "initial navigation state read", - NAVIGATION_STATE_READ_TIMEOUT, + INITIAL_NAVIGATION_STATE_READ_TIMEOUT, )); } } } }; - if let Ok(result) = tokio::time::timeout(DOCUMENT_READY_TIMEOUT, attempt).await { + if let Ok(result) = tokio::time::timeout(INITIAL_DOCUMENT_READY_TIMEOUT, attempt).await { return result; } @@ -2424,7 +2447,7 @@ fn navigation_display_url(url: &str) -> &str { fn initial_document_ready_timeout_reason(last_state_error: Option<&str>) -> String { let mut reason = format!( "initial document exhausted {} ready-state budget", - timeout_budget_label(DOCUMENT_READY_TIMEOUT) + timeout_budget_label(INITIAL_DOCUMENT_READY_TIMEOUT) ); if let Some(err) = last_state_error { reason.push_str("; last initial navigation state read failed: "); @@ -4639,6 +4662,18 @@ mod tests { assert!(reason.contains("last navigation state read failed")); } + #[test] + fn initial_document_timeout_reason_uses_short_bootstrap_budget() { + let reason = super::initial_document_ready_timeout_reason(Some( + "initial navigation state read exceeded 500ms budget", + )); + + assert!(reason.contains("exhausted 2s ready-state budget")); + assert!(reason.contains( + "last initial navigation state read failed: initial navigation state read exceeded 500ms budget" + )); + } + #[test] fn navigation_display_url_redacts_data_urls() { assert_eq!( From d49b9dd756776b7bad63d3f1331ccb9db1dbafb5 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 08:06:25 -0600 Subject: [PATCH 23/50] fix(cdp): avoid bootstrap navigation probes --- crates/plumb-cdp/src/lib.rs | 151 ++---------------------------------- 1 file changed, 8 insertions(+), 143 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 1f6ccbc..eeacb9a 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -113,8 +113,7 @@ const PAGE_COMMAND_TIMEOUT: Duration = Duration::from_secs(25); const PAGE_ENABLE_TIMEOUT: Duration = Duration::from_secs(5); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); -const INITIAL_DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(2); -const INITIAL_NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_millis(500); +const INITIAL_DOCUMENT_SETTLE_DELAY: Duration = Duration::from_millis(100); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(25); const TRANSIENT_CAPTURE_RETRIES: usize = 1; @@ -1011,7 +1010,7 @@ async fn capture_target_raw( ) .await?; - best_effort_wait_for_initial_document_ready_raw(cdp, &page).await; + settle_initial_document().await; capture_on_raw_page(cdp, &page, target, options, apply_viewport_override).await } @@ -1586,7 +1585,7 @@ impl PersistentBrowser { hidden: None, }; let page = create_page_without_load_wait(&self.inner.browser, create_params).await?; - best_effort_wait_for_initial_document_ready(&page).await; + settle_initial_document().await; capture_on_page(&page, target, &self.inner.options, true).await } .await; @@ -2057,91 +2056,11 @@ async fn wait_for_document_ready( Err(CdpError::Driver(Box::new(io::Error::other(reason)))) } -async fn best_effort_wait_for_initial_document_ready(page: &Page) { - if let Err(err) = wait_for_initial_document_ready(page).await { - tracing::debug!( - error = %err, - "initial document did not report ready before navigation; continuing" - ); - } -} - -async fn wait_for_initial_document_ready(page: &Page) -> Result<(), CdpError> { - let mut last_state_error = None; - let attempt = async { - loop { - tokio::time::sleep(Duration::from_millis(50)).await; - match tokio::time::timeout( - INITIAL_NAVIGATION_STATE_READ_TIMEOUT, - read_navigation_state(page), - ) - .await - { - Ok(Ok(state)) if initial_document_is_ready(&state) => return Ok(()), - Ok(Ok(_)) => {} - Ok(Err(err)) => last_state_error = Some(err.to_string()), - Err(_) => { - last_state_error = Some(timeout_reason( - "initial navigation state read", - INITIAL_NAVIGATION_STATE_READ_TIMEOUT, - )); - } - } - } - }; - - if let Ok(result) = tokio::time::timeout(INITIAL_DOCUMENT_READY_TIMEOUT, attempt).await { - return result; - } - - Err(CdpError::Driver(Box::new(io::Error::other( - initial_document_ready_timeout_reason(last_state_error.as_deref()), - )))) -} - -async fn best_effort_wait_for_initial_document_ready_raw(cdp: &mut RawCdpClient, page: &RawPage) { - if let Err(err) = wait_for_initial_document_ready_raw(cdp, page).await { - tracing::debug!( - error = %err, - "initial raw document did not report ready before navigation; continuing" - ); - } -} - -async fn wait_for_initial_document_ready_raw( - cdp: &mut RawCdpClient, - page: &RawPage, -) -> Result<(), CdpError> { - let mut last_state_error = None; - let attempt = async { - loop { - tokio::time::sleep(Duration::from_millis(50)).await; - match tokio::time::timeout( - INITIAL_NAVIGATION_STATE_READ_TIMEOUT, - read_navigation_state_raw(cdp, page), - ) - .await - { - Ok(Ok(state)) if initial_document_is_ready(&state) => return Ok(()), - Ok(Ok(_)) => {} - Ok(Err(err)) => last_state_error = Some(err.to_string()), - Err(_) => { - last_state_error = Some(timeout_reason( - "initial navigation state read", - INITIAL_NAVIGATION_STATE_READ_TIMEOUT, - )); - } - } - } - }; - - if let Ok(result) = tokio::time::timeout(INITIAL_DOCUMENT_READY_TIMEOUT, attempt).await { - return result; - } - - Err(CdpError::Driver(Box::new(io::Error::other( - initial_document_ready_timeout_reason(last_state_error.as_deref()), - )))) +async fn settle_initial_document() { + // Avoid Runtime.evaluate on the bootstrap page. Chrome for Testing + // 150 on macOS can leave that probe in flight before the real + // navigation, which makes the subsequent Page.navigate unreliable. + tokio::time::sleep(INITIAL_DOCUMENT_SETTLE_DELAY).await; } async fn wait_for_document_ready_raw( @@ -2356,10 +2275,6 @@ fn document_is_ready_for_capture(state: &NavigationState, allow_interactive: boo || (allow_interactive && state.ready_state == "interactive")) } -fn initial_document_is_ready(state: &NavigationState) -> bool { - !state.is_chrome_error_page && state.ready_state == "complete" -} - fn document_has_navigated(state: &NavigationState) -> bool { url_has_navigated(&state.href) && !state.is_chrome_error_page } @@ -2444,18 +2359,6 @@ fn navigation_display_url(url: &str) -> &str { } } -fn initial_document_ready_timeout_reason(last_state_error: Option<&str>) -> String { - let mut reason = format!( - "initial document exhausted {} ready-state budget", - timeout_budget_label(INITIAL_DOCUMENT_READY_TIMEOUT) - ); - if let Some(err) = last_state_error { - reason.push_str("; last initial navigation state read failed: "); - reason.push_str(err); - } - reason -} - /// Wait stages that must run *after* navigation. PRD §15 — `--wait-for` /// and `--wait-ms`. /// @@ -4416,32 +4319,6 @@ mod tests { assert!(!super::INITIAL_PAGE_URL.contains("about:blank")); } - #[test] - fn initial_document_ready_accepts_loaded_initial_url() { - let state = super::NavigationState { - href: super::INITIAL_PAGE_URL.to_string(), - ready_state: "complete".to_string(), - is_chrome_error_page: false, - }; - - assert!(super::initial_document_is_ready(&state)); - assert!(!super::document_is_loaded(&state)); - } - - #[test] - fn initial_document_ready_rejects_partial_or_error_documents() { - assert!(!super::initial_document_is_ready(&super::NavigationState { - href: super::INITIAL_PAGE_URL.to_string(), - ready_state: "interactive".to_string(), - is_chrome_error_page: false, - })); - assert!(!super::initial_document_is_ready(&super::NavigationState { - href: "chrome-error://chromewebdata/".to_string(), - ready_state: "complete".to_string(), - is_chrome_error_page: true, - })); - } - #[test] fn add_script_params_registers_for_future_documents_only() { let params = super::add_script_to_evaluate_params("window.__plumb = true;"); @@ -4662,18 +4539,6 @@ mod tests { assert!(reason.contains("last navigation state read failed")); } - #[test] - fn initial_document_timeout_reason_uses_short_bootstrap_budget() { - let reason = super::initial_document_ready_timeout_reason(Some( - "initial navigation state read exceeded 500ms budget", - )); - - assert!(reason.contains("exhausted 2s ready-state budget")); - assert!(reason.contains( - "last initial navigation state read failed: initial navigation state read exceeded 500ms budget" - )); - } - #[test] fn navigation_display_url_redacts_data_urls() { assert_eq!( From b999b6c835ee340c73f85aba3be4bd10bb7b9bdb Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 08:25:27 -0600 Subject: [PATCH 24/50] fix(cdp): use blank bootstrap page --- crates/plumb-cdp/src/lib.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index eeacb9a..ca8da40 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -117,7 +117,7 @@ const INITIAL_DOCUMENT_SETTLE_DELAY: Duration = Duration::from_millis(100); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(25); const TRANSIENT_CAPTURE_RETRIES: usize = 1; -const INITIAL_PAGE_URL: &str = "data:text/html,%3C!doctype%20html%3E%3Ctitle%3Eplumb%3C%2Ftitle%3E"; +const INITIAL_PAGE_URL: &str = "about:blank"; /// CSS property whitelist passed to `DOMSnapshot.captureSnapshot` as the /// `computedStyles` argument. @@ -2057,9 +2057,9 @@ async fn wait_for_document_ready( } async fn settle_initial_document() { - // Avoid Runtime.evaluate on the bootstrap page. Chrome for Testing - // 150 on macOS can leave that probe in flight before the real - // navigation, which makes the subsequent Page.navigate unreliable. + // Avoid probing the bootstrap page before the real navigation. On + // macOS CFT 150, pre-navigation probes and interrupted data: loads + // can make the subsequent Page.navigate unreliable. tokio::time::sleep(INITIAL_DOCUMENT_SETTLE_DELAY).await; } @@ -2280,7 +2280,7 @@ fn document_has_navigated(state: &NavigationState) -> bool { } fn url_has_navigated(url: &str) -> bool { - url != "about:blank" && url != INITIAL_PAGE_URL + url != INITIAL_PAGE_URL } async fn read_navigation_state(page: &Page) -> Result { @@ -4314,9 +4314,8 @@ mod tests { } #[test] - fn initial_page_url_uses_loaded_non_blank_document() { - assert!(super::INITIAL_PAGE_URL.starts_with("data:text/html,")); - assert!(!super::INITIAL_PAGE_URL.contains("about:blank")); + fn initial_page_url_uses_blank_bootstrap_document() { + assert_eq!(super::INITIAL_PAGE_URL, "about:blank"); } #[test] @@ -4416,11 +4415,6 @@ mod tests { ready_state: "interactive".to_string(), is_chrome_error_page: false, })); - assert!(!super::document_is_loaded(&super::NavigationState { - href: "about:blank".to_string(), - ready_state: "complete".to_string(), - is_chrome_error_page: false, - })); assert!(!super::document_is_loaded(&super::NavigationState { href: super::INITIAL_PAGE_URL.to_string(), ready_state: "complete".to_string(), From 6af08c980ceb1996f3986bdfe9167c46ff87c349 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 08:43:44 -0600 Subject: [PATCH 25/50] fix(cdp): retry startup navigation aborts --- crates/plumb-cdp/src/lib.rs | 39 ++++++++++++++++++++++++++++++++++--- 1 file changed, 36 insertions(+), 3 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index ca8da40..27e7379 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -3078,9 +3078,21 @@ fn is_retryable_capture_timeout(err: &CdpError) -> bool { return true; } - source - .downcast_ref::() - .is_some_and(|err| err.kind() == io::ErrorKind::TimedOut) + source.downcast_ref::().is_some_and(|err| { + err.kind() == io::ErrorKind::TimedOut || is_startup_navigation_abort(err) + }) +} + +fn is_startup_navigation_abort(err: &io::Error) -> bool { + if err.kind() != io::ErrorKind::Other { + return false; + } + + let message = err.to_string(); + message.contains("exhausted 30s ready-state budget") + && message.contains("after initial location assignment failed:") + && message.contains("Page.navigate failed: net::ERR_ABORTED") + && message.contains("last navigation state read failed: navigation state read exceeded") } fn timeout_error(operation: &str, timeout: Duration) -> CdpError { @@ -4603,6 +4615,27 @@ mod tests { assert!(super::is_retryable_capture_timeout(&err)); } + #[test] + fn retryable_capture_timeout_accepts_startup_navigation_abort() { + let err = CdpError::Driver(Box::new(io::Error::other( + "navigation to `http://127.0.0.1:49216/` exhausted 30s ready-state budget \ + after initial location assignment failed: driver failure: Page.navigate failed: \ + net::ERR_ABORTED; last navigation state read failed: navigation state read \ + exceeded 2s budget", + ))); + + assert!(super::is_retryable_capture_timeout(&err)); + } + + #[test] + fn retryable_capture_timeout_rejects_bare_navigation_abort() { + let err = CdpError::Driver(Box::new(io::Error::other( + "Page.navigate failed: net::ERR_ABORTED", + ))); + + assert!(!super::is_retryable_capture_timeout(&err)); + } + #[test] fn retryable_capture_timeout_rejects_non_timeout_errors() { let err = CdpError::MalformedSnapshot { From 48768bc3f42a788d5a861dc75fa3082927faccb8 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 09:03:24 -0600 Subject: [PATCH 26/50] fix(cdp): use script navigation for raw web targets --- crates/plumb-cdp/src/lib.rs | 130 ++++++++++++++++++++++++++++++++---- 1 file changed, 116 insertions(+), 14 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 27e7379..4acc78f 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1317,6 +1317,31 @@ impl RawPage { } Ok(()) } + + async fn evaluate_unit_collecting_page_events( + &self, + cdp: &mut RawCdpClient, + operation: &str, + timeout: Duration, + expression: &str, + events: &mut RawNavigationEvents, + ) -> Result<(), CdpError> { + let params = EvaluateParams::builder() + .expression(expression) + .await_promise(true) + .return_by_value(true) + .build() + .map_err(driver_message)?; + let result = self + .execute_collecting_page_events(cdp, operation, timeout, params, events) + .await?; + if let Some(exception) = result.exception_details { + return Err(driver_error( + chromiumoxide::error::CdpError::JavascriptException(Box::new(exception)), + )); + } + Ok(()) + } } async fn capture_on_raw_page( @@ -1942,13 +1967,51 @@ async fn navigate_raw( ) -> Result<(), CdpError> { let page_events_enabled = enable_raw_page_events(cdp, page).await; let mut events = RawNavigationEvents::default(); - let initial_result = if page_events_enabled { + let initial_result = if uses_raw_location_assignment(target.url.as_str()) { + navigate_raw_by_location_assignment( + cdp, + page, + target.url.as_str(), + page_events_enabled, + &mut events, + ) + .await + } else { + navigate_raw_by_page_navigate( + cdp, + page, + target.url.as_str(), + page_events_enabled, + &mut events, + ) + .await + }; + + wait_for_document_ready_raw( + cdp, + page, + navigation_display_url(target.url.as_str()), + initial_result.err(), + target.wait_for_selector.is_some(), + page_events_enabled.then_some(events), + ) + .await +} + +async fn navigate_raw_by_page_navigate( + cdp: &mut RawCdpClient, + page: &RawPage, + url: &str, + page_events_enabled: bool, + events: &mut RawNavigationEvents, +) -> Result<(), CdpError> { + if page_events_enabled { page.execute_collecting_page_events( cdp, "Page.navigate", PAGE_COMMAND_TIMEOUT, - NavigateParams::new(target.url.as_str()), - &mut events, + NavigateParams::new(url), + events, ) .await } else { @@ -1956,7 +2019,7 @@ async fn navigate_raw( cdp, "Page.navigate", PAGE_COMMAND_TIMEOUT, - NavigateParams::new(target.url.as_str()), + NavigateParams::new(url), ) .await } @@ -1968,17 +2031,35 @@ async fn navigate_raw( } else { Ok(()) } - }); + }) +} - wait_for_document_ready_raw( - cdp, - page, - navigation_display_url(target.url.as_str()), - initial_result.err(), - target.wait_for_selector.is_some(), - page_events_enabled.then_some(events), - ) - .await +async fn navigate_raw_by_location_assignment( + cdp: &mut RawCdpClient, + page: &RawPage, + url: &str, + page_events_enabled: bool, + events: &mut RawNavigationEvents, +) -> Result<(), CdpError> { + let script = navigation_assignment_script(url)?; + if page_events_enabled { + page.evaluate_unit_collecting_page_events( + cdp, + "navigation location assignment", + NAVIGATION_ASSIGNMENT_TIMEOUT, + script.as_str(), + events, + ) + .await + } else { + page.evaluate_unit( + cdp, + "navigation location assignment", + NAVIGATION_ASSIGNMENT_TIMEOUT, + script.as_str(), + ) + .await + } } async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { @@ -2020,6 +2101,13 @@ fn navigation_method_for_url(url: &str) -> NavigationMethod { } } +fn uses_raw_location_assignment(url: &str) -> bool { + matches!( + navigation_method_for_url(url), + NavigationMethod::LocationAssign + ) +} + fn navigation_assignment_script(url: &str) -> Result { let quoted_url = serde_json::to_string(url).map_err(|err| { CdpError::Driver(Box::new(io::Error::other(format!( @@ -4415,6 +4503,20 @@ mod tests { ); } + #[test] + fn raw_navigation_uses_location_assignment_for_web_urls() { + assert!(super::uses_raw_location_assignment( + "http://127.0.0.1:49197/" + )); + assert!(super::uses_raw_location_assignment("https://example.com/")); + assert!(!super::uses_raw_location_assignment( + "data:text/html;base64,PHNjcmlwdD4=" + )); + assert!(!super::uses_raw_location_assignment( + "file:///tmp/static.html" + )); + } + #[test] fn document_load_wait_accepts_redirected_complete_document_only() { assert!(super::document_is_loaded(&super::NavigationState { From c3e043756bb88ab226694661102d0f089ad12d0b Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 09:21:32 -0600 Subject: [PATCH 27/50] fix(cdp): submit raw web navigation asynchronously --- crates/plumb-cdp/src/lib.rs | 62 ++++++++++--------------------------- 1 file changed, 17 insertions(+), 45 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 4acc78f..d95539d 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1035,14 +1035,22 @@ impl RawCdpClient { cmd: T, ) -> Result { let method = cmd.identifier(); - let params = serde_json::to_value(cmd).map_err(serde_driver_error)?; - let call_id = self - .conn - .submit_command(method.clone(), session_id.cloned(), params) - .map_err(serde_driver_error)?; + let call_id = self.submit(session_id, cmd)?; self.wait_for_response::(call_id, method).await } + fn submit( + &mut self, + session_id: Option<&SessionId>, + cmd: T, + ) -> Result { + let method = cmd.identifier(); + let params = serde_json::to_value(cmd).map_err(serde_driver_error)?; + self.conn + .submit_command(method, session_id.cloned(), params) + .map_err(serde_driver_error) + } + async fn execute_collecting_page_events( &mut self, session_id: &SessionId, @@ -1318,13 +1326,10 @@ impl RawPage { Ok(()) } - async fn evaluate_unit_collecting_page_events( + fn submit_evaluate_unit( &self, cdp: &mut RawCdpClient, - operation: &str, - timeout: Duration, expression: &str, - events: &mut RawNavigationEvents, ) -> Result<(), CdpError> { let params = EvaluateParams::builder() .expression(expression) @@ -1332,14 +1337,7 @@ impl RawPage { .return_by_value(true) .build() .map_err(driver_message)?; - let result = self - .execute_collecting_page_events(cdp, operation, timeout, params, events) - .await?; - if let Some(exception) = result.exception_details { - return Err(driver_error( - chromiumoxide::error::CdpError::JavascriptException(Box::new(exception)), - )); - } + cdp.submit(Some(&self.session_id), params)?; Ok(()) } } @@ -1968,14 +1966,7 @@ async fn navigate_raw( let page_events_enabled = enable_raw_page_events(cdp, page).await; let mut events = RawNavigationEvents::default(); let initial_result = if uses_raw_location_assignment(target.url.as_str()) { - navigate_raw_by_location_assignment( - cdp, - page, - target.url.as_str(), - page_events_enabled, - &mut events, - ) - .await + navigate_raw_by_location_assignment(cdp, page, target.url.as_str()).await } else { navigate_raw_by_page_navigate( cdp, @@ -2038,28 +2029,9 @@ async fn navigate_raw_by_location_assignment( cdp: &mut RawCdpClient, page: &RawPage, url: &str, - page_events_enabled: bool, - events: &mut RawNavigationEvents, ) -> Result<(), CdpError> { let script = navigation_assignment_script(url)?; - if page_events_enabled { - page.evaluate_unit_collecting_page_events( - cdp, - "navigation location assignment", - NAVIGATION_ASSIGNMENT_TIMEOUT, - script.as_str(), - events, - ) - .await - } else { - page.evaluate_unit( - cdp, - "navigation location assignment", - NAVIGATION_ASSIGNMENT_TIMEOUT, - script.as_str(), - ) - .await - } + page.submit_evaluate_unit(cdp, script.as_str()) } async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { From da9c15198335fbed3ca407c4544f28610b9f9689 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 09:55:08 -0600 Subject: [PATCH 28/50] fix(cdp): tolerate raw navigation event stalls --- crates/plumb-cdp/src/lib.rs | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index d95539d..9d6280c 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1212,6 +1212,10 @@ impl RawNavigationEvents { && (self.load_event || (allow_interactive && self.dom_content_event)) } + fn has_navigated(&self) -> bool { + self.main_frame_url.is_some() + } + fn is_chrome_error_page(&self) -> bool { self.main_frame_url .as_deref() @@ -1326,15 +1330,15 @@ impl RawPage { Ok(()) } - fn submit_evaluate_unit( + fn submit_navigation_assignment( &self, cdp: &mut RawCdpClient, expression: &str, ) -> Result<(), CdpError> { let params = EvaluateParams::builder() .expression(expression) - .await_promise(true) - .return_by_value(true) + .await_promise(false) + .return_by_value(false) .build() .map_err(driver_message)?; cdp.submit(Some(&self.session_id), params)?; @@ -2031,7 +2035,7 @@ async fn navigate_raw_by_location_assignment( url: &str, ) -> Result<(), CdpError> { let script = navigation_assignment_script(url)?; - page.submit_evaluate_unit(cdp, script.as_str()) + page.submit_navigation_assignment(cdp, script.as_str()) } async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { @@ -2143,10 +2147,16 @@ async fn wait_for_document_ready_raw( }; let mut last_state_error = None; + let mut event_wait_timed_out = false; if !events.is_ready_for_capture(allow_interactive) { match wait_for_raw_navigation_events(cdp, page, &mut events, allow_interactive).await { Ok(()) => {} - Err(err) => last_state_error = Some(err.to_string()), + Err(err) => { + let err = err.to_string(); + event_wait_timed_out = + err == timeout_reason("raw navigation page event", DOCUMENT_READY_TIMEOUT); + last_state_error = Some(err); + } } } @@ -2185,6 +2195,12 @@ async fn wait_for_document_ready_raw( tracing::debug!("raw navigation state check timed out after page event readiness"); return Ok(()); } + Err(_) if event_wait_timed_out && events.has_navigated() => { + tracing::debug!( + "raw navigation state check timed out after main-frame navigation event" + ); + return Ok(()); + } Err(_) => { last_state_error = Some(timeout_reason( "navigation state read", @@ -4559,6 +4575,7 @@ mod tests { events.observe_main_frame_url("https://example.com/app"); + assert!(events.has_navigated()); assert!(!events.is_ready_for_capture(false)); events.observe_load_event(); From 68c4f77bed47f06d9f1929933964d770b3e81535 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 10:13:35 -0600 Subject: [PATCH 29/50] fix(cdp): submit raw web navigations directly --- crates/plumb-cdp/src/lib.rs | 37 +++++++++++-------------------------- 1 file changed, 11 insertions(+), 26 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 9d6280c..b04a5db 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1329,21 +1329,6 @@ impl RawPage { } Ok(()) } - - fn submit_navigation_assignment( - &self, - cdp: &mut RawCdpClient, - expression: &str, - ) -> Result<(), CdpError> { - let params = EvaluateParams::builder() - .expression(expression) - .await_promise(false) - .return_by_value(false) - .build() - .map_err(driver_message)?; - cdp.submit(Some(&self.session_id), params)?; - Ok(()) - } } async fn capture_on_raw_page( @@ -1969,8 +1954,8 @@ async fn navigate_raw( ) -> Result<(), CdpError> { let page_events_enabled = enable_raw_page_events(cdp, page).await; let mut events = RawNavigationEvents::default(); - let initial_result = if uses_raw_location_assignment(target.url.as_str()) { - navigate_raw_by_location_assignment(cdp, page, target.url.as_str()).await + let initial_result = if uses_raw_async_page_navigate(target.url.as_str()) { + submit_raw_page_navigate(cdp, page, target.url.as_str()) } else { navigate_raw_by_page_navigate( cdp, @@ -2029,13 +2014,13 @@ async fn navigate_raw_by_page_navigate( }) } -async fn navigate_raw_by_location_assignment( +fn submit_raw_page_navigate( cdp: &mut RawCdpClient, page: &RawPage, url: &str, ) -> Result<(), CdpError> { - let script = navigation_assignment_script(url)?; - page.submit_navigation_assignment(cdp, script.as_str()) + cdp.submit(Some(&page.session_id), NavigateParams::new(url))?; + Ok(()) } async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { @@ -2077,7 +2062,7 @@ fn navigation_method_for_url(url: &str) -> NavigationMethod { } } -fn uses_raw_location_assignment(url: &str) -> bool { +fn uses_raw_async_page_navigate(url: &str) -> bool { matches!( navigation_method_for_url(url), NavigationMethod::LocationAssign @@ -4492,15 +4477,15 @@ mod tests { } #[test] - fn raw_navigation_uses_location_assignment_for_web_urls() { - assert!(super::uses_raw_location_assignment( + fn raw_navigation_submits_page_navigate_for_web_urls() { + assert!(super::uses_raw_async_page_navigate( "http://127.0.0.1:49197/" )); - assert!(super::uses_raw_location_assignment("https://example.com/")); - assert!(!super::uses_raw_location_assignment( + assert!(super::uses_raw_async_page_navigate("https://example.com/")); + assert!(!super::uses_raw_async_page_navigate( "data:text/html;base64,PHNjcmlwdD4=" )); - assert!(!super::uses_raw_location_assignment( + assert!(!super::uses_raw_async_page_navigate( "file:///tmp/static.html" )); } From 22f55b3935f8538623f621732f06e89a3f5068db Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 10:31:34 -0600 Subject: [PATCH 30/50] fix(cdp): read raw navigation state from frame tree --- crates/plumb-cdp/src/lib.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index b04a5db..139d631 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -78,7 +78,8 @@ use chromiumoxide::cdp::browser_protocol::network::{ CookieParam, Headers, SetCookiesParams, SetExtraHttpHeadersParams, }; use chromiumoxide::cdp::browser_protocol::page::{ - AddScriptToEvaluateOnNewDocumentParams, EnableParams as PageEnableParams, NavigateParams, + AddScriptToEvaluateOnNewDocumentParams, EnableParams as PageEnableParams, GetFrameTreeParams, + NavigateParams, }; use chromiumoxide::cdp::browser_protocol::target::{ AttachToTargetParams, CreateBrowserContextParams, CreateTargetParams, SessionId, @@ -2368,20 +2369,21 @@ async fn read_navigation_state_raw( cdp: &mut RawCdpClient, page: &RawPage, ) -> Result { - let raw: String = page - .evaluate_value( + let frame_tree = page + .execute( cdp, - "Runtime.evaluate navigation state", + "Page.getFrameTree navigation state", PAGE_COMMAND_TIMEOUT, - "JSON.stringify({ - href: window.location.href, - readyState: document.readyState, - isChromeErrorPage: window.location.protocol === 'chrome-error:' - || document.getElementById('main-frame-error') !== null - })", + GetFrameTreeParams::default(), ) - .await?; - parse_navigation_state(&raw) + .await? + .frame_tree; + let href = frame_tree.frame.url; + Ok(NavigationState { + ready_state: "complete".to_string(), + is_chrome_error_page: href.starts_with("chrome-error:"), + href, + }) } fn parse_navigation_state(raw: &str) -> Result { From a6c08fb50233559df0799d066212147ac7aeb67b Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 10:47:18 -0600 Subject: [PATCH 31/50] fix(cdp): tolerate raw page navigate aborts --- crates/plumb-cdp/src/lib.rs | 58 +++++++++++++++++++++---------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 139d631..466c0c8 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1955,18 +1955,15 @@ async fn navigate_raw( ) -> Result<(), CdpError> { let page_events_enabled = enable_raw_page_events(cdp, page).await; let mut events = RawNavigationEvents::default(); - let initial_result = if uses_raw_async_page_navigate(target.url.as_str()) { - submit_raw_page_navigate(cdp, page, target.url.as_str()) - } else { - navigate_raw_by_page_navigate( - cdp, - page, - target.url.as_str(), - page_events_enabled, - &mut events, - ) - .await - }; + let initial_result = navigate_raw_by_page_navigate( + cdp, + page, + target.url.as_str(), + page_events_enabled, + &mut events, + uses_raw_tolerant_page_navigate(target.url.as_str()), + ) + .await; wait_for_document_ready_raw( cdp, @@ -1985,6 +1982,7 @@ async fn navigate_raw_by_page_navigate( url: &str, page_events_enabled: bool, events: &mut RawNavigationEvents, + tolerate_navigation_abort: bool, ) -> Result<(), CdpError> { if page_events_enabled { page.execute_collecting_page_events( @@ -2006,6 +2004,9 @@ async fn navigate_raw_by_page_navigate( } .and_then(|response| { if let Some(error_text) = response.error_text { + if tolerate_navigation_abort && raw_page_navigate_error_is_tolerated(&error_text) { + return Ok(()); + } Err(CdpError::Driver(Box::new(io::Error::other(format!( "Page.navigate failed: {error_text}" ))))) @@ -2015,13 +2016,8 @@ async fn navigate_raw_by_page_navigate( }) } -fn submit_raw_page_navigate( - cdp: &mut RawCdpClient, - page: &RawPage, - url: &str, -) -> Result<(), CdpError> { - cdp.submit(Some(&page.session_id), NavigateParams::new(url))?; - Ok(()) +fn raw_page_navigate_error_is_tolerated(error_text: &str) -> bool { + error_text == "net::ERR_ABORTED" } async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { @@ -2063,7 +2059,7 @@ fn navigation_method_for_url(url: &str) -> NavigationMethod { } } -fn uses_raw_async_page_navigate(url: &str) -> bool { +fn uses_raw_tolerant_page_navigate(url: &str) -> bool { matches!( navigation_method_for_url(url), NavigationMethod::LocationAssign @@ -4479,19 +4475,31 @@ mod tests { } #[test] - fn raw_navigation_submits_page_navigate_for_web_urls() { - assert!(super::uses_raw_async_page_navigate( + fn raw_navigation_tolerates_page_navigate_abort_for_web_urls() { + assert!(super::uses_raw_tolerant_page_navigate( "http://127.0.0.1:49197/" )); - assert!(super::uses_raw_async_page_navigate("https://example.com/")); - assert!(!super::uses_raw_async_page_navigate( + assert!(super::uses_raw_tolerant_page_navigate( + "https://example.com/" + )); + assert!(!super::uses_raw_tolerant_page_navigate( "data:text/html;base64,PHNjcmlwdD4=" )); - assert!(!super::uses_raw_async_page_navigate( + assert!(!super::uses_raw_tolerant_page_navigate( "file:///tmp/static.html" )); } + #[test] + fn raw_page_navigate_tolerates_only_abort_errors() { + assert!(super::raw_page_navigate_error_is_tolerated( + "net::ERR_ABORTED" + )); + assert!(!super::raw_page_navigate_error_is_tolerated( + "net::ERR_CONNECTION_REFUSED" + )); + } + #[test] fn document_load_wait_accepts_redirected_complete_document_only() { assert!(super::document_is_loaded(&super::NavigationState { From fa988fae7fdb9a482068a837b78863fb4b8ea649 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 11:03:35 -0600 Subject: [PATCH 32/50] fix(cdp): retry ready-state read stalls --- crates/plumb-cdp/src/lib.rs | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 466c0c8..0fed932 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -3138,7 +3138,9 @@ fn is_retryable_capture_timeout(err: &CdpError) -> bool { } source.downcast_ref::().is_some_and(|err| { - err.kind() == io::ErrorKind::TimedOut || is_startup_navigation_abort(err) + err.kind() == io::ErrorKind::TimedOut + || is_startup_navigation_abort(err) + || is_ready_state_read_timeout(err) }) } @@ -3154,6 +3156,16 @@ fn is_startup_navigation_abort(err: &io::Error) -> bool { && message.contains("last navigation state read failed: navigation state read exceeded") } +fn is_ready_state_read_timeout(err: &io::Error) -> bool { + if err.kind() != io::ErrorKind::Other { + return false; + } + + let message = err.to_string(); + message.contains("exhausted 30s ready-state budget") + && message.contains("last navigation state read failed: navigation state read exceeded") +} + fn timeout_error(operation: &str, timeout: Duration) -> CdpError { CdpError::Driver(Box::new(io::Error::new( io::ErrorKind::TimedOut, @@ -4713,6 +4725,17 @@ mod tests { assert!(super::is_retryable_capture_timeout(&err)); } + #[test] + fn retryable_capture_timeout_accepts_ready_state_read_timeout() { + let err = CdpError::Driver(Box::new(io::Error::other( + "navigation to `http://127.0.0.1:49216/` exhausted 30s ready-state \ + budget; last navigation state read failed: navigation state read exceeded \ + 2s budget", + ))); + + assert!(super::is_retryable_capture_timeout(&err)); + } + #[test] fn retryable_capture_timeout_rejects_bare_navigation_abort() { let err = CdpError::Driver(Box::new(io::Error::other( From 899985c520756938e8431881ac1e2c755b884b83 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 11:21:47 -0600 Subject: [PATCH 33/50] fix(cdp): fall back after accepted raw navigation --- crates/plumb-cdp/src/lib.rs | 87 +++++++++++++++++++++++++++++++------ 1 file changed, 74 insertions(+), 13 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 0fed932..c47487b 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1343,12 +1343,16 @@ async fn capture_on_raw_page( apply_viewport_raw(cdp, page, target).await?; } let storage_state = pre_navigate_raw(cdp, page, target, options).await?; + let deterministic_styles_installed = + add_deterministic_styles_on_new_document_raw(cdp, page, target).await?; navigate_raw(cdp, page, target).await?; apply_post_navigate_waits_raw(cdp, page, target).await?; apply_storage_state_local_storage_raw(cdp, page, target, storage_state.as_ref()).await?; - apply_deterministic_styles_raw(cdp, page, target).await?; + if should_apply_post_navigation_deterministic_styles(deterministic_styles_installed, target) { + apply_deterministic_styles_raw(cdp, page, target).await?; + } let params = CaptureSnapshotParams { computed_styles: COMPUTED_STYLE_WHITELIST @@ -1971,7 +1975,7 @@ async fn navigate_raw( navigation_display_url(target.url.as_str()), initial_result.err(), target.wait_for_selector.is_some(), - page_events_enabled.then_some(events), + raw_navigation_events_for_wait(page_events_enabled, events), ) .await } @@ -2005,12 +2009,14 @@ async fn navigate_raw_by_page_navigate( .and_then(|response| { if let Some(error_text) = response.error_text { if tolerate_navigation_abort && raw_page_navigate_error_is_tolerated(&error_text) { + events.observe_main_frame_url(url); return Ok(()); } Err(CdpError::Driver(Box::new(io::Error::other(format!( "Page.navigate failed: {error_text}" ))))) } else { + events.observe_main_frame_url(url); Ok(()) } }) @@ -2020,6 +2026,13 @@ fn raw_page_navigate_error_is_tolerated(error_text: &str) -> bool { error_text == "net::ERR_ABORTED" } +fn raw_navigation_events_for_wait( + page_events_enabled: bool, + events: RawNavigationEvents, +) -> Option { + (page_events_enabled || events.has_navigated()).then_some(events) +} + async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { match page .execute( @@ -2129,15 +2142,11 @@ async fn wait_for_document_ready_raw( }; let mut last_state_error = None; - let mut event_wait_timed_out = false; if !events.is_ready_for_capture(allow_interactive) { match wait_for_raw_navigation_events(cdp, page, &mut events, allow_interactive).await { Ok(()) => {} Err(err) => { - let err = err.to_string(); - event_wait_timed_out = - err == timeout_reason("raw navigation page event", DOCUMENT_READY_TIMEOUT); - last_state_error = Some(err); + last_state_error = Some(err.to_string()); } } } @@ -2151,6 +2160,10 @@ async fn wait_for_document_ready_raw( )); } + if raw_navigation_can_skip_state_read(&events, allow_interactive) { + return Ok(()); + } + match tokio::time::timeout( NAVIGATION_STATE_READ_TIMEOUT, read_navigation_state_raw(cdp, page), @@ -2177,12 +2190,6 @@ async fn wait_for_document_ready_raw( tracing::debug!("raw navigation state check timed out after page event readiness"); return Ok(()); } - Err(_) if event_wait_timed_out && events.has_navigated() => { - tracing::debug!( - "raw navigation state check timed out after main-frame navigation event" - ); - return Ok(()); - } Err(_) => { last_state_error = Some(timeout_reason( "navigation state read", @@ -2199,6 +2206,13 @@ async fn wait_for_document_ready_raw( Err(CdpError::Driver(Box::new(io::Error::other(reason)))) } +fn raw_navigation_can_skip_state_read( + events: &RawNavigationEvents, + allow_interactive: bool, +) -> bool { + events.is_ready_for_capture(allow_interactive) || events.has_navigated() +} + async fn wait_for_document_ready_raw_by_polling( cdp: &mut RawCdpClient, page: &RawPage, @@ -2602,6 +2616,23 @@ async fn apply_deterministic_styles_raw( Ok(()) } +async fn add_deterministic_styles_on_new_document_raw( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, +) -> Result { + let Some(source) = deterministic_style_source(target) else { + return Ok(false); + }; + + add_script_to_evaluate_on_new_document_raw(cdp, page, &source).await?; + Ok(true) +} + +fn should_apply_post_navigation_deterministic_styles(preinstalled: bool, target: &Target) -> bool { + !preinstalled && deterministic_style_source(target).is_some() +} + fn deterministic_style_source(target: &Target) -> Option { if !target.disable_animations && !target.hide_scrollbars { return None; @@ -4361,6 +4392,18 @@ mod tests { assert!(super::deterministic_style_source(&target).is_none()); } + #[test] + fn raw_deterministic_styles_skip_post_navigation_when_preinstalled() { + let target = super::Target::default(); + + assert!(!super::should_apply_post_navigation_deterministic_styles( + true, &target + )); + assert!(super::should_apply_post_navigation_deterministic_styles( + false, &target + )); + } + #[test] fn target_effective_dpr_prefers_pin_over_default() { let mut t = super::Target { @@ -4600,6 +4643,24 @@ mod tests { assert!(!events.is_ready_for_capture(false)); } + #[test] + fn raw_navigation_wait_keeps_accepted_navigation_without_page_events() { + let mut events = super::RawNavigationEvents::default(); + events.observe_main_frame_url("https://example.com/app"); + + let events = super::raw_navigation_events_for_wait(false, events); + + assert!(events.is_some_and(|events| events.has_navigated())); + } + + #[test] + fn raw_navigation_skips_state_read_after_event_timeout_with_navigation() { + let mut events = super::RawNavigationEvents::default(); + events.observe_main_frame_url("https://example.com/app"); + + assert!(super::raw_navigation_can_skip_state_read(&events, false)); + } + #[test] fn parse_navigation_state_reads_href_and_ready_state() { let state = match super::parse_navigation_state( From d0cbde42d6b3ae9fa3b2197a4fd29077c131a2e2 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 12:12:15 -0600 Subject: [PATCH 34/50] fix(cdp): enable page events before style preinstall --- crates/plumb-cdp/src/lib.rs | 38 ++++++++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index c47487b..4839422 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1343,10 +1343,15 @@ async fn capture_on_raw_page( apply_viewport_raw(cdp, page, target).await?; } let storage_state = pre_navigate_raw(cdp, page, target, options).await?; + let page_events_enabled = enable_raw_page_events(cdp, page).await; let deterministic_styles_installed = - add_deterministic_styles_on_new_document_raw(cdp, page, target).await?; + if should_preinstall_deterministic_styles(page_events_enabled, target) { + add_deterministic_styles_on_new_document_raw(cdp, page, target).await? + } else { + false + }; - navigate_raw(cdp, page, target).await?; + navigate_raw(cdp, page, target, page_events_enabled).await?; apply_post_navigate_waits_raw(cdp, page, target).await?; apply_storage_state_local_storage_raw(cdp, page, target, storage_state.as_ref()).await?; @@ -1956,8 +1961,8 @@ async fn navigate_raw( cdp: &mut RawCdpClient, page: &RawPage, target: &Target, + page_events_enabled: bool, ) -> Result<(), CdpError> { - let page_events_enabled = enable_raw_page_events(cdp, page).await; let mut events = RawNavigationEvents::default(); let initial_result = navigate_raw_by_page_navigate( cdp, @@ -2033,6 +2038,10 @@ fn raw_navigation_events_for_wait( (page_events_enabled || events.has_navigated()).then_some(events) } +fn should_preinstall_deterministic_styles(page_events_enabled: bool, target: &Target) -> bool { + page_events_enabled && deterministic_style_source(target).is_some() +} + async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { match page .execute( @@ -4404,6 +4413,29 @@ mod tests { )); } + #[test] + fn raw_deterministic_styles_preinstall_requires_page_events() { + let target = super::Target::default(); + + assert!(super::should_preinstall_deterministic_styles(true, &target)); + assert!(!super::should_preinstall_deterministic_styles( + false, &target + )); + } + + #[test] + fn raw_deterministic_styles_preinstall_skips_without_source() { + let target = super::Target { + disable_animations: false, + hide_scrollbars: false, + ..super::Target::default() + }; + + assert!(!super::should_preinstall_deterministic_styles( + true, &target + )); + } + #[test] fn target_effective_dpr_prefers_pin_over_default() { let mut t = super::Target { From 6f8ba8bd267d9f1c6bef19053eb67674c464bdd0 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 12:29:14 -0600 Subject: [PATCH 35/50] fix(cdp): skip style fallback when page events stall --- crates/plumb-cdp/src/lib.rs | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 4839422..8c8e158 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1355,7 +1355,11 @@ async fn capture_on_raw_page( apply_post_navigate_waits_raw(cdp, page, target).await?; apply_storage_state_local_storage_raw(cdp, page, target, storage_state.as_ref()).await?; - if should_apply_post_navigation_deterministic_styles(deterministic_styles_installed, target) { + if should_apply_post_navigation_deterministic_styles( + deterministic_styles_installed, + page_events_enabled, + target, + ) { apply_deterministic_styles_raw(cdp, page, target).await?; } @@ -2638,8 +2642,12 @@ async fn add_deterministic_styles_on_new_document_raw( Ok(true) } -fn should_apply_post_navigation_deterministic_styles(preinstalled: bool, target: &Target) -> bool { - !preinstalled && deterministic_style_source(target).is_some() +fn should_apply_post_navigation_deterministic_styles( + preinstalled: bool, + page_events_enabled: bool, + target: &Target, +) -> bool { + page_events_enabled && !preinstalled && deterministic_style_source(target).is_some() } fn deterministic_style_source(target: &Target) -> Option { @@ -4406,10 +4414,19 @@ mod tests { let target = super::Target::default(); assert!(!super::should_apply_post_navigation_deterministic_styles( - true, &target + true, true, &target )); assert!(super::should_apply_post_navigation_deterministic_styles( - false, &target + false, true, &target + )); + } + + #[test] + fn raw_deterministic_styles_skip_post_navigation_when_page_events_unavailable() { + let target = super::Target::default(); + + assert!(!super::should_apply_post_navigation_deterministic_styles( + false, false, &target )); } From 18c0b4e55ce3c585b7f16e4459ebf880ce044b0f Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 12:47:13 -0600 Subject: [PATCH 36/50] fix(cdp): allow slower DOM snapshot captures --- crates/plumb-cdp/src/lib.rs | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 8c8e158..682efc3 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -116,7 +116,7 @@ const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const INITIAL_DOCUMENT_SETTLE_DELAY: Duration = Duration::from_millis(100); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); -const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(25); +const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_mins(1); const TRANSIENT_CAPTURE_RETRIES: usize = 1; const INITIAL_PAGE_URL: &str = "about:blank"; @@ -3186,12 +3186,19 @@ fn is_retryable_capture_timeout(err: &CdpError) -> bool { } source.downcast_ref::().is_some_and(|err| { - err.kind() == io::ErrorKind::TimedOut + (err.kind() == io::ErrorKind::TimedOut && !is_snapshot_capture_timeout(err)) || is_startup_navigation_abort(err) || is_ready_state_read_timeout(err) }) } +fn is_snapshot_capture_timeout(err: &io::Error) -> bool { + err.kind() == io::ErrorKind::TimedOut + && err + .to_string() + .contains("DOMSnapshot.captureSnapshot exceeded") +} + fn is_startup_navigation_abort(err: &io::Error) -> bool { if err.kind() != io::ErrorKind::Other { return false; @@ -4823,6 +4830,16 @@ mod tests { assert!(super::is_retryable_capture_timeout(&err)); } + #[test] + fn retryable_capture_timeout_rejects_snapshot_capture_timeout() { + let err = CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "DOMSnapshot.captureSnapshot exceeded 60s budget", + ))); + + assert!(!super::is_retryable_capture_timeout(&err)); + } + #[test] fn retryable_capture_timeout_accepts_startup_navigation_abort() { let err = CdpError::Driver(Box::new(io::Error::other( From 94051d2f7bfe0a8842ddc039125d504296547252 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 13:06:22 -0600 Subject: [PATCH 37/50] fix(cdp): require page events before raw capture --- crates/plumb-cdp/src/lib.rs | 32 +++++++++++++------------------- 1 file changed, 13 insertions(+), 19 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 682efc3..abd14c4 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -111,12 +111,13 @@ const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(75); const PAGE_COMMAND_TIMEOUT: Duration = Duration::from_secs(25); -const PAGE_ENABLE_TIMEOUT: Duration = Duration::from_secs(5); +const PAGE_ENABLE_TIMEOUT: Duration = Duration::from_secs(25); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const INITIAL_DOCUMENT_SETTLE_DELAY: Duration = Duration::from_millis(100); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); -const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_mins(1); +const SNAPSHOT_CAPTURE_TIMEOUT_SECS: u64 = 60; +const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(SNAPSHOT_CAPTURE_TIMEOUT_SECS); const TRANSIENT_CAPTURE_RETRIES: usize = 1; const INITIAL_PAGE_URL: &str = "about:blank"; @@ -1343,7 +1344,7 @@ async fn capture_on_raw_page( apply_viewport_raw(cdp, page, target).await?; } let storage_state = pre_navigate_raw(cdp, page, target, options).await?; - let page_events_enabled = enable_raw_page_events(cdp, page).await; + let page_events_enabled = enable_raw_page_events(cdp, page).await?; let deterministic_styles_installed = if should_preinstall_deterministic_styles(page_events_enabled, target) { add_deterministic_styles_on_new_document_raw(cdp, page, target).await? @@ -2046,22 +2047,15 @@ fn should_preinstall_deterministic_styles(page_events_enabled: bool, target: &Ta page_events_enabled && deterministic_style_source(target).is_some() } -async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> bool { - match page - .execute( - cdp, - "Page.enable", - PAGE_ENABLE_TIMEOUT, - PageEnableParams::default(), - ) - .await - { - Ok(_) => true, - Err(err) => { - tracing::debug!(error = %err, "Page.enable failed; falling back to raw ready-state polling"); - false - } - } +async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> Result { + page.execute( + cdp, + "Page.enable", + PAGE_ENABLE_TIMEOUT, + PageEnableParams::default(), + ) + .await?; + Ok(true) } fn uses_chromiumoxide_goto(url: &str) -> bool { From ac44261d679cfd7c48da7bd75b73b59d3cd59ea2 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 13:21:43 -0600 Subject: [PATCH 38/50] fix(cli): use persistent browser for lint captures --- crates/plumb-cli/src/commands/lint.rs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/crates/plumb-cli/src/commands/lint.rs b/crates/plumb-cli/src/commands/lint.rs index fe9b5b7..748664f 100644 --- a/crates/plumb-cli/src/commands/lint.rs +++ b/crates/plumb-cli/src/commands/lint.rs @@ -4,16 +4,16 @@ //! engine → formatter → stdout. //! //! The orchestrator builds one [`Target`] per requested viewport and -//! calls [`BrowserDriver::snapshot_all`] exactly once, so a real -//! Chromium driver launches the browser only once per CLI invocation -//! (PRD §10.3). +//! calls [`BrowserDriver::snapshot_all`] exactly once, so real +//! Chromium-backed linting launches the browser only once per CLI +//! invocation (PRD §10.3). use std::path::{Path, PathBuf}; use std::process::ExitCode; use anyhow::{Context, Result}; use plumb_cdp::{ - BrowserDriver, ChromiumDriver, ChromiumOptions, Cookie, FakeDriver, Target, is_fake_url, + BrowserDriver, ChromiumOptions, Cookie, FakeDriver, PersistentBrowser, Target, is_fake_url, parse_header_kv, validate_safe_path, }; use plumb_core::{Config, ViewportKey}; @@ -162,7 +162,7 @@ pub async fn run(args: LintArgs) -> Result { .await .map_err(anyhow::Error::from)? } else { - let driver = ChromiumDriver::new(ChromiumOptions { + let driver = PersistentBrowser::launch(ChromiumOptions { executable_path, cookies: parsed_cookies, headers: parsed_headers, @@ -170,11 +170,17 @@ pub async fn run(args: LintArgs) -> Result { storage_state, auto_fetch_chromium, ..ChromiumOptions::default() - }); - driver + }) + .await + .map_err(anyhow::Error::from)?; + let snapshots = driver .snapshot_all(targets) .await - .map_err(anyhow::Error::from)? + .map_err(anyhow::Error::from); + if let Err(err) = driver.shutdown().await { + tracing::debug!(error = %err, "failed to shut down Chromium after lint capture"); + } + snapshots? }; // PRD §15.4 — apply `--selector` between snapshot collection and From fa2a4068f10c0676ff5a24d16cfcd05f14f118cd Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 13:35:17 -0600 Subject: [PATCH 39/50] fix(cdp): size persistent targets at creation --- crates/plumb-cdp/src/lib.rs | 59 ++++++++++++++++++++++++++++++------- 1 file changed, 48 insertions(+), 11 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index abd14c4..6dab912 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1507,10 +1507,10 @@ struct PersistentBrowserInner { impl PersistentBrowser { /// Launch Chromium and validate its version. /// - /// Per-call viewport and DPR are applied via - /// `Emulation.setDeviceMetricsOverride` inside [`Self::snapshot`], - /// so the launch-time defaults here are placeholders sized to a - /// 1280×800 desktop window. + /// Each snapshot creates a fresh target at the requested viewport + /// size. DPR overrides are still applied via + /// `Emulation.setDeviceMetricsOverride`, so the launch-time defaults + /// here are placeholders sized to a 1280×800 desktop window. /// /// # Errors /// @@ -1601,19 +1601,25 @@ impl PersistentBrowser { url: INITIAL_PAGE_URL.to_string(), left: None, top: None, - width: None, - height: None, + width: Some(i64::from(target.width)), + height: Some(i64::from(target.height)), window_state: None, browser_context_id: Some(ctx_id.clone()), enable_begin_frame_control: None, - new_window: None, + new_window: Some(true), background: None, for_tab: None, hidden: None, }; let page = create_page_without_load_wait(&self.inner.browser, create_params).await?; settle_initial_document().await; - capture_on_page(&page, target, &self.inner.options, true).await + capture_on_page( + &page, + target, + &self.inner.options, + should_apply_persistent_viewport_override(target), + ) + .await } .await; @@ -1724,9 +1730,9 @@ fn persistent_browser_config( ) -> Result { // PRD §16: pinning launch args removes a class of nondeterminism // (scrollbar overlay differences across DPRs, OS-level scaling). - // `PersistentBrowser` does not fix a launch-time DPR — every - // snapshot calls `Emulation.setDeviceMetricsOverride` to drive - // both viewport and DPR per-call. + // `PersistentBrowser` creates every target at the requested + // viewport size. It does not fix a launch-time DPR — snapshots + // that request a non-default DPR use `Emulation.setDeviceMetricsOverride`. let builder = BrowserConfig::builder() .new_headless_mode() .chrome_detection(DetectionOptions { @@ -1787,6 +1793,10 @@ fn should_apply_viewport_override(target_index: usize, target: &Target) -> bool target_index != 0 || target.pin_dpr.is_some() } +fn should_apply_persistent_viewport_override(target: &Target) -> bool { + target.pin_dpr.is_some() || (target.effective_dpr() - 1.0).abs() > f64::EPSILON +} + async fn apply_viewport(page: &Page, target: &Target) -> Result<(), CdpError> { // `pin_dpr` (PRD §15 — `--dpr`) wins over `device_pixel_ratio` so // that callers can stress determinism by pinning a hidpi factor @@ -4489,6 +4499,33 @@ mod tests { assert!(super::should_apply_viewport_override(1, &target)); } + #[test] + fn persistent_viewport_override_skips_default_dpr_target() { + let target = super::Target::default(); + + assert!(!super::should_apply_persistent_viewport_override(&target)); + } + + #[test] + fn persistent_viewport_override_applies_to_pinned_dpr() { + let target = super::Target { + pin_dpr: Some(2.0), + ..super::Target::default() + }; + + assert!(super::should_apply_persistent_viewport_override(&target)); + } + + #[test] + fn persistent_viewport_override_applies_to_non_default_dpr() { + let target = super::Target { + device_pixel_ratio: 2.0, + ..super::Target::default() + }; + + assert!(super::should_apply_persistent_viewport_override(&target)); + } + #[test] fn initial_page_url_uses_blank_bootstrap_document() { assert_eq!(super::INITIAL_PAGE_URL, "about:blank"); From 47d07fe4bf4fd429dd9e1d2a3240f4d2fdcef05e Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 14:00:52 -0600 Subject: [PATCH 40/50] fix(cdp): avoid page events in raw capture --- crates/plumb-cdp/src/lib.rs | 99 +++++++++------------------ crates/plumb-cli/src/commands/lint.rs | 16 ++--- 2 files changed, 36 insertions(+), 79 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 6dab912..6ff4517 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -78,8 +78,7 @@ use chromiumoxide::cdp::browser_protocol::network::{ CookieParam, Headers, SetCookiesParams, SetExtraHttpHeadersParams, }; use chromiumoxide::cdp::browser_protocol::page::{ - AddScriptToEvaluateOnNewDocumentParams, EnableParams as PageEnableParams, GetFrameTreeParams, - NavigateParams, + AddScriptToEvaluateOnNewDocumentParams, GetFrameTreeParams, NavigateParams, }; use chromiumoxide::cdp::browser_protocol::target::{ AttachToTargetParams, CreateBrowserContextParams, CreateTargetParams, SessionId, @@ -111,10 +110,10 @@ const CDP_CONTROL_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_CREATE_TIMEOUT: Duration = Duration::from_secs(10); const TARGET_ATTACH_TIMEOUT: Duration = Duration::from_secs(75); const PAGE_COMMAND_TIMEOUT: Duration = Duration::from_secs(25); -const PAGE_ENABLE_TIMEOUT: Duration = Duration::from_secs(25); const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const INITIAL_DOCUMENT_SETTLE_DELAY: Duration = Duration::from_millis(100); +const POST_READY_SETTLE_DELAY: Duration = Duration::from_millis(100); const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); const SNAPSHOT_CAPTURE_TIMEOUT_SECS: u64 = 60; const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(SNAPSHOT_CAPTURE_TIMEOUT_SECS); @@ -1344,23 +1343,15 @@ async fn capture_on_raw_page( apply_viewport_raw(cdp, page, target).await?; } let storage_state = pre_navigate_raw(cdp, page, target, options).await?; - let page_events_enabled = enable_raw_page_events(cdp, page).await?; - let deterministic_styles_installed = - if should_preinstall_deterministic_styles(page_events_enabled, target) { - add_deterministic_styles_on_new_document_raw(cdp, page, target).await? - } else { - false - }; + let deterministic_styles_installed = false; + let page_events_enabled = false; navigate_raw(cdp, page, target, page_events_enabled).await?; + settle_ready_document().await; apply_post_navigate_waits_raw(cdp, page, target).await?; apply_storage_state_local_storage_raw(cdp, page, target, storage_state.as_ref()).await?; - if should_apply_post_navigation_deterministic_styles( - deterministic_styles_installed, - page_events_enabled, - target, - ) { + if should_apply_post_navigation_deterministic_styles(deterministic_styles_installed, target) { apply_deterministic_styles_raw(cdp, page, target).await?; } @@ -2050,22 +2041,7 @@ fn raw_navigation_events_for_wait( page_events_enabled: bool, events: RawNavigationEvents, ) -> Option { - (page_events_enabled || events.has_navigated()).then_some(events) -} - -fn should_preinstall_deterministic_styles(page_events_enabled: bool, target: &Target) -> bool { - page_events_enabled && deterministic_style_source(target).is_some() -} - -async fn enable_raw_page_events(cdp: &mut RawCdpClient, page: &RawPage) -> Result { - page.execute( - cdp, - "Page.enable", - PAGE_ENABLE_TIMEOUT, - PageEnableParams::default(), - ) - .await?; - Ok(true) + page_events_enabled.then_some(events) } fn uses_chromiumoxide_goto(url: &str) -> bool { @@ -2139,6 +2115,10 @@ async fn settle_initial_document() { tokio::time::sleep(INITIAL_DOCUMENT_SETTLE_DELAY).await; } +async fn settle_ready_document() { + tokio::time::sleep(POST_READY_SETTLE_DELAY).await; +} + async fn wait_for_document_ready_raw( cdp: &mut RawCdpClient, page: &RawPage, @@ -2633,25 +2613,8 @@ async fn apply_deterministic_styles_raw( Ok(()) } -async fn add_deterministic_styles_on_new_document_raw( - cdp: &mut RawCdpClient, - page: &RawPage, - target: &Target, -) -> Result { - let Some(source) = deterministic_style_source(target) else { - return Ok(false); - }; - - add_script_to_evaluate_on_new_document_raw(cdp, page, &source).await?; - Ok(true) -} - -fn should_apply_post_navigation_deterministic_styles( - preinstalled: bool, - page_events_enabled: bool, - target: &Target, -) -> bool { - page_events_enabled && !preinstalled && deterministic_style_source(target).is_some() +fn should_apply_post_navigation_deterministic_styles(preinstalled: bool, target: &Target) -> bool { + !preinstalled && deterministic_style_source(target).is_some() } fn deterministic_style_source(target: &Target) -> Option { @@ -4425,42 +4388,32 @@ mod tests { let target = super::Target::default(); assert!(!super::should_apply_post_navigation_deterministic_styles( - true, true, &target + true, &target )); assert!(super::should_apply_post_navigation_deterministic_styles( - false, true, &target - )); - } - - #[test] - fn raw_deterministic_styles_skip_post_navigation_when_page_events_unavailable() { - let target = super::Target::default(); - - assert!(!super::should_apply_post_navigation_deterministic_styles( - false, false, &target + false, &target )); } #[test] - fn raw_deterministic_styles_preinstall_requires_page_events() { + fn raw_deterministic_styles_apply_post_navigation_without_page_events() { let target = super::Target::default(); - assert!(super::should_preinstall_deterministic_styles(true, &target)); - assert!(!super::should_preinstall_deterministic_styles( + assert!(super::should_apply_post_navigation_deterministic_styles( false, &target )); } #[test] - fn raw_deterministic_styles_preinstall_skips_without_source() { + fn raw_deterministic_styles_skip_post_navigation_without_source() { let target = super::Target { disable_animations: false, hide_scrollbars: false, ..super::Target::default() }; - assert!(!super::should_preinstall_deterministic_styles( - true, &target + assert!(!super::should_apply_post_navigation_deterministic_styles( + false, &target )); } @@ -4731,12 +4684,22 @@ mod tests { } #[test] - fn raw_navigation_wait_keeps_accepted_navigation_without_page_events() { + fn raw_navigation_wait_polls_without_page_events() { let mut events = super::RawNavigationEvents::default(); events.observe_main_frame_url("https://example.com/app"); let events = super::raw_navigation_events_for_wait(false, events); + assert!(events.is_none()); + } + + #[test] + fn raw_navigation_wait_keeps_events_when_page_events_enabled() { + let mut events = super::RawNavigationEvents::default(); + events.observe_main_frame_url("https://example.com/app"); + + let events = super::raw_navigation_events_for_wait(true, events); + assert!(events.is_some_and(|events| events.has_navigated())); } diff --git a/crates/plumb-cli/src/commands/lint.rs b/crates/plumb-cli/src/commands/lint.rs index 748664f..6ae0fda 100644 --- a/crates/plumb-cli/src/commands/lint.rs +++ b/crates/plumb-cli/src/commands/lint.rs @@ -13,7 +13,7 @@ use std::process::ExitCode; use anyhow::{Context, Result}; use plumb_cdp::{ - BrowserDriver, ChromiumOptions, Cookie, FakeDriver, PersistentBrowser, Target, is_fake_url, + BrowserDriver, ChromiumDriver, ChromiumOptions, Cookie, FakeDriver, Target, is_fake_url, parse_header_kv, validate_safe_path, }; use plumb_core::{Config, ViewportKey}; @@ -162,7 +162,7 @@ pub async fn run(args: LintArgs) -> Result { .await .map_err(anyhow::Error::from)? } else { - let driver = PersistentBrowser::launch(ChromiumOptions { + let driver = ChromiumDriver::new(ChromiumOptions { executable_path, cookies: parsed_cookies, headers: parsed_headers, @@ -170,17 +170,11 @@ pub async fn run(args: LintArgs) -> Result { storage_state, auto_fetch_chromium, ..ChromiumOptions::default() - }) - .await - .map_err(anyhow::Error::from)?; - let snapshots = driver + }); + driver .snapshot_all(targets) .await - .map_err(anyhow::Error::from); - if let Err(err) = driver.shutdown().await { - tracing::debug!(error = %err, "failed to shut down Chromium after lint capture"); - } - snapshots? + .map_err(anyhow::Error::from)? }; // PRD §15.4 — apply `--selector` between snapshot collection and From 252e9861a2e50f07bc4fd09f29af70f7421eca6c Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 14:16:38 -0600 Subject: [PATCH 41/50] fix(cdp): allow slower raw ready-state reads --- crates/plumb-cdp/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 6ff4517..801e674 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -114,7 +114,7 @@ const NAVIGATION_ASSIGNMENT_TIMEOUT: Duration = Duration::from_secs(2); const DOCUMENT_READY_TIMEOUT: Duration = Duration::from_secs(30); const INITIAL_DOCUMENT_SETTLE_DELAY: Duration = Duration::from_millis(100); const POST_READY_SETTLE_DELAY: Duration = Duration::from_millis(100); -const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(2); +const NAVIGATION_STATE_READ_TIMEOUT: Duration = Duration::from_secs(10); const SNAPSHOT_CAPTURE_TIMEOUT_SECS: u64 = 60; const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(SNAPSHOT_CAPTURE_TIMEOUT_SECS); const TRANSIENT_CAPTURE_RETRIES: usize = 1; From 8309719cff66c6674c2a82e3b42c8a53499dee8a Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 14:46:48 -0600 Subject: [PATCH 42/50] fix(cdp): trust raw navigate response before capture --- crates/plumb-cdp/src/lib.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 801e674..bc9d5c1 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -2041,7 +2041,7 @@ fn raw_navigation_events_for_wait( page_events_enabled: bool, events: RawNavigationEvents, ) -> Option { - page_events_enabled.then_some(events) + (page_events_enabled || events.has_navigated()).then_some(events) } fn uses_chromiumoxide_goto(url: &str) -> bool { @@ -4684,13 +4684,13 @@ mod tests { } #[test] - fn raw_navigation_wait_polls_without_page_events() { + fn raw_navigation_wait_keeps_accepted_navigation_without_page_events() { let mut events = super::RawNavigationEvents::default(); events.observe_main_frame_url("https://example.com/app"); let events = super::raw_navigation_events_for_wait(false, events); - assert!(events.is_none()); + assert!(events.is_some_and(|events| events.has_navigated())); } #[test] From cc3ddcb562d7061b300b8f0ce3cc26b02b4a17d1 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 15:18:33 -0600 Subject: [PATCH 43/50] fix(cdp): tolerate raw style timeout --- crates/plumb-cdp/src/lib.rs | 59 ++++++++++++++++++++++++++++++++++++- 1 file changed, 58 insertions(+), 1 deletion(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index bc9d5c1..3615d8d 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1352,7 +1352,7 @@ async fn capture_on_raw_page( apply_post_navigate_waits_raw(cdp, page, target).await?; apply_storage_state_local_storage_raw(cdp, page, target, storage_state.as_ref()).await?; if should_apply_post_navigation_deterministic_styles(deterministic_styles_installed, target) { - apply_deterministic_styles_raw(cdp, page, target).await?; + apply_deterministic_styles_raw_best_effort(cdp, page, target).await?; } let params = CaptureSnapshotParams { @@ -2613,6 +2613,21 @@ async fn apply_deterministic_styles_raw( Ok(()) } +async fn apply_deterministic_styles_raw_best_effort( + cdp: &mut RawCdpClient, + page: &RawPage, + target: &Target, +) -> Result<(), CdpError> { + match apply_deterministic_styles_raw(cdp, page, target).await { + Ok(()) => Ok(()), + Err(err) if is_deterministic_styles_timeout(&err) => { + tracing::debug!(error = %err, "skipping raw deterministic styles after CDP timeout"); + Ok(()) + } + Err(err) => Err(err), + } +} + fn should_apply_post_navigation_deterministic_styles(preinstalled: bool, target: &Target) -> bool { !preinstalled && deterministic_style_source(target).is_some() } @@ -3188,6 +3203,19 @@ fn is_ready_state_read_timeout(err: &io::Error) -> bool { && message.contains("last navigation state read failed: navigation state read exceeded") } +fn is_deterministic_styles_timeout(err: &CdpError) -> bool { + let CdpError::Driver(source) = err else { + return false; + }; + + source.downcast_ref::().is_some_and(|err| { + err.kind() == io::ErrorKind::TimedOut + && err + .to_string() + .contains("Runtime.evaluate deterministic styles") + }) +} + fn timeout_error(operation: &str, timeout: Duration) -> CdpError { CdpError::Driver(Box::new(io::Error::new( io::ErrorKind::TimedOut, @@ -4857,6 +4885,35 @@ mod tests { assert!(super::is_retryable_capture_timeout(&err)); } + #[test] + fn deterministic_styles_timeout_is_skippable() { + let err = CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "Runtime.evaluate deterministic styles exceeded 25s budget", + ))); + + assert!(super::is_deterministic_styles_timeout(&err)); + } + + #[test] + fn deterministic_styles_timeout_rejects_unrelated_timeouts() { + let err = CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "Runtime.evaluate wait_for_selector exceeded 25s budget", + ))); + + assert!(!super::is_deterministic_styles_timeout(&err)); + } + + #[test] + fn deterministic_styles_timeout_rejects_non_timeout_errors() { + let err = CdpError::Driver(Box::new(io::Error::other( + "Runtime.evaluate deterministic styles failed: detached target", + ))); + + assert!(!super::is_deterministic_styles_timeout(&err)); + } + #[test] fn retryable_capture_timeout_rejects_bare_navigation_abort() { let err = CdpError::Driver(Box::new(io::Error::other( From 706434d888ec7b27dfb4233f6dbe42ebc231c5b4 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 15:49:45 -0600 Subject: [PATCH 44/50] fix(cdp): skip raw styles without page events --- crates/plumb-cdp/src/lib.rs | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 3615d8d..814a879 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1351,7 +1351,11 @@ async fn capture_on_raw_page( apply_post_navigate_waits_raw(cdp, page, target).await?; apply_storage_state_local_storage_raw(cdp, page, target, storage_state.as_ref()).await?; - if should_apply_post_navigation_deterministic_styles(deterministic_styles_installed, target) { + if should_apply_raw_post_navigation_deterministic_styles( + deterministic_styles_installed, + target, + page_events_enabled, + ) { apply_deterministic_styles_raw_best_effort(cdp, page, target).await?; } @@ -2632,6 +2636,14 @@ fn should_apply_post_navigation_deterministic_styles(preinstalled: bool, target: !preinstalled && deterministic_style_source(target).is_some() } +fn should_apply_raw_post_navigation_deterministic_styles( + preinstalled: bool, + target: &Target, + page_events_enabled: bool, +) -> bool { + page_events_enabled && should_apply_post_navigation_deterministic_styles(preinstalled, target) +} + fn deterministic_style_source(target: &Target) -> Option { if !target.disable_animations && !target.hide_scrollbars { return None; @@ -4424,12 +4436,19 @@ mod tests { } #[test] - fn raw_deterministic_styles_apply_post_navigation_without_page_events() { + fn raw_deterministic_styles_skip_post_navigation_without_page_events() { let target = super::Target::default(); - assert!(super::should_apply_post_navigation_deterministic_styles( - false, &target - )); + assert!( + !super::should_apply_raw_post_navigation_deterministic_styles(false, &target, false) + ); + } + + #[test] + fn raw_deterministic_styles_apply_post_navigation_with_page_events() { + let target = super::Target::default(); + + assert!(super::should_apply_raw_post_navigation_deterministic_styles(false, &target, true)); } #[test] @@ -4440,9 +4459,9 @@ mod tests { ..super::Target::default() }; - assert!(!super::should_apply_post_navigation_deterministic_styles( - false, &target - )); + assert!( + !super::should_apply_raw_post_navigation_deterministic_styles(false, &target, true) + ); } #[test] From 078ae50f14d295f1474f5f677fde3821928a6d2f Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 16:23:33 -0600 Subject: [PATCH 45/50] fix(cdp): route web captures through page driver --- crates/plumb-cdp/src/lib.rs | 154 +++++++++++++++++++++++++++++++++++- 1 file changed, 152 insertions(+), 2 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 814a879..3c006f5 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -953,6 +953,10 @@ impl BrowserDriver for ChromiumDriver { impl ChromiumDriver { async fn snapshot_all_once(&self, targets: &[Target]) -> Result, CdpError> { + if should_use_raw_capture_path(targets) { + return self.snapshot_all_once_raw(targets).await; + } + // Use the first target's dimensions and DPR for the initial // launch. Chromiumoxide's built-in viewport emulation is // disabled in `browser_config`; otherwise it sends its own @@ -960,6 +964,42 @@ impl ChromiumDriver { // initialization. The first unpinned target can reuse the // launch-pinned window/DPR, while later targets and explicit // `--dpr` pins still use Plumb's bounded/labeled override. + let first = &targets[0]; + let resolved_executable = resolve_auto_fetch(&self.options).await?; + let launch = self.browser_config(first, resolved_executable.as_deref())?; + let mut session = ChromiumSession::launch(launch).await?; + + let result: Result, CdpError> = async { + validate_browser_version(&session.browser).await?; + let mut snapshots = Vec::with_capacity(targets.len()); + for (target_index, target) in targets.iter().enumerate() { + let snap = capture_target( + &session.browser, + target, + &self.options, + should_apply_viewport_override(target_index, target), + ) + .await?; + snapshots.push(snap); + } + Ok(snapshots) + } + .await; + + if let Err(cleanup_err) = session.shutdown().await { + tracing::debug!(error = %cleanup_err, "failed to clean up Chromium session"); + if result.is_ok() { + return Err(cleanup_err); + } + } + + result + } + + async fn snapshot_all_once_raw( + &self, + targets: &[Target], + ) -> Result, CdpError> { let first = &targets[0]; let resolved_executable = resolve_auto_fetch(&self.options).await?; let launch = self.browser_config(first, resolved_executable.as_deref())?; @@ -984,7 +1024,7 @@ impl ChromiumDriver { .await; if let Err(cleanup_err) = session.shutdown(&mut raw).await { - tracing::debug!(error = %cleanup_err, "failed to clean up Chromium session"); + tracing::debug!(error = %cleanup_err, "failed to clean up raw Chromium session"); if result.is_ok() { return Err(cleanup_err); } @@ -994,6 +1034,33 @@ impl ChromiumDriver { } } +fn should_use_raw_capture_path(targets: &[Target]) -> bool { + targets.iter().any(|target| { + target.wait_for_selector.is_some() + || matches!( + navigation_method_for_url(target.url.as_str()), + NavigationMethod::CdpNavigate + ) + }) +} + +async fn capture_target( + browser: &Browser, + target: &Target, + options: &ChromiumOptions, + apply_viewport_override: bool, +) -> Result { + let page = with_timeout("Browser.new_page", TARGET_ATTACH_TIMEOUT, async { + browser + .new_page(INITIAL_PAGE_URL) + .await + .map_err(driver_error) + }) + .await?; + + capture_on_page(&page, target, options, apply_viewport_override).await +} + async fn capture_target_raw( cdp: &mut RawCdpClient, target: &Target, @@ -1924,7 +1991,7 @@ struct NavigationState { } async fn navigate_page(page: &Page, url: &str) -> Result<(), CdpError> { - let initial_result = match navigation_method_for_url(url) { + let initial_result = match page_navigation_method_for_url(url) { NavigationMethod::ChromiumoxideGoto => { with_timeout("Page.navigate", DOCUMENT_READY_TIMEOUT, async { page.goto(url).await.map(|_| ()).map_err(driver_error) @@ -1967,6 +2034,14 @@ async fn navigate_page(page: &Page, url: &str) -> Result<(), CdpError> { wait_for_document_ready(page, navigation_display_url(url), initial_result.err()).await } +fn page_navigation_method_for_url(url: &str) -> NavigationMethod { + if url.starts_with("data:") { + NavigationMethod::CdpNavigate + } else { + NavigationMethod::ChromiumoxideGoto + } +} + async fn navigate_raw( cdp: &mut RawCdpClient, page: &RawPage, @@ -3096,6 +3171,43 @@ fn chromium_install_hint() -> String { ) } +struct ChromiumSession { + browser: Browser, + handler_task: Option>, + profile_dir: Option, +} + +impl ChromiumSession { + async fn launch(launch: ChromiumLaunch) -> Result { + let (browser, handler) = with_timeout("Chromium launch", BROWSER_LAUNCH_TIMEOUT, async { + Browser::launch(launch.config) + .await + .map_err(map_launch_error) + }) + .await?; + let handler_task = poll_handler(handler); + Ok(Self { + browser, + handler_task: Some(handler_task), + profile_dir: launch.profile_dir, + }) + } + + async fn shutdown(&mut self) -> Result<(), CdpError> { + let close_result = close_browser_best_effort(&mut self.browser).await; + if let Some(task) = self.handler_task.take() { + task.abort(); + if let Err(join_err) = task.await + && !join_err.is_cancelled() + { + tracing::debug!(error = %join_err, "Chromium handler task failed"); + } + } + let _profile_dir = self.profile_dir.take(); + close_result + } +} + struct RawChromiumSession { browser: Browser, profile_dir: Option, @@ -4616,6 +4728,44 @@ mod tests { ); } + #[test] + fn page_navigation_uses_chromiumoxide_goto_for_web_urls() { + assert_eq!( + super::page_navigation_method_for_url("http://127.0.0.1:49197/"), + super::NavigationMethod::ChromiumoxideGoto + ); + assert_eq!( + super::page_navigation_method_for_url("https://example.com/"), + super::NavigationMethod::ChromiumoxideGoto + ); + assert_eq!( + super::page_navigation_method_for_url("data:text/html;base64,PHNjcmlwdD4="), + super::NavigationMethod::CdpNavigate + ); + } + + #[test] + fn raw_capture_path_is_reserved_for_data_urls_and_selector_gated_pages() { + let web_target = super::Target { + url: "https://example.com/".to_owned(), + ..super::Target::default() + }; + assert!(!super::should_use_raw_capture_path(&[web_target])); + + let app_target = super::Target { + url: "http://127.0.0.1:49197/".to_owned(), + wait_for_selector: Some("html[data-plumb-ready=\"true\"]".to_owned()), + ..super::Target::default() + }; + assert!(super::should_use_raw_capture_path(&[app_target])); + + let data_target = super::Target { + url: "data:text/html;base64,PHNjcmlwdD4=".to_owned(), + ..super::Target::default() + }; + assert!(super::should_use_raw_capture_path(&[data_target])); + } + #[test] fn raw_navigation_tolerates_page_navigate_abort_for_web_urls() { assert!(super::uses_raw_tolerant_page_navigate( From 855204ba871ae4850d0f8fbbe0e1db189d116ec7 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 16:34:00 -0600 Subject: [PATCH 46/50] ci(e2e): limit chrome matrix parallelism --- .github/workflows/e2e-sites.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/e2e-sites.yml b/.github/workflows/e2e-sites.yml index 365faa7..a008734 100644 --- a/.github/workflows/e2e-sites.yml +++ b/.github/workflows/e2e-sites.yml @@ -26,6 +26,7 @@ jobs: timeout-minutes: 25 strategy: fail-fast: false + max-parallel: 3 matrix: os: [ubuntu-latest, macos-latest, windows-latest] framework: From b85601c6f2666773ac29a6382116db8ae048e158 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 17:07:33 -0600 Subject: [PATCH 47/50] fix(cdp): fall back after page creation timeouts --- crates/plumb-cdp/src/lib.rs | 63 +++++++++++++++++++++++++++++++++---- 1 file changed, 57 insertions(+), 6 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 3c006f5..37c5cc9 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -932,8 +932,13 @@ impl BrowserDriver for ChromiumDriver { } let mut attempts = 0; + let mut use_raw_capture = should_use_raw_capture_path(&targets); loop { - let result = self.snapshot_all_once(&targets).await; + let result = if use_raw_capture { + self.snapshot_all_once_raw(&targets).await + } else { + self.snapshot_all_once_page(&targets).await + }; if attempts < TRANSIENT_CAPTURE_RETRIES && result .as_ref() @@ -942,6 +947,13 @@ impl BrowserDriver for ChromiumDriver { { if let Err(err) = &result { tracing::debug!(attempt = attempts + 1, error = %err, "retrying Chromium capture after transient timeout"); + if should_fallback_to_raw_capture(err) { + tracing::debug!( + attempt = attempts + 1, + "retrying Chromium capture through raw CDP page path" + ); + use_raw_capture = true; + } } attempts += 1; continue; @@ -952,11 +964,10 @@ impl BrowserDriver for ChromiumDriver { } impl ChromiumDriver { - async fn snapshot_all_once(&self, targets: &[Target]) -> Result, CdpError> { - if should_use_raw_capture_path(targets) { - return self.snapshot_all_once_raw(targets).await; - } - + async fn snapshot_all_once_page( + &self, + targets: &[Target], + ) -> Result, CdpError> { // Use the first target's dimensions and DPR for the initial // launch. Chromiumoxide's built-in viewport emulation is // disabled in `browser_config`; otherwise it sends its own @@ -1044,6 +1055,16 @@ fn should_use_raw_capture_path(targets: &[Target]) -> bool { }) } +fn should_fallback_to_raw_capture(err: &CdpError) -> bool { + let CdpError::Driver(source) = err else { + return false; + }; + + source.downcast_ref::().is_some_and(|err| { + err.kind() == io::ErrorKind::TimedOut && err.to_string().contains("Browser.new_page") + }) +} + async fn capture_target( browser: &Browser, target: &Target, @@ -4766,6 +4787,36 @@ mod tests { assert!(super::should_use_raw_capture_path(&[data_target])); } + #[test] + fn raw_capture_fallback_accepts_browser_new_page_timeout() { + let err = CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "Browser.new_page exceeded 75s budget", + ))); + + assert!(super::should_fallback_to_raw_capture(&err)); + } + + #[test] + fn raw_capture_fallback_accepts_browser_new_page_request_budget() { + let err = CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "Browser.new_page hit Chromiumoxide request budget (60s)", + ))); + + assert!(super::should_fallback_to_raw_capture(&err)); + } + + #[test] + fn raw_capture_fallback_rejects_unrelated_timeouts() { + let err = CdpError::Driver(Box::new(io::Error::new( + io::ErrorKind::TimedOut, + "DOMSnapshot.captureSnapshot exceeded 60s budget", + ))); + + assert!(!super::should_fallback_to_raw_capture(&err)); + } + #[test] fn raw_navigation_tolerates_page_navigate_abort_for_web_urls() { assert!(super::uses_raw_tolerant_page_navigate( From 80123d9bcc57c34fc39c247d14e1d4abdbcb8fb9 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 17:40:05 -0600 Subject: [PATCH 48/50] fix(cdp): avoid load wait in one-shot page creation --- crates/plumb-cdp/src/lib.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 37c5cc9..48840e6 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -1071,14 +1071,18 @@ async fn capture_target( options: &ChromiumOptions, apply_viewport_override: bool, ) -> Result { - let page = with_timeout("Browser.new_page", TARGET_ATTACH_TIMEOUT, async { - browser - .new_page(INITIAL_PAGE_URL) - .await - .map_err(driver_error) - }) + let page = create_page_without_load_wait( + browser, + CreateTargetParams { + width: Some(i64::from(target.width)), + height: Some(i64::from(target.height)), + new_window: Some(true), + ..CreateTargetParams::new(INITIAL_PAGE_URL) + }, + ) .await?; + settle_initial_document().await; capture_on_page(&page, target, options, apply_viewport_override).await } From c27a16ccc7817b78d7972badcb12e87ce79f4283 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 18:19:55 -0600 Subject: [PATCH 49/50] fix(cdp): raw retry after page navigation stalls --- crates/plumb-cdp/src/lib.rs | 50 ++++++++++++++++++++++++++++++++++--- 1 file changed, 46 insertions(+), 4 deletions(-) diff --git a/crates/plumb-cdp/src/lib.rs b/crates/plumb-cdp/src/lib.rs index 48840e6..83855fa 100644 --- a/crates/plumb-cdp/src/lib.rs +++ b/crates/plumb-cdp/src/lib.rs @@ -119,6 +119,7 @@ const SNAPSHOT_CAPTURE_TIMEOUT_SECS: u64 = 60; const SNAPSHOT_CAPTURE_TIMEOUT: Duration = Duration::from_secs(SNAPSHOT_CAPTURE_TIMEOUT_SECS); const TRANSIENT_CAPTURE_RETRIES: usize = 1; const INITIAL_PAGE_URL: &str = "about:blank"; +const RAW_DEFAULT_READY_SELECTOR: &str = "body"; /// CSS property whitelist passed to `DOMSnapshot.captureSnapshot` as the /// `computedStyles` argument. @@ -1061,10 +1062,17 @@ fn should_fallback_to_raw_capture(err: &CdpError) -> bool { }; source.downcast_ref::().is_some_and(|err| { - err.kind() == io::ErrorKind::TimedOut && err.to_string().contains("Browser.new_page") + let message = err.to_string(); + (err.kind() == io::ErrorKind::TimedOut && message.contains("Browser.new_page")) + || is_page_navigation_init_timeout(&message) }) } +fn is_page_navigation_init_timeout(message: &str) -> bool { + message.contains("exhausted 30s ready-state budget") + && message.contains("Page.navigate exceeded 30s budget") +} + async fn capture_target( browser: &Browser, target: &Target, @@ -2555,15 +2563,20 @@ async fn apply_post_navigate_waits_raw( page: &RawPage, target: &Target, ) -> Result<(), CdpError> { - if let Some(selector) = target.wait_for_selector.as_deref() { - wait_for_selector_raw(cdp, page, selector).await?; - } + wait_for_selector_raw(cdp, page, raw_ready_selector(target)).await?; if let Some(ms) = target.wait_ms { tokio::time::sleep(std::time::Duration::from_millis(ms)).await; } Ok(()) } +fn raw_ready_selector(target: &Target) -> &str { + target + .wait_for_selector + .as_deref() + .unwrap_or(RAW_DEFAULT_READY_SELECTOR) +} + /// Install localStorage entries from an already-parsed Playwright /// storage-state. /// @@ -4811,6 +4824,18 @@ mod tests { assert!(super::should_fallback_to_raw_capture(&err)); } + #[test] + fn raw_capture_fallback_accepts_page_navigation_init_timeout() { + let err = CdpError::Driver(Box::new(io::Error::other( + "navigation to `http://127.0.0.1:49196/` exhausted 30s ready-state \ + budget after initial location assignment failed: driver failure: \ + Page.navigate exceeded 30s budget; last navigation state read failed: \ + navigation state read exceeded 10s budget", + ))); + + assert!(super::should_fallback_to_raw_capture(&err)); + } + #[test] fn raw_capture_fallback_rejects_unrelated_timeouts() { let err = CdpError::Driver(Box::new(io::Error::new( @@ -4821,6 +4846,23 @@ mod tests { assert!(!super::should_fallback_to_raw_capture(&err)); } + #[test] + fn raw_ready_selector_defaults_to_body() { + let target = super::Target::default(); + + assert_eq!(super::raw_ready_selector(&target), "body"); + } + + #[test] + fn raw_ready_selector_preserves_user_selector() { + let target = super::Target { + wait_for_selector: Some("#app-ready".to_owned()), + ..super::Target::default() + }; + + assert_eq!(super::raw_ready_selector(&target), "#app-ready"); + } + #[test] fn raw_navigation_tolerates_page_navigate_abort_for_web_urls() { assert!(super::uses_raw_tolerant_page_navigate( From 5ec11b1d3fffcefc96aba91cf5db8b152cc6a741 Mon Sep 17 00:00:00 2001 From: Aram Hammoudeh Date: Sat, 20 Jun 2026 18:31:07 -0600 Subject: [PATCH 50/50] ci(claude): increase review turn budget --- .github/workflows/claude-code-review.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/claude-code-review.yml b/.github/workflows/claude-code-review.yml index a7ae9ab..f309770 100644 --- a/.github/workflows/claude-code-review.yml +++ b/.github/workflows/claude-code-review.yml @@ -39,7 +39,7 @@ jobs: github.actor != 'dependabot[bot]' && github.actor != 'renovate[bot]' runs-on: ubuntu-latest - timeout-minutes: 20 + timeout-minutes: 30 steps: - uses: actions/checkout@v6 with: @@ -76,7 +76,7 @@ jobs: references. Skip nits unless they stack; focus on substantive issues. claude_args: | - --max-turns 20 + --max-turns 40 --model claude-sonnet-4-6 --allowed-tools "Bash(cargo *),Bash(git diff:*),Bash(git log:*),Read,Glob,Grep" --disallowed-tools "Bash(git push:*),Bash(git commit:*)"