Improve plan mode and ease tool call restrictions #191
Conversation
- Rewrite planning mode guide with clear phases (Understand, Explore, Decide, Present Plan), conditional language, preview protocol, and a self‑audit checklist. - Clarify tool docs with “absolute path” requirements and usage details (e.g., max_depth, include, all_occurrences); emphasize read‑only behavior where applicable. - Update filesystem feature docs to note that paths outside the workspace require approval.
- Add centralized path gating - Remove in-handler workspace-only checks; approval now guards cross‑workspace paths. - Plan behavior: allow safe tools (read/grep/dir‑tree/preview) and pwd; improve deny rules for shell. - Validate required parameters before dispatch in tools/call-tool! (native and MCP) and return INVALID_ARGS on missing keys. - Update tests to reflect approval gating and new preview messaging.
| === "Filesystem" | ||
|
|
||
| Provides access to filesystem under workspace root, listing, reading and writing files, important for agentic operations. | ||
| Provides access to the filesystem for listing, reading, writing, editing and moving files. Operates primarily on workspace files; paths outside the workspace require approval. |
|
|
||
| --- | ||
|
|
||
| ## Self-Audit Checklist (run before sending) |
There was a problem hiding this comment.
Interesting, in the future we could use this + a TODO tool to present to user this whole plan status
There was a problem hiding this comment.
Yes, TODO tool might be nice. But it doesn't really make sense once you do a preview, because that's what the LLM will implement.
TODO is high-level and LLM is still doing some exploration / decisions while implementing the points.
(Self-Audit Checklist is mainly to make some instructions stronger for some model.)
| :toolCall {:approval {:deny {"eca_shell_command" | ||
| {:argsMatchers {"command" [".*>.*", | ||
| :toolCall {:approval {:allow {"eca_shell_command" | ||
| {:argsMatchers {"command" ["pwd"]}} |
There was a problem hiding this comment.
from my test this didn't seem to work, it asked for approval to run pwd
There was a problem hiding this comment.
Weird, it never asks me. Can you check if you have some global settings that overrides this?
There was a problem hiding this comment.
not really, I have default toolCall approval config.
There was a problem hiding this comment.
weird, it worked now, will accept that for now hehe
ericdallo
left a comment
ericdallo
left a comment
There was a problem hiding this comment.
codewise looks good! I'm doing some tests yet
|
I gave it a test to implement #142, LMK what you think, I pasted the chat there as well: #192 Overall looks good to me, besides when I told to implement it tried to use clojureMCP tools and they failed a lot before working, but that's a different issue (especially in clojureMCP, failed tools don't return error: true). |
|
works really great! thank you! |
Improve planning prompt and tool docs
Allow tool calls outside workspace; validate required params
Add centralized path gating
Remove in-handler workspace-only checks; approval now guards cross‑workspace paths.
Plan behavior: allow safe tools (read/grep/dir‑tree/preview) and pwd; improve deny rules for shell.
Validate required parameters before dispatch in tools/call-tool! (native and MCP) and return INVALID_ARGS on missing keys.
Update tests to reflect approval gating and new preview messaging.
I added a entry in changelog under unreleased section.