feat(acp-nats): minimal initialize-only ACP-NATS bridge

[yordis]

Summary

Minimal ACP-NATS bridge that implements only the initialize handler. Other Agent trait methods are stubbed with "not yet implemented".

Scope

• Bridge: NATS-backed Agent implementation with initialize fully functional
• Config: acp_prefix, operation_timeout; prefix validation and subject capacity checks
• Subjects: Only agent::initialize(prefix) -> {prefix}.agent.initialize
• Error: log_and_map_nats_error for consistent NATS error handling
• Tests: 2 initialize tests + 9 config validation tests

Deferred to follow-up PRs

• Metrics/telemetry
• Client module, session handlers, prompt, cancel, ext_method, ext_notification
• validate_session_id, validate_method_name
• Config fields: prompt_timeout, max_concurrent_client_tasks, max_nats_payload_bytes
• Other subject builders (client, agent session, wildcards)
• acp-nats-stdio binary

Verification

• cargo check -p acp-nats
• cargo test -p acp-nats
• cargo clippy -p acp-nats -- -D warnings

[cursor]

PR Summary

Medium Risk
Adds a new NATS-facing integration layer and changes release panic strategy to unwinding, which can affect runtime behavior and binary characteristics across the workspace.

Overview
Introduces a new acp-nats crate that implements an ACP Agent bridge backed by NATS, with initialize fully wired to request_with_timeout on the {prefix}.agent.initialize subject and explicit mapping of trogon-nats::NatsError into ACP error codes (including retryable AGENT_UNAVAILABLE).

Adds validated configuration (AcpPrefix, operation timeout) and a small subject builder module, plus tests covering prefix validation and initialize success/error paths; other Agent methods are stubbed as "not yet implemented".

Workspace-level changes remove panic = "abort" from release profile (to allow catch_unwind usage in bridge crates) and extend AGENTS.md conventions; trogon-nats only gets minor test/comment cleanup.

^{Written by Cursor Bugbot for commit 9b48358. This will update automatically on new commits. Configure here.}

[coderabbitai]

Warning

Rate limit exceeded

@yordis has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 0 minutes and 14 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 44dec1e and 9b48358.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (11)

• rsworkspace/Cargo.toml
• rsworkspace/crates/AGENTS.md
• rsworkspace/crates/acp-nats/Cargo.toml
• rsworkspace/crates/acp-nats/src/agent/initialize.rs
• rsworkspace/crates/acp-nats/src/agent/mod.rs
• rsworkspace/crates/acp-nats/src/config.rs
• rsworkspace/crates/acp-nats/src/error.rs
• rsworkspace/crates/acp-nats/src/lib.rs
• rsworkspace/crates/acp-nats/src/nats/mod.rs
• rsworkspace/crates/acp-nats/src/nats/subjects.rs
• rsworkspace/crates/trogon-nats/src/connect.rs

Walkthrough

Introduces a new acp-nats crate enabling NATS-based agent client protocol support with configuration validation, a Bridge agent implementation, and initialization request handling. Updates workspace panic configuration and documents module conventions for telemetry separation.

Changes

Cohort / File(s)	Summary
Workspace Configuration rsworkspace/Cargo.toml	Removed per-package panic abort override in release profile; clarified that bridge crates use catch_unwind for isolation and workspace-wide unwinding will be used.
Documentation rsworkspace/crates/AGENTS.md	Added module conventions section recommending a telemetry submodule to separate observability from domain logic; enhanced validation guidance.
Core Crate Setup rsworkspace/crates/acp-nats/Cargo.toml, rsworkspace/crates/acp-nats/src/lib.rs	New acp-nats crate with dependencies on agent-client-protocol, async-nats, tokio, tracing, and trogon-nats; establishes public API exports for Bridge, Config, validation, and NATS client traits.
Configuration Module rsworkspace/crates/acp-nats/src/config.rs	Introduces ValidationError, AcpPrefix (with validation rules for wildcards, whitespace, boundary dots), and Config structs with operation timeout configuration and builder-style customization.
Agent Bridge & Initialization rsworkspace/crates/acp-nats/src/agent/mod.rs, rsworkspace/crates/acp-nats/src/agent/initialize.rs	Implements Bridge generic struct wrapping NATS client traits; delegates initialize to a handler that forwards requests with timeout and error mapping; stubs remaining Agent methods as "not yet implemented."
NATS Subject Utilities rsworkspace/crates/acp-nats/src/nats/mod.rs, rsworkspace/crates/acp-nats/src/nats/subjects.rs	Introduces subjects module with agent submodule exposing an initialize function that constructs prefixed NATS subject names.
Minor Cleanup rsworkspace/crates/trogon-nats/src/connect.rs	Removed documentation comment above connect function; cleaned up inline comments in unit tests.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant Bridge
    participant NATS as NATS Broker
    participant Agent

    Client->>Bridge: initialize(request)
    Note over Bridge: Logs client name<br/>Builds subject from AcpPrefix
    Bridge->>NATS: request_with_timeout<br/>(subject, args, timeout)
    NATS->>Agent: forward request
    Agent->>NATS: send InitializeResponse
    NATS->>Bridge: response (with timeout/error handling)
    alt Success
        Bridge->>Bridge: map response
        Bridge->>Client: Ok(InitializeResponse)
    else Error
        Bridge->>Bridge: map_initialize_error<br/>(NatsError)
        Bridge->>Client: Err(Error with code)
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• feat(trogon-nats): add trogon-nats pkg #4: Directly consumed by this PR; acp-nats depends on trogon-nats types (RequestClient, PublishClient, FlushClient) and utilities (request_with_timeout, NatsConfig, NatsAuth).

