Add lazy per-port nREPL initialization with graceful degradation #135
Conversation
- Add lazy initialization of nREPL connections per-port in nrepl.clj - Create unified start entry point in main.clj - Add nrepl-available? check to bash tool for graceful fallback - Add nrepl-available? check to inspect-project for graceful degradation - Fix test and lint commands in CLAUDE.md
Catch ConnectException and SocketException in execute-tool and return a clear error message that includes the port number and suggests checking that an nREPL server is running.
WalkthroughAdds lazy, per-port nREPL initialization and per-port state; moves dialect runtime execution into the nREPL layer (dialects now only provide expressions); introduces optional :project-dir startup path and lightweight start wrappers; updates tools and tests to be nREPL-port-aware. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / Tool
participant Core as core.clj
participant nREPL as nrepl.clj
participant Dialects as dialects.clj
participant REPL as Remote nREPL
rect rgba(220,240,220,0.6)
Note over User,Core: Start with :project-dir provided
User->>Core: start/build-and-start-mcp-server(opts with :project-dir)
Core->>nREPL: create-and-start-nrepl-connection(:project-dir)
Core->>Core: load config from project-dir (no REPL)
end
rect rgba(240,220,220,0.6)
Note over User,REPL: Start without :project-dir (lazy)
User->>Core: start/build-and-start-mcp-server(opts without project-dir)
Core->>nREPL: create-and-start-nrepl-connection(no project-dir)
Core->>nREPL: ensure-port-if-needed()
Note over User,nREPL: Initialization deferred until first eval/use
User->>nREPL: eval-code() or tool request (first use)
nREPL->>nREPL: ensure-port-initialized! (acquire per-port lock)
nREPL->>REPL: describe() — detect env type
REPL-->>nREPL: env-type
nREPL->>Dialects: fetch-project-directory-exp(env-type)
Dialects-->>nREPL: expression template
nREPL->>REPL: eval(expression) — get project-dir
REPL-->>nREPL: project-dir
nREPL->>nREPL: initialize-environment / load-repl-helpers (eval helpers)
nREPL->>nREPL: mark port initialized (cache env-type, project-dir)
nREPL->>User: perform requested eval/command (using initialized port)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~55 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Add start function to sse_main.clj that sets project-dir from CWD - Update :mcp-sse alias to use new start function (port now optional) - Update :mcp and :dev-mcp aliases to use lazy initialization
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
deps.edn (2)
40-40: Update the comment to reflect that port is now optional.The comment states "it needs an nrepl port to talk to," but per the PR objectives, port is now optional when project-dir is provided, and initialization is lazy.
Consider updating to:
- ;; it needs an nrepl port to talk to + ;; optional nrepl port for REPL connection (uses lazy initialization)
73-73: Update the comment to reflect that port is now optional.The comment states "it needs an nrepl port to talk to," but this is outdated given the lazy initialization changes.
Consider updating to:
- ;; it needs an nrepl port to talk to + ;; optional nrepl port for REPL connection (uses lazy initialization)src/clojure_mcp/sse_main.clj (1)
37-41: Consider dissoc-ing :not-cwd before delegating.The :not-cwd key is passed through to start-sse-mcp-server and eventually to lower layers where it's not needed. While harmless if ignored, removing internal control flags improves clarity.
Apply this diff to clean up the control flag:
(defn start [opts] (let [not-cwd? (get opts :not-cwd false) opts' (if not-cwd? opts - (assoc opts :project-dir (System/getProperty "user.dir")))] - (start-sse-mcp-server opts'))) + (assoc opts :project-dir (System/getProperty "user.dir"))) + opts'' (dissoc opts' :not-cwd)] + (start-sse-mcp-server opts'')))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
deps.edn(2 hunks)src/clojure_mcp/sse_main.clj(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{clj,cljc}: Use:requirewith ns aliases for imports (e.g.,[clojure.string :as string])
Include clear tool:descriptionfor LLM guidance
Validate inputs and provide helpful error messages in MCP tools
Return structured data with both result and error status in MCP tools
Maintain atom-based state for consistent service access in MCP tools
Files:
src/clojure_mcp/sse_main.clj
**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{clj,cljc}: Use kebab-case for vars/functions; end predicates with?(e.g.,is-top-level-form?)
Usetry/catchwith specific exception handling; use atom for tracking errors
Use 2-space indentation and maintain whitespace in edited forms
Align namespaces with directory structure (e.g.,clojure-mcp.repl-tools)
Files:
src/clojure_mcp/sse_main.clj
🧠 Learnings (4)
📓 Common learnings
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Maintain atom-based state for consistent service access in MCP tools
Applied to files:
deps.edn
📚 Learning: 2025-08-18T00:39:24.837Z
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
Applied to files:
deps.edn
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Return structured data with both result and error status in MCP tools
Applied to files:
deps.edn
🔇 Additional comments (3)
deps.edn (2)
48-49: LGTM!The function rename to
clojure-mcp.sse-main/startaligns with the unified start entry point pattern, and using:mcp-sse-portclearly distinguishes the SSE server port from the nREPL port.
72-72: LGTM!The function rename to
clojure-mcp.main/start, commenting out the port, and adding:enable-logging? trueall align with the PR objectives of lazy initialization and making port optional.Also applies to: 74-75
src/clojure_mcp/sse_main.clj (1)
24-41: LGTM! Clear implementation of lazy initialization entry point.The function correctly implements the lazy REPL initialization pattern documented in the PR objectives. The logic properly defaults :project-dir to the current working directory while allowing users to opt out via :not-cwd. The docstring clearly documents the behavior and supported options.
When no port is provided or configured, the eval tool now checks for a .nrepl-port file in the project directory. This allows automatic connection to any nREPL server that writes its port to this standard location.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/clojure_mcp/tools/eval/tool.clj (3)
11-22: Add port range validation to prevent invalid port numbers.The function parses the port but doesn't validate that it's within the valid port range (1-65535). Invalid port numbers could cause connection failures later with less clear error messages.
Apply this diff to add port validation:
(defn- read-nrepl-port-file "Reads the .nrepl-port file from the given directory. Returns the port number if found and valid, nil otherwise." [dir] (when dir (let [port-file (io/file dir ".nrepl-port")] (when (.exists port-file) (try - (-> (slurp port-file) - string/trim - Integer/parseInt) + (let [port (-> (slurp port-file) + string/trim + Integer/parseInt)] + (when (and (pos-int? port) (<= port 65535)) + port)) (catch Exception _ nil))))))
86-88: Validate port number is within valid range.The validation checks that port is a positive integer but doesn't enforce the valid port range (1-65535). This is inconsistent with the validation added to
read-nrepl-port-file(if that suggestion is applied).Apply this diff to add upper bound validation:
- (when (and port (not (pos-int? port))) - (throw (ex-info (str "Error parameter must be positive integer: port " (pr-str inputs)) + (when (and port (not (and (pos-int? port) (<= port 65535)))) + (throw (ex-info (str "Error parameter must be valid port number (1-65535): port " (pr-str inputs)) {:inputs inputs})))
113-119: Clarify client selection logic with a comment.The conditional logic for selecting whether to use
with-port-initializedorensure-port-initialized!is not immediately clear. The condition(or port (not (:port base-client)))could benefit from an explanatory comment.Consider adding a clarifying comment:
(try - (let [client (if (or port (not (:port base-client))) - ;; Use specified port or discovered port - (nrepl/with-port-initialized base-client effective-port) - (do - ;; For default port, ensure it's initialized (should already be, but safe) - (nrepl/ensure-port-initialized! base-client) - base-client))] + (let [client (if (or port (not (:port base-client))) + ;; Non-default port case: user specified a port OR no default port configured. + ;; Create/get a port-specific client via lazy initialization. + (nrepl/with-port-initialized base-client effective-port) + ;; Default port case: port matches configured default. + ;; Ensure default port is initialized and return base client. + (do + (nrepl/ensure-port-initialized! base-client) + base-client))]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/clojure_mcp/tools/eval/tool.clj(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{clj,cljc}: Use:requirewith ns aliases for imports (e.g.,[clojure.string :as string])
Include clear tool:descriptionfor LLM guidance
Validate inputs and provide helpful error messages in MCP tools
Return structured data with both result and error status in MCP tools
Maintain atom-based state for consistent service access in MCP tools
Files:
src/clojure_mcp/tools/eval/tool.clj
**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{clj,cljc}: Use kebab-case for vars/functions; end predicates with?(e.g.,is-top-level-form?)
Usetry/catchwith specific exception handling; use atom for tracking errors
Use 2-space indentation and maintain whitespace in edited forms
Align namespaces with directory structure (e.g.,clojure-mcp.repl-tools)
Files:
src/clojure_mcp/tools/eval/tool.clj
🧠 Learnings (5)
📓 Common learnings
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to **/*.{clj,cljc} : Align namespaces with directory structure (e.g., `clojure-mcp.repl-tools`)
📚 Learning: 2025-08-18T00:39:24.837Z
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
Applied to files:
src/clojure_mcp/tools/eval/tool.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to **/*.{clj,cljc} : Align namespaces with directory structure (e.g., `clojure-mcp.repl-tools`)
Applied to files:
src/clojure_mcp/tools/eval/tool.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Prefer REPL-driven development for rapid iteration and feedback
Applied to files:
src/clojure_mcp/tools/eval/tool.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Use `:require` with ns aliases for imports (e.g., `[clojure.string :as string]`)
Applied to files:
src/clojure_mcp/tools/eval/tool.clj
🔇 Additional comments (1)
src/clojure_mcp/tools/eval/tool.clj (1)
124-131: Good error handling for connection failures.The exception handling for
ConnectExceptionandSocketExceptionprovides clear, actionable error messages that include the port number and suggest troubleshooting steps. This aligns well with the graceful degradation objectives.
Move port resolution logic (provided, configured, or .nrepl-port file) to validate-inputs and pass the resolved port to execute-tool via the inputs map. This removes duplicate port resolution code.
- Move read-nrepl-port-file to nrepl.clj as shared utility - Update eval tool to discover port from .nrepl-port when not configured - Update bash tool to use discovered port for nREPL execution - Update inspect-project to fetch describe info using discovered port - Update eval tool test to expect resolved port in validate-inputs
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/clojure_mcp/tools/eval/tool.clj (1)
74-85: Extract duplicated port resolution logic.The port resolution logic at lines 77-79 is duplicated in
execute-tool. As noted in previous review comments, this duplication should be extracted into a helper function to ensure consistency if the logic needs to be updated.Consider extracting into a helper function as suggested in the previous review:
(defn- determine-effective-port "Determines the effective port from multiple sources in priority order: 1. Explicitly provided port 2. Configured port from service 3. Port from .nrepl-port file in project directory" [service provided-port] (or provided-port (:port service) (nrepl/read-nrepl-port-file (config/get-nrepl-user-dir service))))Then use it in both locations to eliminate duplication.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/clojure_mcp/nrepl.clj(5 hunks)src/clojure_mcp/tools/bash/core.clj(2 hunks)src/clojure_mcp/tools/bash/tool.clj(2 hunks)src/clojure_mcp/tools/eval/tool.clj(3 hunks)src/clojure_mcp/tools/project/core.clj(2 hunks)test/clojure_mcp/tools/eval/tool_test.clj(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{clj,cljc}: Use kebab-case for vars/functions; end predicates with?(e.g.,is-top-level-form?)
Usetry/catchwith specific exception handling; use atom for tracking errors
Use 2-space indentation and maintain whitespace in edited forms
Align namespaces with directory structure (e.g.,clojure-mcp.repl-tools)
Files:
test/clojure_mcp/tools/eval/tool_test.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/tools/eval/tool.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/nrepl.clj
test/**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
deftestwith descriptive names;testingfor subsections;isfor assertions in tests
Files:
test/clojure_mcp/tools/eval/tool_test.clj
src/**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{clj,cljc}: Use:requirewith ns aliases for imports (e.g.,[clojure.string :as string])
Include clear tool:descriptionfor LLM guidance
Validate inputs and provide helpful error messages in MCP tools
Return structured data with both result and error status in MCP tools
Maintain atom-based state for consistent service access in MCP tools
Files:
src/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/tools/eval/tool.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/nrepl.clj
🧠 Learnings (9)
📓 Common learnings
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Validate inputs and provide helpful error messages in MCP tools
Applied to files:
test/clojure_mcp/tools/eval/tool_test.cljsrc/clojure_mcp/tools/bash/tool.clj
📚 Learning: 2025-08-18T00:39:24.837Z
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
Applied to files:
src/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/tools/eval/tool.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/nrepl.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to **/*.{clj,cljc} : Align namespaces with directory structure (e.g., `clojure-mcp.repl-tools`)
Applied to files:
src/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/eval/tool.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Return structured data with both result and error status in MCP tools
Applied to files:
src/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/project/core.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Maintain atom-based state for consistent service access in MCP tools
Applied to files:
src/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/nrepl.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Include clear tool `:description` for LLM guidance
Applied to files:
src/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/project/core.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Prefer REPL-driven development for rapid iteration and feedback
Applied to files:
src/clojure_mcp/tools/eval/tool.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Use `:require` with ns aliases for imports (e.g., `[clojure.string :as string]`)
Applied to files:
src/clojure_mcp/tools/eval/tool.clj
🔇 Additional comments (21)
test/clojure_mcp/tools/eval/tool_test.clj (1)
60-64: LGTM! Test correctly validates port resolution.The test expectations properly verify that
validate-inputsnow returns both the code string and a resolved port as a positive integer, aligning with the per-port evaluation changes introduced in this PR.src/clojure_mcp/tools/bash/tool.clj (2)
6-6: LGTM! Import required for port-file reading.The nREPL namespace import is necessary for the
read-nrepl-port-filefunction used in the port resolution logic below.
118-130: LGTM! Graceful degradation for bash tool.The execute-tool implementation correctly implements graceful degradation by:
- Resolving the effective port from configured port or .nrepl-port file
- Using nREPL execution only when both session-type and port are available
- Falling back to local execution when no port is configured
This aligns with the PR's objectives for graceful degradation.
src/clojure_mcp/tools/project/core.clj (4)
186-187: LGTM! Enables Babashka detection without REPL.Reading
bb.ednunconditionally allows project inspection to detect Babashka projects via file presence, supporting graceful degradation when no REPL connection is available.
345-348: LGTM! Guard clause prevents invalid configuration.The guard clause properly checks for required configuration before attempting project inspection, returning a clear error message when configuration is missing.
350-356: LGTM! Graceful degradation for project inspection.The port resolution and conditional
describe-infofetching correctly implement graceful degradation:
- Only attempts to fetch version information when a port is available
- Allows project inspection to proceed without a REPL connection
- Passes potentially nil describe-info to
format-project-infoThe code properly supports the PR's graceful degradation objectives.
358-361: LGTM! format-project-info handles nil describe-info.The call to
format-project-infowith potentially nildescribe-infois safe, as the formatting function useswhenguards (lines 215-223) to conditionally include version information only when available.src/clojure_mcp/tools/bash/core.clj (2)
169-169: LGTM! Port parameter added for per-port evaluation.The function signature correctly adds
portto the destructured parameters, enabling per-port nREPL evaluation as required by the lazy initialization changes.
182-186: LGTM! Port override logic is clear and correct.The conditional port override using
cond->is idiomatic and clearly implements the intended behavior: using the provided port when available, otherwise using the client's configured port.src/clojure_mcp/tools/eval/tool.clj (4)
5-7: LGTM! Required imports for port-aware evaluation.The imports for
configandnreplnamespaces are necessary for the port resolution and initialization logic introduced in this PR.
32-33: LGTM! Clear documentation for port parameter.The PORT PARAMETER documentation clearly explains the use case for evaluating on different nREPL servers and mentions lazy initialization behavior, providing helpful guidance for tool users.
59-60: LGTM! Schema correctly defines port property.The schema properly defines the
portproperty as an optional integer with a clear description of its purpose and use case.
87-104: LGTM! Excellent error handling for connection failures.The execute-tool implementation properly:
- Uses lazy port initialization via
with-port-initialized- Delegates to the core evaluation function with appropriate options
- Catches connection exceptions and returns structured error responses with clear, actionable messages
The error messages help users understand connection failures and guide them toward resolution.
src/clojure_mcp/nrepl.clj (8)
3-9: LGTM! Necessary imports for per-port functionality.The added imports support the new functionality for port file reading, dialect detection, and initialization logging introduced in this PR.
15-22: LGTM! Well-structured port entry model.The
make-port-entryfunction properly initializes per-port state with all necessary fields for tracking sessions, namespaces, environment type, initialization status, and project directory.
24-32: LGTM! Create function supports optional port initialization.The updated
createfunction properly supports lazy per-port initialization by conditionally creating an initial port entry when a port is provided, while remaining backward compatible when port is nil.
99-152: LGTM! Consistent per-port state accessors.The per-port state accessor functions follow Clojure naming conventions and provide a clean API for managing port-specific state (env-type, initialized?, project-dir). The
ensure-port-entry!function properly handles lazy port entry creation.
179-199: LGTM! Clean separation for lazy initialization.The refactor properly separates:
eval-code-internal: for use during initialization (avoids circular calls)eval-code: public API that triggers lazy initializationThe forward declaration is necessary and clearly documented. This design prevents initialization loops while ensuring ports are initialized before use.
225-293: LGTM! Dialect functions moved to break circular dependencies.The dialect-aware functions are correctly moved to the nREPL namespace to avoid circular dependencies. The use of
eval-code-internalin these functions properly avoids triggering full initialization during the initialization process itself, preventing infinite loops.
313-366: LGTM! Well-designed lazy initialization with proper locking.The initialization functions properly implement lazy, thread-safe per-port initialization:
detect-and-store-env-type!: idempotent environment detectioninitialize-port!: runs initialization steps only when neededensure-port-initialized!: provides thread safety via per-port lockingwith-portandwith-port-initialized: clean API for port lifecycle managementThe separation between
with-port(entry creation) andwith-port-initialized(full init) provides flexibility while ensuring thread safety.
368-379: LGTM! Port file reading supports graceful degradation.The
read-nrepl-port-filefunction properly reads the.nrepl-portfile with appropriate error handling, returning nil on failure to support graceful degradation when port discovery fails.
The locking mechanism was unnecessary because: - swap! on atoms is already atomic - Initialization operations are idempotent - The port-initialized? check provides sufficient protection Simplified ensure-port-initialized! to use cond->.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/clojure_mcp/nrepl.clj (2)
229-238: Consider handling nil describe response explicitly.If
describereturnsnil(e.g., connection failure),detect-nrepl-env-typereturnsnilrather than:unknown. This could propagate a nil env-type through the system.(defn detect-nrepl-env-type "Detects the nREPL environment type by querying the server's describe op." [service] - (when-let [{:keys [versions]} (describe service)] - (cond - (get versions :clojure) :clj - (get versions :babashka) :bb - (get versions :basilisp) :basilisp - (get versions :sci-nrepl) :scittle - :else :unknown))) + (if-let [{:keys [versions]} (describe service)] + (cond + (get versions :clojure) :clj + (get versions :babashka) :bb + (get versions :basilisp) :basilisp + (get versions :sci-nrepl) :scittle + :else :unknown) + :unknown))
308-321: Consider error handling for partial initialization failure.If
initialize-environmentorload-repl-helpersthrows afterdetect-and-store-env-type!succeeds, the port is left withenv-typeset but not markedinitialized?. Subsequent calls will retry initialization from the environment step (which is reasonable), but if the issue is transient, the cached env-type prevents re-detection.This may be acceptable if env-type detection is idempotent and reliable. If you want stricter guarantees, consider wrapping in try/catch and clearing env-type on failure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/clojure_mcp/nrepl.clj(5 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
src/**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
src/**/*.{clj,cljc}: Use:requirewith ns aliases for imports (e.g.,[clojure.string :as string])
Include clear tool:descriptionfor LLM guidance
Validate inputs and provide helpful error messages in MCP tools
Return structured data with both result and error status in MCP tools
Maintain atom-based state for consistent service access in MCP tools
Files:
src/clojure_mcp/nrepl.clj
**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{clj,cljc}: Use kebab-case for vars/functions; end predicates with?(e.g.,is-top-level-form?)
Usetry/catchwith specific exception handling; use atom for tracking errors
Use 2-space indentation and maintain whitespace in edited forms
Align namespaces with directory structure (e.g.,clojure-mcp.repl-tools)
Files:
src/clojure_mcp/nrepl.clj
🧠 Learnings (3)
📓 Common learnings
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Maintain atom-based state for consistent service access in MCP tools
📚 Learning: 2025-08-18T00:39:24.837Z
Learnt from: hugoduncan
Repo: bhauman/clojure-mcp PR: 86
File: src/clojure_mcp/subprocess.clj:0-0
Timestamp: 2025-08-18T00:39:24.837Z
Learning: In the clojure-mcp project, the user prefers to only parse stdout for nREPL port discovery in the subprocess.clj module, and explicitly does not want to parse stderr for port information.
Applied to files:
src/clojure_mcp/nrepl.clj
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-08-02T20:23:28.499Z
Learning: Applies to src/**/*.{clj,cljc} : Maintain atom-based state for consistent service access in MCP tools
Applied to files:
src/clojure_mcp/nrepl.clj
🔇 Additional comments (8)
src/clojure_mcp/nrepl.clj (8)
1-10: LGTM!Namespace declaration follows coding guidelines with proper
:requireand idiomatic aliases (io,str,log).
15-23: LGTM!Clean factory function with sensible defaults. The
:initialized?key follows Clojure predicate naming convention.
24-32: LGTM!The conditional port entry initialization aligns with the lazy init pattern. Maintains atom-based state as per coding guidelines.
99-153: LGTM!Per-port state accessors are well-structured with consistent patterns. The
ensure-port-entry!correctly usesswap!with a conditional update to handle concurrent access atomically. Naming follows kebab-case conventions.
179-199: LGTM!Clean separation between internal eval (for initialization code) and public eval (with lazy init). The forward declaration and split prevent circular initialization calls.
261-272: LGTM with a note on canonicalization.The vector-unwrapping and quote-stripping logic correctly handles nREPL's string return format. The
getCanonicalPathcall could throw if given an invalid path, but this would surface as a clear error during initialization.
330-348: LGTM!
with-porthandles edge cases (nil port, same port) correctly. Theensure-port-entry!uses atomicswap!for safe concurrent entry creation.
349-360: LGTM!Defensive implementation with appropriate fallback to
nilfor missing or invalid port files. The silent error handling is reasonable since missing.nrepl-portfiles are expected in some workflows.
Summary
nrepl.cljstartentry point inmain.cljthat sets project-dir from CWDKey Changes
Lazy nREPL Initialization
eval-codecallenv-type,initialized?,project-dirGraceful Degradation
bashtool falls back to local execution when no default portinspect-projectworks without REPL (skips describe info)clojure_evalshows clear error when connection failsNew Entry Point
clojure-mcp.main/startsets:project-dirto CWD by defaultTest plan
clojure_evalwith valid port - lazy init worksclojure_evalwith invalid port - clear error messageclojure_inspect_projectwithout REPL - graceful degradationclojure_inspect_projectwith REPL - shows Clojure/Java versionsbashtool without REPL - local execution fallbackbashtool with REPL - executes over nREPL when configuredSummary by CodeRabbit
Documentation
New Features
Improvements
Tests
Chores
✏️ Tip: You can customize this high-level summary in your review settings.