Skip to content

Conversation

@bhauman
Copy link
Owner

@bhauman bhauman commented Jan 2, 2026

Summary

  • Add ENABLE_TOOLS and DISABLE_TOOLS environment variables that override config file settings
  • Accept comma-separated tool names with optional whitespace (e.g., "bash, eval_code, file_edit")
  • Normalize hyphens to underscores for user convenience (e.g., file-editfile_edit)

Test plan

  • Added comprehensive tests for ENABLE_TOOLS env var behavior
  • Added comprehensive tests for DISABLE_TOOLS env var behavior
  • Added tests for hyphen-to-underscore normalization
  • Added tests for combined env var and config interactions
  • All existing tests continue to pass

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Tools can now be configured via ENABLE_TOOLS and DISABLE_TOOLS environment variables, with settings taking precedence over configuration files.
    • Improved handling of tool identifier formats, including hyphenated and space-separated variations.
  • Tests

    • Added comprehensive test coverage for environment variable-based tool configuration scenarios and precedence rules.

✏️ Tip: You can customize this high-level summary in your review settings.

Environment variables override config file settings for tool filtering.
Accepts comma-separated tool names with optional whitespace.
Hyphens are normalized to underscores (e.g., file-edit -> file_edit).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
@coderabbitai
Copy link

coderabbitai bot commented Jan 2, 2026

📝 Walkthrough

Walkthrough

The changes introduce environment variable override functionality for tool enable/disable configuration. A dynamic testing override mechanism and helper functions parse ENABLE_TOOLS and DISABLE_TOOLS environment variables, allowing env-based configuration to take precedence over file-based config.

Changes

Cohort / File(s) Summary
Config Implementation
src/clojure_mcp/config.clj
Added string namespace import. Introduced dynamic var *env-overrides* for test overrides, private helper get-env to check overrides first then System/getenv, and parse-tools-env-var to parse env vars into keyword vectors with hyphen normalization. Updated get-enable-tools and get-disable-tools to respect env var overrides while maintaining same signature.
Comprehensive Test Suite
test/clojure_mcp/config/tool_filter_test.clj
Added extensive unit tests covering env var overriding config, empty/ignored values, interaction scenarios with both ENABLE_TOOLS and DISABLE_TOOLS set, mixed env and config precedence, and string-to-keyword conversion.

Sequence Diagram(s)

sequenceDiagram
    participant Caller
    participant ConfigModule as Config Module
    participant Environment
    
    Caller->>ConfigModule: get-enable-tools(nrepl-client-map)
    activate ConfigModule
    
    ConfigModule->>ConfigModule: get-env("ENABLE_TOOLS")
    activate ConfigModule
    Note over ConfigModule: Check *env-overrides* first
    ConfigModule-->>ConfigModule: check System/getenv
    deactivate ConfigModule
    
    alt ENABLE_TOOLS set
        ConfigModule->>ConfigModule: parse-tools-env-var(value)
        Note over ConfigModule: Parse and normalize<br/>to keywords
        ConfigModule-->>Caller: Return env-based tools
    else ENABLE_TOOLS not set
        ConfigModule-->>Caller: Fall back to config file
    end
    deactivate ConfigModule
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops with glee through env vars so bright,
Overrides dance from morning to night,
Tools enabled, disabled, and free,
Configuration bends, tests dance with glee!

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and directly describes the main change: adding ENABLE_TOOLS and DISABLE_TOOLS environment variable support.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c2f4954 and 27107bc.

📒 Files selected for processing (2)
  • src/clojure_mcp/config.clj
  • test/clojure_mcp/config/tool_filter_test.clj
🧰 Additional context used
📓 Path-based instructions (2)
**/*.clj

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.clj: Use :require with ns aliases in Clojure imports (e.g., [clojure.string :as string])
Use kebab-case for variable and function names in Clojure
End Clojure predicate functions with ? (e.g., is-top-level-form?)
Use try/catch with specific exception handling in Clojure; use atom for tracking errors
Use 2-space indentation in Clojure code and maintain whitespace in edited forms
Align Clojure namespaces with directory structure (e.g., clojure-mcp.repl-tools)
Include clear tool :description in MCP tools for LLM guidance
Validate inputs and provide helpful error messages in MCP tool implementations
Return structured data with both result and error status in MCP tool responses
Maintain atom-based state for consistent service access in MCP tool implementations

Files:

  • test/clojure_mcp/config/tool_filter_test.clj
  • src/clojure_mcp/config.clj
**/*test*.clj

📄 CodeRabbit inference engine (CLAUDE.md)

Use deftest with descriptive names in Clojure tests; use testing for subsections and is for assertions

Files:

  • test/clojure_mcp/config/tool_filter_test.clj