Poem

🐰 A bridge to agents, built with care,
Through NATS the messages now flow fair,
With prefixes validated, timeouts set just right,
The protocol speaks—initialization takes flight! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 41.18% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'feat(acp-nats): minimal initialize-only ACP-NATS bridge' directly and specifically describes the main change—introducing a minimal NATS-backed ACP Agent bridge focused on the initialize handler.
Description check	✅ Passed	The description is well-related to the changeset, providing a clear summary of the minimal initialize-only ACP-NATS bridge, scope of changes, deferred items, and verification steps performed.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch add-acp-nats-init

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[coderabbitai]

🧹 Nitpick comments (3)

rsworkspace/crates/acp-nats/src/agent/mod.rs (1)

34-90: Consolidate repeated “not yet implemented” errors.

A small helper keeps these stubs consistent and easier to update later.

♻️ Suggested refactor

+fn not_implemented() -> Error {
+    Error::new(ErrorCode::InternalError.into(), "not yet implemented")
+}
+
 #[async_trait::async_trait(?Send)]
 impl<N: RequestClient + PublishClient + FlushClient> Agent for Bridge<N> {
     async fn initialize(&self, args: InitializeRequest) -> Result<InitializeResponse> {
         initialize::handle(self, args).await
     }
 
     async fn authenticate(&self, _args: AuthenticateRequest) -> Result<AuthenticateResponse> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 
     async fn new_session(&self, _args: NewSessionRequest) -> Result<NewSessionResponse> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 
     async fn load_session(&self, _args: LoadSessionRequest) -> Result<LoadSessionResponse> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 
     async fn set_session_mode(
         &self,
         _args: SetSessionModeRequest,
     ) -> Result<SetSessionModeResponse> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 
     async fn prompt(&self, _args: PromptRequest) -> Result<PromptResponse> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 
     async fn cancel(&self, _args: CancelNotification) -> Result<()> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 
     async fn ext_method(&self, _args: ExtRequest) -> Result<ExtResponse> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 
     async fn ext_notification(&self, _args: ExtNotification) -> Result<()> {
-        Err(Error::new(
-            ErrorCode::InternalError.into(),
-            "not yet implemented",
-        ))
+        Err(not_implemented())
     }
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@rsworkspace/crates/acp-nats/src/agent/mod.rs` around lines 34 - 90, Multiple
RPC stubs repeat the same Err(Error::new(ErrorCode::InternalError.into(), "not
yet implemented")); create a small helper (e.g., fn not_implemented_err() ->
Error or async fn not_implemented<T>() -> Result<T, Error>) and replace the
repeated expressions in authenticate, new_session, load_session,
set_session_mode, prompt, cancel, ext_method, and ext_notification with calls to
that helper so all stubs are consistent and easier to update later.

rsworkspace/crates/acp-nats/src/config.rs (1)

15-24: Avoid hard-coded max length in the error message.

Keeps the display text consistent if the constant changes.

♻️ Suggested refactor

-            ValidationError::TooLong(field, len) => {
-                write!(f, "{} is too long: {} bytes (max 128)", field, len)
-            }
+            ValidationError::TooLong(field, len) => {
+                write!(f, "{} is too long: {} bytes (max {})", field, len, MAX_PREFIX_LENGTH)
+            }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@rsworkspace/crates/acp-nats/src/config.rs` around lines 15 - 24, The Display
impl for ValidationError hard-codes "128" in the TooLong message; update the
write! call inside impl std::fmt::Display for ValidationError (the match arm for
ValidationError::TooLong(field, len)) to use the module constant (e.g., MAX_LEN
or whatever constant defines the max length in this module) instead of the
literal 128, e.g. change the format string to include the constant and pass it
as an argument so the message reads "{} is too long: {} bytes (max {})", field,
len, MAX_LEN.

rsworkspace/crates/acp-nats/src/agent/initialize.rs (1)

86-132: Avoid hard-coded subject strings in tests.

Using the subject builder keeps tests aligned with the production format.

♻️ Suggested refactor

     #[tokio::test]
     async fn initialize_forwards_request_and_returns_response() {
         let (mock, bridge) = mock_bridge();
         let expected = InitializeResponse::new(ProtocolVersion::LATEST);
-        set_json_response(&mock, "acp.agent.initialize", &expected);
+        let subject = crate::nats::agent::initialize("acp");
+        set_json_response(&mock, &subject, &expected);
 
         let request = InitializeRequest::new(ProtocolVersion::LATEST);
         let result = bridge.initialize(request).await;
@@
     async fn initialize_logs_client_name_when_client_info_provided() {
         let (mock, bridge) = mock_bridge();
         let expected = InitializeResponse::new(ProtocolVersion::LATEST);
-        set_json_response(&mock, "acp.agent.initialize", &expected);
+        let subject = crate::nats::agent::initialize("acp");
+        set_json_response(&mock, &subject, &expected);
@@
     async fn initialize_returns_error_when_response_is_invalid_json() {
         let (mock, bridge) = mock_bridge();
-        mock.set_response("acp.agent.initialize", "not json".into());
+        let subject = crate::nats::agent::initialize("acp");
+        mock.set_response(&subject, "not json".into());

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@rsworkspace/crates/acp-nats/src/agent/initialize.rs` around lines 86 - 132,
Tests are using the hard-coded subject string "acp.agent.initialize" (seen in
set_json_response(&mock, "acp.agent.initialize", ...),
mock.set_response("acp.agent.initialize", ...)) which can drift from production
formatting; replace those literal strings with the central subject builder used
by the codebase (the same helper used in production to construct agent subjects)
so tests derive the subject the same way as runtime (use the project's subject
builder function where you call set_json_response, mock.set_response, and any
other mock methods referencing the subject).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@rsworkspace/crates/acp-nats/src/agent/initialize.rs`:
- Around line 86-132: Tests are using the hard-coded subject string
"acp.agent.initialize" (seen in set_json_response(&mock, "acp.agent.initialize",
...), mock.set_response("acp.agent.initialize", ...)) which can drift from
production formatting; replace those literal strings with the central subject
builder used by the codebase (the same helper used in production to construct
agent subjects) so tests derive the subject the same way as runtime (use the
project's subject builder function where you call set_json_response,
mock.set_response, and any other mock methods referencing the subject).

In `@rsworkspace/crates/acp-nats/src/agent/mod.rs`:
- Around line 34-90: Multiple RPC stubs repeat the same
Err(Error::new(ErrorCode::InternalError.into(), "not yet implemented")); create
a small helper (e.g., fn not_implemented_err() -> Error or async fn
not_implemented<T>() -> Result<T, Error>) and replace the repeated expressions
in authenticate, new_session, load_session, set_session_mode, prompt, cancel,
ext_method, and ext_notification with calls to that helper so all stubs are
consistent and easier to update later.

In `@rsworkspace/crates/acp-nats/src/config.rs`:
- Around line 15-24: The Display impl for ValidationError hard-codes "128" in
the TooLong message; update the write! call inside impl std::fmt::Display for
ValidationError (the match arm for ValidationError::TooLong(field, len)) to use
the module constant (e.g., MAX_LEN or whatever constant defines the max length
in this module) instead of the literal 128, e.g. change the format string to
include the constant and pass it as an argument so the message reads "{} is too
long: {} bytes (max {})", field, len, MAX_LEN.

---

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5f80ac and 44dec1e.

⛔ Files ignored due to path filters (1)

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

📒 Files selected for processing (10)

• rsworkspace/Cargo.toml
• rsworkspace/crates/AGENTS.md
• rsworkspace/crates/acp-nats/Cargo.toml
• rsworkspace/crates/acp-nats/src/agent/initialize.rs
• rsworkspace/crates/acp-nats/src/agent/mod.rs
• rsworkspace/crates/acp-nats/src/config.rs
• rsworkspace/crates/acp-nats/src/lib.rs
• rsworkspace/crates/acp-nats/src/nats/mod.rs
• rsworkspace/crates/acp-nats/src/nats/subjects.rs
• rsworkspace/crates/trogon-nats/src/connect.rs