You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Mostly a clean removal of the browser-harness benchmark integration (~1 080 lines deleted) with a small focused fix in src/lib.rs for recording frame URLs on same-document navigations (when loader_id is absent), plus pytest.skip guards for optional reference packages. No stealth/anti-detection claims are added and docs stay in sync.
🔴 Critical Issues (1)
Missing iframe same-document navigation fix in RustwrightPage (Node.js binding) PyPage receives two fixes for the loader_id.is_none() early-return path: one for the main-frame goto (~L3837) and one for the iframe/OOPIF goto path (~L3911, which passes session_id). The Node.js RustwrightPage (~L7092) only gets the main-frame fix. If the Node.js binding supports frame-level navigation (or OOPIFs), same-document navigations in sub-frames will leave stale URLs in frame_state. If the Node path intentionally doesn't support sub-frame goto today, add a comment explaining why, so it doesn't look like an accidental omission.
🟡 Suggestions (2)
No test coverage for the new record_frame_navigation_url path.
The src/lib.rs change is the only substantive behavioral fix in this PR, but the test changes only add require_reference_module guards — no new case exercises the same-document navigation code path (e.g. page.goto("…#hash"), history.pushState, or a data-URL with an anchor). A single pytest case that navigates to a hash fragment and asserts page.url returns the post-navigation URL would pin this behaviour and prevent regressions.
require_reference_module exit-code 3 is a magic number.
The sentinel 3 (module not found) vs 0 (found) is correct but undocumented. A comment in the helper or a named constant (MODULE_NOT_FOUND_EXIT = 3) would make the intent clear to future readers, since exit code 3 has no standard meaning.
📝 Minor / Style (1)
find_chromium_executable error message was updated from "for browser-harness comparison" to "for the Puppeteer comparison" — correct, but the function is also used by the TypeScript-Playwright runner. A more neutral message like "Could not find a Chromium executable for the Node.js runner" would be accurate across all callers.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
https://github.com/Skyvern-AI/rustwright-cloud/pull/19