UN-3702 [FEAT] Hide deprecated runner tools from new workflows and block adding them - #2298
Deepak-Kesavan wants to merge 2 commits into
Conversation
…l list after failed switches
Frontend Lint Report (Biome)✅ All checks passed! No linting or formatting issues found. |
|
|
| patch.object( | ||
| ToolProcessor, | ||
| "_get_tool_ids_in_workflow", | ||
| MagicMock(return_value=tool_ids_in_workflow), |
There was a problem hiding this comment.
Existing-tool lookup lacks coverage The retention tests mock the lookup for tool IDs in a workflow, so they check the list filter but not the new database query. A regression in that query could hide a deprecated tool from a workflow that already uses it without failing these tests. Please add a database-backed test for an accessible workflow with an existing registry tool and one without it.
Prompt To Fix With AI
This is a comment left during a code review.
Path: backend/tool_instance_v2/tests/test_deprecated_registry_tools.py
Line: 73-76
Comment:
**Existing-tool lookup lacks coverage** The retention tests mock the lookup for tool IDs in a workflow, so they check the list filter but not the new database query. A regression in that query could hide a deprecated tool from a workflow that already uses it without failing these tests. Please add a database-backed test for an accessible workflow with an existing registry tool and one without it.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Unstract test resultsPer-group results
Critical paths
|



What
classify, Text Extractortext_extractor, and any private registry tools) can no longer be added to a workflow.Why
How
GET tool/takes an optionalworkflow_idquery param.ToolProcessor.get_tool_listkeeps a registry tool only if that workflow already has an instance of it. The workflow is looked up throughWorkflow.objects.for_user, and a malformedworkflow_idreturns a 400.ToolInstanceSerializer.validate_tool_idrejects registry tools. It runs on both create and update, so aPATCHcan't switch an instance to one either. It allows an instance to keep the registry tool it already has.ToolProcessor.is_registry_tooldecides what counts as a registry tool: the uid resolves in the tool registry (public_tools.json/private_tools.json). Prompt Studio and agentic tools are keyed by UUIDs, so they're unaffected.Agency.jsx)workflow_idwhen fetching the tool list.Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)
tool_settings_schema) and execution are unchanged. Those still resolve the tool throughget_tool_by_uid, which this PR doesn't touch, and the tool stays in their list.GET tool/withoutworkflow_id: they no longer get registry tools. The only other caller,CreateApiDeploymentFromPromptStudio.jsx, looks for Prompt Studio tools only, so it's unaffected.Database Migrations
Env Config
Relevant Docs
Related Issues or PRs
Dependencies Versions
Notes on Testing
backend/tool_instance_v2/tests/test_deprecated_registry_tools.py:workflow_idparsing in the view: a UUID is passed through, a missing value lists without a workflow, and a malformed value returns a 400.validate_tool_idthroughis_valid()for create, for update switching to a registry tool (rejected), and for an instance keeping its own registry tool (allowed).is_registry_toolagainst the repo's realpublic_tools.json.tool_instance_v2pass locally. Removing the check makes the new validation tests fail.Screenshots
Checklist
I have read and understood the Contribution Guidelines.