[wak327]

🐛 Bug-fix PR

📌 Summary

Tool conflict checks compared raw tool names with the stored DbTool.name value. DbTool.name holds the slug of the custom name, with the gateway prefix for a gateway tool. Names that differ only in case or separator characters passed the check. One visibility scope then held two tools with the same invocation name, and tools/call cannot tell them apart.

This PR makes tool creation, tool update, and configuration import compare the stored name in the target visibility scope.

Refs #6189

🔁 Reproduction Steps

Issue #6189 lists the gaps. These steps use the default GATEWAY_TOOL_NAME_SEPARATOR (-) on main:

1. Send POST /tools with a public tool named my-tool. Then send POST /tools with a public tool named My_Tool. Both requests succeed, and two rows store my-tool.
2. Send POST /tools twice with a public tool named get_weather. Both requests succeed, because the check compares get_weather with the stored get-weather.
3. Create two private tools with the same name for the same owner. Both requests succeed, because register_tool() has no private-scope branch.
4. Send PUT /tools/{id} with {"custom_name": "My_Tool"} for a second public tool. The rename succeeds, and two rows store my-tool.
5. Import a configuration that contains My_Tool and my-tool. Both rows are created.

🐞 Root Cause

All paths are in mcpgateway/services/tool_service.py. The set_custom_name_and_slug listener in mcpgateway/db.py computes DbTool.name at flush time.

• register_tool() compared the raw tool.name with DbTool.name. It had no branch for private visibility.
• update_tool() compared the raw custom_name with DbTool.custom_name for a tool without a gateway. A rename to a slug-equivalent name did not match. A change of custom_name alone ran no check for such a tool.
• _process_tool_chunk() compared raw names with DbTool.name, and only with existing rows. Two slug-equivalent names in one import batch were both added.
• The public, team, and private scope filter existed in four copies with different fallbacks.

💡 Fix Description

All source changes are in mcpgateway/services/tool_service.py.

