Conversation
…ent session - Introduce OutputFormat (Text/Json/Toon) with --toon flag alongside --json. TOON produces compact tabular output for LLM agents (toon-format crate). - Add `console`, `network`, and `sw-logs` commands for inspecting page console messages, network requests, and extension service worker logs. - Add persistent CDP session that stays attached to the current page so events accumulate across commands; `console`/`network` drain on demand instead of using blocking --duration windows. - Add --block-url/--allow-url under `emulate` (+ as global flags) for URL blocking via Network.setBlockedURLs; patterns persist in the daemon. - Add `kill-daemon` subcommand for stopping the background daemon cleanly. - Rewrite skill/chrome-devtools/SKILL.md with correct target-name workflow, pattern-based usage examples, and critical gotchas.
|
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:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces an ChangesCDP CLI Expansion: OutputFormat, Persistent Sessions, New Commands, URL Blocklist
Sequence DiagramssequenceDiagram
participant Executor
participant CdpClient
participant CDP as Chrome CDP
participant Format as format_structured
Executor->>CdpClient: ensure_persistent_session(target_id)
CdpClient->>CDP: Target.attachToTarget (flatten)
CdpClient->>CDP: Network.enable / Runtime.enable
CdpClient->>CDP: Network.setBlockedURLs(blocklist)
alt console/network with duration_ms == 0 (drain)
Executor->>CdpClient: drain_console_events() / drain_network_events()
CdpClient-->>Executor: Vec from persistent buffer
else console/network with duration_ms > 0 (live)
Executor->>CdpClient: read_events_for(duration_ms)
loop until timeout
CDP-->>CdpClient: Runtime/Network event
CdpClient->>CdpClient: push_event → buffer
end
Executor->>CdpClient: drain_*_events()
else sw-logs
Executor->>CdpClient: get_all_targets()
CdpClient-->>Executor: all targets including service_worker
loop per service_worker
Executor->>CDP: Target.attachToTarget
Executor->>CDP: Runtime.enable
end
Executor->>CdpClient: read_events_for(duration_ms)
end
Executor->>Executor: process_*_events(raw CDP)
Executor->>Format: format_structured(events, req.format())
Format-->>Executor: formatted String
Executor-->>Executor: CommandResult::output(formatted)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 persistent CDP sessions for continuous event collection, adds new commands for inspecting console logs, network requests, and extension service worker logs, and implements a compact, LLM-friendly TOON output format. The review feedback highlights critical issues that need to be addressed before merging: specifically, the event router in push_event and the service worker log processor both lack session ID filtering, which leads to event pollution from unrelated sessions. Additionally, several commands (sw-logs, console, and network) fail to disable domains or detach sessions if an error occurs during event reading, resulting in potential resource and session leaks.
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: 7
🤖 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/cdp.rs`:
- Around line 560-570: The match expression in the `read_events_for` function
combines timeout errors and socket/read errors into the same case, causing real
WebSocket failures to be silently treated as normal timeout completion and
partial data returned as success. Differentiate the two error cases: handle the
timeout case (when tokio::time::timeout returns Err) by breaking the loop
normally, and handle the actual read/socket error case (when self.read_text()
returns Ok(Err)) separately to either propagate the error, log it, or return an
appropriate result instead of silently breaking with partial data.
- Around line 146-152: When detaching from an old target in the
`ensure_persistent_session` method, the event buffers for `network_events` and
`console_events` are not being cleared, causing stale events from the previous
target to persist and be drained under the new page context. In the block where
`persistent_session.take()` is called and we detach from the target, add
clearing of both `network_events` and `console_events` buffers alongside the
`persistent_target_id = None` assignment. Apply this same fix at the other
affected location mentioned in the comment (lines 174-176).
- Around line 212-233: The persistent event buffering logic is capturing Network
and Runtime events without verifying they belong to the active persistent
session, causing events from different sessions to be mixed together. Modify the
event handling in the block where you check for Network.requestWillBeSent,
Network.responseReceived, Network.loadingFinished, Network.loadingFailed,
Runtime.consoleAPICalled, and Runtime.exceptionThrown to extract the sessionId
from the incoming event and compare it against the sessionId stored in
self.persistent_session before appending to self.network_events or
self.console_events. Only push events to these buffers if the sessionIds match.
- Around line 166-172: The Network.enable and Runtime.enable calls for
persistent session setup are being silently ignored with let _ = discards, which
means failures go undetected and the session is still marked as persistent even
when critical domain initialization fails. Remove the let _ = pattern and
instead check the results of both send_to_target calls for "Network.enable" and
"Runtime.enable" - either propagate any errors returned or at minimum log them
and prevent the session from being marked as persistent if either call fails.
This ensures downstream collectors cannot skip per-command enables and miss
events due to uninitialized domains.
In `@src/commands/emulation.rs`:
- Around line 102-114: The allow_url processing block removes patterns from
client.blocklist but never adds them to client.allowlist, so allowed rules are
not persisted as daemon state. When iterating through params.allow_url, add each
pattern to client.allowlist to ensure the allow rules are durably stored and
properly included in status output. Apply the same fix to any other location
where allow_url patterns are processed (such as the similar code block around
line 193-207).
In `@src/commands/executor.rs`:
- Around line 412-457: The executor uses inconsistent default values for the
duration parameter compared to the CLI. In both the "console" and "network"
command handlers, change the unwrap_or(3000) calls to unwrap_or(0) to match the
CLI defaults defined in src/lib.rs, ensuring that direct daemon callers sending
raw JSON requests without a duration field get 0ms (instant drain mode) instead
of 3000ms, which aligns with the documented behavior that omitting the duration
field means "drain instantly".
In `@src/commands/sw_logs.rs`:
- Around line 68-79: The code processes Runtime events for all sessionIds
without filtering out events that don't belong to attached service-worker
sessions. Add a validation check immediately after determining the source
variable (after the find operation on the sessions iterator) to skip processing
events where the sessionId is not found in the attached sessions. Specifically,
if source resolves to "unknown" (indicating the sessionId was not matched),
continue to the next event iteration to prevent unrelated page/session logs from
being included in the sw-logs output.
🪄 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: af8ddc19-cdd3-4738-88ef-c3cb09704232
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (17)
Cargo.tomlskill/chrome-devtools/SKILL.mdsrc/cdp.rssrc/commands/console.rssrc/commands/emulation.rssrc/commands/evaluate.rssrc/commands/executor.rssrc/commands/mod.rssrc/commands/network.rssrc/commands/pages.rssrc/commands/snapshot.rssrc/commands/sw_logs.rssrc/commands/third_party.rssrc/format.rssrc/lib.rssrc/protocol.rstests/executor_tests.rs
Correctness - cdp::push_event now gates persistent-buffer routing on the event's sessionId matching the persistent page session, so sw-logs and ad-hoc live sessions no longer leak into the page's console_events / network_events buffers. - sw_logs now filters out events that don't match an attached service worker session, preventing page/other-session events from appearing as SW logs. Cleanup (URL blocking) - Removed the dead `allowlist` field on CdpClient, the `--clear-allows` flag, and the unreachable "Allowed URLs:" display branch. The doc comments claimed allowlist semantics that were never implemented; --allow-url already just removed entries from the blocklist. - Renamed --allow-url to --unblock-url to match its actual semantics (and reword --block-url's help to no longer reference --allow-url). Robustness - sw-logs / console / network now capture read_events_for's result, run session detach / Runtime.disable / Network.disable cleanup, and only then `?`-propagate the error. read_events_for is currently infallible, so this is dead-code hygiene — but removes a latent trap the moment it gains a fallible path. - kill-daemon now uses libc::kill(SIGTERM) directly instead of shelling out to /usr/bin/kill, and distinguishes ESRCH (stale PID file) from real signalling failures. Tests (19 → 31 tests) - format::from_flags precedence (toon wins over json) - DaemonRequest::format() precedence (output_format wins over legacy json_output bool) - format_structured round-trips Json / Toon / Text - process_console_events: arg joining, description fallback, exception mapping, type-filter include/exclude, `error` filter includes exceptions, ignores unknown methods - process_network_events: request+response merge by requestId, loadingFailed status, type filter, URL sort, ignores unknown methods, orphan response dropped - extract_extension_id: with path, no path, trailing slash, non-extension URL, empty string, prefix-only edge case Misc - Executor duration fallback for console/network changed 3000 → 0 so hand-crafted / malformed requests default to drain mode consistent with the CLI's documented contract. - Console/Network --duration help clarified: events collected in live mode remain available to a later drain. - skill/chrome-devtools/SKILL.md updated for --unblock-url rename.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces significant enhancements to the Chrome DevTools CLI, including a persistent CDP session for continuous Network and Console event collection, new commands (console, network, sw-logs, and kill-daemon), network URL blocking capabilities, and support for the compact TOON output format. The review feedback highlights critical issues regarding duplicate event delivery in live mode, a failure of URL blocking in direct/fallback mode, and opportunities to make the code more idiomatic and efficient by avoiding unnecessary cloning of session variables.
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 |
There was a problem hiding this comment.
Code Review
This pull request enhances the Chrome DevTools CLI by introducing persistent CDP sessions for continuous network and console event collection, adding new commands (console, network, sw-logs, and kill-daemon), implementing URL blocking, and supporting a compact TOON output format. The code review identified several important issues to address: redundant subcommand arguments shadowing global flags, a potential CDP session leak if initialization fails, fragile parsing of non-string primitive values in console and service worker logs, event buffer pollution from service worker logs, and premature deletion of socket/PID files when daemon termination fails due to permission 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.
…k-url flags Page-level commands previously attached a throwaway per-command CDP session while blocklist/emulation state lived on the long-lived persistent session, so that state never governed the commands that actually ran. Unify on the persistent session (falling back to a per-command attach only when it is unavailable), and never detach the persistent session during cleanup. This fixes `emulate --clear-viewport` / `--clear-geolocation` being silent no-ops: the override was set on one short-lived session and the clear ran on a different one. Set and clear now run on the same session, so clearing works. It also lets `--block-url` govern the page's subresource requests during navigation. Also dedupe the network flags. `--block-url` / `--unblock-url` were declared both as global flags and as `emulate` subcommand flags, which shadowed each other and forced a workaround in the executor. They are now defined once (global) and inherited by `emulate` via clap; the emulate handler still applies them itself, ordered before `--clear-blocks` (clear, then add), which is why the generic merge keeps skipping "emulate". cdp: store the persistent session id before enabling Network/Runtime so events arriving during enable are routed to the persistent buffers; reset the session id and detach if either enable fails. docs: - Document that `--block-url` blocks subresources only (images, scripts, fetch/XHR, CDN, trackers) and not top-level navigations — a Chrome `Network.setBlockedURLs` limitation, not a CLI bug. - Add README reference for console/network/sw-logs/kill-daemon. - Correct the live-mode description: `--duration` consumes events, so they do not reappear on a later drain. Verified against live (headless) Chrome on google.com: navigate, evaluate, snapshot, network, console, fill, screenshot, dialog handling, subresource blocking, and the `--clear-viewport` fix (756 -> 800 -> 756). cargo build clean, cargo test 31 passed.
|
/gemini review |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request introduces persistent CDP sessions to continuously collect network and console events, adds new commands (console, network, sw-logs, and kill-daemon), and implements URL blocking capabilities along with a compact TOON output format for LLM agents. The reviewer feedback highlights critical issues with URL blocking in direct/fallback mode, where the blocklist is silently ignored when a persistent session is unavailable. To resolve this, the reviewer suggests refactoring apply_network_rules to accept an optional session ID and applying it during fallback session attachment. Additionally, the reviewer recommends preserving the chronological order of network requests instead of sorting them alphabetically by URL to improve sequence analysis.
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: 3
♻️ Duplicate comments (1)
src/cdp.rs (1)
591-601:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t treat WebSocket read failures as normal timeout completion.
At Line 600,
Ok(Err(_)) | Err(_) => breakmasks transport errors and returns partial results as success.Proposed fix
- Ok(Err(_)) | Err(_) => break, + Ok(Err(e)) => return Err(e), + Err(_) => break,🤖 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/cdp.rs` around lines 591 - 601, The current match statement at the timeout call site is masking transport errors by treating both timeout expiration (Err(_)) and read failures (Ok(Err(_))) identically by just breaking out of the loop. Refactor the match statement to distinguish between these two cases: when the timeout expires (Err(_)), you can safely break to indicate normal timeout completion; however, when read_text() actually fails (Ok(Err(_))), this represents a real transport error that should be propagated or logged as an error rather than silently returning partial results as success. This ensures that legitimate WebSocket failures are not hidden from the caller.
🤖 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/cdp.rs`:
- Around line 203-211: The `apply_network_rules_internal` method is currently
discarding the result of the `send_to_target` call using `let _ = `, which means
failures to apply the Network.setBlockedURLs command to Chrome are silently
ignored and the caller cannot detect failures. Remove the `let _ = ` discard
pattern and instead propagate or handle the error result appropriately, either
by returning the Result type or checking and logging failures. Apply this same
fix to the other similar location mentioned at lines 189-193 where network rule
application also discards results.
In `@src/lib.rs`:
- Around line 685-689: The direct fallback mode for page navigation is
hardcoding empty vectors for block_url and unblock_url instead of using the
global blocklist configuration, causing command-line flags like --block-url and
--unblock-url to be silently ignored. Replace the Vec::new() calls with the
appropriate values from the global blocklist configuration so that the direct
fallback mode respects the same URL filtering rules as daemon mode. This issue
occurs at two locations: around line 685 (for new-page fallback) and around line
787 (for navigate fallback), both of which need to be updated consistently to
use the actual blocklist values instead of empty vectors.
- Around line 556-567: The PID is parsed as u32 but then cast to libc::pid_t (a
signed integer type) before being passed to the libc::kill function. On systems
where pid_t is i32, u32 values exceeding i32::MAX will wrap to negative numbers,
causing unexpected behavior. Add a validation check after parsing the PID string
into the pid variable to ensure the parsed value does not exceed the maximum
value representable by libc::pid_t (typically i32::MAX). If the PID is out of
range, return an error similar to the existing invalid PID error handling. This
validation should occur before the unsafe call to libc::kill to prevent
incorrect signal delivery.
---
Duplicate comments:
In `@src/cdp.rs`:
- Around line 591-601: The current match statement at the timeout call site is
masking transport errors by treating both timeout expiration (Err(_)) and read
failures (Ok(Err(_))) identically by just breaking out of the loop. Refactor the
match statement to distinguish between these two cases: when the timeout expires
(Err(_)), you can safely break to indicate normal timeout completion; however,
when read_text() actually fails (Ok(Err(_))), this represents a real transport
error that should be propagated or logged as an error rather than silently
returning partial results as success. This ensures that legitimate WebSocket
failures are not hidden from the caller.
🪄 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: 7f330c19-8245-4c64-a916-c502a65b96fd
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
Cargo.tomlREADME.mdskill/chrome-devtools/SKILL.mdsrc/cdp.rssrc/commands/console.rssrc/commands/emulation.rssrc/commands/evaluate.rssrc/commands/executor.rssrc/commands/input.rssrc/commands/mod.rssrc/commands/navigate.rssrc/commands/network.rssrc/commands/pages.rssrc/commands/snapshot.rssrc/commands/sw_logs.rssrc/commands/third_party.rssrc/format.rssrc/lib.rssrc/protocol.rstests/executor_tests.rstests/telemetry_tests.rs
…onologically Executor - When the persistent CDP session is unavailable (failed to attach or lost), the fallback fresh session now re-applies `Network.setBlockedURLs` using the daemon's blocklist. Without this, `--block-url` was silently ignored on the degraded path because ensure_persistent_session is what normally calls setBlockedURLs. - `apply_network_rules_internal` promoted to pub(crate) so executor.rs can call it on the fresh session. Network command - Drop the alphabetical-by-URL sort on `network` output. Events now appear in first-seen (chronological) order, with redirects reusing the existing requestId in place rather than being repositioned. - Test `test_process_network_events_sorts_by_url` renamed to `test_process_network_events_preserves_chronological_order` and expectations flipped (z.example.com before a.example.com).
…ndling Network rule application used to swallow failures. `apply_network_rules` and `apply_network_rules_internal` now return `Result` and propagate `Network.setBlockedURLs` errors; the callers (`emulate`, the executor's global block-flag handler, and the fallback-session apply) propagate with `?` instead of discarding the result, so a failed blocklist update surfaces to the user. `read_events_for` no longer treats a real WebSocket read error the same as the collection-window timeout. A timeout is the normal completion path; an `Ok(Err(_))` from `read_text()` is a transport failure and is now logged as a warning before stopping, so a partial result isn't silently presented as a clean end-of-window. `kill-daemon` now validates that the PID from the PID file fits in `libc::pid_t` (i32) before signaling. A corrupted PID file could otherwise wrap to a negative value via `as`, and `kill(-pid)` signals an entire process group — potentially hitting unrelated processes. The guard lives inside `#[cfg(unix)]` so non-unix builds stay warning-clean. Also document, via comments, that direct-mode `new-page` / `navigate` intentionally do not carry the URL blocklist: blocking is daemon-only state and there is no persistent session to apply it to in the one-shot direct path.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request significantly expands the Chrome DevTools CLI by introducing a persistent CDP session in the background daemon for continuous event tracking. It adds new commands to drain and live-monitor console logs (console), network requests (network), and extension service worker logs (sw-logs), alongside a kill-daemon command. It also implements URL blocking capabilities and introduces a compact, LLM-friendly TOON output format. The review feedback identifies that while the documentation states that running emulate with no flags displays all active overrides, the current implementation only shows blocked URLs. The reviewer provides actionable suggestions to store and display active viewport and geolocation overrides within the persistent CdpClient session.
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.
…tion per tab `emulate` with no flags is documented to show all active overrides, but only the blocklist was tracked — viewport and geolocation weren't stored, so they could never be displayed. Store them as structured overrides (`ViewportOverride`, `GeolocationOverride`) on the client: set on apply, reset on `--clear-viewport` / `--clear-geolocation` / `--clear-all`, and render alongside the blocklist in the no-flags output. Make all emulation (viewport, geolocation, URL blocklist) **per-tab** instead of a single daemon-wide value. The active tab's state lives in the `blocklist`/`viewport`/`geolocation` fields; `ensure_persistent_session` saves that state under the outgoing target id and restores the incoming target's saved state (or empty) on every target switch, then applies it to the new session. Each tab keeps its own overrides — they persist across navigation within a tab and don't leak to other tabs, so e.g. a mobile viewport with images blocked on one tab and a desktop baseline on another can be held at the same time. State is kept in exactly one place per tab (active fields when current, the saved map otherwise); restore removes from the map and save writes the leaving tab's own state, so there's no duplication. - Re-apply viewport/geolocation on target switch via `apply_emulation_internal`, mirroring the existing blocklist re-apply, so overrides survive switches. - `close-page` calls `forget_target` to drop a closed tab's saved state (and reset the active fields + invalid persistent session if it was the active tab). - Only save tabs that actually have overrides, so the saved-state map doesn't accumulate an empty entry for every tab visited. Docs (README, SKILL) updated from daemon-wide to per-tab semantics. Verified on live Chrome: tab A held 375px + `*.png` blocked (fetch blocked) while tab B held 1440px + no block (fetch OK), concurrently, while interleaving commands; `--clear-*` removes the right override; target_id is stable across cross-origin navigation so a tab's state stays attached to it. cargo build clean, cargo test 31 passed.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces persistent CDP sessions to continuously collect network and console events, adding new commands (console, network, sw-logs, and kill-daemon) and per-tab emulation state isolation (viewport, geolocation, and URL blocking). It also adds support for the compact TOON output format. Feedback focuses on a security vulnerability in kill-daemon where a PID of 0 could terminate the entire process group, and performance optimizations in src/cdp.rs to replace Vec with VecDeque for more efficient event buffer capping.
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.
Code Review
This pull request significantly enhances the Chrome DevTools CLI by introducing persistent CDP sessions in the daemon for continuous event collection, adding new commands (console, network, sw-logs, and kill-daemon), implementing per-tab emulation and URL blocking isolation, and adding a compact --toon output format for LLM agents. The review feedback correctly identifies a critical inconsistency in direct mode (run_direct), where global URL blocking flags are not applied to all page-level commands (like Click or Fill), potentially allowing triggered network requests to bypass the blocklist. Implementing the suggested fix to apply the blocklist across all non-emulation page-level commands in direct mode will ensure behavioral consistency with daemon mode.
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 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a persistent CDP session to continuously collect network and console events, enabling per-tab emulation isolation (viewport, geolocation, and URL blocking) and new inspection commands (console, network, sw-logs, and kill-daemon). It also adds support for TOON, a compact output format designed for LLM agents. The review feedback highlights two important issues in the fallback/direct modes: first, event duplication occurs during subsequent drains because Network and Runtime events are incorrectly pushed to the generic event buffer; second, active viewport and geolocation overrides are lost in the degraded executor path because only the network blocklist is re-applied.
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 enhance session management
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces persistent CDP sessions to enable continuous background collection of network and console events, per-tab emulation isolation, and URL blocking. It adds new commands (console, network, sw-logs, and kill-daemon) and supports a new compact output format (TOON) tailored for LLM agents. Feedback on the changes highlights two issues in src/cdp.rs: first, in direct mode, the Network domain must be explicitly enabled before applying URL blocking rules; second, in read_events_for, ad-hoc session events can leak into the general events buffer when a persistent session is active, requiring stricter filtering based on the session ID.
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.
…f events in CdpClient
|
@coderabbitai full review |
|
/gemini review |
✅ Action performedFull review finished. |
|
Warning Gemini encountered an error creating the review. You can try again by commenting |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/commands/evaluate.rs (1)
54-77:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep non-text
evaluateoutput machine-parseable.The DOM-traversal hint is appended even in Json/Toon modes, which turns structured output into mixed text and breaks downstream parsers.
💡 Suggested fix
- if expression.contains("querySelector") - || expression.contains("document.body") - || expression.contains("getElementById") - || expression.contains("getElementsBy") - { + if format.is_text() + && (expression.contains("querySelector") + || expression.contains("document.body") + || expression.contains("getElementById") + || expression.contains("getElementsBy")) + { output_hint.push_str("\n\n[HINT: Avoid using `evaluate` for DOM traversal. Use the `snapshot` command to get a clean accessibility tree of the page, then use `click` or `fill`.]"); }🤖 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 54 - 77, The DOM-traversal hint is being unconditionally appended to output_hint after the format check block, which causes text hints to be added to structured JSON and Toon output, breaking machine-parseable formats. Move the entire hint-appending block (the condition checking for "querySelector", "document.body", "getElementById", "getElementsBy" and the output_hint.push_str call) inside the `if format.is_text()` block so the hint is only appended for text-format output.
♻️ Duplicate comments (1)
skill/chrome-devtools/SKILL.md (1)
35-40:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd language tag to unlabeled fenced code block (MD040).
The output example block is missing a language identifier on the opening fence.
🔧 Proposed fix
**Step 1: Run `list-pages` to see what's open and get target names** ```bash chrome-devtools list-pagesOutput:
-+text
[0] (warm-squid) Your Repositories — https://github.com/aeroxy
[1] (pink-hen) Gmail — https://mail.google.com
[2] (hazy-vole) Example — https://example.com🤖 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 `@skill/chrome-devtools/SKILL.md` around lines 35 - 40, The output example block in the SKILL.md file has a fenced code block that is missing a language identifier, which violates markdown linting rule MD040. Locate the output example section showing the three repository links (warm-squid, pink-hen, hazy-vole) and add the language tag "text" to the opening fence of that code block by changing the opening ``` to ```text.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In `@src/commands/evaluate.rs`:
- Around line 54-77: The DOM-traversal hint is being unconditionally appended to
output_hint after the format check block, which causes text hints to be added to
structured JSON and Toon output, breaking machine-parseable formats. Move the
entire hint-appending block (the condition checking for "querySelector",
"document.body", "getElementById", "getElementsBy" and the output_hint.push_str
call) inside the `if format.is_text()` block so the hint is only appended for
text-format output.
---
Duplicate comments:
In `@skill/chrome-devtools/SKILL.md`:
- Around line 35-40: The output example block in the SKILL.md file has a fenced
code block that is missing a language identifier, which violates markdown
linting rule MD040. Locate the output example section showing the three
repository links (warm-squid, pink-hen, hazy-vole) and add the language tag
"text" to the opening fence of that code block by changing the opening ``` to
```text.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4210a06d-a6af-4b94-99da-57b2b9958eda
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
Cargo.tomlREADME.mdskill/chrome-devtools/SKILL.mdsrc/cdp.rssrc/commands/console.rssrc/commands/emulation.rssrc/commands/evaluate.rssrc/commands/executor.rssrc/commands/input.rssrc/commands/mod.rssrc/commands/navigate.rssrc/commands/network.rssrc/commands/pages.rssrc/commands/snapshot.rssrc/commands/sw_logs.rssrc/commands/third_party.rssrc/format.rssrc/lib.rssrc/protocol.rstests/executor_tests.rstests/telemetry_tests.rs
|
/gemini review |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Code Review
This pull request enhances the Chrome DevTools CLI by introducing persistent CDP sessions for continuous console and network event collection, per-tab emulation and URL blocking isolation, extension service worker log collection (sw-logs), a kill-daemon command, and a compact TOON output format. The reviewer pointed out an issue in src/cdp.rs where setup results are evaluated within an array literal, which prevents proper short-circuiting on failure. Handling these sequentially was recommended to avoid executing subsequent setup steps if an earlier one fails.
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: 2
🤖 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/cdp.rs`:
- Around line 272-284: The network and emulation rules are being applied
concurrently via array evaluation, which means if the network rules fail, the
emulation is still applied, leaving the target partially configured. Replace the
array literal containing both await calls with sequential if let Err blocks.
First, await the result of apply_network_rules_internal and check for errors
with an if let Err block that performs the cleanup and returns. Then, await
apply_emulation_internal and perform the same error handling in a separate if
let Err block. This ensures that if network rules fail, emulation rules are
never applied, preventing partial configuration state.
In `@src/commands/executor.rs`:
- Around line 228-240: The block/unblock URL logic currently applies to the
navigate command through the generic merge before emulation, but navigate
emulation can clear these rules before navigation because empty blocklist fields
are passed to EmulateParams. Modify the code to skip the generic block/unblock
merge (the conditional block checking cmd != "emulate" && cmd != "close-page" &&
cmd != "select-page") for the navigate command as well, and instead pass
req.block_url and req.unblock_url into the navigate EmulateParams structure.
This ensures block/unblock rules are applied during the navigate emulation phase
with the correct clear/apply ordering, matching the behavior of the emulate
command.
🪄 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: bc4cec51-0b19-44a9-9a53-714bdd6c5aeb
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
Cargo.tomlREADME.mdskill/chrome-devtools/SKILL.mdsrc/cdp.rssrc/commands/console.rssrc/commands/emulation.rssrc/commands/evaluate.rssrc/commands/executor.rssrc/commands/input.rssrc/commands/mod.rssrc/commands/navigate.rssrc/commands/network.rssrc/commands/pages.rssrc/commands/snapshot.rssrc/commands/sw_logs.rssrc/commands/third_party.rssrc/format.rssrc/lib.rssrc/protocol.rstests/executor_tests.rstests/telemetry_tests.rs
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a persistent CDP session architecture to the background daemon, enabling continuous background collection of network and console events. It adds several new inspection commands (console, network, sw-logs) to drain or live-monitor these events, alongside a kill-daemon command for clean daemon shutdown. Additionally, it implements per-tab emulation isolation (viewport, geolocation, and URL blocking via --block-url/--unblock-url) and introduces structured output formatting options, including JSON and TOON (Token-Oriented Object Notation). No review comments were provided, so there is no additional feedback to address.
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.
1 participant
console,network, andsw-logscommands for inspecting page console messages, network requests, and extension service worker logs.console/networkdrain on demand instead of using blocking --duration windows.emulate(+ as global flags) for URL blocking via Network.setBlockedURLs; patterns persist in the daemon.kill-daemonsubcommand for stopping the background daemon cleanly.Summary by CodeRabbit
Release Notes
console,network, andsw-logscommands with optional type filtering and duration-based live collection (plus quick drain mode).kill-daemonand persistent CDP event buffering to reuse inspection state across page targets.--block-url/--unblock-urland clearing options (--clear-blocks/--clear-all).--json/--toonacross key commands.