Rescue validation race in McpServerBlock.block!
What does this MR do and why?
Closes https://gitlab.com/gitlab-org/gitlab/-/work_items/622706
Ai::Catalog::McpServerBlock.block! (ee/app/models/ai/catalog/mcp_server_block.rb) was documented as race-safe but only rescued ActiveRecord::RecordNotUnique. The model validates uniqueness of namespace_id scoped to ai_catalog_mcp_server_id, and that validation runs before the insert reaches the database.
When two concurrent requests race, the loser can pass find_or_create_by!'s initial find, then have the winner commit its row before the loser's own uniqueness validation runs. The loser's validation catches the now-existing row and raises ActiveRecord::RecordInvalid ("Namespace has already been taken"), which was unrescued and surfaced as a 500 from the aiCatalogMcpServerSetBlock GraphQL mutation.
Data was never wrong: exactly one block row is created either way, and enforcement applies as expected. Only the losing caller's response was wrong — it reported failure for an action that had actually succeeded. Severity 4, shipped in commit 20f38338b9c9 (2026-06-28), unflagged since.
The fix adds a second rescue clause. It re-raises unless the error is specifically the namespace_id "taken" uniqueness error — so genuine validation failures still surface — and otherwise re-reads the winning row with find_by!. This follows the existing pattern in ee/app/models/search/zoekt/replica.rb, which uses the identical errors.of_kind?(:namespace_id, :taken) guard, and matches the of_kind?(..., :taken) idiom used in services like app/services/packages/npm/create_temporary_package_service.rb to distinguish a uniqueness race from a real validation failure.
No feature flag: this corrects a response for a benign race and doesn't change behavior for sequential callers.
How to validate locally
-
A deterministic 5-thread concurrency repro is available at
ai-governance-notes/qa-scripts/mcp_audit_events_deep.rb(section T4), run viabin/rails runner. Requires GDK postgres and redis running. Without the fix, at least one of the 5 concurrent calls raises; with the fix, all 5 return success with exactly one block row. -
Three examples were added to
ee/spec/models/ai/catalog/mcp_server_block_spec.rb:- the validation race returns the existing row
- the unique-index race returns the existing row
- a
RecordInvalidcarrying any other error still re-raises
The validation-race example was confirmed to fail without the fix (it raised
RecordInvalidfromblock!) before being asserted to pass. -
27 examples pass across
ee/spec/models/ai/catalog/mcp_server_block_spec.rbandee/spec/services/ai/catalog/mcp_servers/set_block_service_spec.rb. -
RuboCop clean.
Database
No database review or query plan needed. The new find_by!(attrs) call runs the same query the existing RecordNotUnique branch already runs, so this introduces no new SQL and no new scopes.
Notes for reviewers
- There is currently no audit-event emission for MCP server blocks on master. That's being added by !251763 (merged), still open. Once that merges, this fix won't cause duplicate audit events: the row recovered via
find_by!is notpreviously_new_record?, which is the condition that service gates audit emission on, so only the winner emits.
typebug backend groupcompliance devopssecurity governance sectionsec severity4