Add user-defined-tool operations to the agent-operations layer#22625
Open
dannon wants to merge 8 commits intogalaxyproject:devfrom
Open
Add user-defined-tool operations to the agent-operations layer#22625dannon wants to merge 8 commits intogalaxyproject:devfrom
dannon wants to merge 8 commits intogalaxyproject:devfrom
Conversation
Wraps DynamicToolManager.list_unprivileged_tools so the in-process MCP server can enumerate a user's user-defined tools, matching the parity gap with the standalone galaxy-mcp server. Also wires up the lazy DynamicToolManager property on AgentOperationsManager and a _setup_udt_user helper on the smoke test class for the rest of the UDT work.
Validates the representation through DynamicUnprivilegedToolCreatePayload and delegates to DynamicToolManager.create_unprivileged_tool, mirroring the POST /api/unprivileged_tools endpoint. The MCP wrapper carries the full GalaxyUserTool schema in its docstring (required fields, the common container-as-string mistake, a worked example) so agents can construct valid representations without round-tripping.
Looks up the UDT by UUID, then deactivates it via
DynamicToolManager.deactivate_unprivileged_tool. Mirrors DELETE
/api/unprivileged_tools/{uuid}.
Mirrors run_tool but passes tool_uuid in the payload, which tools_service._create routes to the toolbox's unprivileged-tool resolver. Closes UDT parity with the standalone galaxy-mcp server.
deactivate_unprivileged_tool deliberately only flips the per-user UserDynamicToolAssociation.active flag, leaving DynamicTool.active intact so other users with associations to the same DynamicTool aren't affected (the model schema permits many-to-many, even though the current create path is 1:1). That means a user who deactivates "their" UDT can still resolve it by UUID through the toolbox -- and run it via tools_service._create -- because get_unprivileged_tool_by_uuid doesn't filter by association.active either. Add a runtime preflight in run_user_tool that fails the call when either the underlying tool or the calling user's association is inactive. Also surfaces unauthenticated and unowned errors as clean ValueErrors before reaching the deeper service layer. Tightening the chokepoint (DynamicToolManager.get_unprivileged_tool_by_uuid) to filter by association.active would close this across all entry points but is a meaningful behavior change for the existing UnprivilegedToolsApi endpoints; leaving that for a separate review.
test_mcp_run_user_tool was building its history and input dataset with the default test interactor, while the MCP-side run authenticated as a freshly-provisioned UDT user -- so the MCP call was reaching for resources owned by a different user. Switch to DatasetPopulator(self._get_interactor(api_key=api_key)) so populator- side and MCP-side calls share an identity. Also assert on the run-tool response shape (jobs[].tool_id, outputs[].output_name) in addition to the end-to-end content check. test_mcp_delete_user_tool now also tries to run the UDT after deletion and asserts the call errors with "deactivated" -- regression test for the deactivation guard in run_user_tool.
mvdbeek
reviewed
May 2, 2026
mvdbeek
reviewed
May 2, 2026
mvdbeek
reviewed
May 2, 2026
mvdbeek
reviewed
May 2, 2026
Per Marius' review on PR 22625 -- Galaxy only uses local imports when there's a real reason (circular dep, optional dep). None of these had one, so they belong at the top of the file alongside the rest.
dannon
added a commit
to dannon/galaxy
that referenced
this pull request
May 2, 2026
CI on PR galaxyproject#22625 caught this -- packages/test.sh installs galaxy-app on its own and runs pytest with --doctest-modules, which imports every source file at collection time. The earlier "Hoist agent-operations local imports to module level" commit moved InvocationIndexPayload/WorkflowIndexPayload up alongside the rest, but galaxy.webapps lives in galaxy-webapps and isn't a galaxy-app dependency, so those two imports blow up collection with ModuleNotFoundError. Restoring just those two as method-local imports (with a comment so we don't try to hoist them again) gets the package test back to green without dragging the rest of the cleanup back down.
WorkflowIndexPayload and InvocationIndexPayload were defined in the webapps service layer, which forced operations.py (in galaxy-app) to reach across the package boundary into galaxy-webapps just to construct them. They're plain Pydantic models that extend a base already living in galaxy.schema.schema, so move them next to their parents. Services keep the same names via re-export so external consumers don't break, and operations.py can now import them at module level alongside the rest of the galaxy.schema imports.
dannon
force-pushed
the
agent-ops-udt-parity
branch
from
160cfed to
2cb7189
Compare
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
2 participants
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
The standalone galaxy-mcp server has grown four user-defined-tool (UDT)
tools -- create, list, delete, run -- that the in-process MCP server
exposed at /api/mcp didn't have. This adds them, so an agent talking
to the built-in MCP can manage UDTs through the same surface as the
external server.
The implementation is thin -- each operation is a wrapper over the
existing DynamicToolManager (for create/list/delete) and
tools_service._create (for run, which already accepts tool_uuid in its
payload and routes to the toolbox's unprivileged-tool resolver). No
new infrastructure or config; the existing /api/unprivileged_tools
endpoints are the reference for shape and semantics.
The MCP-wrapper docstrings carry agent-facing UX detail: required-field
schemas for create_user_tool (including the common "container must be
a string, not a dict" mistake), worked examples, and NEXT STEPS
pointers. Tool descriptions are how an LLM picks tools and fills
arguments, so this is functional rather than decorative.
A deactivation question for review
While wiring up run_user_tool I noticed that a UDT deactivated through
DELETE /api/unprivileged_tools/{uuid} can still be resolved and run
by UUID via tools_service._create -- because
deactivate_unprivileged_tool only flips the per-user
UserDynamicToolAssociation.active, leaving DynamicTool.active intact,
and get_unprivileged_tool_by_uuid doesn't filter by
association.active.
Reading the original split between deactivate() (admin path, flips
DynamicTool.active) and deactivate_unprivileged_tool() (user path,
flips only the association), and the fact that the schema permits
many-to-many UserDynamicToolAssociation -> DynamicTool, that asymmetry
looks deliberate -- "deactivated" is a per-user UI-facing state, not
a hard runtime gate, so we don't break other users sharing the
underlying tool.
So I left the manager alone and added a per-user runtime guard inside
run_user_tool only (checks both DynamicTool.active and the calling
user's association.active). That fixes the agent-operations entry
point without touching shared state.
The broader question -- whether get_unprivileged_tool_by_uuid should
also filter by association.active so the standard tools API run path
respects per-user deactivation -- is out of scope for this PR but
worth a separate look.
Out of scope
rather than parity work)
can land separately)
Test plan
pytest test/integration/test_agents.py::TestMCPServerSmoke -vpytest lib/galaxy_test/api/test_unprivileged_tools.py -venable_mcp_server=trueandenable_beta_tool_formats=true, confirm the four*_user_toolentries appear in
tools/listover MCP and that create -> run ->delete -> attempt-run-after-delete behaves as expected