MCP engine test suite

[adessy]

The MCP engine was built velocity-first with minimal test coverage. This PR adds a systematic test suite, plus fixes for two real bugs the new specs surfaced.

Production fixes (found while writing the suite)

• Missing authorization guards — update_project, update_phase, update_resource and replace_form_fields called neither authorize_project! nor Pundit authorize, so the draft-only constraint was not enforced for updates: a super admin could mutate published projects through the MCP. Both guards are now in place, matching every other mutating tool.
• Silent empty list for a bad parent id — list_events and list_causes filtered without looking up the parent, so a nonexistent project_id/phase_id returned [] (an LLM reads that as "no events" rather than "wrong id"). They now return the standard not-found error like their siblings.
• No way to remove a project header background — remote_header_bg_url was a non-nullable string; it now accepts null, which update_project translates into remove_header_bg! (CarrierWave ignores plain nil assignment), mirroring the web API's remove_image_if_requested!.
• Opaque replace_form_fields validation errors — form-level error keys (stale_data, locked_deletion, …) were relayed as a raw JSON dump; they now map to actionable prose (re-fetch and retry, echo locked fields back, structural page rules), with the raw errors still in structuredContent.
• Group visibility was invisible and unreachable — with visible_to: 'groups', the project payload carried no group information, the schemas had no way to set the group list, and list_groups' project_id filter matched smart-group rules referencing the project rather than the groups that can see it. The project schemas now accept group_ids (replace-wholesale, like area_ids), the serializer surfaces group_ids, and the misleading filter is removed (read ids from the payload, resolve names via list_groups).
• Broken get_form_fields → replace_form_fields round-trip — Serializers::CustomField emitted a constraints key (from the web serializer) that is not a CustomField attribute; echoing fetched fields back into replace_form_fields crashed UpdateAllService with ActiveModel::UnknownAttributeError. The MCP serializer now drops the key (constraints are reported at the top level of the get_form_fields response). The round-trip is covered by a spec.

Test strategy

Depth follows risk: complex tools (create_phase's feature-flag-driven schema, update_phase_permission's demographic_questions semantics, form-fields round-trip/guards, destroy_resource cascades) get deep bespoke specs; thin list tools get short, deliberately uniform specs.

Similar tools are tested with an identical structure ("canvas" per category: list / create / update / get / attach), documented in the engine CLAUDE.md §12 (kept in the private claude repo since **/CLAUDE.md is gitignored here). New helpers:

• be_not_found('Label') matcher (spec/support/matchers/be_not_found.rb)
• it_behaves_like 'a paginated list tool' shared examples (spec/support/mcp_server/shared_examples/paginated_list_tool.rb), used by all 13 paginated list specs
• stub_failing_remote_download next to the existing download stubs

Shared examples deliberately not created (over-factorization would hurt more than help):

• draft-only tool: setup and no-side-effect assertion vary too much per tool; the published context stays inline (6 lines).
• multiloc merge: mechanics unit-tested once in multiloc_merge_spec.rb; each update tool keeps one small integration example proving it routes through the merge.
• not-found: the be_not_found matcher reduces it to a 3-line example.

Serializers: no direct specs for domain serializers — they're exercised through get_resource_spec (all 9 types), list-spec shape assertions and the form-fields specs. Only Serializers::Base and McpServer::Jsonapi (where the logic lives) have unit specs.

New layers beyond tools:

• spec/requests/mcp_spec.rb — the only place the HTTP gating is exercised: Doorkeeper mcp:access scope, McpPolicy (super admin + feature flag), RFC 9728 WWW-Authenticate challenge, end-to-end tools/list / tools/call with a real bearer token. Doubles as the McpPolicy spec.
• Unit specs for BaseTool::Pagination, MultilocMerge, ResponseHelpers, Multiloc, Jsonapi, Serializers::Base; suite-wide definition invariants in base_tool_spec.rb.

Coverage (lines, engines/commercial/mcp_server/app/**)

	Baseline	This PR
Total	740/887 = 83.4%	923/928 = 99.5%
Examples	76	252

Remaining uncovered lines are defensive: the CarrierWave::DownloadError rescues in the attach tools (download failures actually surface as model validation errors — covered as such) and Runner#run's NotImplementedError (covered) — plus BaseTool::Authorization#pundit_user (see below).

Notes / follow-ups (not addressed here)

• List tools should go through Pundit policy scopes (policy_scope) instead of querying unscoped (Group.all, Volunteering::Cause.where(...), …). Today McpPolicy restricts the endpoint to super admins so nothing leaks, but the tools silently rely on that outer gate; scoping them makes the invariant local and future-proofs any loosening of endpoint access.

• update_phase_permission replaces access_denied_explanation_multiloc wholesale instead of merging like other update tools — the spec encodes current behavior with an explicit test name; consider aligning.

• BaseTool::Authorization#pundit_user is dead code: the included hook includes Pundit::Authorization afterwards, whose identical pundit_user shadows it.

• Remaining "settable but not clearable" fields (same class as the header_bg fix): update_phase can't clear end_at (the only way to make the last phase open-ended); update_phase_permission can't clear access_denied_explanation_multiloc (schema rejects null and {}, and the Runner's .compact drops nils); update_resource silently no-ops on remote_image_url: null for causes (CarrierWave ignores nil — needs remove_image!). The embed-URL fields were checked and are fine as is (presence validations make clearing invalid where they matter).

• Mass-assignment gaps (implemented, currently stashed — git stash): create_phase splats params into Phase.new without additionalProperties: false; replace_form_fields' field/option/statement item schemas accept undeclared keys that reach CustomField.new/assign_attributes, including resource_id (re-parenting a field to another form). The stash closes both schemas, declares the read-only keys for round-tripping and strips them in the tool before saving.

• Runner::NOT_DRAFT_MESSAGE ends with a period, and unauthorized_message appends another one → "…via MCP..".

• Feature-gated participation methods are enforced only by schema validation at MCP dispatch (the gem's validate_tool_call_arguments default; pinned by a request-spec example) — consider a runtime flag check in CreatePhase's Runner as defense-in-depth.

🤖 Generated with Claude Code