Add dry_run parameter to file_write tool

[bhauman]

Summary

Adds optional dry_run parameter to the file_write tool, allowing previewing changes without writing to disk. This brings consistency with the other file editing tools (file_edit, clojure_edit, clojure_edit_replace_sexp) that already support this parameter.

Changes

• src/clojure_mcp/tools/file_write/tool.clj

  • Updated validate-inputs to extract and pass through dry_run
  • Modified execute-tool to pass dry_run and skip timestamp update in dry_run mode
  • Updated format-results to handle new-source output

• src/clojure_mcp/tools/file_write/core.clj

  • Updated write-clojure-file to accept dry_run parameter and skip file save when set
  • Updated write-text-file to accept dry_run parameter and conditionally write
  • Updated write-file dispatcher to pass dry_run through

• test/clojure_mcp/tools/file_write/core_test.clj

  • Fixed all test calls to pass nil for dry_run parameter

Dry Run Modes

• dry_run="diff" - Returns unified diff without writing to file
• dry_run="new-source" - Returns complete file content without writing
• No dry_run parameter - Normal operation, writes to file

Testing

✅ All 365 tests pass with 0 failures and 0 errors
✅ Verified all three modes work correctly (diff, new-source, and normal write)

Notes

The dry_run parameter is for internal use only and is not exposed in the tool schema.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features
  • Added dry-run mode for file write operations, allowing users to preview changes before persisting them.
  • Dry-run mode returns either the new file content or a diff showing proposed changes.
  • File timestamps remain unchanged during dry-run operations.

[coderabbitai]

Warning

Rate limit exceeded

@bhauman has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 6 minutes and 45 seconds before requesting another review.

⌛ How to resolve this issue?

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

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

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

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

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between d51faa4 and 594825a.

📒 Files selected for processing (2)

• src/clojure_mcp/tools/file_write/core.clj (4 hunks)
• src/clojure_mcp/tools/file_write/tool.clj (3 hunks)

Walkthrough

The pull request introduces a unified dry_run parameter throughout the file-write module to enable preview modes. Functions in the core layer now accept this parameter to optionally suppress file persistence and timestamp updates, with conditional formatting of results based on the dry_run mode (returning new-source or diff payloads without writing to disk).

Changes

Cohort / File(s)	Summary
Core file-write logic enhancement src/clojure_mcp/tools/file_write/core.clj	Added is-clojure-file? helper method. Updated write-clojure-file, write-text-file, and write-file to accept optional dry_run parameter. When dry_run = "new-source", returns new source payload; otherwise returns diff without persisting changes.
Tool layer parameter threading src/clojure_mcp/tools/file_write/tool.clj	Updated validate-inputs, execute-tool, and format-results methods to destructure and thread dry_run through the tool flow. Conditional behavior: timestamp updates skipped on dry runs; new-source payload returned directly when present.
Test call sites test/clojure_mcp/tools/file_write/core_test.clj	Updated all invocations of write-text-file, write-clojure-file, and write-file to pass nil for the new dry_run parameter, maintaining test semantics.

Sequence Diagram

sequenceDiagram
    participant Tool as Tool Layer
    participant Core as Core Logic
    participant FS as File System
    
    Tool->>Tool: validate-inputs (extract dry_run)
    Tool->>Core: execute-tool (pass dry_run)
    
    alt dry_run = "new-source"
        Core->>Core: compute new_source
        Core-->>Tool: return {new-source: ...}
    else dry_run = true
        Core->>Core: compute diff
        Core-->>Tool: return {diff: ...}
    else dry_run = nil
        Core->>FS: write-file
        Core->>FS: update timestamp
        Core-->>Tool: return {type: ..., file-path: ...}
    end
    
    Tool->>Tool: format-results (check for new-source)
    alt new-source present
        Tool-->>Client: return new-source directly
    else
        Tool-->>Client: return formatted result
    end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

The changes follow a consistent pattern: adding an optional dry_run parameter across three functions and threading it through the tool layer. While there are multiple files involved, the modifications are largely homogeneous (parameter passing), with straightforward conditional logic for handling the dry_run modes. No complex algorithmic changes or error-handling modifications are present.

Poem

🐰 A dry run, oh what delight!
Preview changes, see what's right,
No files harmed in testing's play,
Just diffs and sources on display! ✨
Write safely, review with care,
Before committing to the air!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The pull request title "Add dry_run parameter to file_write tool" directly and accurately summarizes the main change in the changeset. The PR focuses on adding an optional dry_run parameter across the file_write tool's validate-inputs, execute-tool, and format-results methods, as well as the underlying write functions in core.clj. The title is concise, specific, and clear enough that a teammate reviewing the history would immediately understand the primary purpose of the change without requiring ambiguous phrasing or vague terms.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

---

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

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 0

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/clojure_mcp/tools/file_write/tool.clj (1)