🧠 Learnings (9)
📓 Common learnings
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*.clj : Include clear tool `:description` in MCP tools for LLM guidance
📚 Learning: 2026-01-02T16:18:02.394Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*.clj : Include clear tool `:description` in MCP tools for LLM guidance

Applied to files:

  • test/clojure_mcp/config/tool_filter_test.clj
  • src/clojure_mcp/config.clj
📚 Learning: 2026-01-02T16:18:02.394Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*test*.clj : Use `deftest` with descriptive names in Clojure tests; use `testing` for subsections and `is` for assertions

Applied to files:

  • test/clojure_mcp/config/tool_filter_test.clj
📚 Learning: 2025-12-27T06:54:07.157Z
Learnt from: nandoolle
Repo: bhauman/clojure-mcp PR: 138
File: src/clojure_mcp/agent/langchain/model.clj:91-97
Timestamp: 2025-12-27T06:54:07.157Z
Learning: When reviewing Clojure code (e.g., in src/clojure_mcp/... ), verify calls to presets/get-preset follow the two arities: [preset-key] and [preset-key overrides]. The 2-arg version should merge the provided overrides map with the preset defaults and return the resulting configuration. Check for correct map merging, preservation of immutability, and validation of overrides keys to ensure predictable configuration outcomes.

Applied to files:

  • test/clojure_mcp/config/tool_filter_test.clj
  • src/clojure_mcp/config.clj
📚 Learning: 2026-01-02T16:18:02.394Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*.clj : Align Clojure namespaces with directory structure (e.g., `clojure-mcp.repl-tools`)

Applied to files:

  • src/clojure_mcp/config.clj
📚 Learning: 2026-01-02T16:18:02.394Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*.clj : Maintain atom-based state for consistent service access in MCP tool implementations

Applied to files:

  • src/clojure_mcp/config.clj
📚 Learning: 2026-01-02T16:18:02.394Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*.clj : Validate inputs and provide helpful error messages in MCP tool implementations

Applied to files:

  • src/clojure_mcp/config.clj
📚 Learning: 2026-01-02T16:18:02.394Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*.clj : Use `:require` with ns aliases in Clojure imports (e.g., `[clojure.string :as string]`)

Applied to files:

  • src/clojure_mcp/config.clj
📚 Learning: 2026-01-02T16:18:02.394Z
Learnt from: CR
Repo: bhauman/clojure-mcp PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-02T16:18:02.394Z
Learning: Applies to **/*.clj : Return structured data with both result and error status in MCP tool responses

Applied to files:

  • src/clojure_mcp/config.clj
🔇 Additional comments (9)
src/clojure_mcp/config.clj (6)

4-4: LGTM! Import follows conventions.

The clojure.string import with :as string alias follows Clojure best practices and coding guidelines.


315-318: LGTM! Well-designed testing override mechanism.

The dynamic var *env-overrides* provides a clean way to override environment variables in tests without affecting the actual system environment. The naming convention with earmuffs and clear docstring are excellent.


320-325: LGTM! Clean environment abstraction.

The get-env helper properly checks the test override first before falling back to System/getenv, enabling deterministic testing.


327-338: LGTM! Robust parsing with normalization.

The parsing logic correctly handles:

  • Empty/unset environment variables (returns nil, falling back to config)
  • Whitespace around commas (trimmed)
  • Hyphen-to-underscore normalization (for convenience)
  • Empty entries after splitting (removed)

The threading macro makes the transformation pipeline clear and readable.


340-346: LGTM! Environment override precedence is correct.

The function properly prioritizes ENABLE_TOOLS environment variable over config file settings, with clear fallback behavior when the env var is unset or empty.


348-354: LGTM! Consistent with enable-tools implementation.

The get-disable-tools function mirrors the get-enable-tools pattern, providing consistent environment variable override behavior.

test/clojure_mcp/config/tool_filter_test.clj (3)

7-48: LGTM! Comprehensive ENABLE_TOOLS test coverage.

The test suite thoroughly validates:

  • Environment variable override of config settings
  • Whitespace handling around commas
  • Hyphen-to-underscore normalization
  • Empty string behavior (falls back to config)
  • Absence of env var (uses config)

Test structure follows guidelines with descriptive deftest names, testing blocks, and clear assertions.


50-80: LGTM! Thorough DISABLE_TOOLS test coverage.

The tests mirror the ENABLE_TOOLS test patterns, ensuring symmetric behavior for both environment variables with consistent parsing and override logic.


82-108: LGTM! Excellent interaction testing.

These tests validate critical scenarios where both environment variables and config settings interact:

  • Both env vars set (disable takes precedence)
  • Mixed env var and config (env var overrides for its domain)
  • Correct precedence rules maintained

The test cases confirm that the implementation correctly handles the composition of enable/disable lists from different sources.


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

❤️ Share

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

@bhauman bhauman merged commit 5810ff6 into main Jan 2, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants