Skip to content

fix(cdp): keep default raw readiness best effort - #310

Merged
aram-devdocs merged 1 commit into
mainfrom
codex/fix-macos-raw-readiness
Jun 21, 2026
Merged

fix(cdp): keep default raw readiness best effort#310
aram-devdocs merged 1 commit into
mainfrom
codex/fix-macos-raw-readiness

Conversation

@aram-devdocs

Copy link
Copy Markdown
Owner

Summary

  • keep explicit raw --wait-for selectors strict
  • make the synthetic raw body readiness probe best-effort before DOM snapshot capture
  • preserves the raw fallback path while avoiding the macOS CI failure where wait_for_selector "body" exhausted its 10s budget after a page-init stall

Validation

  • cargo fmt --all -- --check
  • cargo test -p plumb-cdp --lib raw_
  • cargo clippy -p plumb-cdp -p plumb-cli --all-targets --all-features -- -D warnings
  • cargo build --release -p plumb-cli
  • cargo run --release -p plumb-e2e -- --site html-css --site tailwind-html --site react-vite --no-build --plumb-bin target/release/plumb --chrome-path "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome"

@aram-devdocs
aram-devdocs merged commit 8c952d5 into main Jun 21, 2026
20 of 25 checks passed
@aram-devdocs
aram-devdocs deleted the codex/fix-macos-raw-readiness branch June 21, 2026 01:01
@github-actions

Copy link
Copy Markdown
Contributor

I have everything I need. Here is the full review.


PR #310 — fix(cdp): keep default raw readiness best effort

Scope: crates/plumb-cdp/src/lib.rs only (+24 / -10). Single-crate, no public API additions, no dependency changes.


1. Determinism

No violations. The change touches only CDP I/O orchestration; nothing flows into PlumbSnapshot output. tokio::time::sleep already exists in wait_for_selector_raw — no new wall-clock source. tracing::debug! is the correct I/O (not println!/eprintln!), consistent with plumb-cdp CLAUDE.md.

2. Workspace layering

Clean. All changes stay inside plumb-cdp. No new cross-crate dependencies, no new unsafe blocks, no println!/eprintln! added.

3. Error handling

The semantic split is sound:

  • Explicit user selector (wait_for_selector set) → strict: error propagated with ?. Breaking a user's explicit wait contract is always fatal.
  • Default body probe → best-effort: error swallowed after tracing::debug!. A macOS page-init stall should not abort the whole capture for a probe that was already a heuristic.

wait_for_default_ready_selector_raw_best_effort returns () — the name makes the contract explicit and the log line preserves observability for debugging. No unwrap/expect anywhere.

4. Test coverage

The two renamed unit tests (raw_strict_ready_selector_is_absent_without_user_selector, raw_strict_ready_selector_preserves_user_selector) correctly cover the refactored raw_strict_ready_selector helper. wait_for_default_ready_selector_raw_best_effort has no direct unit test, but it's an async function that requires a live RawCdpClient/RawPage — mocking would violate Plumb's "don't mock CDP I/O" convention. The e2e run cited in the PR description (--site html-css --site tailwind-html --site react-vite) is the appropriate coverage tier.

5. Documentation

All modified/added functions are private. missing_docs does not apply. No public items changed.


Punch list

No blockers or warnings found. The change is a minimal, well-scoped behavioural fix with the semantics clearly encoded in the function names and the log message.

Verdict: APPROVE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant