-
-
Notifications
You must be signed in to change notification settings - Fork 61
Add new prompt to save custom user prompts to config #117
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 a new public prompt var Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User / Agent
participant P as Prompts Module
U->>P: Request "save-new-prompt"(prompt_name, prompt_text)
activate P
P->>P: Build user-facing guidance message (how to create/update and save under :prompts)
P-->>U: Return guidance message
deactivate P
note over U: Persistence to config (e.g., writing to .clojure-mcp/config.edn) is performed by the caller/tool using the returned guidance, not by the Prompts Module.
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 (1)
src/clojure_mcp/prompts.clj(2 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/prompts.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/prompts.clj
🔇 Additional comments (1)
src/clojure_mcp/prompts.clj (1)
363-363: Addition to default-prompts is consistent with current implementation.The addition of
save-new-promptto the prompts list is correct for the current implementation. However, if the prompt is refactored to acceptnrepl-client-atom(as suggested in the previous comment), this line would need to be updated to(save-new-prompt nrepl-client-atom)to match the pattern used by other action prompts likeadd-dirandscratch-pad-load.
src/clojure_mcp/prompts.clj
Outdated
| (def save-new-prompt | ||
| {:name "save_new_prompt" | ||
| :description "Asks the user for a new prompt and a name, and saves them to their user config" | ||
| :arguments [{:name "prompt_name" | ||
| :description "Name for this prompt" | ||
| :required? true} | ||
| {:name "prompt_text" | ||
| :description "Text content for this prompt" | ||
| :required? true}] | ||
| :prompt-fn (fn [_ request-args clj-result-k] | ||
| (let [prompt-name (get request-args "prompt_name") | ||
| prompt-text (get request-args "prompt_text")] | ||
| (clj-result-k | ||
| {:description (str "Create prompt: " prompt-name) | ||
| :messages [{:role :user | ||
| :content (format | ||
| "Save %s in the `.clojure-mcp` project folder, in the `config.edn` file under the `:prompts` key using the STRING key %s, using the following format: | ||
| `:description` - \"Custom user-added prompt\" | ||
| `:content` - %s | ||
| " | ||
| prompt-name prompt-name prompt-text)}]})))}) |
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.
Critical: Prompt doesn't actually save; format mismatch with expected config structure.
The prompt has several critical issues:
-
No actual save operation: The prompt-fn only constructs a user message instructing the LLM to manually edit
config.edn, rather than programmatically persisting the data usingconfig/set-config!(as seen inadd-dirat line 172). This approach is unreliable and won't work consistently. -
Format mismatch: The format specified in the message (
:descriptionand:content) doesn't match the structure expected bycreate-prompt-from-config(lines 309-324), which requires:description,:args, and either:file-pathor:content. This means even if the LLM follows the instructions, the saved prompt won't be usable. -
Misleading description: Line 285 claims this prompt "saves them to their user config", but it only requests the LLM to save, not performing the operation itself.
-
No validation or error handling: Unlike other action prompts in this file (e.g.,
add-dir,scratch-pad-save-as), there's no input validation or try/catch blocks.
As per coding guidelines: "Validate inputs and provide helpful error messages in MCP tools" and "Use try/catch with specific exception handling".
Consider this approach instead:
-(def save-new-prompt
- {:name "save_new_prompt"
- :description "Asks the user for a new prompt and a name, and saves them to their user config"
- :arguments [{:name "prompt_name"
- :description "Name for this prompt"
- :required? true}
- {:name "prompt_text"
- :description "Text content for this prompt"
- :required? true}]
- :prompt-fn (fn [_ request-args clj-result-k]
- (let [prompt-name (get request-args "prompt_name")
- prompt-text (get request-args "prompt_text")]
- (clj-result-k
- {:description (str "Create prompt: " prompt-name)
- :messages [{:role :user
- :content (format
- "Save %s in the `.clojure-mcp` project folder, in the `config.edn` file under the `:prompts` key using the STRING key %s, using the following format:
-`:description` - \"Custom user-added prompt\"
-`:content` - %s
-"
- prompt-name prompt-name prompt-text)}]})))})
+(defn save-new-prompt [nrepl-client-atom]
+ {:name "ACT/save_new_prompt"
+ :description "Saves a new custom prompt to the user config"
+ :arguments [{:name "prompt_name"
+ :description "Name for this prompt (kebab-case recommended)"
+ :required? true}
+ {:name "prompt_text"
+ :description "Text content for this prompt"
+ :required? true}]
+ :prompt-fn (fn [_ request-args clj-result-k]
+ (let [prompt-name (get request-args "prompt_name")
+ prompt-text (get request-args "prompt_text")]
+ (if (or (str/blank? prompt-name) (str/blank? prompt-text))
+ (clj-result-k
+ {:description "Missing required arguments"
+ :messages [{:role :assistant
+ :content "Error: Both prompt_name and prompt_text are required and cannot be blank."}]})
+ (try
+ (let [current-prompts (config/get-prompts @nrepl-client-atom)
+ prompt-key (keyword prompt-name)
+ new-prompt-config {:description "Custom user-added prompt"
+ :content prompt-text
+ :args []}
+ updated-prompts (assoc current-prompts prompt-key new-prompt-config)]
+ (config/set-config! nrepl-client-atom :prompts updated-prompts)
+ (clj-result-k
+ {:description (str "Saved prompt: " prompt-name)
+ :messages [{:role :assistant
+ :content (format "Successfully saved custom prompt '%s' to config.edn under :prompts key.\n\nYou can now use this prompt with the name: %s"
+ prompt-name
+ prompt-name)}]}))
+ (catch Exception e
+ (clj-result-k
+ {:description (str "Error saving prompt: " (.getMessage e))
+ :messages [{:role :assistant
+ :content (format "Failed to save prompt '%s'.\nError: %s"
+ prompt-name
+ (.getMessage e))}]}))))))})Then update line 363 to pass the atom:
- save-new-prompt
+ (save-new-prompt nrepl-client-atom)Would you like me to open an issue to track implementing actual save functionality with proper validation and error handling?
|
@bhauman New version pushed |
Summary by CodeRabbit