66-92: Clarify dry_run access control and add validation.

The PR description states that dry_run is "for internal use and not exposed in the tool schema." However, the code extracts dry_run from inputs (line 66), which means external callers can provide it even though it's not in the schema. This creates an undocumented API surface.

Additionally, there's no validation that dry_run contains valid values ("diff" or "new-source").

Consider one of these approaches:

Option 1: Make it truly internal
If dry_run is only for internal tool usage, extract it from a different source (e.g., tool config) rather than from user inputs:

 (defmethod tool-system/validate-inputs :file-write [{:keys [nrepl-client-atom]} inputs]
-  (let [{:keys [file_path content dry_run]} inputs
+  (let [{:keys [file_path content]} inputs
         nrepl-client (and nrepl-client-atom @nrepl-client-atom)]

Option 2: Make it officially supported
If external callers should be able to use dry_run, add it to the schema and validate it:

 (defmethod tool-system/tool-schema :file-write [_]
   {:type :object
    :properties {:file_path {:type :string
                             :description "The absolute path to the file to write (must be absolute, not relative)"}
                 :content {:type :string
-                          :description "The content to write to the file"}}
+                          :description "The content to write to the file"}
+                :dry_run {:type :string
+                          :enum ["diff" "new-source"]
+                          :description "Preview mode: 'diff' shows changes, 'new-source' shows complete content. Does not write to disk."}}
    :required [:file_path :content]})

And add validation:

+ (when (and dry_run (not (contains? #{"diff" "new-source"} dry_run)))
+   (throw (ex-info (str "Invalid dry_run value: " dry_run " - must be 'diff' or 'new-source'")
+                   {:dry_run dry_run})))

🧹 Nitpick comments (3)

test/clojure_mcp/tools/file_write/core_test.clj (1)

69-69: Consider adding test coverage for dry_run functionality.

All test calls have been updated to pass nil for the new dry_run parameter, which correctly maintains existing behavior. However, there's no test coverage for the new dry_run modes ("diff" and "new-source").

Consider adding tests to verify:

• dry_run="diff" returns diff without writing file
• dry_run="new-source" returns complete file content without writing
• File is not modified when dry_run is set
• File timestamp is not updated when dry_run is set

Do you want me to generate test cases for the dry_run functionality?

Also applies to: 82-82, 96-97, 111-112, 125-126, 138-139, 153-154, 169-170, 185-186, 197-198

src/clojure_mcp/tools/file_write/core.clj (2)

32-75: Consider validating dry_run parameter values.

The function correctly threads dry_run through the pipeline and handles both "new-source" and diff modes. However, there's no validation that dry_run is one of the expected values (nil, "diff", or "new-source"). Invalid values would fall through to the :else branch, potentially causing confusion.

Consider adding validation at the function entry:

 [nrepl-client-atom file-path content dry_run]
+ (when (and dry_run (not (contains? #{"diff" "new-source"} dry_run)))
+   (throw (ex-info (str "Invalid dry_run value: " dry_run)
+                   {:dry_run dry_run
+                    :expected #{"diff" "new-source"}})))
  (let [file (io/file file-path)

---

87-112: Same validation consideration applies here.

The write-text-file function has the same dry_run handling pattern as write-clojure-file and would benefit from the same parameter validation.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c102347 and d51faa4.

📒 Files selected for processing (3)

• src/clojure_mcp/tools/file_write/core.clj (4 hunks)
• src/clojure_mcp/tools/file_write/tool.clj (4 hunks)
• test/clojure_mcp/tools/file_write/core_test.clj (10 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

src/**/*.{clj,cljc}

📄 CodeRabbit inference engine (CLAUDE.md)

src/**/*.{clj,cljc}: Use :require with ns aliases for imports (e.g., [clojure.string :as string])
Include clear tool :description for 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/file_write/core.clj
• src/clojure_mcp/tools/file_write/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?)
Use try/catch with 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/file_write/core.clj
• test/clojure_mcp/tools/file_write/core_test.clj
• src/clojure_mcp/tools/file_write/tool.clj

test/**/*.{clj,cljc}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• test/clojure_mcp/tools/file_write/core_test.clj

🔇 Additional comments (4)

src/clojure_mcp/tools/file_write/core.clj (2)

12-20: LGTM!

The is-clojure-file? predicate correctly follows Clojure naming conventions and provides a clean abstraction for file type detection.

---

129-132: LGTM!

The write-file dispatcher correctly threads the dry_run parameter to both write-clojure-file and write-text-file paths.

src/clojure_mcp/tools/file_write/tool.clj (2)

94-113: LGTM!

The execute-tool implementation correctly passes dry_run through to the core write function and appropriately skips the timestamp update when in dry-run mode.

---

115-133: LGTM!

The format-results method correctly handles the new :new-source result for dry-run mode while preserving the existing diff-based formatting for normal operations.