refactor: try to reland inspector changes

[bartlomieju]

For denoland/deno_core#1253

[coderabbitai]

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request patches the deno_core dependency to use a git source instead of the crates.io registry in Cargo.toml. In runtime/inspector_server.rs, the inspector session handling is refactored to replace per-session channel plumbing with a shared Arc-wrapped InspectorIoDelegate. The InspectorInfo struct is updated to expose an io_delegate field instead of separate new_session_tx and deregister_rx fields. The constructor signature for InspectorInfo::new is simplified accordingly. Session creation and deregistration now use the shared io_delegate instead of individual channel mechanisms.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main refactoring work: relanding inspector changes with updated architecture for session and deregistration handling.
Description check	✅ Passed	The description references the related deno_core PR, providing context for why this change is needed, though minimal detail is given.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

runtime/inspector_server.rs (1)

258-261: Address or track the TODO comment.

The deregister logic works but the TODO indicates room for simplification. Consider either refactoring this now or opening an issue to track the improvement.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 99a8dbf and 0d5148a.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• Cargo.toml (1 hunks)
• runtime/inspector_server.rs (10 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.rs

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.rs: For debugging Rust code, set breakpoints in IDE debuggers (VS Code with rust-analyzer, IntelliJ IDEA) or use lldb directly
Use eprintln!() or dbg!() macros for debug prints in Rust code

Files:

• runtime/inspector_server.rs

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)

• GitHub Check: test release linux-x86_64
• GitHub Check: test debug linux-aarch64
• GitHub Check: test debug macos-aarch64
• GitHub Check: test debug macos-x86_64
• GitHub Check: test debug linux-x86_64
• GitHub Check: test debug windows-x86_64
• GitHub Check: test release macos-x86_64
• GitHub Check: build libs
• GitHub Check: lint debug linux-x86_64
• GitHub Check: lint debug windows-x86_64
• GitHub Check: lint debug macos-x86_64

🔇 Additional comments (8)

runtime/inspector_server.rs (7)

11-11: LGTM!

The new imports support the refactored inspector design using shared InspectorIoDelegate.

Also applies to: 15-15

---

105-108: LGTM!

The refactored registration flow cleanly creates the io_delegate and passes it to InspectorInfo. This simplifies the API by replacing separate channel plumbing.

---

147-159: LGTM!

The scoped borrow and Arc cloning properly manage lifetimes and enable concurrent session handling.

---

193-193: Good observability improvement.

The new log message helps track session lifecycle.

---

284-285: LGTM!

The _fut suffix improves readability by making it explicit that these are futures being polled in the select macro.

Also applies to: 288-288, 298-298

---

448-448: LGTM!

The InspectorInfo refactoring simplifies the API by replacing multiple channel fields with a single Arc-wrapped delegate.

Also applies to: 456-456, 464-464

---

190-200: Verify error handling for new_remote_session.

The call to io_delegate.new_remote_session doesn't have explicit error handling. Confirm whether this method can fail and if so, whether errors should be logged or propagated.

Cargo.toml (1)

535-536: Confirm this git patch is temporary before merge.

This Cargo.toml patch overrides deno_core with a personal fork (bartlomieju/deno_core.git on branch reland_inspector). Ensure this is explicitly documented as temporary for testing inspector changes and will be reverted to the upstream version before merging to main.