-
-
Notifications
You must be signed in to change notification settings - Fork 60
Prompt management tool #116
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds public project/home config save/load utilities and exposes the home-config loader. Introduces a new manage-prompts tool (add/remove/list/get) wired into experimental tools and using project config for prompt storage. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant TS as ToolSystem
participant MPT as ManagePromptsTool
participant CFG as ConfigStore
U->>TS: invoke manage-prompts (op, prompt_name, prompt_content)
TS->>MPT: dispatch(inputs, context)
MPT->>MPT: validate inputs (op-specific)
alt operations that read config
MPT->>CFG: load-project-config(working-dir)
CFG-->>MPT: project config
end
alt add_prompt
MPT->>CFG: save-project-config(updated :prompts, working-dir)
CFG-->>MPT: ok
else remove_prompt
MPT->>CFG: save-project-config(updated :prompts, working-dir)
CFG-->>MPT: ok / not-found
else list_prompts/get_prompt
MPT-->>TS: formatted result (prompts or single prompt)
end
TS-->>U: formatted response (success / not-found / error)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/clojure_mcp/config.clj(7 hunks)src/clojure_mcp/tools.clj(3 hunks)src/clojure_mcp/tools/manage_prompts/tool.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/tools.cljsrc/clojure_mcp/config.cljsrc/clojure_mcp/tools/manage_prompts/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.cljsrc/clojure_mcp/config.cljsrc/clojure_mcp/tools/manage_prompts/tool.clj
🔇 Additional comments (15)
src/clojure_mcp/tools.clj (1)
33-34: LGTM!The manage-prompts tool symbol is correctly added to the experimental tools collection, following the established naming convention.
src/clojure_mcp/config.clj (6)
5-5: LGTM!The
clojure.pprintimport is correctly aliased and used in thesave-config-filefunction for pretty-printing EDN output.
38-43: LGTM!Making
load-home-configpublic is intentional based on the PR objectives, enabling external access to home config loading.
45-59: LGTM!The
save-config-fileimplementation correctly:
- Creates parent directories when needed
- Pretty-prints EDN for readability
- Logs success/failure appropriately
- Returns a boolean indicating success
The error handling with try/catch is appropriate for file I/O operations.
61-64: LGTM!The
get-project-config-pathfunction correctly constructs the project config path following the established.clojure-mcp/config.ednconvention.
66-71: LGTM!The
load-project-configfunction correctly reuses the existingload-config-fileutility and follows the same pattern asload-home-config.
73-76: LGTM!The
save-project-configfunction is a clean wrapper that composessave-config-filewithget-project-config-path, maintaining good separation of concerns.src/clojure_mcp/tools/manage_prompts/tool.clj (8)
1-6: LGTM!The namespace declaration follows Clojure conventions:
- Uses kebab-case naming
- Properly requires dependencies with aliases
- Includes clear docstring
As per coding guidelines.
8-9: LGTM!The tool name correctly uses snake_case for MCP protocol compatibility.
11-32: LGTM!The tool description is comprehensive and follows coding guidelines:
- Clear explanation of operations
- Helpful examples
- Guidance on when to use the tool
As per coding guidelines requiring clear tool descriptions for LLM guidance.
34-43: LGTM!The JSON schema correctly defines:
- Enum constraint for operations
- Clear property descriptions
- Required fields
45-65: LGTM!The input validation is thorough and follows coding guidelines:
- Validates required parameters
- Provides helpful error messages with context
- Uses specific exception handling with ex-info
As per coding guidelines requiring validation with helpful error messages.
67-106: LGTM!The execution logic correctly:
- Handles all four operations with appropriate logic
- Uses try/catch for error handling with logging
- Returns structured data with :result and :error status
- Leverages project config utilities appropriately
The prompt names are stored as strings in the :prompts map as intended (matching the schema definition).
As per coding guidelines requiring structured data with result and error status.
147-151: LGTM!The
create-manage-prompts-toolfunction correctly extracts the working directory from the nREPL client atom and constructs the tool configuration map.
153-159: LGTM!The
manage-prompts-toolfunction properly wraps the tool creation and returns a registration map using the tool-system API.
|
@bhauman This is a first round effort, and just lists custom prompts. If I'm on the right track, I'll proceed with adding/editing next. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/clojure_mcp/tools/manage_prompts/tool.clj (1)
135-141: Consider filtering nil values before joining.The
whenforms in the vector can returnnilif their conditions are false, resulting in blank lines in the output when joined. For cleaner formatting, filter outnilvalues before joining.Apply this diff to remove blank lines:
(str "Prompt: " (:prompt_name result) "\n\n" (if (map? content) - (str/join "\n" - [(when (:description content) - (str "Description: " (:description content))) - (when (:content content) - (str "Content:\n" (:content content))) - (when (:file-path content) - (str "File: " (:file-path content)))]) + (str/join "\n" + (filter some? + [(when (:description content) + (str "Description: " (:description content))) + (when (:content content) + (str "Content:\n" (:content content))) + (when (:file-path content) + (str "File: " (:file-path content)))])) content))))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/clojure_mcp/tools/manage_prompts/tool.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/tools/manage_prompts/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/manage_prompts/tool.clj
🧠 Learnings (1)
📚 Learning: 2025-08-02T20:23:28.499Z
Learnt from: CR
PR: bhauman/clojure-mcp#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/manage_prompts/tool.clj
🔇 Additional comments (1)
src/clojure_mcp/tools/manage_prompts/tool.clj (1)
148-152: get-nrepl-user-dir always returns a valid directory. create-and-start-nrepl-connection initializes the client map with a canonical :nrepl-user-dir, so working-dir cannot be nil.Likely an incorrect or invalid review comment.
Summary by CodeRabbit