Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR adds two new CLI subcommands, Changesrun-script and adapter commands
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as "CLI (lib.rs)"
participant Executor as "executor.rs"
participant Evaluate as "evaluate.rs"
participant Page as "Page Context"
User->>CLI: run-script/adapter file_path --arg key=value
CLI->>CLI: parse_args() into JSON script_args
CLI->>Executor: build_request() / run_direct()
Executor->>Executor: script_exec_args() validate file_path, output, track_navigation
Executor->>Evaluate: run_script()/run_adapter()
alt run-script
Evaluate->>Evaluate: parse_script_url_marker() for `@url/`@navigate
Evaluate->>Page: navigate if URL derived and mismatched
Evaluate->>Evaluate: build_ctx_object()
Evaluate->>Page: inject ctx + script, evaluate()
else adapter
Evaluate->>Evaluate: parse_adapter_domains() for `@domain`
Evaluate->>Page: check current domain, auto-navigate if mismatched
Evaluate->>Evaluate: strip_export_keywords(), is_valid_js_identifier()
Evaluate->>Page: inject ctx + adapter code, call function_name(ctx)
end
Page-->>Evaluate: result / exceptionDetails
Evaluate-->>Executor: CommandResult
Executor-->>CLI: response
CLI-->>User: output (text/json)
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces two new commands, run-script and adapter, to the chrome-devtools CLI, enabling the execution of local JavaScript files and site-specific custom adapters with injected helper utilities and domain protection. Feedback on these changes suggests extracting the duplicated ctx helper definition and the hardcoded 100ms polling interval into shared constants or helpers to improve maintainability. Additionally, it is recommended to avoid prepending www. when constructing target URLs for domain protection to prevent navigation failures on domains that do not use the subdomain.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@skill/chrome-devtools/examples/deepwiki_adapter.js`:
- Around line 33-34: The readWiki flow currently performs navigation and then
waits in the same execution context, which breaks because setting
window.location.href tears down the Runtime.evaluate context. Update readWiki in
deepwiki_adapter.js so it only scrapes the current page and does not navigate or
call ctx.waitForSelector; move the wiki navigation responsibility to run_adapter
or its caller, and keep readWiki focused on reading article/wiki-content from
the already loaded page.
In `@skill/chrome-devtools/examples/search_deepwiki.js`:
- Around line 4-33: The search_deepwiki example is incompatible with the
run-script wrapper because it re-wraps execution in a nested async IIFE, which
causes the returned value to be lost. Update the script to match the injected
async context used by run_script, using the existing ctx helpers directly in the
top-level flow of search_deepwiki.js. Also remove the window.location.href
navigation fallback, since changing pages breaks the evaluation context;
instead, validate the current DeepWiki page up front and fail fast or move this
example to an adapter-based approach.
In `@src/commands/evaluate.rs`:
- Around line 203-208: The fallback navigation in evaluate::evaluate is
rewriting bare domains too aggressively by forcing every non-HTTP target into
https://www.<domain>. Update the target_url construction for domains[0] so it
preserves the original host when no scheme is present, and only prepends a
scheme or otherwise normalizes the input without assuming a www subdomain. Keep
the logic localized around the target_domain/target_url branch so adapters with
apex hosts or existing subdomains navigate correctly.
- Around line 228-268: The wrapper built in evaluate.rs injects script_content
directly into an async IIFE, so exported adapter definitions like export async
function will fail to parse before the {function_name} existence check runs.
Update the evaluation path around the iife/script_content handling to either
execute adapters as modules or preprocess the loaded source to remove export
keywords before injection, and make the supported adapter format explicit in the
evaluate flow.
- Around line 110-149: The injected `script_content` in `evaluate` is executed
as a bare statement inside the async IIFE, so any returned value from a user
script is lost. Update the `iife` construction in `evaluate` to return or await
the injected expression so `run-script` preserves the result from scripts like
the documented async IIFE, while keeping the surrounding `ctx` helpers
unchanged.
In `@src/lib.rs`:
- Around line 435-459: `parse_args` currently ignores malformed tokens and empty
keys, which lets bad `--arg` input slip through and fail later with a misleading
error. Update `parse_args` in `src/lib.rs` to return
`anyhow::Result<serde_json::Value>`, validate every argument has a non-empty key
before inserting into the map, and return descriptive `anyhow!` errors for
tokens without `=` or invalid keys. Then update the caller that builds the
request from `parse_args` to handle the `Result` and surface the error
immediately.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 606dbb73-c001-49af-be28-c6dd8211bb2f
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (9)
Cargo.tomlskill/chrome-devtools/SKILL.mdskill/chrome-devtools/examples/deepwiki_adapter.jsskill/chrome-devtools/examples/search_deepwiki.jssrc/commands/evaluate.rssrc/commands/executor.rssrc/lib.rswiki/adapter.mdwiki/run-script.md
* remove in-adapter navigation from deepwiki examples to avoid tearing down execution context * require pre-navigation and simplify async script structure with direct ctx usage * add `strip_export_keywords` to support ES module-style adapters without syntax errors * fix domain auto-navigation to preserve exact host (avoid forcing `www`) * make `parse_args` return `Result`, validate key=value format, and reject malformed inputs * update callers to handle parsing errors and add tests for new validations
* introduce `POLL_INTERVAL_MS` constant for configurable polling frequency * replace hardcoded 100ms delays in `waitForText` and `waitForSelector` * ensure consistent polling behavior across `run_script` and `run_adapter`
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces the run-script and adapter commands to the chrome-devtools CLI, enabling the execution of local JavaScript files and site-specific custom adapters with injected helper utilities and domain protection. The review feedback highlights several opportunities to improve robustness, including refining domain matching to handle ports and schemes, restricting comment parsing to actual comment lines, preserving leading zeros in CLI arguments, making export keyword stripping safer, supporting HTTP for local development, validating function names to prevent arbitrary code execution, extending the ctx.fill helper to support checkboxes and radio buttons, and making script_args optional.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
src/commands/evaluate.rs (1)
111-147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the injected
ctxwrapper into one testable helper.The wrapper string is duplicated almost verbatim in
run_scriptandrun_adapter, so helper changes now have to stay in sync in two places. As per coding guidelines, "Pure conversion and formatting logic should be extracted into testable functions".♻️ Suggested shape
+fn build_ctx_wrapper(script_args: &serde_json::Value, body: &str) -> Result<String> { + let args_str = serde_json::to_string(script_args)?; + Ok(format!( + r#"(async () => {{ + const ctx = {{ + args: {args_str}, + wait: async (ms) => new Promise(r => setTimeout(r, ms)), + waitForText: async (text, timeout = 30000) => {{ /* ... */ }}, + waitForSelector: async (selector, timeout = 30000) => {{ /* ... */ }}, + click: async (selector) => {{ /* ... */ }}, + fill: async (selector, value) => {{ /* ... */ }} + }}; + + {body} + }})()"# + )) +}Also applies to: 262-303
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/evaluate.rs` around lines 111 - 147, The injected ctx wrapper is duplicated between run_script and run_adapter, so extract the shared IIFE/context-building string into one testable helper and have both call sites reuse it. Move the wrapper construction (including args_str, waitForText, waitForSelector, click, and fill) into a dedicated function or helper near evaluate::run_script/run_adapter, then call that helper from both paths so future changes only happen in one place.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/evaluate.rs`:
- Around line 168-181: Normalize the adapter domain before comparison in
url_matches_domain so scheme and any path are stripped just like the current URL
host. Update the matching logic to compare only the hostname from the raw
`@domain` text against the host extracted from url_matches_domain, and keep the
existing exact/subdomain match behavior. This should be handled within
url_matches_domain in evaluate.rs so run_adapter() stops auto-navigating when
the adapter domain is written as https://example.com or includes a path.
- Around line 153-165: The parse_adapter_domains helper is matching any `@domain`
substring, which can pick up strings or comments inside adapter code instead of
only metadata. Update parse_adapter_domains to only accept JSDoc metadata lines
that actually declare `@domain` in the adapter header, using a stricter pattern or
by limiting parsing to the comment block associated with the adapter. Keep the
existing function name and domain extraction flow, but ensure only genuine
adapter metadata lines are collected.
In `@wiki/adapter.md`:
- Around line 69-81: The Wiki docs still show the outdated readWiki flow, so
update the example to match the fixed implementation in
skill/chrome-devtools/examples/deepwiki_adapter.js: in readWiki, remove the
ctx.args.url handling and the window.location.href navigation, and keep only the
scraping logic that reads the already-loaded page. Make sure the documented
readWiki signature and behavior match the actual adapter example so the wiki no
longer instructs users to tear down the Runtime.evaluate context before
ctx.waitForSelector runs.
In `@wiki/run-script.md`:
- Around line 28-69: Update the wiki’s Search DeepWiki example to match the
fixed behavior in search_deepwiki.js: remove the nested async IIFE wrapper, use
the top-level ctx-driven script structure that returns results directly, and
replace the in-script window.location.href navigation with an explicit “navigate
first” requirement. Keep the unique flow around ctx.args.query, ctx.fill,
ctx.click, ctx.waitForSelector, and the final return results aligned with the
actual example file so readers don’t copy the broken pattern.
---
Nitpick comments:
In `@src/commands/evaluate.rs`:
- Around line 111-147: The injected ctx wrapper is duplicated between run_script
and run_adapter, so extract the shared IIFE/context-building string into one
testable helper and have both call sites reuse it. Move the wrapper construction
(including args_str, waitForText, waitForSelector, click, and fill) into a
dedicated function or helper near evaluate::run_script/run_adapter, then call
that helper from both paths so future changes only happen in one place.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd31527e-f906-4ecf-a845-b0ff586f05f0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlskill/chrome-devtools/SKILL.mdskill/chrome-devtools/examples/deepwiki_adapter.jsskill/chrome-devtools/examples/search_deepwiki.jssrc/commands/evaluate.rssrc/commands/executor.rssrc/constants.rssrc/lib.rswiki/adapter.mdwiki/run-script.md
…RL matching, and update docs/tests * extract shared `build_ctx_object` for consistent ctx helpers across run-script and adapter * harden `@domain` parsing to only accept valid comment metadata and ignore false positives * add hostname normalization and improve domain matching logic (scheme/path/port aware) * simplify run-script execution flow and remove duplicated inline ctx code * expand tests for parsing, normalization, and ctx helper generation * update docs to clarify async execution model and navigation requirements
… reliability * handle checkbox/radio inputs correctly in `fill` helper * validate adapter function names as safe JS identifiers to prevent injection * refine export stripping to only affect real declarations * normalize hosts and detect local domains to choose correct navigation scheme * default `script_args` to empty object when omitted * preserve non-canonical numeric args (e.g., leading zeros, `+` prefix) * add tests covering identifier validation, local host detection, and parsing edge cases
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces the run-script and adapter commands to evaluate local JavaScript files and custom site adapters inside the browser context, complete with injected automation helpers (ctx), domain protection, and CLI argument parsing. The review feedback highlights an opportunity to support contenteditable elements in the ctx.fill helper and identifies a bug in normalize_host where IPv6 addresses are incorrectly truncated when stripping ports, suggesting a robust fix and additional unit tests.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/evaluate.rs`:
- Around line 340-373: The auto-navigation in evaluate::run (the block that
calls crate::commands::navigate::navigate after building target_url) should
re-check the current URL against domains before continuing. After navigation
completes, call client.current_url(session_id).await? again and verify it still
matches at least one entry via url_matches_domain; if it does not, abort instead
of running the adapter on the wrong page.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6f3b9cbc-3e8e-4d9c-841d-54d587eb727d
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlskill/chrome-devtools/SKILL.mdskill/chrome-devtools/examples/deepwiki_adapter.jsskill/chrome-devtools/examples/search_deepwiki.jssrc/commands/evaluate.rssrc/commands/executor.rssrc/constants.rssrc/lib.rswiki/adapter.mdwiki/run-script.md
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the run-script and adapter commands to the chrome-devtools CLI, enabling the execution of local JavaScript files and site-specific adapters with injected automation helpers (ctx) and automatic domain-based navigation. Feedback on these changes highlights several improvement opportunities: float arguments in parse_args lack a round-trip check, which can mutate formatted decimals; ctx.fill should explicitly guard against programmatically filling file inputs to avoid browser security errors; normalize_host strips ports, which can cause different local development ports to incorrectly match; and strip_export_keywords naively strips exports line-by-line, potentially corrupting multi-line string literals or comments.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Introduced CUSTOM_SCRIPTING.md guide for creating `run-script` and `adapter` files - Added flexible argument parsing (positional + named) with automatic mapping to ctx.args - Implemented comment-based @url/@Navigate auto-navigation for scripts - Added domain-aware adapter execution with @Domain protection - Enhanced ctx helpers (wait, waitForSelector, click, fill) for SPA compatibility - Updated SKILL.md examples to reference new scripting/adapters - Replaced DeepWiki examples with Hacker News search scripts/adapters - Improved argument parsing to support trailing raw args (`query`, `_0`, `_1`, etc.) - Added URL encoding for safe interpolation in auto-navigation - Refined evaluate.rs to remove intrusive hints and improve DOM input handling
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/evaluate.rs`:
- Around line 213-236: `run-script` in `evaluate.rs` is only checking
`url_matches_domain()`/host equality before skipping navigation, which can leave
the tab on the wrong query or path. Update the `current_url` vs
`interpolated_url` check in `run_script` to compare against the fully built
target navigation URL (the same `nav_url` logic used before
`crate::commands::navigate::navigate`), and after navigating re-read the page
URL and verify it matches that full target before continuing.
In `@src/lib.rs`:
- Around line 452-456: The float parsing branch in the argument/value parser is
rewriting non-canonical numeric tokens instead of preserving them like the
integer branch does. Update the logic in the function that matches
`v.parse::<f64>()` to only emit `serde_json::Value::Number` when the original
text is canonical, and otherwise fall back to
`serde_json::Value::String(v.to_string())` just like the integer path. Add a
regression test near `test_parse_args_preserves_leading_zeros` covering cases
such as `01.50`, `1e3`, and `+3.5` to ensure they remain strings when
non-canonical.
- Around line 340-347: The CLI parsing for raw_args in the main args struct is
swallowing later flags because trailing_var_arg and allow_hyphen_values make
option parsing stop after the first positional. Update the parsing shape around
raw_args, output, and track_navigation so those flags remain reachable after
script arguments, either by requiring explicit -- delimiter/options-first
ordering or by changing the argument definition to keep options parsed
separately. Locate the fix in the struct that defines raw_args, output, and
track_navigation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d644c000-0340-448e-9d05-c863ebc2fc68
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
Cargo.tomlREADME.mdskill/chrome-devtools/CUSTOM_SCRIPTING.mdskill/chrome-devtools/SKILL.mdskill/chrome-devtools/examples/hn_adapter.jsskill/chrome-devtools/examples/search_hn.jssrc/commands/evaluate.rssrc/commands/executor.rssrc/constants.rssrc/lib.rswiki/adapter.mdwiki/run-script.md
💤 Files with no reviewable changes (1)
- README.md
|
/gemini review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces two new features to the Chrome DevTools CLI: run-script for executing local JavaScript files with injected helpers and auto-navigation, and adapter for running domain-protected custom site adapter functions. It adds corresponding documentation, examples, and CLI argument parsing logic. The review feedback highlights two valuable improvements: traversing the prototype chain in the fill helper to reliably locate the native value setter for custom elements, and correctly stripping basic auth credentials in normalize_host before extracting the hostname.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…ion with additional tests
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces two new commands, run-script and adapter, to execute local JavaScript files and site-specific custom adapters inside the browser page context. It adds an injected helper context (ctx) with automation utilities, parses flexible CLI arguments into typed JS values, and supports comment-based auto-navigation and domain protection. Comprehensive documentation, examples, and unit tests are also provided. Feedback on the changes suggests improving parse_adapter_domains to stop scanning at the first non-comment line and to support block comments, preventing potential false positives.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…and add tests for block comments
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for running custom local JavaScript scripts (run-script) and site-specific custom adapters (adapter) with built-in domain protection, auto-navigation, and injected automation helpers. It includes comprehensive documentation, real-world examples, CLI argument parsing, and unit tests. Feedback on these changes highlights a few areas for improvement: first, the newly added toml dependency in Cargo.toml appears to be unused and should be removed; second, synchronous file I/O (std::fs::read_to_string) is used within the asynchronous run_script and run_adapter functions in src/commands/evaluate.rs, which blocks the Tokio executor thread and should be replaced with tokio::fs::read_to_string.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
* drop `toml` crate and related transitive dependencies from Cargo files * clean up `Cargo.lock` by removing unused packages (toml, serde\_spanned, winnow, etc.) * switch script and adapter file reads to `tokio::fs::read_to_string` with async/await * ensure non-blocking I/O in `evaluate.rs` command execution paths
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
wiki/run-script.md (1)
68-107: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winWiki example doesn't match the actual
search_hn.jsfile.The embedded script here still shows
//@urlhttps://hn.algolia.complus actx.fill("input.SearchInput", query)call. The actualskill/chrome-devtools/examples/search_hn.jsfile in this same PR uses//@urlhttps://hn.algolia.com/?query={query}(interpolating the query directly into the URL) and has noctx.fillstep at all. This is the same doc/example desync pattern flagged and fixed twice already in this PR (forreadWikiandsearch_deepwiki.js) — please sync this example too.📝 Proposed fix to match actual example
### Script file (`skill/chrome-devtools/examples/search_hn.js`) ```javascript -// `@url` https://hn.algolia.com +// `@url` https://hn.algolia.com/?query={query} // search_hn.js // Run with: chrome-devtools run-script skill/chrome-devtools/examples/search_hn.js -a query="Rust" // // run-script injects `ctx` and runs this file inside an async context. -// Setting `@url` above tells the CLI to automatically navigate to the target site first! +// Setting `@url` above tells the CLI to automatically navigate to the pre-rendered query URL first! const query = ctx.args.query; if (!query) { throw new Error("Query argument is required. Pass it with '-a query=...'"); } -// Fill in search input (the SPA will fetch and render results dynamically) -await ctx.fill("input.SearchInput", query); - // Wait for results to update/load await ctx.waitForSelector("article.Story", 10000);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@wiki/run-script.md` around lines 68 - 107, The wiki example for search_hn.js is out of sync with the actual script, so update the embedded snippet and CLI note to match the real behavior in search_hn.js. Replace the generic `@url` and the ctx.fill("input.SearchInput", query) step with the query-interpolated URL approach, and keep the rest of the example aligned with the script’s actual flow around ctx.args.query and waitForSelector.
🧹 Nitpick comments (1)
src/lib.rs (1)
481-523: 🎯 Functional Correctness | 🔵 TrivialDuplicated key=value parsing between
named_argsandraw_argsloops.The split/trim/empty-key-check logic (lines 486-494 and 500-506) is repeated. Consider extracting a small helper, e.g.
fn parse_kv(arg: &str) -> Result<(String, serde_json::Value)>, used by both loops.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib.rs` around lines 481 - 523, The key=value parsing in parse_args is duplicated across the named_args and raw_args loops, so extract the split/trim/empty-key validation into a small helper such as parse_kv or equivalent near parse_args and reuse it in both paths. Keep the existing positional handling in parse_args intact, but have both loops call the shared helper to build the key and parsed JSON value consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/lib.rs`:
- Around line 497-520: The raw_args parser in lib.rs is treating every token
containing “=” as a key=value pair, which misclassifies literal positional
values like URLs or other strings with query parameters. Update the parsing
logic in the raw_args loop to only enter the key/value branch when the part
before “=” is a valid identifier-like key, and otherwise preserve the token as a
positional argument. Keep the positional handling that populates _0, _1, etc.,
and ensure the first positional still maps to query in the same way.
- Around line 455-479: The float handling in parse_json_value is using
f.to_string() for the canonicality check, which can misclassify values like 1.0
or 100.0 as strings. Update the float branch in parse_json_value to compare
against the parsed serde_json::Number’s canonical string form instead, and keep
the existing serde_json::Number::from_f64 path for valid numbers. Add a
regression test covering an input like val5=1.0 to verify it is parsed as a JSON
number.
---
Duplicate comments:
In `@wiki/run-script.md`:
- Around line 68-107: The wiki example for search_hn.js is out of sync with the
actual script, so update the embedded snippet and CLI note to match the real
behavior in search_hn.js. Replace the generic `@url` and the
ctx.fill("input.SearchInput", query) step with the query-interpolated URL
approach, and keep the rest of the example aligned with the script’s actual flow
around ctx.args.query and waitForSelector.
---
Nitpick comments:
In `@src/lib.rs`:
- Around line 481-523: The key=value parsing in parse_args is duplicated across
the named_args and raw_args loops, so extract the split/trim/empty-key
validation into a small helper such as parse_kv or equivalent near parse_args
and reuse it in both paths. Keep the existing positional handling in parse_args
intact, but have both loops call the shared helper to build the key and parsed
JSON value consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a5508685-8213-4156-a1e1-43c7d65160ac
📒 Files selected for processing (11)
README.mdskill/chrome-devtools/CUSTOM_SCRIPTING.mdskill/chrome-devtools/SKILL.mdskill/chrome-devtools/examples/hn_adapter.jsskill/chrome-devtools/examples/search_hn.jssrc/commands/evaluate.rssrc/commands/executor.rssrc/constants.rssrc/lib.rswiki/adapter.mdwiki/run-script.md
💤 Files with no reviewable changes (1)
- README.md
… '=' and support canonical floats
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces custom scripting (run-script) and domain-aware adapters (adapter) to the Chrome DevTools CLI, allowing users to execute local JavaScript files inside the browser context with injected automation helpers. The review feedback highlights several critical issues and improvement opportunities: compilation errors in src/commands/evaluate.rs due to passing Option<&String> instead of Option<&str> to the navigate function; React/Vue state desynchronization when the fill helper directly toggles checkboxes and radio buttons without using their prototype setters; an inability to distinguish between different local ports on localhost because normalize_host unconditionally strips ports; early termination of @domain scanning if files contain directives or imports before the JSDoc metadata block; and potential runtime SyntaxErrors from unhandled named export blocks in strip_export_keywords.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
…t stripping * use native property setters via prototype chain to ensure React/Vue detect input updates * extend checkbox/radio handling to set `checked` via native setter * enhance `parse_adapter_domains` to skip leading imports and `"use strict"` directives * refine `strip_export_keywords` to drop bare re-exports while preserving non-declaration exports * add tests for import skipping, re-export stripping, and updated fill behavior
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces custom scripting (run-script) and domain-aware adapters (adapter) to the Chrome DevTools CLI, allowing users to evaluate local JavaScript files with injected helper utilities (ctx) and parsed dynamic arguments. It includes comprehensive documentation, real-world examples, and robust CLI argument parsing. The review feedback suggests two key improvements: refactoring the waitForText and waitForSelector helpers to ensure the DOM is checked at least once even when the timeout is set to zero, and extending the JSON argument parser to support null values.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
… null parsing * update wait loops to always perform an initial condition check (handle `timeout = 0`) * restructure polling logic to break only after timeout threshold * extend argument parsing to treat `"null"` as JSON null value * add tests for null parsing and wait loop behavior
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for running custom local JavaScript scripts (run-script) and domain-aware site adapters (adapter) with injected automation helpers (ctx) and automatic navigation capabilities. It includes comprehensive documentation, real-world examples, CLI argument parsing, and unit tests. Feedback on the changes identifies two critical compilation errors in src/commands/evaluate.rs where Option<&String> is passed to the navigate function instead of the expected Option<&str>; using .as_str() on the string variables is suggested to resolve these errors.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
/gemini review |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
1 participant
Summary by CodeRabbit
New Features
Documentation
Bug Fixes