feat: Implement dynamic impulse radius classification of tools - #236
marcelklehr wants to merge 18 commits into
Conversation
to automatically judge need for user confirmation Assisted-by: ClaudeCode:claude-opus-4-8 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
…citly destructive tools Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
…lassification Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
lukasdotcom
left a comment
lukasdotcom
left a comment
There was a problem hiding this comment.
I'll do a more in depth review tomorrow, but here are some things that I found for the tool categorizations. Feel free to ignore them if you disagree with any of these.
- update_page_content for collectives should be destructive. You could also just make sure it is adding text and only mark it destructive then.
- For duckduckgo and youtube it does not have an impulse radius. Before this they didn't require a confirmation, but now they would require confirmation.
- For remove_reaction I wouldn't consider that destructive.
…lementing the confirmation dialog Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Marcel Klehr <mklehr@gmx.net>
julien-nc
left a comment
julien-nc
left a comment
There was a problem hiding this comment.
How does it work with MCP tools? They default to EXTERNAL. Can an admin assign a radius later? If not, won't they stick with "always_confirm"?
|
|
||
| @tool | ||
| @dangerous_tool | ||
| @impulse(ImpulseRadius.SELF) |
There was a problem hiding this comment.
Should be destructive or always_confirm, no?
|
|
||
| @tool | ||
| @safe_tool | ||
| @impulse(ImpulseRadius.SELF) |
There was a problem hiding this comment.
It is ambiguous but since it can overwrite existing files, should it be considered as destructive like files.upoad_file for example? (@destructive_if(path_taken))
|
|
||
| @tool | ||
| @dangerous_tool | ||
| @impulse(transfer_radius) |
There was a problem hiding this comment.
Spotted by Qwen:
when destination_path is omitted the conversion writes over/near the source; no destructive_if (compare with copy_file, which has destination_taken). Verify in-place semantics and mark accordingly.
|
I'm also not currently seeing the impulse radii being passed to the Assistant chat. Do they need to be added here, perhaps? context_agent/ex_app/lib/nc_model.py Line 263 in 3c973a1 |
4 participants
to automatically judge need for user confirmation
🤖 AI (if applicable)