Migrate from clojure.tools.logging to Timbre #131
Conversation
Replace clojure.tools.logging with taoensso.timbre for pure Clojure logging. Changes: - Create src/clojure_mcp/logging.clj with centralized Timbre configuration - Add logging initialization to all main entry points (main.clj, sse_main.clj, figwheel_main.clj, shadow_main.clj) - Update all 26 source files to use [taoensso.timbre :as log] - Add test/clojure_mcp/test_helper.clj to suppress logging during tests - Update :test alias in deps.edn to use -M flag with test helper - Add helpful error message when using -X:test instead of -M:test - Remove clojure.tools.logging and logback dependencies - Add slf4j-nop to main deps to suppress Java library logging - Delete dev/logback.xml (no longer needed) Benefits: - Pure Clojure logging solution (no Java dependencies) - Simplified configuration - Better REPL integration - Clean test output with zero logging noise - Consistent logging across all entry points
WalkthroughThis pull request migrates the codebase from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. 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 |
![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 (9)
src/clojure_mcp/main_examples/figwheel_main.clj (1)
40-46: LGTM: Proper logging configuration and option sanitization.The logging setup correctly:
- Configures logging before server startup
- Provides sensible defaults (logging disabled by default, debug level)
- Sanitizes options by removing logging-related keys before passing to core
Note: This pattern is duplicated in
shadow_main.clj. Consider extracting a shared helper function if more entry points are added.src/clojure_mcp/main_examples/shadow_main.clj (1)
86-92: LGTM: Consistent logging configuration pattern.The logging setup follows the same pattern as
figwheel_main.clj, properly configuring logging before server startup and sanitizing options.Note: This configuration block is identical to the one in
figwheel_main.clj(lines 40-46). Consider extracting a shared helper function if additional entry points are added.test/clojure_mcp/test_helper.clj (1)
9-17: Consider sending the -X:test error message to STDERR and avoiding emojiThe guidance for incorrect
-X:testusage is helpful, but you might want to:
- Print to
*err*instead of STDOUT.- Use plain text instead of an emoji for maximum portability in CI logs.
For example:
-(defn run-tests-with-exec - "Called when using -X:test (incorrect usage). - Prints helpful message directing user to use -M:test instead." - [& _args] - (println "\n⚠️ Error: The :test alias requires the -M flag, not -X") - (println "\nPlease run tests using:") - (println " clojure -M:test") - (println "\nThe -M flag is required to suppress logging during test runs.") - (System/exit 1)) +(defn run-tests-with-exec + "Called when using -X:test (incorrect usage). + Prints helpful message directing user to use -M:test instead." + [& _args] + (binding [*out* *err*] + (println "\nWARNING: The :test alias requires the -M flag, not -X") + (println "\nPlease run tests using:") + (println " clojure -M:test") + (println "\nThe -M flag is required to suppress logging during test runs.")) + (System/exit 1))src/clojure_mcp/sse_main.clj (1)
8-22: SSE entrypoint logging setup and opts sanitization look correctConfiguring logging up front with
logging/configure-logging!and then passing(dissoc opts :log-file :log-level :enable-logging?)intobuild-and-start-mcp-servercleanly separates logging concerns from server options and matches the pattern inclojure-mcp.main/start-mcp-server.If this pattern is repeated in several entrypoints (main, SSE, figwheel, shadow), consider a small helper like
logging/configure-from-opts!to avoid drift across call sites over time.src/clojure_mcp/main.clj (1)
3-6: Main entrypoint now cleanly configures logging before server startupUsing
logging/configure-logging!with sane defaults and then stripping:log-file,:log-level, and:enable-logging?fromoptsbefore passing tocore/build-and-start-mcp-serveris a good separation of concerns and keeps the core API free of logging-specific keys.Given this same pattern appears in
clojure-mcp.sse-main(and likely the example mains), a small helper (e.g.logging/configure-from-opts!) could reduce duplication and make future logging option changes one-place.Also applies to: 36-46
deps.edn (2)
38-67: MCP entrypoint aliases now consistently use exec-fn-based startupThe
:mcp,:mcp-sse,:mcp-figwheel, and:mcp-shadow*aliases all delegate via:exec-fnto the appropriatestart-*-mcp-serverfunctions with minimal:exec-args, which pairs well with the new logging configuration logic in those entrypoints.You might consider expanding the comments here to briefly mention the new logging-related opts (
:log-file,:enable-logging?,:log-level) so users discover them fromdeps.ednalone.
108-116: :test alias wiring matches the new test helper behaviorThe
:testalias setup works nicely withclojure-mcp.test-helper:
-X:testinvokesrun-tests-with-execvia:exec-fn, which prints guidance and exits with a non-zero status.-M:testfollows:main-opts, first requiringclojure-mcp.test-helper(to suppress logging) and then runningcognitect.test-runner.The added
:extra-depsensure the runner and test tooling are on the classpath.If you want to avoid duplicated
nrepl/nrepldeclarations, you could rely on the top-level:depsentry and drop it from:test’s:extra-deps, unless you specifically want to pin a different version for tests.src/clojure_mcp/logging.clj (2)
33-42: Make Timbre config derive fromdefault-configand simplify:ns-filter
configure-logging!currently callstimbre/set-config!with a minimal map that only defines:appenders. This replaces Timbre’s entire*config*and drops other defaults like:min-level,:ns-filter,:timestamp-opts, and:output-fn. It works as long as Timbre tolerates these asnil, but it’s brittle and bypasses future defaults and environment-based config merging. (taoensso.github.io)Given this is the central logging entry point, it’d be more robust to start from
timbre/default-config(or the existing*config*) and override only what you care about. That also lets you put:min-leveland:ns-filterat the top level instead of per-appender.For example:
- (timbre/set-config! - {:appenders (if enable-logging? - {:spit (assoc - (timbre/spit-appender {:fname log-file}) - :enabled? enable-logging? - :min-level (or log-level :report) - :ns-filter (if enable-logging? - {:allow #{"clojure-mcp.*"}} - {:deny #{"*"}}))} - {})})) + (timbre/set-config! + (assoc timbre/default-config + :min-level (or log-level :debug) + :ns-filter (if enable-logging? + {:allow #{"clojure-mcp.*"}} + {:deny #{"*"}}) + :appenders (if enable-logging? + {:spit (assoc (timbre/spit-appender {:fname log-file}) + :enabled? true)} + {})))This keeps Timbre’s standard behavior (output-fn, timestamp options, etc.) while still enforcing your namespace filter and appender choice. It also removes the redundant inner
if enable-logging?on:ns-filter.
9-9: Clarify and/or harden default log file path handling
default-log-filepoints to".clojure-mcp/clojure-mcp.log", whileconfigure-dev-logging!writes to"logs/clojure-mcp.log". Neither path ensures that the parent directory exists beforespit-appendertries to write, which can throw if the directory is missing.Two possible follow-ups:
- Explicitly document that callers must ensure the containing directory exists, or
- Proactively create the parent directory when
enable-logging?is true.For example, you could (optionally) add a small helper invoked before
timbre/spit-appenderthat(.mkdirs (io/file parent-dir)).Also applies to: 48-50
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (33)
deps.edn(2 hunks)dev/logback.xml(0 hunks)src/clojure_mcp/agent/general_agent.clj(1 hunks)src/clojure_mcp/agent/langchain.clj(1 hunks)src/clojure_mcp/agent/langchain/chat_listener.clj(1 hunks)src/clojure_mcp/agent/langchain/model.clj(1 hunks)src/clojure_mcp/config.clj(1 hunks)src/clojure_mcp/core.clj(1 hunks)src/clojure_mcp/dialects.clj(1 hunks)src/clojure_mcp/logging.clj(1 hunks)src/clojure_mcp/main.clj(2 hunks)src/clojure_mcp/main_examples/figwheel_main.clj(2 hunks)src/clojure_mcp/main_examples/shadow_main.clj(2 hunks)src/clojure_mcp/nrepl.clj(1 hunks)src/clojure_mcp/nrepl_launcher.clj(1 hunks)src/clojure_mcp/sse_core.clj(1 hunks)src/clojure_mcp/sse_main.clj(1 hunks)src/clojure_mcp/tool_system.clj(1 hunks)src/clojure_mcp/tools/agent_tool_builder/core.clj(1 hunks)src/clojure_mcp/tools/agent_tool_builder/file_changes.clj(1 hunks)src/clojure_mcp/tools/agent_tool_builder/tool.clj(1 hunks)src/clojure_mcp/tools/bash/core.clj(1 hunks)src/clojure_mcp/tools/bash/tool.clj(1 hunks)src/clojure_mcp/tools/eval/core.clj(1 hunks)src/clojure_mcp/tools/figwheel/tool.clj(1 hunks)src/clojure_mcp/tools/glob_files/core.clj(1 hunks)src/clojure_mcp/tools/grep/core.clj(1 hunks)src/clojure_mcp/tools/project/core.clj(1 hunks)src/clojure_mcp/tools/scratch_pad/tool.clj(1 hunks)src/clojure_mcp/tools/unified_clojure_edit/tool.clj(1 hunks)src/clojure_mcp/tools/unified_read_file/file_timestamps.clj(1 hunks)src/clojure_mcp/tools/unified_read_file/tool.clj(1 hunks)test/clojure_mcp/test_helper.clj(1 hunks)
💤 Files with no reviewable changes (1)
- dev/logback.xml
🧰 Additional context used
📓 Path-based instructions (3)
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/agent_tool_builder/core.cljsrc/clojure_mcp/core.cljsrc/clojure_mcp/tools/glob_files/core.cljsrc/clojure_mcp/tool_system.cljsrc/clojure_mcp/agent/langchain.cljsrc/clojure_mcp/agent/general_agent.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/main.cljsrc/clojure_mcp/dialects.cljsrc/clojure_mcp/tools/agent_tool_builder/file_changes.cljsrc/clojure_mcp/main_examples/figwheel_main.cljsrc/clojure_mcp/agent/langchain/chat_listener.cljsrc/clojure_mcp/nrepl.cljsrc/clojure_mcp/tools/agent_tool_builder/tool.cljsrc/clojure_mcp/nrepl_launcher.cljsrc/clojure_mcp/tools/figwheel/tool.cljsrc/clojure_mcp/tools/unified_read_file/file_timestamps.cljsrc/clojure_mcp/sse_core.cljsrc/clojure_mcp/logging.cljsrc/clojure_mcp/sse_main.cljsrc/clojure_mcp/tools/grep/core.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/agent/langchain/model.cljsrc/clojure_mcp/tools/scratch_pad/tool.cljsrc/clojure_mcp/tools/unified_read_file/tool.cljsrc/clojure_mcp/config.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/unified_clojure_edit/tool.cljsrc/clojure_mcp/main_examples/shadow_main.cljsrc/clojure_mcp/tools/eval/core.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/agent_tool_builder/core.cljsrc/clojure_mcp/core.cljsrc/clojure_mcp/tools/glob_files/core.cljsrc/clojure_mcp/tool_system.cljsrc/clojure_mcp/agent/langchain.cljsrc/clojure_mcp/agent/general_agent.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/main.cljsrc/clojure_mcp/dialects.cljsrc/clojure_mcp/tools/agent_tool_builder/file_changes.cljsrc/clojure_mcp/main_examples/figwheel_main.cljsrc/clojure_mcp/agent/langchain/chat_listener.cljsrc/clojure_mcp/nrepl.cljsrc/clojure_mcp/tools/agent_tool_builder/tool.cljsrc/clojure_mcp/nrepl_launcher.cljsrc/clojure_mcp/tools/figwheel/tool.cljsrc/clojure_mcp/tools/unified_read_file/file_timestamps.cljsrc/clojure_mcp/sse_core.cljsrc/clojure_mcp/logging.cljsrc/clojure_mcp/sse_main.cljtest/clojure_mcp/test_helper.cljsrc/clojure_mcp/tools/grep/core.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/agent/langchain/model.cljsrc/clojure_mcp/tools/scratch_pad/tool.cljsrc/clojure_mcp/tools/unified_read_file/tool.cljsrc/clojure_mcp/config.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/unified_clojure_edit/tool.cljsrc/clojure_mcp/main_examples/shadow_main.cljsrc/clojure_mcp/tools/eval/core.clj
test/**/*.{clj,cljc}
📄 CodeRabbit inference engine (CLAUDE.md)
Use
deftestwith descriptive names;testingfor subsections;isfor assertions in tests
Files:
test/clojure_mcp/test_helper.clj
🧠 Learnings (8)
📚 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/core.cljsrc/clojure_mcp/tool_system.cljsrc/clojure_mcp/agent/langchain.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/main.cljsrc/clojure_mcp/dialects.cljtest/clojure_mcp/test_helper.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/tools/scratch_pad/tool.cljsrc/clojure_mcp/config.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/unified_clojure_edit/tool.cljsrc/clojure_mcp/main_examples/shadow_main.cljsrc/clojure_mcp/tools/eval/core.cljdeps.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:
src/clojure_mcp/core.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/main.cljsrc/clojure_mcp/dialects.cljsrc/clojure_mcp/main_examples/figwheel_main.cljsrc/clojure_mcp/sse_core.cljsrc/clojure_mcp/sse_main.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/main_examples/shadow_main.cljdeps.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} : Validate inputs and provide helpful error messages in MCP tools
Applied to files:
src/clojure_mcp/core.cljsrc/clojure_mcp/tool_system.cljsrc/clojure_mcp/agent/langchain.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/main.cljsrc/clojure_mcp/dialects.cljsrc/clojure_mcp/main_examples/figwheel_main.cljtest/clojure_mcp/test_helper.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/tools/scratch_pad/tool.cljsrc/clojure_mcp/config.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/unified_clojure_edit/tool.cljsrc/clojure_mcp/tools/eval/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/core.cljsrc/clojure_mcp/agent/langchain.cljsrc/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/main.cljsrc/clojure_mcp/dialects.cljsrc/clojure_mcp/tools/agent_tool_builder/file_changes.cljsrc/clojure_mcp/main_examples/figwheel_main.cljsrc/clojure_mcp/sse_core.cljsrc/clojure_mcp/sse_main.cljtest/clojure_mcp/test_helper.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/agent/langchain/model.cljsrc/clojure_mcp/tools/scratch_pad/tool.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/main_examples/shadow_main.cljdeps.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 **/*.{clj,cljc} : Align namespaces with directory structure (e.g., `clojure-mcp.repl-tools`)
Applied to files:
src/clojure_mcp/core.cljsrc/clojure_mcp/tools/agent_tool_builder/file_changes.cljtest/clojure_mcp/test_helper.cljsrc/clojure_mcp/tools/project/core.cljsrc/clojure_mcp/tools/scratch_pad/tool.cljsrc/clojure_mcp/config.cljsrc/clojure_mcp/tools/bash/tool.cljsrc/clojure_mcp/tools/unified_clojure_edit/tool.cljdeps.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} : Include clear tool `:description` for LLM guidance
Applied to files:
src/clojure_mcp/tool_system.cljsrc/clojure_mcp/tools/agent_tool_builder/file_changes.cljtest/clojure_mcp/test_helper.cljsrc/clojure_mcp/tools/grep/core.cljsrc/clojure_mcp/tools/scratch_pad/tool.cljsrc/clojure_mcp/tools/unified_clojure_edit/tool.cljsrc/clojure_mcp/tools/eval/core.cljdeps.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} : Use `:require` with ns aliases for imports (e.g., `[clojure.string :as string]`)
Applied to files:
src/clojure_mcp/tools/bash/core.cljsrc/clojure_mcp/nrepl.cljsrc/clojure_mcp/tools/unified_clojure_edit/tool.cljsrc/clojure_mcp/tools/eval/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 test/**/*.{clj,cljc} : Use `deftest` with descriptive names; `testing` for subsections; `is` for assertions in tests
Applied to files:
test/clojure_mcp/test_helper.clj
🔇 Additional comments (31)
src/clojure_mcp/tools/unified_clojure_edit/tool.clj (1)
9-9: LGTM! Clean logging library migration.The import change from
clojure.tools.loggingtotaoensso.timbreis correct and maintains API compatibility. Thelogalias remains unchanged, so the existinglog/infocall at line 203 works without modification. This change aligns with the PR's systematic migration to Timbre across the codebase.src/clojure_mcp/agent/langchain/model.clj (1)
5-5: Timbre import andlog/*usages look correct and non-breakingSwitching the require to
[taoensso.timbre :as log]keeps thelog/infoandlog/warncall sites valid, including the(log/warn e (str ...))pattern inget-tool-model, so behavior should carry over cleanly to the new backend while satisfying the aliasing guideline. As per coding guidelines.Also applies to: 570-573
src/clojure_mcp/tools/agent_tool_builder/core.clj (1)
11-11: LGTM! Clean logging backend migration.The switch from
clojure.tools.loggingtotaoensso.timbreis consistent with the project-wide logging migration. Preserving thelogalias ensures all existing logging calls remain compatible.src/clojure_mcp/tools/bash/core.clj (1)
7-7: LGTM! Logging backend successfully migrated.The Timbre migration is correctly applied with the preserved
logalias. All existing logging calls (debug, warn, error) throughout the file remain compatible.src/clojure_mcp/agent/langchain.clj (1)
5-5: LGTM! Logging backend migration applied correctly.The switch to Timbre maintains the
logalias, ensuring seamless compatibility with existing logging statements in the LangChain integration code.src/clojure_mcp/nrepl_launcher.clj (1)
8-8: LGTM! Logging migration successful.The Timbre backend swap is properly implemented. This launcher module's extensive logging (debug, info, warn, error) will continue to function correctly with the preserved
logalias.src/clojure_mcp/nrepl.clj (1)
7-7: LGTM! Logging backend updated correctly.The Timbre migration maintains compatibility with the nREPL client's error and info logging. The preserved
logalias ensures no changes are needed to the connection management code.src/clojure_mcp/tools/bash/tool.clj (1)
9-9: LGTM! Logging backend migration complete.The Timbre switch is correctly applied. The bash tool's error and debug logging will continue to work seamlessly with the preserved
logalias.src/clojure_mcp/tools/unified_read_file/file_timestamps.clj (1)
6-6: LGTM! Logging migration implemented correctly.The Timbre backend swap maintains full compatibility with the file timestamp tracking logic. The preserved
logalias ensures all debug, info, and warn calls continue to function properly.src/clojure_mcp/agent/general_agent.clj (1)
5-5: LGTM! Logging backend successfully updated.The Timbre migration is properly applied to the general agent module. Error logging for agent creation and chat operations will continue to work correctly with the preserved
logalias.src/clojure_mcp/tools/figwheel/tool.clj (1)
3-3: Clean logging backend migration.The swap from
clojure.tools.loggingtotaoensso.timbreis consistent with the project-wide migration. Thelogalias is preserved, so all existing logging calls continue to work without modification.src/clojure_mcp/tools/glob_files/core.clj (1)
7-7: Clean logging backend migration.The namespace change from
clojure.tools.loggingtotaoensso.timbreis straightforward and consistent with the project-wide logging migration. All logging calls throughout the file continue to work via the preservedlogalias.src/clojure_mcp/tools/eval/core.clj (1)
9-9: Clean logging backend migration.The logging namespace swap to
taoensso.timbremaintains thelogalias, ensuring all existing logging calls (such as Line 90'slog/error) continue to function without modification.src/clojure_mcp/agent/langchain/chat_listener.clj (1)
5-5: Clean logging backend migration.The switch to
taoensso.timbreis consistent with the project-wide migration. Thelogalias is preserved, ensuring all logging calls (including those in thelogging-listenerfunction) continue to work seamlessly.src/clojure_mcp/core.clj (1)
5-5: Clean logging backend migration.The migration to
taoensso.timbreis well-executed. The preservedlogalias ensures all logging calls throughout this core file (lines 94, 218, 231, 234, 260, 262, 284, etc.) continue to function without modification.src/clojure_mcp/tools/agent_tool_builder/tool.clj (1)
8-8: Clean logging backend migration.The namespace change to
taoensso.timbrealigns with the project-wide migration. Thelogalias is preserved, maintaining compatibility with existing logging calls in thecreate-agent-toolsfunction.src/clojure_mcp/dialects.clj (1)
8-8: Clean logging backend migration.The migration to
taoensso.timbreis straightforward and consistent. The preservedlogalias ensures existing logging calls (lines 85, 107) continue to work without modification.src/clojure_mcp/tool_system.clj (1)
8-8: Clean logging backend migration.The namespace swap to
taoensso.timbreis consistent with the project-wide migration. Thelogalias is preserved, ensuring the error logging call (line 110) continues to work seamlessly.src/clojure_mcp/config.clj (1)
7-7: LGTM: Clean logging backend migration.The import swap from
clojure.tools.loggingtotaoensso.timbremaintains the samelogalias, ensuring all existing log calls remain unchanged and compatible.src/clojure_mcp/sse_core.clj (1)
6-6: LGTM: Logging backend successfully migrated.The Timbre import preserves the
logalias, maintaining compatibility with all existing logging calls.src/clojure_mcp/tools/unified_read_file/tool.clj (1)
13-13: LGTM: Consistent logging migration.Import change aligns with the project-wide migration to Timbre while preserving existing log call syntax.
src/clojure_mcp/main_examples/figwheel_main.clj (1)
13-13: LGTM: Centralized logging integration.The import of the new
clojure-mcp.loggingnamespace enables centralized logging configuration for entry points.src/clojure_mcp/tools/scratch_pad/tool.clj (1)
7-7: LGTM: Logging backend migrated successfully.The import swap maintains the
logalias, ensuring compatibility with all existing logging statements throughout the file.src/clojure_mcp/main_examples/shadow_main.clj (1)
14-16: LGTM: Logging integration complete.Both the centralized logging namespace and Timbre backend are properly imported for logging configuration and usage.
src/clojure_mcp/tools/grep/core.clj (1)
8-8: LGTM: Logging migration complete.The import change aligns with the project-wide migration to Timbre, maintaining compatibility with all existing log calls.
src/clojure_mcp/tools/agent_tool_builder/file_changes.clj (1)
8-8: LGTM: Logging backend updated.The import swap completes the migration to Timbre while preserving the
logalias and existing logging calls.test/clojure_mcp/test_helper.clj (1)
1-8: Test helper logging suppression is wired correctlyRequiring
clojure-mcp.logginghere and callinglogging/configure-test-logging!at load time ensures tests run with logging suppressed regardless of how the test runner is invoked, which matches the new :test alias behavior.deps.edn (2)
68-98: Dev aliases correctly mirror the main MCP entrypoints with dev/test pathsThe dev aliases (
:prompt-cli,:dev-mcp*) now consistently:
- Add
["dev" "test"]to:extra-paths, and- Use the same
start-*-mcp-serverfunctions as the non-dev aliases,which should give a predictable dev experience while still honoring the centralized logging configuration.
29-35: Logging migration verified—old backend fully removed, new stack correctly in placeVerification confirms the migration is complete:
- Timbre 6.8.0 and slf4j-nop 2.0.16 are present in deps.edn (lines 29, 32)
- No logback-classic, java-logging, log4j, or log4j2 in deps.edn
- No logback.xml or logback-classic code references
- No conflicting SLF4J bindings
The changes are correct as-is.
src/clojure_mcp/tools/project/core.clj (1)
12-12: Timbre logging alias swap verified as correctThe migration to
[taoensso.timbre :as log]at line 12 is complete and consistent. The dependency is present in deps.edn (6.8.0), all 12+ existinglog/debugandlog/warncall sites throughout the file remain valid, and no old logging backend references persist in the codebase. The namespace declaration follows coding guidelines with proper:requireformatting and ns aliases.src/clojure_mcp/logging.clj (1)
44-63: Helper logging modes and test suppression look consistentThe dev/prod/test helpers and
suppress-logging-for-tests!cleanly wrapconfigure-logging!and match the PR intent: simple dev file logging, and logging suppressed by default and in tests via env var or system property.No functional issues spotted here.
Also applies to: 64-71
| Options: | ||
| - :log-file Path to log file (default: 'logs/clojure-mcp.log') | ||
| - :enable-logging? Whether to enable logging (default: true) | ||
| - :log-level Minimum log level (default: :debug) | ||
| Valid values: :trace :debug :info :warn :error :fatal :report |
There was a problem hiding this comment.
Fix configure-logging! option docs to match actual defaults
The option docs and the implementation diverge:
- Doc says
:log-filedefault is'logs/clojure-mcp.log', but:orusesdefault-log-filewhich is".clojure-mcp/clojure-mcp.log". - Doc says
:enable-logging?default istrue, but the implementation defaults it tofalse.
Given this is a new public API, this mismatch will confuse users configuring logging from the CLI/entry points.
Consider updating the docstring to describe the real behavior:
- - :log-file Path to log file (default: 'logs/clojure-mcp.log')
- - :enable-logging? Whether to enable logging (default: true)
- - :log-level Minimum log level (default: :debug)
+ - :log-file Path to log file (default: '.clojure-mcp/clojure-mcp.log')
+ - :enable-logging? Whether to enable logging (default: false)
+ - :log-level Minimum log level when logging is enabled (default: :debug)Also applies to: 29-32
🤖 Prompt for AI Agents
In src/clojure_mcp/logging.clj around lines 14-18 and 29-32, the docstring
defaults for :log-file and :enable-logging? do not match the implementation;
update the two documentation blocks to state the actual defaults (use
default-log-file value ".clojure-mcp/clojure-mcp.log" for :log-file and set
:enable-logging? default to false), and ensure the :log-level default text
remains correct (e.g., :debug) — or alternatively change the implementation to
match the previously documented defaults; pick one approach and make both doc
blocks consistent with the code.
Summary
Changes
src/clojure_mcp/logging.clj- Centralized Timbre configuration with convenience functionstest/clojure_mcp/test_helper.clj- Test logging suppressionclojure.tools.loggingtotaoensso.timbreConfiguration
Logging is now configured via options at entry points:
:log-file- Path to log file (default:.clojure-mcp/clojure-mcp.log):enable-logging?- Whether to enable logging (default:false):log-level- Minimum log level (default::debug)Test Results
All tests pass with zero logging noise: 293 tests, 2062 assertions, 0 failures
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.