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/locks; moves dialect runtime execution into the nREPL layer (dialects now only provide expressions); introduces optional :project-dir startup path (direct config load) and small CLI/start wrappers; makes tools and tests 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 rgb(220,240,220)
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 rgb(240,220,220)
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.
| :aliases | ||
|
|
||
| {:mcp | ||
| {:exec-fn clojure-mcp.main/start-mcp-server |
There was a problem hiding this comment.
Update the exec-fn to match the new unified start entry point.
The :dev-mcp alias (line 72) was updated to use clojure-mcp.main/start, but this :mcp alias still references clojure-mcp.main/start-mcp-server. Per the PR objectives introducing a unified start entry point, this should be updated to start for consistency.
Apply this diff:
- {:exec-fn clojure-mcp.main/start-mcp-server
+ {:exec-fn clojure-mcp.main/start📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {:exec-fn clojure-mcp.main/start-mcp-server | |
| {:exec-fn clojure-mcp.main/start |
🤖 Prompt for AI Agents
In deps.edn around line 39 (alias :mcp), the :exec-fn is still set to
clojure-mcp.main/start-mcp-server; update that value to clojure-mcp.main/start
so the :mcp alias uses the new unified start entry point (mirror the change made
for :dev-mcp), ensuring the alias references clojure-mcp.main/start instead of
start-mcp-server.
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.
src/clojure_mcp/tools/eval/tool.clj
Outdated
| (let [service @nrepl-client-atom | ||
| project-dir (config/get-nrepl-user-dir service) | ||
| effective-port (or port | ||
| (:port service) | ||
| (read-nrepl-port-file project-dir))] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Extract duplicated effective-port determination logic.
The logic to determine the effective port (from provided port, configured port, or .nrepl-port file) is duplicated between validate-inputs (lines 90-94) and execute-tool (lines 109-111). This duplication could lead to inconsistency if one location is updated without the other.
Extract this logic into a helper function:
(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)
(read-nrepl-port-file (config/get-nrepl-user-dir service))))Then use it in both locations:
(defmethod tool-system/validate-inputs ::clojure-eval [{:keys [nrepl-client-atom]} inputs]
(let [{:keys [code timeout_ms port]} inputs]
...
;; Check that a port is available
(let [service @nrepl-client-atom
- project-dir (config/get-nrepl-user-dir service)
- effective-port (or port
- (:port service)
- (read-nrepl-port-file project-dir))]
+ effective-port (determine-effective-port service port)]
(when-not effective-port
(throw (ex-info "No nREPL port available..."
- {:inputs inputs
- :project-dir project-dir}))))
+ {:inputs inputs
+ :project-dir (config/get-nrepl-user-dir service)}))))
inputs))
(defmethod tool-system/execute-tool ::clojure-eval [{:keys [nrepl-client-atom timeout session-type]}
{:keys [timeout_ms port] :as inputs}]
(let [base-client @nrepl-client-atom
- project-dir (config/get-nrepl-user-dir base-client)
- effective-port (or port
- (:port base-client)
- (read-nrepl-port-file project-dir))]
+ effective-port (determine-effective-port base-client port)]
...))Also applies to: 109-111
🤖 Prompt for AI Agents
In src/clojure_mcp/tools/eval/tool.clj around lines 90-94 and 109-111, the
effective-port selection logic is duplicated; extract it into a private helper
(e.g., determine-effective-port) that accepts the service and provided-port and
returns (or provided-port, (:port service), (read-nrepl-port-file
(config/get-nrepl-user-dir service))); replace the duplicated expressions in
validate-inputs and execute-tool with calls to this helper so both sites share
the same logic.
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.
| (defn- get-lock-for-port | ||
| "Gets or creates a lock object for the given port." | ||
| [port] | ||
| (or (get @init-locks port) | ||
| (let [lock (Object.)] | ||
| (swap! init-locks (fn [locks] | ||
| (if (contains? locks port) | ||
| locks | ||
| (assoc locks port lock)))) | ||
| (get @init-locks port)))) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/clojure_mcp/nrepl.clj | sed -n '298,315p'Repository: bhauman/clojure-mcp
Length of output: 843
🏁 Script executed:
rg -nP --type=clj -C3 '\b(future|thread|concurrent|parallel)\b' src/Repository: bhauman/clojure-mcp
Length of output: 90
🏁 Script executed:
find . -name "*test*.clj" -o -name "*_test.clj" | head -20Repository: bhauman/clojure-mcp
Length of output: 976
🏁 Script executed:
cat test/clojure_mcp/nrepl_launcher_test.clj | head -100Repository: bhauman/clojure-mcp
Length of output: 4318
🏁 Script executed:
rg -n "get-lock-for-port" src/clojure_mcp/nrepl.cljRepository: bhauman/clojure-mcp
Length of output: 136
🏁 Script executed:
rg -n "future\|pmap\|async" test/clojure_mcp/nrepl_launcher_test.cljRepository: bhauman/clojure-mcp
Length of output: 45
🏁 Script executed:
cat -n src/clojure_mcp/nrepl.clj | sed -n '340,350p'Repository: bhauman/clojure-mcp
Length of output: 444
🏁 Script executed:
rg -B5 -A5 "get-lock-for-port" src/clojure_mcp/nrepl.cljRepository: bhauman/clojure-mcp
Length of output: 684
Simplify lock retrieval to eliminate unnecessary object creation.
The get-lock-for-port function may create redundant lock objects under concurrent access. When multiple threads call it simultaneously for the same port, each creates a lock object before any swap! occurs. While the swap's internal check ensures only one lock is stored per port, this still wastes the discarded objects.
Additionally, the final (get @init-locks port) on line 311 retrieves from the atom after swap! completes, which is safe but creates a small window of redundancy.
Simplify by returning the lock directly from the swap result:
(defn- get-lock-for-port
"Gets or creates a lock object for the given port."
[port]
(let [lock (Object.)]
(get (swap! init-locks
(fn [locks]
(if (contains? locks port)
locks
(assoc locks port lock))))
port)))This ensures the returned lock is guaranteed to be in the atom and eliminates the redundant second dereference.
🤖 Prompt for AI Agents
In src/clojure_mcp/nrepl.clj around lines 302 to 311, avoid creating redundant
lock objects under concurrent access by allocating the lock before the swap! and
returning the lock from the swap! result instead of dereferencing the atom
again; specifically, create a lock object, call (swap! init-locks (fn [locks]
(if (contains? locks port) locks (assoc locks port lock)))) and then return (get
swapped-map port) (i.e., the value from the swap! return) so only one lock is
retained and no extra dereference is needed.
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.