fix(tools): wire manage_rag into tool dispatch (#5557)#5595
Open
RavinduPabasara wants to merge 1 commit into
Open
fix(tools): wire manage_rag into tool dispatch (#5557)#5595RavinduPabasara wants to merge 1 commit into
RavinduPabasara wants to merge 1 commit into
Conversation
manage_rag was advertised in the UI's TOOL_GROUPS and had a maintained handler (do_manage_rag, last touched by the odysseus-dev#1660 data-loss fix), but was absent from TOOL_TAGS and from every dispatch site, so agents had no working path to it: fence blocks naming it were rejected as unknown before reaching a handler, and models looped hunting for workarounds. The unwiring appears accidental (registry-era fallout, cf odysseus-dev#4277): the handler kept receiving fixes after odysseus-dev#1660, the UI kept advertising the tool, and its tests kept passing against the orphaned function. Wiring, following the manage_memory pattern exactly: - TOOL_TAGS: recognize the manage_rag fence tag - tool_execution: route manage_rag through dispatch_ai_tool - ai_interaction.dispatch_ai_tool: add the manage_rag branch Also add manage_rag to NON_ADMIN_BLOCKED_TOOLS: add_directory indexes arbitrary server paths into the shared RAG index, the same sensitivity class as manage_memory/read_file, which are both gated. Without this, re-wiring the tool would open a new non-admin capability. Tests: tag recognition, dispatch reaches the handler with session scope, public-user gating, and user-disable handling.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
manage_ragwas advertised to users (UITOOL_GROUPS) and to nobody's benefit: it was absent fromTOOL_TAGSand from every dispatch site, so agent tool blocks naming it were rejected as unknown before reaching a handler — models loop hunting for workarounds, as documented in #5557. The handler itself was alive and maintained (it received the #1660 data-loss fix), so the unwiring looks like accidental registry-era fallout (cf. #4277) rather than a deliberate removal — this PR takes the issue's "re-register" direction: recognize the fence tag, route it throughdispatch_ai_toolnext tomanage_memory, and addmanage_ragtoNON_ADMIN_BLOCKED_TOOLSso re-wiring doesn't open a new non-admin capability (add_directoryindexes arbitrary server paths — same sensitivity class asmanage_memory/read_file, both gated).Target branch
dev, notmain.Linked Issue
Fixes #5557
Type of Change
Checklist
devuvicorn app:app) and verified the change works — details and the one thing I did not drive are in How to Test.How to Test
python -m pytest tests/test_manage_rag_dispatch.py— 4 new regression tests: fence tag recognized, dispatch reachesdo_manage_ragwith session scope, public-user gating, user-disable handling. Hermetic (no chromadb), same style astest_rag_remove_directory_scope.py.python -m pytest— 4,613 passed. The 4 failures inTestHostDockerAccess/test_cookbook_docker_access.pyoccur identically on cleandevon this machine (Docker-socket environment tests, macOS) — not introduced here.uvicorn app:app, fresh sqlite env) — boots cleanly with the change, and exercising the real dispatch chain in the running install now reaches the handler:Note: the destructive remove_directory action stays available to admin/single-user as-is, since #1660's collection-wipe bug is already fixed inside the handler. Happy to restrict dispatch to list/add_directory if you'd prefer the conservative surface.
No UI changes — nothing that renders was touched.