• _tool_name_scope() returns the filter for one namespace: all public tools, the team tools of one team, or the private tools of one owner. It returns None when the team or the owner is missing. Every conflict check uses it.
• _stored_tool_name() computes the stored name of a new tool the same way as the listener. It uses slugify() for a tool without a gateway and build_gateway_tool_invocation_name() for a gateway tool.
• _check_new_tool_name_conflict() runs in register_tool(). It uses LIMIT 1, so legacy duplicate rows raise ToolNameConflictError (HTTP 409) and not MultipleResultsFound (HTTP 400). The private scope uses owner_email, or created_by when owner_email is empty. The new row uses the same owner.
• update_tool() computes the final custom name and the final stored name. It runs _check_tool_name_conflict() on a rename, a custom_name change, or a visibility change. That helper now compares DbTool.name and replaces _check_gateway_tool_invocation_name_conflict(). It keeps the get_for_update() row lock and excludes the tool itself.
• _process_tool_chunk(), _process_single_tool_for_bulk() and _create_tool_object() now annotate the team ID as Optional[str]. register_tools_bulk() already takes team_id: Optional[str], and team IDs are strings.
• _process_tool_chunk() computes one stored name for each incoming tool and matches by position. A tool that the chunk added earlier counts as an existing tool, so the conflict strategy applies to it. With update, the later entry updates the pending row. _process_single_tool_for_bulk() now receives one existing_tool and not a name map.
• _load_authorized_tool_conflicts() and _existing_tools_query() (added on main by fix: enforce ownership on configuration import updates #7243) look up stored names in the scope from _tool_name_scope(). The import ownership check and the late-collision check of fix: enforce ownership on configuration import updates #7243 therefore cover slug-equivalent names. A non-owner who imports Shared_Tool with conflict_strategy=update gets Permission denied for the stored shared-tool row.

Tests:

• tests/unit/mcpgateway/services/test_tool_service.py::TestStoredToolNameConflict uses the real test database. It covers the public, team, and private scopes, the created_by owner fallback, legacy duplicate rows, and gateway-prefixed names. It also covers renames through name and custom_name, a visibility change, and a rename that keeps the stored name of the tool itself. For bulk import, it covers existing rows, two names in one batch, and one name on two gateways. Expected names come from slugify() and build_gateway_tool_invocation_name(), so the tests pass for any separator.
• tests/live_gateway/e2e/test_e2e.py::TestToolNameConflict sends POST /tools (HTTP 409) and PUT /tools/{id} (HTTP 400) to a live gateway. It reads GATEWAY_TOOL_NAME_SEPARATOR to build the expected stored name.
• The existing _process_single_tool_for_bulk() tests pass existing_tool. The update_tool() doctest sets name, custom_name, and visibility to None, so the mock does not reach slugify().

Behavior changes

• POST /tools, the Admin UI add form, and configuration import reject a name whose stored name a tool in the same scope already uses.
• PUT /tools/{id} and the Admin UI edit form reject a rename or a visibility change to a stored name in use. PUT /tools/{id} keeps its existing HTTP 400 for an update conflict. The Admin UI edit form keeps HTTP 409.
• On update, a tool without a gateway and a gateway tool with the same custom name no longer conflict. Their stored names differ, so their invocation names differ.
• Bulk import with visibility="team" and no team_id runs no conflict query. Before, it queried the private tools of the owner, which is the wrong scope. register_tool() already skipped this case.

Deviations from the issue

• The issue asks for unique custom_name values. This PR compares DbTool.name, the stored invocation name. tools/call resolves tools by that value. Two gateways can each expose search. Their tools store <gateway-a>-search and <gateway-b>-search, so they do not collide. A custom_name comparison rejects that valid case.
• Gap 1 (federation sync) is fixed on main by Fix for Internal 453 #6534. This PR reuses its gateway-prefixed name helper.
• Gap 4 (a database unique constraint per visibility scope) needs an Alembic migration, a plan for existing duplicate rows, and a maintainer decision. This PR does not include it.

Known residual gaps

• No database constraint backs the scope check. Two concurrent requests for the same unused stored name can both commit (gap 4).
• gRPC reflection sync in mcpgateway/services/grpc_service.py adds DbTool rows without a scope check. The feature is off by default (MCPGATEWAY_GRPC_ENABLED=false), and the issue does not list it.
• update_tool() reads the conflict row through get_for_update(), which expects one row. If two legacy duplicate rows already use the target stored name, the update fails with MultipleResultsFound (HTTP 400) and not with ToolNameConflictError.
• The bulk rename strategy does not check the generated <name>_imported_<timestamp> name again. This behavior is unchanged.

📏 Reviewability

• This PR has one clear purpose
• The linked issue is not labeled triage
• Unrelated bugs or improvements are tracked in separate issues/PRs
• Tests are included with the code they validate
• If AI-assisted, I understand and can explain the generated changes

🧪 Verification

All commands ran from the repository root. python is the project virtual environment interpreter.

Check	Command	Status
Ruff lint	uv tool run ruff==0.15.20 check mcpgateway/services/tool_service.py tests/unit/mcpgateway/services/test_tool_service.py tests/unit/mcpgateway/services/test_tool_service_coverage.py tests/live_gateway/e2e/test_e2e.py	tool_service.py clean. 4 findings on lines this PR does not change: 3 useless-import-alias in test_tool_service_coverage.py, 1 blank-line rule at test_e2e.py:3955
Ruff format	uv tool run ruff==0.15.20 format --check on the same files	tool_service.py formatted. The 3 test files have format diffs only in hunks this PR does not change
Pylint, source	make pylint TARGET=mcpgateway/services/tool_service.py	10.00/10
Pylint, tests	make pylint TARGET="tests/unit/mcpgateway/services/test_tool_service.py tests/unit/mcpgateway/services/test_tool_service_coverage.py tests/live_gateway/e2e/test_e2e.py"	8.17/10. On changed lines, the only message is W0621 redefined-outer-name for pytest fixtures, as in the rest of these files
Docstring coverage	uv tool run interrogate==1.7.0 -v mcpgateway/services/tool_service.py	100%
mypy	python -m mypy mcpgateway/services/tool_service.py --follow-imports=silent	189 errors in the file, 198 on main (same local mypy version). Three messages are new. Each is on an existing assignment in the bulk update strategy, for example existing_tool.input_schema = tool.input_schema. mypy checks these lines now because existing_tool has the DbTool type, not Any
Affected unit tests	python -m pytest tests/unit/mcpgateway/services/test_tool_service.py tests/unit/mcpgateway/services/test_tool_service_coverage.py tests/unit/mcpgateway/services/test_row_level_locking.py tests/unit/mcpgateway/services/test_tool_audit_no_db.py tests/unit/mcpgateway/test_email_extraction_consistency.py -n 8	884 passed, 3 skipped
Other separator	GATEWAY_TOOL_NAME_SEPARATOR=-- python -m pytest tests/unit/mcpgateway/services/test_tool_service.py -k TestStoredToolNameConflict	17 passed
New tests against main	The -k TestStoredToolNameConflict run with tool_service.py from main	15 failed, 2 passed (the two allow-path cases)
Doctests	python -m pytest --doctest-modules mcpgateway/services/tool_service.py mcpgateway/utils/create_slug.py --no-cov, with test-only values for JWT_SECRET_KEY, AUTH_ENCRYPTION_SECRET, PLATFORM_ADMIN_PASSWORD, DEFAULT_USER_PASSWORD, and BASIC_AUTH_PASSWORD	23 passed, 1 skipped
Full unit suite	DATABASE_URL='sqlite:///:memory:' TEST_DATABASE_URL='sqlite:///:memory:' ARGON2ID_TIME_COST=1 ARGON2ID_MEMORY_COST=1024 python -m pytest -n auto --maxfail=0 --ignore=tests/fuzz --ignore=tests/manual --ignore=test.py --ignore=tests/live_gateway (the make test environment and ignore list)	23756 passed, 926 skipped, 2 xfailed
Diff coverage	The affected unit tests with --cov=mcpgateway --cov-report=xml:<file>, then diff-cover <file> --compare-branch=main --fail-under=90	tool_service.py: 55 changed lines, 100% covered
Secrets	uv tool run --from 'git+https://github.com/ibm/detect-secrets.git@076672a9a01abdfc7ecee2e7d14f08cdccb73976' detect-secrets scan --use-all-plugins on the 5 changed files	No findings on changed lines
After rebase onto a211ab7d	The affected unit tests command above plus tests/unit/mcpgateway/services/test_import_service.py	1020 passed, 3 skipped
Ownership merge, red check	test_register_tools_bulk_update_ownership_matches_stored_name with tool_service.py from main	Failed: the import got Tool name conflict and not Permission denied
Doctests after rebase	python -m pytest --doctest-modules mcpgateway/services/tool_service.py with test-only secrets	22 passed, 1 skipped
Live gateway e2e	python -m pytest -p playwright --collect-only tests/live_gateway/e2e/test_e2e.py -k TestToolNameConflict	3 tests collected. Not executed, see below

Checks not run:

• make docker-nuke docker-prod-rust testing-up RUST_MCP_MODE= and make test-e2e: no gateway stack runs on the test host. make docker-nuke removes local Docker state, and the host runs unrelated containers. The new live-gateway tests are collected but not executed.
• make coverage: the diff coverage above comes from the affected unit tests, not from the full suite.
• make detect-secrets-scan: this target rewrites .secrets.baseline. The scan above reads only the changed files.
• Manual regression in the Admin UI and with PostgreSQL: not run.

📐 MCP Compliance (if relevant)

• Matches current MCP spec
• No breaking change to MCP clients

✅ Checklist

• Code formatted (make black isort pre-commit)
• No secrets/credentials committed