Let MCP tools declare which argument names their namespace

Raounak Sharmarequested to merge
623328-mcp-tools-declare-namespace-argument into master

What does this MR do and why?

Split out of !252615 (closed), which was closed as too large. This is part 1 of 2 and adds no behaviour of its own.

Governing the GitLab MCP server means knowing which project or group a tools/call acts on, and that has to be read out of the call's arguments. The tools do not agree on what that argument is called. Of the 22 tools that carry a rule under their own name:

Declared arguments Tools Example
project_id / group_id 11 get_work_item_types
id (REST convention) 8 manage_pipeline, get_job
project_ids / group_ids (lists of Global IDs) 1 attach_scan_profile
full_path (project or group) 1 search_labels
none 1 get_mcp_server_version

A single central list of argument names would have to assert that id always means a project. That is true of every tool today and is a guess about every tool added after. Resolving the wrong container is worse than resolving none, because then another namespace's rules decide the call. So each tool declares its own argument, and Mcp::Tools::Concerns::GovernanceNamespaceResolver turns that declaration into containers.

get_mcp_server_version is the one tool that cannot be governed — a version string is not a namespaced resource — so it is declared ungovernable explicitly rather than by omission.

Global IDs carry a class as well as a number. Projects and groups share a number space, so the class is checked before the number is used: a gid://gitlab/Group/1 passed to a project argument resolves to nothing, not to project 1.

governable_tools_namespace_spec walks every governable tool and fails the build if one declares no namespace argument, or declares one that is not in its own input schema. Without it, a tool added later is served ungoverned silently, and only in production.

Safe to merge on its own

Nothing calls resolve_governance_containers in this MR — the only caller arrives in part 2. The concern is inert:

  • No production code path reaches it.
  • None of the added method names (namespace_arguments, resolve_governance_containers) exist anywhere on master, so nothing is shadowed or overridden.
  • Gitlab::ResourceLookup is already included by these classes elsewhere, so including it again is a no-op.

Verified by running the whole MCP and tool-rules suite — including the enforcement specs that are not part of this MR — against a branch containing only these 12 files: 1734 examples, 0 failures, 4 pending.

How to reproduce the issue

There is no user-visible defect to reproduce here; this MR is the substrate the enforcement in part 2 stands on. What can be observed today is the inconsistency it addresses:

  1. Check out master.

  2. In rails console, list how the governable tools spell their scope argument:

    ::Mcp::Tools::Registry.new.tools.each do |name, tool|
  puts "#{name}: #{tool.class.input_schema[:properties].keys.join(', ')}"
end

  3. Observe that there is no argument name common to all of them — manage_pipeline and get_job use id, search_labels uses full_path, attach_scan_profile uses project_ids/group_ids. Any caller wanting the namespace of a call has nothing reliable to key on.

How to test the fix

  1. Check out this branch.

  2. In rails console, ask a REST-shaped tool for its containers:

    tool = ::Mcp::Tools::Registry.new.tools['get_job']
tool.send(:resolve_governance_containers, { id: 'your-group/your-project' })
# => [#<Project id: …>]

  3. Check that a wrong-typed Global ID resolves nothing rather than the wrong record:

    tool.send(:resolve_governance_containers, { id: 'gid://gitlab/Group/1' })
# => []

  4. Confirm no behaviour changed — every MCP tool still answers as before:

    bundle exec rspec spec/services/mcp spec/lib/api/mcp \
  ee/spec/lib/ai/tool_rules ee/spec/services/ai/tool_rules \
  ee/spec/lib/ee/api/mcp ee/spec/requests/api/mcp

Test coverage added

  • spec/services/mcp/tools/concerns/governance_namespace_resolver_spec.rb — each argument shape, Global ID class matching, blank and missing values.
  • ee/spec/lib/ai/tool_rules/governable_tools_namespace_spec.rb — the build-failing guardrail over every governable tool.
  • spec/services/mcp/tools/base/api_tool_spec.rb — deriving the container from the route path.

Database

lookup_projects / lookup_groups replace a find_by(id:) per identifier with one id_in. Same index, same filters, same scope — one statement instead of N, not a new access pattern.

SELECT projects.* FROM projects
WHERE projects.pending_delete = FALSE AND projects.hidden = FALSE
  AND projects.id IN (1, 2, 1000000)
Index Scan using projects_pkey on projects  (cost=0.14..2.43 rows=3 width=1448)
  Index Cond: (id = ANY ('{1,2,1000000}'::bigint[]))
  Filter: ((NOT pending_delete) AND (NOT hidden))

Full-path identifiers deliberately keep the per-value lookup: find_by_full_path follows redirects after a rename, which a batched WHERE full_path IN (...) would not. That is bounded because the only unbounded arguments (attach_scan_profile's project_ids / group_ids) are declared as Global IDs; the code comment records the assumption so a future list argument accepting paths does not silently reintroduce the fan-out.

References

Edited by Raounak Sharma

Merge request reports