chore(rust): replace inline_module_block allows with file-backed modules - #387
Merged
Conversation
PR SummaryLow Risk Overview NATS subject layout ( Other extractions: Subject strings and stream assignments stay the same; this is import-path and module-structure cleanup, not wire-format changes. Reviewed by Cursor Bugbot for commit 162e7fd. Bugbot is set up for automated code reviews on this repo. Configure here. |
WalkthroughReplaces legacy Changesacp-nats Subject Module Reorganization
mcp-nats Subject Module Reorganization
Inline Module Extractions
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
|
Code Coverage SummaryDetailsDiff against mainResults for commit: 162e7fd Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
…paths Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
force-pushed
the
yordis/fix-inline-module-block
branch
from
June 23, 2026 03:14
4724e79 to
162e7fd
Compare
yordis
added
the
rust:coverage-baseline-reset
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rsworkspace/crates/mcp-nats/src/nats/subjects/mod.rs (1)
171-179: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd symmetric coverage for
OneServerSubject.This wildcard contract now feeds the server transport path too, but the test only pins
OneClientSubject. Add the matching server-specific assertion so both one-peer subscription subjects are covered.Proposed test addition
assert_eq!( subscriptions::OneClientSubject::new(&p("mcp"), &peer("desktop")).to_string(), "mcp.client.desktop.>" ); + assert_eq!( + subscriptions::OneServerSubject::new(&p("mcp"), &peer("filesystem")).to_string(), + "mcp.server.filesystem.>" + );🤖 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 `@rsworkspace/crates/mcp-nats/src/nats/subjects/mod.rs` around lines 171 - 179, The test currently verifies subject string representations for AllServerSubject, AllClientSubject, and OneClientSubject but lacks the corresponding assertion for OneServerSubject. Add an assert_eq! statement after the OneClientSubject assertion that verifies OneServerSubject::new with the same parameters (mcp prefix and desktop peer) produces the expected server-specific subject string to achieve symmetric test coverage for both one-peer subscription subjects.
🤖 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 `@rsworkspace/crates/mcp-nats/src/lib.rs`:
- Around line 25-26: Before releasing this version, create or update the release
notes to document the breaking change of removing the crate-level mcp_client and
mcp_server re-exports. Include a clear migration guide that shows external
consumers how to update their imports from the old re-export pattern to the new
nats::subjects::{client, server, subscriptions} module structure, with examples
of the old import paths and their new equivalents.
---
Nitpick comments:
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/mod.rs`:
- Around line 171-179: The test currently verifies subject string
representations for AllServerSubject, AllClientSubject, and OneClientSubject but
lacks the corresponding assertion for OneServerSubject. Add an assert_eq!
statement after the OneClientSubject assertion that verifies
OneServerSubject::new with the same parameters (mcp prefix and desktop peer)
produces the expected server-specific subject string to achieve symmetric test
coverage for both one-peer subscription subjects.
🪄 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
Run ID: 92edec5a-2c43-4b00-9784-f633e010b61a
📒 Files selected for processing (31)
rsworkspace/crates/acp-nats-agent/src/connection.rsrsworkspace/crates/acp-nats/src/agent/authenticate.rsrsworkspace/crates/acp-nats/src/agent/bridge.rsrsworkspace/crates/acp-nats/src/agent/cancel.rsrsworkspace/crates/acp-nats/src/agent/close_session.rsrsworkspace/crates/acp-nats/src/agent/ext_method.rsrsworkspace/crates/acp-nats/src/agent/ext_notification.rsrsworkspace/crates/acp-nats/src/agent/fork_session.rsrsworkspace/crates/acp-nats/src/agent/initialize.rsrsworkspace/crates/acp-nats/src/agent/list_sessions.rsrsworkspace/crates/acp-nats/src/agent/load_session.rsrsworkspace/crates/acp-nats/src/agent/logout.rsrsworkspace/crates/acp-nats/src/agent/new_session.rsrsworkspace/crates/acp-nats/src/agent/prompt.rsrsworkspace/crates/acp-nats/src/agent/resume_session.rsrsworkspace/crates/acp-nats/src/agent/set_session_config_option.rsrsworkspace/crates/acp-nats/src/agent/set_session_mode.rsrsworkspace/crates/acp-nats/src/agent/set_session_model.rsrsworkspace/crates/acp-nats/src/client/mod.rsrsworkspace/crates/acp-nats/src/client_proxy.rsrsworkspace/crates/acp-nats/src/lib.rsrsworkspace/crates/acp-nats/src/nats/mod.rsrsworkspace/crates/acp-nats/src/nats/subjects/mod.rsrsworkspace/crates/mcp-nats/src/lib.rsrsworkspace/crates/mcp-nats/src/nats/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/mod.rsrsworkspace/crates/mcp-nats/src/transport.rsrsworkspace/crates/trogon-decider/src/testing.rsrsworkspace/crates/trogon-decider/src/testing/private.rsrsworkspace/crates/trogon-scheduler/src/processor/execution/checkpoints/codec.rsrsworkspace/crates/trogon-scheduler/src/processor/execution/checkpoints/codec/twin.rs
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
allow(inline_module_block)attributes were hiding intent and bypassing a policy lint that the codebase has agreed to enforce; converting them to proper file-backed modules makes the layout match the rule rather than mute it.trogonai-proto::r#genis preserved because the inline modules there are emitted bybuffa-codegenand are not owned source.