feat(mcp-nats): support distributed MCP#149
Merged
Merged
Conversation
Member
yordis
commented
May 4, 2026
yordis
commented
- MCP traffic needs the same distributed transport option ACP already has for remote backends.
- Shared NATS conventions make the protocol bridges easier to operate and reason about together.
- SDK-aligned boundaries reduce protocol drift as MCP evolves.
PR SummaryMedium Risk Overview Introduces two new binaries: Tightens a few supporting pieces: Reviewed by Cursor Bugbot for commit 3b520b7. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
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:
WalkthroughAdds three new workspace crates (mcp-nats, mcp-nats-server, mcp-nats-stdio) implementing MCP-over-NATS transport, typed NATS subject system, NATS transport/telemetry/helpers, stdio↔NATS bridge and HTTP↔NATS server, plus manifests, docs, mocks, and extensive tests and config wiring. MCP NATS Transport and Stdio/HTTP Bridges
sequenceDiagram
participant Local as Local MCP Client (stdio)
participant Bridge as mcp-nats-stdio Bridge
participant NATS as NATS Broker
participant Remote as Remote MCP Server
Local->>Bridge: JSON-RPC Request (stdin)
Bridge->>Bridge: map method -> NATS subject
Bridge->>NATS: request_with_timeout / publish
NATS->>Remote: deliver request
Remote->>NATS: publish response (reply subject)
NATS->>Bridge: deliver response
Bridge->>Local: JSON-RPC Response (stdout)
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
✨ Finishing Touches🧪 Generate unit tests (beta)
|
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
99f547f to
70450e5
Compare
![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
🧹 Nitpick comments (3)
rsworkspace/crates/mcp-nats/src/nats/subjects/server.rs (1)
3-55: ⚡ Quick winSame macro-duplication issue as
client.rs— merge into a single parameterized macro.
server_request_subject!andserver_notification_subject!are identical except for the marker trait. Apply the same$marker:identpattern described in theclient.rscomment.🤖 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/server.rs` around lines 3 - 55, The two macros server_request_subject! and server_notification_subject! are duplicated; replace them with a single parameterized macro (e.g., server_subject!) that takes an additional $marker:ident parameter and uses it to implement the correct marker trait (impl super::markers::$marker for $name {}), keeping the existing struct, new(), and Display implementation unchanged; update invocations to pass the appropriate marker (Requestable or Publishable) so server_request_subject and server_notification_subject behavior is preserved via the unified macro.rsworkspace/crates/mcp-nats/src/nats/subjects/client.rs (1)
3-55: ⚡ Quick winMerge the two macros — they differ only in the marker trait.
client_request_subject!andclient_notification_subject!expand to identical bodies; the sole difference isRequestablevsPublishable. A single parameterized macro eliminates the duplicated maintenance surface.♻️ Proposed refactor
-macro_rules! client_request_subject { - ($name:ident, $suffix:literal) => { - #[derive(Debug, Clone)] - pub struct $name { - prefix: McpPrefix, - client_id: McpPeerId, - } - - impl $name { - pub fn new(prefix: &McpPrefix, client_id: &McpPeerId) -> Self { - Self { - prefix: prefix.clone(), - client_id: client_id.clone(), - } - } - } - - impl std::fmt::Display for $name { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}.client.{}.{}", self.prefix, self.client_id, $suffix) - } - } - - impl super::markers::Requestable for $name {} - }; -} - -macro_rules! client_notification_subject { - ($name:ident, $suffix:literal) => { - #[derive(Debug, Clone)] - pub struct $name { - prefix: McpPrefix, - client_id: McpPeerId, - } - - impl $name { - pub fn new(prefix: &McpPrefix, client_id: &McpPeerId) -> Self { - Self { - prefix: prefix.clone(), - client_id: client_id.clone(), - } - } - } - - impl std::fmt::Display for $name { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "{}.client.{}.{}", self.prefix, self.client_id, $suffix) - } - } - - impl super::markers::Publishable for $name {} - }; -} - -client_request_subject!(PingSubject, "ping"); -client_request_subject!(CreateMessageSubject, "sampling.create_message"); -client_request_subject!(ListRootsSubject, "roots.list"); -client_request_subject!(CreateElicitationSubject, "elicitation.create"); - -client_notification_subject!(CancelledSubject, "notifications.cancelled"); -client_notification_subject!(ProgressSubject, "notifications.progress"); -client_notification_subject!(InitializedSubject, "notifications.initialized"); -client_notification_subject!(RootsListChangedSubject, "notifications.roots.list_changed"); +macro_rules! client_subject { + ($name:ident, $suffix:literal, $marker:ident) => { + #[derive(Debug, Clone)] + pub struct $name { + prefix: McpPrefix, + client_id: McpPeerId, + } + + impl $name { + pub fn new(prefix: &McpPrefix, client_id: &McpPeerId) -> Self { + Self { + prefix: prefix.clone(), + client_id: client_id.clone(), + } + } + } + + impl std::fmt::Display for $name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}.client.{}.{}", self.prefix, self.client_id, $suffix) + } + } + + impl super::markers::$marker for $name {} + }; +} + +client_subject!(PingSubject, "ping", Requestable); +client_subject!(CreateMessageSubject, "sampling.create_message", Requestable); +client_subject!(ListRootsSubject, "roots.list", Requestable); +client_subject!(CreateElicitationSubject, "elicitation.create", Requestable); + +client_subject!(CancelledSubject, "notifications.cancelled", Publishable); +client_subject!(ProgressSubject, "notifications.progress", Publishable); +client_subject!(InitializedSubject, "notifications.initialized", Publishable); +client_subject!(RootsListChangedSubject, "notifications.roots.list_changed", Publishable);🤖 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/client.rs` around lines 3 - 55, client_request_subject! and client_notification_subject! are duplicated except for the marker trait; replace them with a single parameterized macro (e.g., client_subject!) that takes the struct name, suffix literal, and the marker trait identifier (or path) so it generates the same struct, new(), Display impl and then implements super::markers::YourMarker for the type; update existing invocations to pass Requestable or Publishable as the third argument and ensure the macro uses that identifier in the final impl block (impl super::markers::$marker for $name {}).rsworkspace/crates/mcp-nats/src/mcp_peer_id.rs (1)
20-20: ⚡ Quick winImplement
source()to expose the error chain.
impl std::error::Error for McpPeerIdError {}without asource()implementation prevents error chain traversal for tools likeanyhow/eyre. SinceSubjectTokenViolationimplementsstd::error::Errorand is already wrapped as a field, addingsource()is a minimal improvement that ensures error context is fully preserved.Proposed fix
-impl std::error::Error for McpPeerIdError {} +impl std::error::Error for McpPeerIdError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + Some(&self.0) + } +}🤖 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/mcp_peer_id.rs` at line 20, The current Error impl for McpPeerIdError lacks a source() method which prevents error chaining; update the impl std::error::Error for McpPeerIdError to override fn source(&self) -> Option<&(dyn std::error::Error + 'static)> and return the underlying error for the variant/field that wraps SubjectTokenViolation (e.g., match on McpPeerIdError::SubjectTokenViolation(inner) => Some(inner) and return None for other variants), ensuring the wrapped SubjectTokenViolation is exposed as the source.
🤖 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/Cargo.toml`:
- Line 37: The rmcp dependency in Cargo.toml is missing the transport-async-rw
feature required to use AsyncRwTransport; update the rmcp entry (the dependency
line that currently lists features ["client","server","transport-io"]) to
include "transport-async-rw" so the features list becomes
["client","server","transport-io","transport-async-rw"], ensuring
AsyncRwTransport in mcp-nats-stdio/src/main.rs can be compiled.
In `@rsworkspace/crates/mcp-nats-stdio/README.md`:
- Around line 38-39: Update the README statement about NATS authentication to
include a direct cross-reference to the trogon-nats docs: add a relative link to
the trogon-nats README (or the specific authentication section in workspace
docs) next to the sentence "NATS authentication is loaded through
`trogon-nats`..." so readers can immediately find the environment variable names
and setup instructions; reference the module name `trogon-nats` in the link text
so it's discoverable.
In `@rsworkspace/crates/mcp-nats/src/config.rs`:
- Around line 41-44: with_operation_timeout currently accepts any Duration,
allowing invalid (too-small) timeouts into Config; modify pub fn
with_operation_timeout(mut self, timeout: Duration) -> Self to validate the
input against the crate's MIN_TIMEOUT_SECS (or corresponding MIN_DURATION
constant) and enforce the invariant before assigning: if timeout < MIN_TIMEOUT
(or timeout.as_secs_f64() < MIN_TIMEOUT_SECS) set self.operation_timeout =
MIN_TIMEOUT instead (or return a Result with an error if you prefer an explicit
failure path), ensuring operation_timeout can never be set below the minimum;
update references to MIN_TIMEOUT_SECS and the operation_timeout field in Config
accordingly.
In `@rsworkspace/crates/mcp-nats/src/mcp_prefix.rs`:
- Around line 41-58: Add a unit test in the tests module that constructs a
string longer than 128 bytes and calls McpPrefix::new(...) expecting an
Err(McpPrefixError::SubjectTokenViolation(SubjectTokenViolation::TooLong)) (or
equivalent PartialEq match) to assert the TooLong branch; reference
McpPrefix::new, McpPrefixError, and SubjectTokenViolation::TooLong so the test
exercises the 128-byte limit path and verifies the Display/PartialEq behavior.
In `@rsworkspace/crates/mcp-nats/src/transport.rs`:
- Line 30: The code currently removes the RequestId -> reply subject mapping
from pending_replies inside reply_subject_for_response (and similar blocks in
the 123-165 and 230-238 ranges) before attempting serialization/publish/flush,
which causes MissingReplySubject on transient publish failures; change the logic
to lookup (peek/clone) the reply subject from pending_replies without removing
it, use the cloned subject for serialization/publish/flush, and only remove the
entry from pending_replies after the publish/flush succeeds (or on a final
non-retriable error); update functions like reply_subject_for_response and the
corresponding send/publish code paths to perform removal on success and preserve
the mapping on errors so retries can succeed.
- Around line 157-165: Notification and response/error branches in the
JsonRpcMessage match (the Notification arm and the Response/Error arm) call
publish_raw(...).await with no timeout, so a stalled NATS connection can hang
send(); wrap those await calls in the same operation timeout used by the request
path (use tokio::time::timeout with the existing operation_timeout or op_timeout
variable) and propagate a NatsTransportError on timeout; ensure you apply the
same pattern for the other occurrences noted (around lines 203–218), keeping
serialize_message(&item) and reply_subject logic intact and only wrapping the
publish_raw(...).await calls.
---
Nitpick comments:
In `@rsworkspace/crates/mcp-nats/src/mcp_peer_id.rs`:
- Line 20: The current Error impl for McpPeerIdError lacks a source() method
which prevents error chaining; update the impl std::error::Error for
McpPeerIdError to override fn source(&self) -> Option<&(dyn std::error::Error +
'static)> and return the underlying error for the variant/field that wraps
SubjectTokenViolation (e.g., match on
McpPeerIdError::SubjectTokenViolation(inner) => Some(inner) and return None for
other variants), ensuring the wrapped SubjectTokenViolation is exposed as the
source.
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/client.rs`:
- Around line 3-55: client_request_subject! and client_notification_subject! are
duplicated except for the marker trait; replace them with a single parameterized
macro (e.g., client_subject!) that takes the struct name, suffix literal, and
the marker trait identifier (or path) so it generates the same struct, new(),
Display impl and then implements super::markers::YourMarker for the type; update
existing invocations to pass Requestable or Publishable as the third argument
and ensure the macro uses that identifier in the final impl block (impl
super::markers::$marker for $name {}).
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/server.rs`:
- Around line 3-55: The two macros server_request_subject! and
server_notification_subject! are duplicated; replace them with a single
parameterized macro (e.g., server_subject!) that takes an additional
$marker:ident parameter and uses it to implement the correct marker trait (impl
super::markers::$marker for $name {}), keeping the existing struct, new(), and
Display implementation unchanged; update invocations to pass the appropriate
marker (Requestable or Publishable) so server_request_subject and
server_notification_subject behavior is preserved via the unified macro.
🪄 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: 63808272-0a22-4ab1-8350-843746997bb2
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (26)
rsworkspace/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/README.mdrsworkspace/crates/mcp-nats-stdio/src/config.rsrsworkspace/crates/mcp-nats-stdio/src/main.rsrsworkspace/crates/mcp-nats/Cargo.tomlrsworkspace/crates/mcp-nats/README.mdrsworkspace/crates/mcp-nats/src/client.rsrsworkspace/crates/mcp-nats/src/config.rsrsworkspace/crates/mcp-nats/src/constants.rsrsworkspace/crates/mcp-nats/src/jsonrpc.rsrsworkspace/crates/mcp-nats/src/lib.rsrsworkspace/crates/mcp-nats/src/mcp_peer_id.rsrsworkspace/crates/mcp-nats/src/mcp_prefix.rsrsworkspace/crates/mcp-nats/src/nats/mod.rsrsworkspace/crates/mcp-nats/src/nats/parsing.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/markers.rsrsworkspace/crates/mcp-nats/src/nats/subjects/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions.rsrsworkspace/crates/mcp-nats/src/server.rsrsworkspace/crates/mcp-nats/src/telemetry/mod.rsrsworkspace/crates/mcp-nats/src/telemetry/transport.rsrsworkspace/crates/mcp-nats/src/transport.rsrsworkspace/crates/mcp-nats/tests/transport.rs
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
70450e5 to
4577917
Compare
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
2 times, most recently
from
80e53bd to
81111a8
Compare
|
Code Coverage SummaryDetailsDiff against mainResults for commit: 3b520b7 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
81111a8 to
4fc854c
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (5)
rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rs (1)
14-14: 💤 Low valueUse
McpPrefix'sDisplayimplementation directly instead of callingas_str().
McpPrefix::as_str()is defined and the code compiles, but sinceMcpPrefiximplementsDisplay, simplify line 14 towrite!(f, "{}.client.>", self.prefix)to keep the formatting layer thin.🤖 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/subscriptions/all_client.rs` at line 14, Replace the explicit call to McpPrefix::as_str() with McpPrefix's Display so the formatter uses the type's Display impl: change the write! invocation that currently uses self.prefix.as_str() to use self.prefix directly (i.e., write!(f, "{}.client.>", self.prefix)), referencing the write! call and the self.prefix/McpPrefix symbols to locate the change.rsworkspace/crates/mcp-nats/src/nats/subjects/client/ping.rs (1)
1-23: ⚡ Quick winConsider a
macro_rules!to eliminate per-file boilerplate for client subjects.All 8 client subject types (
PingSubject,CreateMessageSubject,ListRootsSubject,CreateElicitationSubject,RootsListChangedSubject,CancelledSubject,InitializedSubject,ProgressSubject) are structurally identical — the only differences are the struct name, the format-string literal, and which marker trait is applied. Keeping each as a separate hand-written file means adding a new subject type requires creating a whole new file, and any future change to the shared pattern (e.g., derivingPartialEq, addingas_ref(), changing the constructor signature) must be applied in 8 places.A single
macro_rules!inmod.rsreduces all of this to one declaration site:♻️ Proposed refactor — define a shared subject macro
// In rsworkspace/crates/mcp-nats/src/nats/subjects/client/mod.rs +macro_rules! define_client_subject { + ($name:ident, $fmt:literal, Requestable) => { + define_client_subject!(`@inner` $name, $fmt, super::super::markers::Requestable); + }; + ($name:ident, $fmt:literal, Publishable) => { + define_client_subject!(`@inner` $name, $fmt, super::super::markers::Publishable); + }; + (`@inner` $name:ident, $fmt:literal, $marker:path) => { + #[derive(Debug, Clone)] + pub struct $name { + prefix: crate::McpPrefix, + client_id: crate::McpPeerId, + } + + impl $name { + pub fn new(prefix: &crate::McpPrefix, client_id: &crate::McpPeerId) -> Self { + Self { + prefix: prefix.clone(), + client_id: client_id.clone(), + } + } + } + + impl std::fmt::Display for $name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, $fmt, self.prefix.as_str(), self.client_id.as_str()) + } + } + + impl $marker for $name {} + }; +} + +define_client_subject!(PingSubject, "{}.client.{}.ping", Requestable); +define_client_subject!(CreateMessageSubject, "{}.client.{}.sampling.create_message", Requestable); +define_client_subject!(ListRootsSubject, "{}.client.{}.roots.list", Requestable); +define_client_subject!(CreateElicitationSubject, "{}.client.{}.elicitation.create", Requestable); +define_client_subject!(InitializedSubject, "{}.client.{}.notifications.initialized", Publishable); +define_client_subject!(CancelledSubject, "{}.client.{}.notifications.cancelled", Publishable); +define_client_subject!(ProgressSubject, "{}.client.{}.notifications.progress", Publishable); +define_client_subject!(RootsListChangedSubject, "{}.client.{}.notifications.roots.list_changed", Publishable);This also allows the 8 individual source files to be deleted entirely.
🤖 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/client/ping.rs` around lines 1 - 23, Replace the per-file boilerplate for client subjects by defining a macro_rules! in mod.rs that generates the struct, new(prefix, client_id) constructor, derives (e.g. Debug, Clone, and optionally PartialEq), the Display impl that uses a provided format string, and implements the appropriate marker trait(s) (e.g. super::super::markers::Requestable); then remove the eight near-identical files such as PingSubject and instead invoke the macro with the unique identifiers (struct name like PingSubject), the format string literal ("{}.client.{}.ping"), and the marker trait(s) to recreate the same types and behavior, ensuring the generated symbols (PingSubject, new, Display, and Requestable) match the originals so callers remain unchanged.rsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rs (1)
1-27: ⚡ Quick winCollapse identical subject boilerplate into a declarative macro.
All 8 reviewed subject types (
CancelTaskSubject,InitializeSubject,ReadResourceSubject,LoggingMessageSubject,ListPromptsSubject,ListToolsSubject,CancelledSubject,PromptListChangedSubject) and every future subject share an identical skeleton: two fields (McpPrefix,McpPeerId), the same constructor, a single-formatDisplay, and one of two marker impls.parsing.rsalready maps ~20 suffixes, so the number of subject files will keep growing under the current approach.A macro collapses each subject to a single line with no duplicated structure:
♻️ Proposed macro in a new
subjects/server/macros.rs// rsworkspace/crates/mcp-nats/src/nats/subjects/server/macros.rs macro_rules! define_subject { ($name:ident, $fmt:literal, requestable) => { #[derive(Debug, Clone)] pub struct $name { prefix: crate::McpPrefix, server_id: crate::McpPeerId, } impl $name { pub fn new(prefix: &crate::McpPrefix, server_id: &crate::McpPeerId) -> Self { Self { prefix: prefix.clone(), server_id: server_id.clone(), } } } impl std::fmt::Display for $name { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { write!(f, $fmt, self.prefix.as_str(), self.server_id.as_str()) } } impl super::super::markers::Requestable for $name {} }; ($name:ident, $fmt:literal, publishable) => { // identical body, impl Publishable instead // … }; } pub(crate) use define_subject;Then each subject file (or a single
all.rs) reduces to:define_subject!(CancelTaskSubject, "{}.server.{}.tasks.cancel", requestable); define_subject!(InitializeSubject, "{}.server.{}.initialize", requestable); define_subject!(ReadResourceSubject, "{}.server.{}.resources.read", requestable); define_subject!(ListPromptsSubject, "{}.server.{}.prompts.list", requestable); define_subject!(ListToolsSubject, "{}.server.{}.tools.list", requestable); define_subject!(LoggingMessageSubject, "{}.server.{}.notifications.message", publishable); define_subject!(CancelledSubject, "{}.server.{}.notifications.cancelled", publishable); define_subject!(PromptListChangedSubject, "{}.server.{}.notifications.prompts.list_changed",publishable);🤖 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/server/cancel_task.rs` around lines 1 - 27, The current many identical subject structs (e.g., CancelTaskSubject, InitializeSubject, ReadResourceSubject, LoggingMessageSubject, etc.) should be replaced by a declarative macro; add a macro named define_subject that generates the struct (with fields prefix: McpPrefix and server_id: McpPeerId), the new() constructor, Display impl using a format literal, and either impl Requestable or impl Publishable depending on a token; then replace each individual subject file with a single invocation like define_subject!(CancelTaskSubject, "{}.server.{}.tasks.cancel", requestable) and update the module that maps suffixes in parsing.rs to use these macro-generated types. Ensure the macro exposes define_subject for use by the server subject module and references the exact type names Requestable and Publishable and methods prefix.as_str()/server_id.as_str() used in the Display implementation.rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rs (1)
1-28: ⚡ Quick winConsider a
macro_rules!to eliminate 8-way boilerplate across all new subject files.All 8 subject types added in this PR (
ListResourceTemplatesSubject,ListTasksSubject,PingSubject,ProgressSubject,CallToolSubject,ResourceListChangedSubject,GetTaskSubject,GetTaskResultSubject) are structurally identical — same two fields, same constructor shape, sameDisplayformat pattern — differing only in struct name, subject suffix string, and marker trait (RequestablevsPublishable). A singlemacro_rules!macro collapses all 8 into ~15 lines and eliminates drift risk if the struct shape or constructor signature ever changes.♻️ Proposed macro consolidation (place in `mod.rs` or a `macros.rs` submodule)
+macro_rules! define_subject { + ($name:ident, $suffix:literal, Requestable) => { + define_subject!($name, $suffix, super::super::markers::Requestable); + impl super::super::markers::Requestable for $name {} + }; + ($name:ident, $suffix:literal, Publishable) => { + define_subject!($name, $suffix); + impl super::super::markers::Publishable for $name {} + }; + ($name:ident, $suffix:literal) => { + #[derive(Debug, Clone)] + pub struct $name { + prefix: crate::McpPrefix, + server_id: crate::McpPeerId, + } + impl $name { + pub fn new(prefix: &crate::McpPrefix, server_id: &crate::McpPeerId) -> Self { + Self { prefix: prefix.clone(), server_id: server_id.clone() } + } + } + impl std::fmt::Display for $name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!(f, "{}.server.{}.{}", self.prefix.as_str(), self.server_id.as_str(), $suffix) + } + } + }; +} + +define_subject!(ListResourceTemplatesSubject, "resources.templates.list", Requestable); +define_subject!(ListTasksSubject, "tasks.list", Requestable); +define_subject!(PingSubject, "ping", Requestable); +define_subject!(CallToolSubject, "tools.call", Requestable); +define_subject!(GetTaskSubject, "tasks.get", Requestable); +define_subject!(GetTaskResultSubject, "tasks.result", Requestable); +define_subject!(ProgressSubject, "notifications.progress", Publishable); +define_subject!(ResourceListChangedSubject, "notifications.resources.list_changed", Publishable);The individual per-operation files can then be removed in favour of a single
subjects/server/mod.rsblock, keeping re-exports unchanged.🤖 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/server/list_resource_templates.rs` around lines 1 - 28, Multiple subject structs (e.g., ListResourceTemplatesSubject, ListTasksSubject, PingSubject, ProgressSubject, CallToolSubject, ResourceListChangedSubject, GetTaskSubject, GetTaskResultSubject) share identical fields, constructor pattern, and Display formatting; replace this boilerplate with a single macro_rules! (e.g., subject_struct!) that accepts the struct name, subject suffix string (like "server.{id}.resources.templates.list"), and the marker trait to implement (super::super::markers::Requestable or Publishable), and generates the struct with fields prefix: crate::McpPrefix and server_id: crate::McpPeerId, a new(prefix, server_id) constructor that clones its args, a Display impl that writes "{}.server.{}.{suffix}" using prefix.as_str() and server_id.as_str(), and the marker impl; then update each subject file to a one-line macro invocation with the proper name, suffix, and trait.rsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_updated.rs (1)
1-28: ⚡ Quick winConsider a
macro_rules!to eliminate boilerplate across all server subject types.All 8 files added in this PR (and likely many more in the broader crate) are structurally identical: same two fields, same constructor, same
Displayskeleton, varying only in the struct name, subject-suffix literal, and marker trait. A single declarative macro collapses the entire family to one-liners.♻️ Suggested macro (in
server/mod.rsor a siblingmacros.rs)+macro_rules! server_subject { + ($name:ident, $suffix:literal, Publishable) => { + server_subject!(`@inner` $name, $suffix, super::super::markers::Publishable); + }; + ($name:ident, $suffix:literal, Requestable) => { + server_subject!(`@inner` $name, $suffix, super::super::markers::Requestable); + }; + (`@inner` $name:ident, $suffix:literal, $marker:path) => { + #[derive(Debug, Clone)] + pub struct $name { + prefix: crate::McpPrefix, + server_id: crate::McpPeerId, + } + + impl $name { + pub fn new(prefix: &crate::McpPrefix, server_id: &crate::McpPeerId) -> Self { + Self { + prefix: prefix.clone(), + server_id: server_id.clone(), + } + } + } + + impl std::fmt::Display for $name { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}.server.{}.{}", + self.prefix.as_str(), + self.server_id.as_str(), + $suffix, + ) + } + } + + impl $marker for $name {} + }; +}Each existing file then reduces to a single invocation:
-#[derive(Debug, Clone)] -pub struct ResourceUpdatedSubject { - prefix: crate::McpPrefix, - server_id: crate::McpPeerId, -} - -impl ResourceUpdatedSubject { - pub fn new(prefix: &crate::McpPrefix, server_id: &crate::McpPeerId) -> Self { - Self { - prefix: prefix.clone(), - server_id: server_id.clone(), - } - } -} - -impl std::fmt::Display for ResourceUpdatedSubject { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{}.server.{}.notifications.resources.updated", - self.prefix.as_str(), - self.server_id.as_str() - ) - } -} - -impl super::super::markers::Publishable for ResourceUpdatedSubject {} +server_subject!(ResourceUpdatedSubject, "notifications.resources.updated", Publishable);This pattern applies identically to all eight files in this PR and any future server subject types. New subjects become a one-liner with no risk of the fields or Display skeleton drifting across copies.
🤖 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/server/resource_updated.rs` around lines 1 - 28, All eight server subject structs repeat the same two fields, constructor, and Display skeleton; create a declarative macro (e.g., in server/mod.rs or server/macros.rs) that takes the struct name, the subject-suffix literal (like "resources.updated"), and the marker trait (e.g., markers::Publishable) and emits: a struct with fields prefix: crate::McpPrefix and server_id: crate::McpPeerId, a new(prefix: &crate::McpPrefix, server_id: &crate::McpPeerId) -> Self that clones the inputs, an impl Display that formats "{}.server.{}.notifications.<suffix>" using prefix.as_str() and server_id.as_str(), and an impl of the provided marker trait; then replace ResourceUpdatedSubject (and the other seven files) with a single invocation of that macro referencing ResourceUpdatedSubject, "resources.updated", and markers::Publishable.
🤖 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.
Nitpick comments:
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/client/ping.rs`:
- Around line 1-23: Replace the per-file boilerplate for client subjects by
defining a macro_rules! in mod.rs that generates the struct, new(prefix,
client_id) constructor, derives (e.g. Debug, Clone, and optionally PartialEq),
the Display impl that uses a provided format string, and implements the
appropriate marker trait(s) (e.g. super::super::markers::Requestable); then
remove the eight near-identical files such as PingSubject and instead invoke the
macro with the unique identifiers (struct name like PingSubject), the format
string literal ("{}.client.{}.ping"), and the marker trait(s) to recreate the
same types and behavior, ensuring the generated symbols (PingSubject, new,
Display, and Requestable) match the originals so callers remain unchanged.
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rs`:
- Around line 1-27: The current many identical subject structs (e.g.,
CancelTaskSubject, InitializeSubject, ReadResourceSubject,
LoggingMessageSubject, etc.) should be replaced by a declarative macro; add a
macro named define_subject that generates the struct (with fields prefix:
McpPrefix and server_id: McpPeerId), the new() constructor, Display impl using a
format literal, and either impl Requestable or impl Publishable depending on a
token; then replace each individual subject file with a single invocation like
define_subject!(CancelTaskSubject, "{}.server.{}.tasks.cancel", requestable) and
update the module that maps suffixes in parsing.rs to use these macro-generated
types. Ensure the macro exposes define_subject for use by the server subject
module and references the exact type names Requestable and Publishable and
methods prefix.as_str()/server_id.as_str() used in the Display implementation.
In
`@rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rs`:
- Around line 1-28: Multiple subject structs (e.g.,
ListResourceTemplatesSubject, ListTasksSubject, PingSubject, ProgressSubject,
CallToolSubject, ResourceListChangedSubject, GetTaskSubject,
GetTaskResultSubject) share identical fields, constructor pattern, and Display
formatting; replace this boilerplate with a single macro_rules! (e.g.,
subject_struct!) that accepts the struct name, subject suffix string (like
"server.{id}.resources.templates.list"), and the marker trait to implement
(super::super::markers::Requestable or Publishable), and generates the struct
with fields prefix: crate::McpPrefix and server_id: crate::McpPeerId, a
new(prefix, server_id) constructor that clones its args, a Display impl that
writes "{}.server.{}.{suffix}" using prefix.as_str() and server_id.as_str(), and
the marker impl; then update each subject file to a one-line macro invocation
with the proper name, suffix, and trait.
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_updated.rs`:
- Around line 1-28: All eight server subject structs repeat the same two fields,
constructor, and Display skeleton; create a declarative macro (e.g., in
server/mod.rs or server/macros.rs) that takes the struct name, the
subject-suffix literal (like "resources.updated"), and the marker trait (e.g.,
markers::Publishable) and emits: a struct with fields prefix: crate::McpPrefix
and server_id: crate::McpPeerId, a new(prefix: &crate::McpPrefix, server_id:
&crate::McpPeerId) -> Self that clones the inputs, an impl Display that formats
"{}.server.{}.notifications.<suffix>" using prefix.as_str() and
server_id.as_str(), and an impl of the provided marker trait; then replace
ResourceUpdatedSubject (and the other seven files) with a single invocation of
that macro referencing ResourceUpdatedSubject, "resources.updated", and
markers::Publishable.
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rs`:
- Line 14: Replace the explicit call to McpPrefix::as_str() with McpPrefix's
Display so the formatter uses the type's Display impl: change the write!
invocation that currently uses self.prefix.as_str() to use self.prefix directly
(i.e., write!(f, "{}.client.>", self.prefix)), referencing the write! call and
the self.prefix/McpPrefix symbols to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a50d6f37-f22d-4fcb-900f-eb890f2cfb9f
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (63)
rsworkspace/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/README.mdrsworkspace/crates/mcp-nats-stdio/src/config.rsrsworkspace/crates/mcp-nats-stdio/src/main.rsrsworkspace/crates/mcp-nats/Cargo.tomlrsworkspace/crates/mcp-nats/README.mdrsworkspace/crates/mcp-nats/src/client.rsrsworkspace/crates/mcp-nats/src/config.rsrsworkspace/crates/mcp-nats/src/constants.rsrsworkspace/crates/mcp-nats/src/jsonrpc.rsrsworkspace/crates/mcp-nats/src/lib.rsrsworkspace/crates/mcp-nats/src/mcp_peer_id.rsrsworkspace/crates/mcp-nats/src/mcp_prefix.rsrsworkspace/crates/mcp-nats/src/nats/mod.rsrsworkspace/crates/mcp-nats/src/nats/parsing.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_elicitation.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/initialized.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/list_roots.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/roots_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/markers.rsrsworkspace/crates/mcp-nats/src/nats/subjects/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/complete.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/elicitation_completed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_prompt.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task_result.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/initialize.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_prompts.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resources.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tasks.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tools.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/logging_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/prompt_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/read_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_updated.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/set_logging_level.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/subscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/tool_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/unsubscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_server.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_server.rsrsworkspace/crates/mcp-nats/src/server.rsrsworkspace/crates/mcp-nats/src/telemetry/mod.rsrsworkspace/crates/mcp-nats/src/telemetry/transport.rsrsworkspace/crates/mcp-nats/src/transport.rsrsworkspace/crates/mcp-nats/tests/transport.rs
✅ Files skipped from review due to trivial changes (11)
- rsworkspace/crates/mcp-nats/src/telemetry/mod.rs
- rsworkspace/crates/mcp-nats-stdio/README.md
- rsworkspace/crates/mcp-nats/src/constants.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rs
- rsworkspace/crates/mcp-nats/src/client.rs
- rsworkspace/crates/mcp-nats/Cargo.toml
- rsworkspace/crates/mcp-nats/src/lib.rs
- rsworkspace/crates/mcp-nats/src/telemetry/transport.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/mod.rs
- rsworkspace/crates/mcp-nats/src/transport.rs
🚧 Files skipped from review as they are similar to previous changes (12)
- rsworkspace/crates/mcp-nats/tests/transport.rs
- rsworkspace/crates/mcp-nats-stdio/Cargo.toml
- rsworkspace/crates/mcp-nats/src/mcp_prefix.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/markers.rs
- rsworkspace/crates/mcp-nats/src/jsonrpc.rs
- rsworkspace/crates/mcp-nats/src/server.rs
- rsworkspace/Cargo.toml
- rsworkspace/crates/mcp-nats-stdio/src/config.rs
- rsworkspace/crates/mcp-nats/src/nats/parsing.rs
- rsworkspace/crates/mcp-nats/src/config.rs
- rsworkspace/crates/mcp-nats/src/nats/mod.rs
- rsworkspace/crates/mcp-nats/src/mcp_peer_id.rs
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
2 times, most recently
from
8b321e9 to
c8c27f5
Compare
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
c8c27f5 to
e1c8a3e
Compare
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
e1c8a3e to
a7e3ef5
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rsworkspace/crates/trogon-nats/src/mocks.rs (1)
145-147: ⚡ Quick winAdd a unit test for
hang_next_publish.Every other mock control path (
fail_next_publish,fail_publish_count,hang_next_request) has a corresponding test, buthang_next_publishdoes not. Given the async-cancellation semantics here (flag cleared beforepending(), subsequent call succeeds), a test asserting that the second call succeeds after the first one is cancelled under a timeout would both document the intended behavior and guard against regressions.✅ Example test skeleton
#[tokio::test] async fn advanced_mock_hang_next_publish_hangs_once_then_succeeds() { let mock = AdvancedMockNatsClient::new(); mock.hang_next_publish(); // First call hangs — cancel it via timeout. let hang = tokio::time::timeout( std::time::Duration::from_millis(10), mock.publish_with_headers("foo", async_nats::HeaderMap::new(), bytes::Bytes::from("x")), ) .await; assert!(hang.is_err(), "expected timeout on hanging publish"); // Flag was consumed; second call should succeed. let second = mock .publish_with_headers("foo", async_nats::HeaderMap::new(), bytes::Bytes::from("y")) .await; assert!(second.is_ok()); assert_eq!(mock.published_messages(), vec!["foo"]); }🤖 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/trogon-nats/src/mocks.rs` around lines 145 - 147, Add a new tokio unit test for AdvancedMockNatsClient that calls hang_next_publish(), then asserts the first publish_with_headers("foo", ...) call is cancelled via tokio::time::timeout (expect Err), and then calls publish_with_headers a second time and asserts it returns Ok and that published_messages() equals vec!["foo"]; this documents that hang_next_publish() consumes the hang flag before awaiting and allows the subsequent call to succeed.
🤖 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/README.md`:
- Around line 45-69: The doctest's hidden async function incorrectly types the
parameter as async_nats::Client and calls client::connect with it; change the
hidden setup to obtain a trogon-compatible client using mcp_nats::nats::connect
(as used in main.rs) so the client satisfies
SubscribeClient+RequestClient+PublishClient+FlushClient—replace the hidden nats
declaration with a call to mcp_nats::nats::connect (passing the
config/mcp.nats() and timeout as appropriate) before calling client::connect and
serving the transport.
---
Nitpick comments:
In `@rsworkspace/crates/trogon-nats/src/mocks.rs`:
- Around line 145-147: Add a new tokio unit test for AdvancedMockNatsClient that
calls hang_next_publish(), then asserts the first publish_with_headers("foo",
...) call is cancelled via tokio::time::timeout (expect Err), and then calls
publish_with_headers a second time and asserts it returns Ok and that
published_messages() equals vec!["foo"]; this documents that hang_next_publish()
consumes the hang flag before awaiting and allows the subsequent call to
succeed.
🪄 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: 8de6e296-2e6c-47b2-9741-bb3eeda2b010
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
rsworkspace/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/README.mdrsworkspace/crates/mcp-nats-stdio/src/config.rsrsworkspace/crates/mcp-nats-stdio/src/main.rsrsworkspace/crates/mcp-nats/Cargo.tomlrsworkspace/crates/mcp-nats/README.mdrsworkspace/crates/mcp-nats/src/client.rsrsworkspace/crates/mcp-nats/src/config.rsrsworkspace/crates/mcp-nats/src/constants.rsrsworkspace/crates/mcp-nats/src/jsonrpc.rsrsworkspace/crates/mcp-nats/src/lib.rsrsworkspace/crates/mcp-nats/src/mcp_peer_id.rsrsworkspace/crates/mcp-nats/src/mcp_prefix.rsrsworkspace/crates/mcp-nats/src/nats/mod.rsrsworkspace/crates/mcp-nats/src/nats/parsing.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_elicitation.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/initialized.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/list_roots.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/roots_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/markers.rsrsworkspace/crates/mcp-nats/src/nats/subjects/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/complete.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/elicitation_completed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_prompt.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task_result.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/initialize.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_prompts.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resources.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tasks.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tools.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/logging_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/prompt_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/read_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_updated.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/set_logging_level.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/subscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/tool_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/unsubscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_server.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_server.rsrsworkspace/crates/mcp-nats/src/server.rsrsworkspace/crates/mcp-nats/src/telemetry/mod.rsrsworkspace/crates/mcp-nats/src/telemetry/transport.rsrsworkspace/crates/mcp-nats/src/transport.rsrsworkspace/crates/mcp-nats/tests/transport.rsrsworkspace/crates/trogon-nats/README.mdrsworkspace/crates/trogon-nats/src/mocks.rs
✅ Files skipped from review due to trivial changes (29)
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/markers.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/initialized.rs
- rsworkspace/crates/mcp-nats/src/telemetry/mod.rs
- rsworkspace/crates/trogon-nats/README.md
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/set_logging_level.rs
- rsworkspace/crates/mcp-nats/Cargo.toml
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/subscribe_resource.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rs
- rsworkspace/crates/mcp-nats-stdio/README.md
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/read_resource.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/get_prompt.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rs
- rsworkspace/crates/mcp-nats/src/telemetry/transport.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/prompt_list_changed.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/logging_message.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/progress.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/cancelled.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/list_roots.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_list_changed.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/unsubscribe_resource.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/elicitation_completed.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/ping.rs
- rsworkspace/crates/mcp-nats/src/constants.rs
- rsworkspace/crates/mcp-nats/src/mcp_prefix.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tasks.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_server.rs
🚧 Files skipped from review as they are similar to previous changes (21)
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_server.rs
- rsworkspace/crates/mcp-nats/src/client.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resources.rs
- rsworkspace/crates/mcp-nats/tests/transport.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/create_message.rs
- rsworkspace/crates/mcp-nats/src/jsonrpc.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tools.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/progress.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_prompts.rs
- rsworkspace/crates/mcp-nats/src/config.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/tool_list_changed.rs
- rsworkspace/crates/mcp-nats/src/mcp_peer_id.rs
- rsworkspace/crates/mcp-nats-stdio/Cargo.toml
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task.rs
- rsworkspace/crates/mcp-nats/src/lib.rs
- rsworkspace/Cargo.toml
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/roots_list_changed.rs
- rsworkspace/crates/mcp-nats/src/nats/parsing.rs
- rsworkspace/crates/mcp-nats/src/transport.rs
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
a7e3ef5 to
d69b264
Compare
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
d69b264 to
8edbfab
Compare
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
2 times, most recently
from
956af0e to
9ce3e65
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (2)
rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rs (1)
7-13: 💤 Low valueOptional: accept owned params to avoid unconditional clones.
newcurrently forces a clone on every call regardless of whether the caller still needs the originals. Accepting owned values lets callers move when they can and clone explicitly when they must.♻️ Proposed refactor
- pub fn new(prefix: &crate::McpPrefix, client_id: &crate::McpPeerId) -> Self { + pub fn new(prefix: crate::McpPrefix, client_id: crate::McpPeerId) -> Self { Self { - prefix: prefix.clone(), - client_id: client_id.clone(), + prefix, + client_id, } }🤖 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/subscriptions/one_client.rs` around lines 7 - 13, The constructor OneClientSubject::new currently forces clones of &crate::McpPrefix and &crate::McpPeerId on every call; change its signature to accept owned/convertible parameters (e.g., pub fn new(prefix: impl Into<crate::McpPrefix>, client_id: impl Into<crate::McpPeerId>) -> Self) and then call prefix.into() and client_id.into() to initialize Self without unconditional clones, so callers can move values or pass references that will be cloned only when necessary.rsworkspace/crates/mcp-nats/src/nats/parsing.rs (1)
24-115: ⚡ Quick winMany
from_suffixarms are not covered by any test, making typos or future edits invisible to CI.Of the 34 total method variants across all four enums, the test suite exercises only a handful:
ListTools,Initialized,CreateMessage,ListRoots,ResourceUpdated, andToolListChanged. The remaining 28 variants — including the fullServerRequestMethodset minusListTools, and all task-related extensions (GetTask,ListTasks,GetTaskResult,CancelTask) — have zero test coverage. A single-character typo in any of thosefrom_suffixmatch arms would silently ship.Consider adding one parameterised/table-driven test per
from_suffixmethod to cover all defined arms:🧪 Sketch of additional coverage
#[test] fn server_request_all_suffixes() { let cases = [ ("initialize", ServerRequestMethod::Initialize), ("ping", ServerRequestMethod::Ping), ("completion.complete", ServerRequestMethod::Complete), ("logging.set_level", ServerRequestMethod::SetLoggingLevel), ("prompts.list", ServerRequestMethod::ListPrompts), ("prompts.get", ServerRequestMethod::GetPrompt), ("resources.list", ServerRequestMethod::ListResources), ("resources.templates.list", ServerRequestMethod::ListResourceTemplates), ("resources.read", ServerRequestMethod::ReadResource), ("resources.subscribe", ServerRequestMethod::SubscribeResource), ("resources.unsubscribe", ServerRequestMethod::UnsubscribeResource), ("tools.list", ServerRequestMethod::ListTools), ("tools.call", ServerRequestMethod::CallTool), ("tasks.get", ServerRequestMethod::GetTask), ("tasks.list", ServerRequestMethod::ListTasks), ("tasks.result", ServerRequestMethod::GetTaskResult), ("tasks.cancel", ServerRequestMethod::CancelTask), ]; for (suffix, expected) in cases { let subject = format!("mcp.server.peer.{suffix}"); assert_eq!( parse_server_subject(&subject).unwrap(), ParsedServerSubject::Request { server_id: peer("peer"), method: expected }, "failed for suffix: {suffix}", ); } } // … similar tables for server_notification_all_suffixes, // client_request_all_suffixes, client_notification_all_suffixes🤖 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/parsing.rs` around lines 24 - 115, The from_suffix match arms across ServerRequestMethod::from_suffix, ServerNotificationMethod::from_suffix, ClientRequestMethod::from_suffix, and ClientNotificationMethod::from_suffix lack test coverage for many variants; add parameterized/table-driven unit tests that iterate each suffix->enum mapping for those four from_suffix functions (e.g., a test like server_request_all_suffixes that asserts parse_server_subject(...).method == ServerRequestMethod::<Variant> for every defined suffix), and analogous tests for server_notification_all_suffixes, client_request_all_suffixes, and client_notification_all_suffixes to ensure every match arm is exercised and will fail on typos or regressions.
🤖 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.
Nitpick comments:
In `@rsworkspace/crates/mcp-nats/src/nats/parsing.rs`:
- Around line 24-115: The from_suffix match arms across
ServerRequestMethod::from_suffix, ServerNotificationMethod::from_suffix,
ClientRequestMethod::from_suffix, and ClientNotificationMethod::from_suffix lack
test coverage for many variants; add parameterized/table-driven unit tests that
iterate each suffix->enum mapping for those four from_suffix functions (e.g., a
test like server_request_all_suffixes that asserts
parse_server_subject(...).method == ServerRequestMethod::<Variant> for every
defined suffix), and analogous tests for server_notification_all_suffixes,
client_request_all_suffixes, and client_notification_all_suffixes to ensure
every match arm is exercised and will fail on typos or regressions.
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rs`:
- Around line 7-13: The constructor OneClientSubject::new currently forces
clones of &crate::McpPrefix and &crate::McpPeerId on every call; change its
signature to accept owned/convertible parameters (e.g., pub fn new(prefix: impl
Into<crate::McpPrefix>, client_id: impl Into<crate::McpPeerId>) -> Self) and
then call prefix.into() and client_id.into() to initialize Self without
unconditional clones, so callers can move values or pass references that will be
cloned only when necessary.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7e17780d-f9be-42ff-866d-18a916673101
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (65)
rsworkspace/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/README.mdrsworkspace/crates/mcp-nats-stdio/src/config.rsrsworkspace/crates/mcp-nats-stdio/src/main.rsrsworkspace/crates/mcp-nats/Cargo.tomlrsworkspace/crates/mcp-nats/README.mdrsworkspace/crates/mcp-nats/src/client.rsrsworkspace/crates/mcp-nats/src/config.rsrsworkspace/crates/mcp-nats/src/constants.rsrsworkspace/crates/mcp-nats/src/jsonrpc.rsrsworkspace/crates/mcp-nats/src/lib.rsrsworkspace/crates/mcp-nats/src/mcp_peer_id.rsrsworkspace/crates/mcp-nats/src/mcp_prefix.rsrsworkspace/crates/mcp-nats/src/nats/mod.rsrsworkspace/crates/mcp-nats/src/nats/parsing.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_elicitation.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/initialized.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/list_roots.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/roots_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/markers.rsrsworkspace/crates/mcp-nats/src/nats/subjects/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/complete.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/elicitation_completed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_prompt.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task_result.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/initialize.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_prompts.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resources.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tasks.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tools.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/logging_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/prompt_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/read_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_updated.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/set_logging_level.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/subscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/tool_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/unsubscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_server.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_server.rsrsworkspace/crates/mcp-nats/src/server.rsrsworkspace/crates/mcp-nats/src/telemetry/mod.rsrsworkspace/crates/mcp-nats/src/telemetry/transport.rsrsworkspace/crates/mcp-nats/src/transport.rsrsworkspace/crates/mcp-nats/tests/transport.rsrsworkspace/crates/trogon-nats/README.mdrsworkspace/crates/trogon-nats/src/mocks.rs
✅ Files skipped from review due to trivial changes (22)
- rsworkspace/crates/mcp-nats/src/nats/subjects/markers.rs
- rsworkspace/crates/mcp-nats-stdio/README.md
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/initialize.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tasks.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/subscribe_resource.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/read_resource.rs
- rsworkspace/crates/trogon-nats/README.md
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_server.rs
- rsworkspace/crates/mcp-nats/tests/transport.rs
- rsworkspace/crates/mcp-nats/src/telemetry/transport.rs
- rsworkspace/crates/mcp-nats/src/constants.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/cancelled.rs
- rsworkspace/crates/mcp-nats/src/telemetry/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rs
- rsworkspace/crates/mcp-nats/src/jsonrpc.rs
- rsworkspace/crates/mcp-nats-stdio/Cargo.toml
- rsworkspace/crates/mcp-nats/src/lib.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/ping.rs
🚧 Files skipped from review as they are similar to previous changes (30)
- rsworkspace/Cargo.toml
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/elicitation_completed.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_updated.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/cancelled.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_server.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task_result.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/progress.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/list_roots.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rs
- rsworkspace/crates/mcp-nats/Cargo.toml
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resources.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/create_message.rs
- rsworkspace/crates/mcp-nats/src/mcp_prefix.rs
- rsworkspace/crates/mcp-nats/src/mcp_peer_id.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/set_logging_level.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/get_prompt.rs
- rsworkspace/crates/mcp-nats/src/server.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/unsubscribe_resource.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/complete.rs
- rsworkspace/crates/mcp-nats/src/nats/mod.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/create_elicitation.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/client/mod.rs
- rsworkspace/crates/mcp-nats/src/client.rs
- rsworkspace/crates/mcp-nats-stdio/src/config.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/list_prompts.rs
- rsworkspace/crates/mcp-nats-stdio/src/main.rs
- rsworkspace/crates/mcp-nats/src/config.rs
- rsworkspace/crates/mcp-nats/src/transport.rs
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
8ee2875 to
000079b
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (8)
rsworkspace/crates/mcp-nats/src/telemetry/transport.rs (1)
7-9: ⚡ Quick winType the transport direction instead of accepting raw strings
Using
&strhere makes invalid direction values representable. Prefer an enum/value-object (e.g.,Inbound|Outbound) and map to stable label text at the edge.As per coding guidelines, "Prefer domain-specific value objects over primitives ... invalid instances should be unrepresentable."
🤖 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/telemetry/transport.rs` around lines 7 - 9, The function record_direction currently accepts a raw &str which allows invalid values; define a TransportDirection enum (e.g., Inbound, Outbound) and add a stable mapping method (e.g., to_label() or impl Display for TransportDirection) that returns the canonical string for the span label, then change record_direction signature to accept TransportDirection and call span.record("mcp.nats.direction", &direction.to_label()/direction.to_string()); update all sites that call record_direction to pass the enum instead of raw strings so invalid directions become unrepresentable.rsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rs (1)
1-2: ⚡ Quick winConsider deriving
PartialEqandEqon subject structs.All six new subject structs in this PR derive only
Debug + Clone. Since bothMcpPrefixandMcpPeerIdalready implementPartialEq + Eq + Hash, addingPartialEqandEqto these structs is trivial and improves ergonomics — enabling directassert_eq!comparisons in tests and deduplication in collections. The same applies toListTasksSubject,PingSubject,ListResourceTemplatesSubject,ListToolsSubject, andProgressSubject.♻️ Proposed change (representative — apply to all 6 subject structs)
-#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct CallToolSubject {🤖 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/server/call_tool.rs` around lines 1 - 2, The subject structs (e.g., CallToolSubject) currently derive only Debug and Clone; update each of the six subject structs — CallToolSubject, ListTasksSubject, PingSubject, ListResourceTemplatesSubject, ListToolsSubject, and ProgressSubject — to also derive PartialEq and Eq so they can be compared and used in equality-based collections; locate the struct declarations and add PartialEq, Eq to the derive attribute alongside Debug and Clone.rsworkspace/crates/mcp-nats/tests/transport.rs (1)
52-55: 💤 Low valueOptional: tighten the response assertion.
The current assertion only checks the variant. Verifying that the response's
idmatches the request'sRequestId::Number(1)would also catch routing/correlation regressions:♻️ Proposed change
- assert!(matches!( - transport.receive().await.unwrap(), - ServerJsonRpcMessage::Response(_) - )); + let received = transport.receive().await.unwrap(); + let ServerJsonRpcMessage::Response(resp) = received else { + panic!("expected Response, got {received:?}"); + }; + assert_eq!(resp.id, RequestId::Number(1));🤖 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/tests/transport.rs` around lines 52 - 55, Tighten the test assertion by matching the received ServerJsonRpcMessage::Response payload and asserting its id equals the original request id (RequestId::Number(1)); update the pattern that currently uses matches!(transport.receive().await.unwrap(), ServerJsonRpcMessage::Response(_)) to destructure the response from transport.receive().await.unwrap() (or match on ServerJsonRpcMessage::Response(resp)) and assert resp.id == RequestId::Number(1) to ensure correct routing/correlation.rsworkspace/crates/mcp-nats/src/nats/parsing.rs (1)
141-171: 💤 Low valueOptional: collapse the duplicate marker scan.
Each entry-point calls
parse_role_subjecttwice, rescanning the whole string and re-validatingMcpPeerId::newfor both request- and notification-method parsers. Threading both parsers into a single scan would halve the work on every routed message:♻️ Sketch
fn parse_role_subject<R, N>( subject: &str, marker: &str, parse_request: impl Fn(&str) -> Option<R>, parse_notification: impl Fn(&str) -> Option<N>, ) -> Option<(McpPeerId, Either<R, N>)> { /* single scan, try both at each candidate */ }🤖 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/parsing.rs` around lines 141 - 171, Both parse_server_subject and parse_client_subject call parse_role_subject twice, causing duplicate scans and duplicate McpPeerId::new validation; refactor parse_role_subject to accept two parser callbacks (e.g., parse_request: Fn(&str)->Option<R> and parse_notification: Fn(&str)->Option<N>) and return Option<(McpPeerId, Either<R,N>)> so a single scan tests both request- and notification-parsers at each candidate; update parse_server_subject and parse_client_subject to call the new parse_role_subject once and map the Either<R,N> into ParsedServerSubject/ParsedClientSubject variants accordingly, preserving existing method constructors like ServerRequestMethod::from_suffix, ClientNotificationMethod::from_suffix, ClientRequestMethod::from_suffix and ServerNotificationMethod::from_suffix.rsworkspace/crates/mcp-nats/src/jsonrpc.rs (1)
13-17: 💤 Low valueAvoid full
Valueparse and clone on every routed payload.
extract_request_idis on the hot path for every NATS-delivered message and it currently (1) deserializes the whole payload into aserde_json::Valueand (2) clones the idValuebefore re-deserializing. A targeted partial deserialize is cheaper and allocates less:♻️ Proposed refactor
pub fn extract_request_id(payload: &[u8]) -> Option<RequestId> { - let value = serde_json::from_slice::<serde_json::Value>(payload).ok()?; - let id = value.get("id")?; - serde_json::from_value::<RequestId>(id.clone()).ok() + #[derive(serde::Deserialize)] + struct IdOnly { + id: Option<RequestId>, + } + serde_json::from_slice::<IdOnly>(payload).ok()?.id }🤖 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/jsonrpc.rs` around lines 13 - 17, extract_request_id currently deserializes the entire payload into serde_json::Value and clones the "id" node; change it to stream-parse only the "id" to avoid full-Value allocation: use serde_json::Deserializer::from_slice to iterate the top-level object, stop once the "id" field is encountered, capture its raw JSON fragment (e.g. via serde_json::value::RawValue or by reading the value token from the Deserializer) and then deserialize that fragment into RequestId; update extract_request_id to use serde_json::Deserializer and RequestId deserialization instead of Value::get and clone.rsworkspace/crates/mcp-nats-server/src/main.rs (1)
364-370: 💤 Low valueSilent drop of server notifications received before peer is bound.
run_proxy_workersubscribes to the client's NATS subject space immediately on start, butpeeris only set on the first HTTP-sideProxyCommand. AnyServerJsonRpcMessage::Notificationarriving in that window is dropped without any log entry — theif let Some(peer) = peer && ...short-circuits silently. The same is true if the HTTP session never issues a request before the server publishes notifications (e.g., reconnect races, or server-initiated subscriptions). At minimum, this should leave a trace.♻️ Proposed log on the dropped-notification path
ServerJsonRpcMessage::Notification(notification) => { - if let Some(peer) = peer - && let Err(error) = peer.send_notification(notification.notification).await - { - warn!(error = %error, "Failed to forward MCP server notification to HTTP client"); + let Some(peer) = peer else { + warn!("Dropping MCP server notification; HTTP client peer not yet bound"); + return; + }; + if let Err(error) = peer.send_notification(notification.notification).await { + warn!(error = %error, "Failed to forward MCP server notification to HTTP client"); } }🤖 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-server/src/main.rs` around lines 364 - 370, run_proxy_worker currently drops ServerJsonRpcMessage::Notification messages silently when peer is None; update the Notification match arm (the block handling ServerJsonRpcMessage::Notification) to detect the peer==None path and emit a warning (include identifying info from the notification, e.g. notification.notification or its topic/subject) before dropping, and also log send errors returned from peer.send_notification(notification.notification). Reference the ServerJsonRpcMessage::Notification branch, the peer variable, and the peer.send_notification(...) call so the changed code both logs dropped-notification events when peer is not bound and preserves the existing error log for failed sends.rsworkspace/crates/mcp-nats/src/transport.rs (1)
307-314: 💤 Low valueAvoid double serialization by reading the method field directly from the typed request/notification.
method_for_messageserializes the entireJsonRpcMessageenum toserde_json::Valuejust to extract themethodstring. The call site insend()already pattern-matchesJsonRpcMessage::Request(_)andJsonRpcMessage::Notification(_), but discards the inner value. Since rmcp'sRequestandNotificationtypes expose a publicmethodfield, you can refactorsubject_for_messageto accept the inner request or notification directly and read.methodwithout serialization. This eliminates the redundantserde_json::to_value()call that runs beforeserialize_message(&item).Refactor the pattern match in
send()to capture the request/notification, then pass it to a specialized helper or inline the method extraction.🤖 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/transport.rs` around lines 307 - 314, The helper method_for_message currently serializes the whole TxJsonRpcMessage to extract "method" — change the approach to read the method directly from the typed inner variant: add a new helper (or overload) like method_for_request_or_notification that takes the inner Request/Notification type (the concrete types inside TxJsonRpcMessage) and returns its .method string without calling serde_json::to_value; update send() where it pattern-matches JsonRpcMessage::Request(_) and JsonRpcMessage::Notification(_) to capture the inner value and pass that inner value into the new helper (or inline the .method read) and keep serialize_message(&item) for the payload only; remove the serde_json::to_value call and the NatsTransportError::Serialize path from method_for_message and adjust error handling to use NatsTransportError::MissingMethod if .method is empty or missing.rsworkspace/crates/mcp-nats-server/src/config.rs (1)
19-37: 💤 Low valueBoundary type naming: consider
ArgsInput(orCliInput).
Argsis the untrusted CLI boundary type that is later converted into the validatedHttpBridgeConfig. Per the guidelines, untrusted boundary types should carry an*Input/*Wire/*Requestsuffix to make the conversion seam explicit and to keep boundary types from leaking into domain code by accident.Argsreads like a domain type today.- pub struct Args { + pub struct ArgsInput {(and update
ParseArgs<Args = Args>and call sites accordingly)As per coding guidelines: "Untrusted input must use distinct
*Input/*Wire/*Requesttypes. Convert those boundary types into domain types exactly once."🤖 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-server/src/config.rs` around lines 19 - 37, Rename the untrusted CLI boundary type Args to a clearly marked input type (e.g., ArgsInput or CliInput) and update all generics and call sites that reference it (for example any ParseArgs<Args = Args> usages) so the conversion to the validated domain type HttpBridgeConfig is explicit and happens exactly once; change the struct name on the #[derive(Parser...)] declaration and then adjust imports, type parameters, and function signatures that mention Args to use the new ArgsInput/CliInput identifier and perform a single conversion step into HttpBridgeConfig where the validation/mapping belongs.
🤖 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-server/src/config.rs`:
- Line 46: Replace the raw Vec<String> used for host filtering with a domain
value object: create a new allowed_host.rs module containing pub struct
AllowedHost(String) with a constructor pub fn new(raw: impl Into<String>) ->
Result<Self, AllowedHostError> that validates host syntax (and implements
as_str(), Clone/Debug/PartialEq/Eq); change the HttpBridgeConfig field
allowed_hosts: Vec<String> to allowed_hosts: Vec<AllowedHost> and update any
construction points (CLI/env parsing) to convert each raw string via
AllowedHost::new, returning or mapping validation errors via AllowedHostError so
invalid hosts cannot be represented downstream.
- Around line 131-135: ConfigError::InvalidEnvVar currently stores the parse
error as a String (via to_string()) which loses typed error context; change the
error variants to preserve the original typed source instead—either (A)
parameterize InvalidEnvVar with a boxed source error (e.g., InvalidEnvVar { key:
&'static str, value: String, source: Box<dyn std::error::Error + Send + Sync> })
and return the concrete T::Err from read_env_var, or (B) implement typed per-key
variants (e.g., InvalidPort(std::num::ParseIntError),
InvalidHost(std::net::AddrParseError)) and replace the generic read_env_var with
dedicated parsers that propagate the concrete parse error; update Display and
source() impls to mention the env key but expose the underlying error.
- Around line 49-75: Extract the McpHttpPath and McpHttpPathError definitions
and all their impl blocks into a new module file named mcp_http_path.rs (keeping
the types, new(), as_str(), Display and Error impls unchanged), then remove the
inlined definitions from config.rs and import/re-export them where needed (e.g.,
add pub mod mcp_http_path; and pub use crate::mcp_http_path::{McpHttpPath,
McpHttpPathError}; or at minimum use crate::mcp_http_path::{McpHttpPath,
McpHttpPathError}; in config.rs) so config.rs only composes validated types and
the value object lives in its own module.
- Around line 52-59: McpHttpPath::new is too permissive; tighten validation so
invalid HTTP paths are unrepresentable by rejecting paths that contain
whitespace, control characters, '?' or '#', any segment equal to "." or "..", or
other illegal chars; implement this in McpHttpPath::new (returning
McpHttpPathError on failure) by iterating the characters (or using a regex) to
ensure the string starts with '/' and every char is a printable ASCII path char
(no control or space), split on '/' to forbid "." and ".." segments, and return
Err(McpHttpPathError) for any violation so callers cannot construct invalid
McpHttpPath instances.
In `@rsworkspace/crates/mcp-nats-stdio/README.md`:
- Around line 16-21: The README's docker run currently blocks the shell; update
the example to run NATS detached so the subsequent cargo/build and binary run
can execute in the same session—modify the docker run line (the current "docker
run -p 4222:4222 nats:latest") to include the detach flag (e.g., add -d and
optionally --name nats) and mention how to stop/remove the container when done
so the build/run steps (cargo build --release -p mcp-nats-stdio and
./target/release/mcp-nats-stdio --server-id filesystem) can be executed
immediately afterward.
---
Nitpick comments:
In `@rsworkspace/crates/mcp-nats-server/src/config.rs`:
- Around line 19-37: Rename the untrusted CLI boundary type Args to a clearly
marked input type (e.g., ArgsInput or CliInput) and update all generics and call
sites that reference it (for example any ParseArgs<Args = Args> usages) so the
conversion to the validated domain type HttpBridgeConfig is explicit and happens
exactly once; change the struct name on the #[derive(Parser...)] declaration and
then adjust imports, type parameters, and function signatures that mention Args
to use the new ArgsInput/CliInput identifier and perform a single conversion
step into HttpBridgeConfig where the validation/mapping belongs.
In `@rsworkspace/crates/mcp-nats-server/src/main.rs`:
- Around line 364-370: run_proxy_worker currently drops
ServerJsonRpcMessage::Notification messages silently when peer is None; update
the Notification match arm (the block handling
ServerJsonRpcMessage::Notification) to detect the peer==None path and emit a
warning (include identifying info from the notification, e.g.
notification.notification or its topic/subject) before dropping, and also log
send errors returned from peer.send_notification(notification.notification).
Reference the ServerJsonRpcMessage::Notification branch, the peer variable, and
the peer.send_notification(...) call so the changed code both logs
dropped-notification events when peer is not bound and preserves the existing
error log for failed sends.
In `@rsworkspace/crates/mcp-nats/src/jsonrpc.rs`:
- Around line 13-17: extract_request_id currently deserializes the entire
payload into serde_json::Value and clones the "id" node; change it to
stream-parse only the "id" to avoid full-Value allocation: use
serde_json::Deserializer::from_slice to iterate the top-level object, stop once
the "id" field is encountered, capture its raw JSON fragment (e.g. via
serde_json::value::RawValue or by reading the value token from the Deserializer)
and then deserialize that fragment into RequestId; update extract_request_id to
use serde_json::Deserializer and RequestId deserialization instead of Value::get
and clone.
In `@rsworkspace/crates/mcp-nats/src/nats/parsing.rs`:
- Around line 141-171: Both parse_server_subject and parse_client_subject call
parse_role_subject twice, causing duplicate scans and duplicate McpPeerId::new
validation; refactor parse_role_subject to accept two parser callbacks (e.g.,
parse_request: Fn(&str)->Option<R> and parse_notification: Fn(&str)->Option<N>)
and return Option<(McpPeerId, Either<R,N>)> so a single scan tests both request-
and notification-parsers at each candidate; update parse_server_subject and
parse_client_subject to call the new parse_role_subject once and map the
Either<R,N> into ParsedServerSubject/ParsedClientSubject variants accordingly,
preserving existing method constructors like ServerRequestMethod::from_suffix,
ClientNotificationMethod::from_suffix, ClientRequestMethod::from_suffix and
ServerNotificationMethod::from_suffix.
In `@rsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rs`:
- Around line 1-2: The subject structs (e.g., CallToolSubject) currently derive
only Debug and Clone; update each of the six subject structs — CallToolSubject,
ListTasksSubject, PingSubject, ListResourceTemplatesSubject, ListToolsSubject,
and ProgressSubject — to also derive PartialEq and Eq so they can be compared
and used in equality-based collections; locate the struct declarations and add
PartialEq, Eq to the derive attribute alongside Debug and Clone.
In `@rsworkspace/crates/mcp-nats/src/telemetry/transport.rs`:
- Around line 7-9: The function record_direction currently accepts a raw &str
which allows invalid values; define a TransportDirection enum (e.g., Inbound,
Outbound) and add a stable mapping method (e.g., to_label() or impl Display for
TransportDirection) that returns the canonical string for the span label, then
change record_direction signature to accept TransportDirection and call
span.record("mcp.nats.direction", &direction.to_label()/direction.to_string());
update all sites that call record_direction to pass the enum instead of raw
strings so invalid directions become unrepresentable.
In `@rsworkspace/crates/mcp-nats/src/transport.rs`:
- Around line 307-314: The helper method_for_message currently serializes the
whole TxJsonRpcMessage to extract "method" — change the approach to read the
method directly from the typed inner variant: add a new helper (or overload)
like method_for_request_or_notification that takes the inner
Request/Notification type (the concrete types inside TxJsonRpcMessage) and
returns its .method string without calling serde_json::to_value; update send()
where it pattern-matches JsonRpcMessage::Request(_) and
JsonRpcMessage::Notification(_) to capture the inner value and pass that inner
value into the new helper (or inline the .method read) and keep
serialize_message(&item) for the payload only; remove the serde_json::to_value
call and the NatsTransportError::Serialize path from method_for_message and
adjust error handling to use NatsTransportError::MissingMethod if .method is
empty or missing.
In `@rsworkspace/crates/mcp-nats/tests/transport.rs`:
- Around line 52-55: Tighten the test assertion by matching the received
ServerJsonRpcMessage::Response payload and asserting its id equals the original
request id (RequestId::Number(1)); update the pattern that currently uses
matches!(transport.receive().await.unwrap(), ServerJsonRpcMessage::Response(_))
to destructure the response from transport.receive().await.unwrap() (or match on
ServerJsonRpcMessage::Response(resp)) and assert resp.id == RequestId::Number(1)
to ensure correct routing/correlation.
🪄 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: fba32e96-a6ee-4ea6-a928-562fa3174d7e
⛔ Files ignored due to path filters (1)
rsworkspace/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (69)
rsworkspace/Cargo.tomlrsworkspace/crates/mcp-nats-server/Cargo.tomlrsworkspace/crates/mcp-nats-server/README.mdrsworkspace/crates/mcp-nats-server/src/config.rsrsworkspace/crates/mcp-nats-server/src/main.rsrsworkspace/crates/mcp-nats-stdio/Cargo.tomlrsworkspace/crates/mcp-nats-stdio/README.mdrsworkspace/crates/mcp-nats-stdio/src/config.rsrsworkspace/crates/mcp-nats-stdio/src/main.rsrsworkspace/crates/mcp-nats/Cargo.tomlrsworkspace/crates/mcp-nats/README.mdrsworkspace/crates/mcp-nats/src/client.rsrsworkspace/crates/mcp-nats/src/config.rsrsworkspace/crates/mcp-nats/src/constants.rsrsworkspace/crates/mcp-nats/src/jsonrpc.rsrsworkspace/crates/mcp-nats/src/lib.rsrsworkspace/crates/mcp-nats/src/mcp_peer_id.rsrsworkspace/crates/mcp-nats/src/mcp_prefix.rsrsworkspace/crates/mcp-nats/src/nats/mod.rsrsworkspace/crates/mcp-nats/src/nats/parsing.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_elicitation.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/create_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/initialized.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/list_roots.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/client/roots_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/markers.rsrsworkspace/crates/mcp-nats/src/nats/subjects/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/call_tool.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancel_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/cancelled.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/complete.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/elicitation_completed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_prompt.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/get_task_result.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/initialize.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_prompts.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resource_templates.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_resources.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tasks.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/list_tools.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/logging_message.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/ping.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/progress.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/prompt_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/read_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/resource_updated.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/set_logging_level.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/subscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/tool_list_changed.rsrsworkspace/crates/mcp-nats/src/nats/subjects/server/unsubscribe_resource.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_server.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/mod.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_client.rsrsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_server.rsrsworkspace/crates/mcp-nats/src/server.rsrsworkspace/crates/mcp-nats/src/telemetry/mod.rsrsworkspace/crates/mcp-nats/src/telemetry/transport.rsrsworkspace/crates/mcp-nats/src/transport.rsrsworkspace/crates/mcp-nats/tests/transport.rsrsworkspace/crates/trogon-nats/README.mdrsworkspace/crates/trogon-nats/src/mocks.rs
✅ Files skipped from review due to trivial changes (3)
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/one_server.rs
- rsworkspace/crates/mcp-nats-server/README.md
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/initialize.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/tool_list_changed.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/subscriptions/all_client.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/cancelled.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/read_resource.rs
- rsworkspace/Cargo.toml
- rsworkspace/crates/mcp-nats/src/client.rs
- rsworkspace/crates/trogon-nats/src/mocks.rs
- rsworkspace/crates/mcp-nats/src/nats/subjects/server/elicitation_completed.rs
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
4ea6501 to
3e97dc4
Compare
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
2 times, most recently
from
02dadf4 to
b7336c7
Compare
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
2a0d5f3 to
1573082
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1573082. Configure here.
yordis
force-pushed
the
yordis/feat-nats-mcp
branch
from
1573082 to
8984367
Compare
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>
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/feat-nats-mcp
branch
from
cebcd73 to
da81775
Compare
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
yordis
added
the
rust:coverage-baseline-reset
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>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
Signed-off-by: Yordis Prieto <yordis.prieto@gmail.com>
mariorha
added a commit
that referenced
this pull request
May 12, 2026
Resolves all conflicts from 5 commits: - refactor(gateway): consolidate source adapters (#153) - feat(mcp-nats): support distributed MCP (#149) - refactor(telemetry): generalize service instrumentation (#152) - fix(signal): share shutdown boundary (#151) - fix(acp-nats): preserve typed NATS subject contracts (#150) Key decisions: - trogon-source-* Cargo.toml files deleted (accepted gateway consolidation) - acp-nats-server: accepted origin/main (registry routing removed by fix #150) - trogon-telemetry: generic resource_attributes API accepted (fix #152) - service_name.rs: combined AcpNatsWs + McpNatsStdio/McpNatsServer variants - Platform crates migrated from acp-telemetry to trogon-telemetry - signal::shutdown_signal moved from acp-telemetry to trogon-std - Orphaned trogon-source-* dirs excluded from workspace (tests preserved)
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.