Skip to content

Commit 6b11d5d

Browse files
adessyclaude
andcommitted
Serialize all paginated payloads through MCP serializers
`paginated_response` had two serialization branches: MCP serializers, and an `as_json(only: ...)` fast path used by three flat-payload tools that also leaked string keys into `structuredContent`. The fast path is gone: `list_groups`, `list_user_custom_fields` and the dormant `list_users` get from-scratch serializers (no `wraps` — the explicit `#attributes` hash keeps the allowlist property, PII allowlist included), and `Serializers::Base#to_h` now enforces the symbols-as-top-level-keys invariant so individual serializers cannot diverge (multiloc values keep their string locale keys). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0451002 commit 6b11d5d

12 files changed

Lines changed: 169 additions & 82 deletions

File tree

back/engines/commercial/mcp_server/app/lib/mcp_server/base_tool/pagination.rb

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ module McpServer::BaseTool::Pagination
1111
per_page: { type: 'integer', description: "Results per page (default: #{DEFAULT_PER_PAGE}, max: #{MAX_PER_PAGE})" }
1212
}.freeze
1313

14-
def paginated_response(label, scope, page: nil, per_page: nil, serializer: nil, params: {}, **json_options)
14+
def paginated_response(label, scope, serializer:, page: nil, per_page: nil, params: {})
1515
page ||= 1
1616
per_page = (per_page || DEFAULT_PER_PAGE).to_i.clamp(1, MAX_PER_PAGE)
1717

@@ -24,12 +24,8 @@ def paginated_response(label, scope, page: nil, per_page: nil, serializer: nil,
2424
total_pages: records.total_pages
2525
}
2626

27-
data = serializer ? serializer.serialize(records, params:) : records.as_json(**json_options)
28-
29-
structured_content = {
30-
data: data,
31-
pagination: pagination
32-
}
27+
data = serializer.serialize(records, params:)
28+
structured_content = { data:, pagination: }
3329

3430
summary = <<~TEXT.squish
3531
Found #{pagination[:total_count]} #{label}

back/engines/commercial/mcp_server/app/lib/mcp_server/serializers/base.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,13 @@ def to_h
9191
.index_by { it[:id] }
9292
end
9393

94-
collection? ? records.map { attributes(it) } : attributes(records.sole)
94+
# Enforce the invariant that serializers return hashes with symbols as
95+
# top-level keys (multiloc values keep their string locale keys).
96+
if collection?
97+
records.map { attributes(it).symbolize_keys }
98+
else
99+
attributes(records.sole).symbolize_keys
100+
end
95101
end
96102

97103
def attributes(record)
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# frozen_string_literal: true
2+
3+
class McpServer::Serializers::Group < McpServer::Serializers::Base
4+
def attributes(record)
5+
record.slice(:id, :title_multiloc, :membership_type, :memberships_count)
6+
end
7+
end
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# frozen_string_literal: true
2+
3+
class McpServer::Serializers::User < McpServer::Serializers::Base
4+
def attributes(record)
5+
record.slice(:id, :first_name, :last_name, :email, :roles)
6+
end
7+
end
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
# frozen_string_literal: true
2+
3+
# Registration (user profile) fields. Not to be confused with Serializers::CustomField,
4+
# which serializes form fields (options and matrix statements inlined).
5+
class McpServer::Serializers::UserCustomField < McpServer::Serializers::Base
6+
def attributes(record)
7+
record.slice(:id, :title_multiloc, :input_type, :code, :required)
8+
end
9+
end

back/engines/commercial/mcp_server/app/lib/mcp_server/tools/list_groups.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ def run
2323
'groups',
2424
scope.order_new,
2525
**params.slice(:page, :per_page),
26-
only: %i[id title_multiloc membership_type memberships_count]
26+
serializer: McpServer::Serializers::Group
2727
)
2828
end
2929
end

back/engines/commercial/mcp_server/app/lib/mcp_server/tools/list_user_custom_fields.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ def run
1212
'user custom fields',
1313
CustomField.registration.enabled.not_hidden.order(:ordering),
1414
**params.slice(:page, :per_page),
15-
only: %i[id title_multiloc input_type code required]
15+
serializer: McpServer::Serializers::UserCustomField
1616
)
1717
end
1818
end

back/engines/commercial/mcp_server/app/lib/mcp_server/tools/list_users.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ def run
2626
'users',
2727
scope,
2828
**params.slice(:page, :per_page),
29-
only: %i[id first_name last_name email roles]
29+
serializer: McpServer::Serializers::User
3030
)
3131
end
3232
end

back/engines/commercial/mcp_server/spec/lib/mcp_server/base_tool/pagination_spec.rb

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,59 +6,60 @@
66
subject(:host) { Class.new { include McpServer::BaseTool::Pagination }.new }
77

88
let_it_be(:areas) { create_list(:area, 3) }
9+
let_it_be(:current_user) { create(:super_admin) }
910

1011
let(:scope) { Area.order(:ordering) }
1112

1213
def paginate(**options)
13-
host.paginated_response('areas', scope, page: nil, per_page: nil, **options)
14+
host.paginated_response(
15+
'areas', scope,
16+
page: nil, per_page: nil,
17+
serializer: McpServer::Serializers::Area, params: { current_user: },
18+
**options
19+
)
1420
end
1521

1622
it 'returns the data and pagination envelope with defaults' do
1723
response = paginate
1824

1925
expect(response).not_to be_error
20-
expect(response.structured_content[:data].size).to eq(3)
21-
expect(response.structured_content[:pagination]).to eq(
22-
page: 1,
23-
per_page: described_class::DEFAULT_PER_PAGE,
24-
total_count: 3,
25-
total_pages: 1
26+
expect(response.structured_content).to match(
27+
data: have_attributes(size: 3),
28+
pagination: {
29+
page: 1,
30+
per_page: described_class::DEFAULT_PER_PAGE,
31+
total_count: 3,
32+
total_pages: 1
33+
}
2634
)
2735
end
2836

2937
it 'summarizes the result in the text block' do
30-
response = paginate
31-
32-
expect(response.content.first[:text])
38+
expect(paginate.content.first[:text])
3339
.to include('Found 3 areas (showing page 1, 20 per page)')
3440
end
3541

3642
it 'serves subsequent pages' do
3743
response = paginate(page: 2, per_page: 2)
3844

39-
expect(response.structured_content[:data].size).to eq(1)
40-
expect(response.structured_content[:pagination]).to eq(
41-
page: 2, per_page: 2, total_count: 3, total_pages: 2
45+
expect(response.structured_content).to match(
46+
data: have_attributes(size: 1),
47+
pagination: { page: 2, per_page: 2, total_count: 3, total_pages: 2 }
4248
)
4349
end
4450

4551
it 'clamps per_page between 1 and MAX_PER_PAGE' do
46-
expect(paginate(per_page: 999).structured_content.dig(:pagination, :per_page))
47-
.to eq(described_class::MAX_PER_PAGE)
48-
expect(paginate(per_page: 0).structured_content.dig(:pagination, :per_page))
49-
.to eq(1)
50-
end
51-
52-
it 'serializes with as_json options when no serializer is given' do
53-
response = paginate(only: %i[id])
52+
per_page = paginate(per_page: 999).structured_content.dig(:pagination, :per_page)
53+
expect(per_page).to eq(described_class::MAX_PER_PAGE)
5454

55-
expect(response.structured_content[:data]).to match_array(areas.map { |a| { 'id' => a.id } })
55+
per_page = paginate(per_page: 0).structured_content.dig(:pagination, :per_page)
56+
expect(per_page).to eq(1)
5657
end
5758

58-
it 'serializes with the given MCP serializer' do
59-
response = paginate(serializer: McpServer::Serializers::Area, params: { current_user: create(:super_admin) })
59+
it 'serializes the records with the given serializer' do
60+
response = paginate
6061

61-
expect(response.structured_content[:data].pluck(:id)).to eq(scope.pluck(:id))
62-
expect(response.structured_content[:data].first).to include(:title_multiloc)
62+
expect(response.structured_content[:data])
63+
.to eq(McpServer::Serializers::Area.serialize(scope, params: { current_user: }))
6364
end
6465
end

back/engines/commercial/mcp_server/spec/lib/mcp_server/serializers/base_spec.rb

Lines changed: 100 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,74 +2,135 @@
22

33
require 'rails_helper'
44

5+
# Tested through anonymous serializers that declare exactly the feature under test,
6+
# so this spec depends on no domain serializer. Integration with the real web
7+
# serializers is covered by the tool specs.
58
describe McpServer::Serializers::Base do
6-
let_it_be(:current_user) { create(:super_admin) }
9+
let_it_be(:record) { create(:custom_field_select, :with_options) }
710

8-
describe 'wrapping a web API serializer' do
9-
it 'flattens the JSONAPI output into a plain hash' do
10-
project = create(:project)
11+
describe '.serialize' do
12+
let(:serializer) do
13+
Class.new(described_class) do
14+
def attributes(record) = { id: record.id }
15+
end
16+
end
1117

12-
serialized = McpServer::Serializers::Project.serialize(project, params: { current_user: })
18+
it 'returns a hash for a single record and an array for a relation' do
19+
expect(serializer.serialize(record)).to be_a(Hash)
20+
expect(serializer.serialize(CustomField.all)).to be_an(Array)
21+
end
1322

14-
expect(serialized).to be_a(Hash)
15-
expect(serialized[:id]).to eq(project.id)
16-
expect(serialized[:title_multiloc]).to eq(project.title_multiloc)
17-
expect(serialized).not_to include(:data, :attributes, :relationships)
23+
it 'returns an empty array for an empty relation' do
24+
expect(serializer.serialize(CustomField.none)).to eq([])
1825
end
26+
end
1927

20-
it 'merges the click-through URLs when the serializer opts in' do
21-
project = create(:project)
28+
describe 'with a wrapped upstream serializer' do
29+
before do
30+
stub_const('UpstreamOptionSerializer', Class.new do
31+
include JSONAPI::Serializer
32+
set_type :option
33+
attribute :option_title, &:title_multiloc
34+
end)
35+
36+
stub_const('UpstreamSerializer', Class.new do
37+
include JSONAPI::Serializer
38+
set_type :probe
39+
attribute :probe_title, &:title_multiloc
40+
attribute(:echo) { |_record, params| params[:probe] }
41+
has_many :options, serializer: UpstreamOptionSerializer
42+
end)
43+
end
2244

23-
serialized = McpServer::Serializers::Project.serialize(project, params: { current_user: })
45+
let(:serializer) { Class.new(described_class) { wraps UpstreamSerializer } }
2446

25-
expect(serialized[:admin_url]).to end_with("/admin/projects/#{project.id}")
26-
expect(serialized[:public_url]).to end_with("/projects/#{project.slug}")
47+
it 'flattens the JSONAPI output into a plain hash' do
48+
serialized = serializer.serialize(record)
49+
50+
expect(serialized).to match(
51+
id: record.id,
52+
probe_title: record.title_multiloc,
53+
echo: nil,
54+
option_ids: match_array(record.option_ids)
55+
)
56+
end
57+
58+
it 'serializes every record of a collection, in order' do
59+
other_record = create(:custom_field)
60+
61+
serialized = serializer.serialize([record, other_record])
62+
63+
expect(serialized).to match([
64+
a_hash_including(id: record.id, probe_title: record.title_multiloc),
65+
a_hash_including(id: other_record.id, probe_title: other_record.title_multiloc)
66+
])
67+
end
68+
69+
it 'forwards params to the upstream serializer' do
70+
serialized = serializer.serialize(record, params: { probe: 'x' })
71+
72+
expect(serialized[:echo]).to eq('x')
2773
end
28-
end
2974

30-
describe 'inline relationships' do
31-
it 'embeds the related resources instead of exposing ids' do
32-
field = create(:custom_field_select, :with_options)
75+
it 'embeds relationships declared as inline instead of exposing ids' do
76+
serializer = Class.new(described_class) do
77+
wraps UpstreamSerializer
78+
inline :options
79+
end
3380

34-
serialized = McpServer::Serializers::CustomField.serialize(field)
81+
serialized = serializer.serialize(record)
3582

36-
expect(serialized[:options]).to be_an(Array)
37-
expect(serialized[:options].pluck(:id)).to match_array(field.options.pluck(:id))
38-
expect(serialized[:options].first).to include(:title_multiloc)
3983
expect(serialized).not_to include(:option_ids)
84+
expect(serialized[:options]).to match_array(
85+
record.options.map { |option| { id: option.id, option_title: option.title_multiloc } }
86+
)
4087
end
4188
end
4289

43-
describe 'building from scratch (no wraps)' do
44-
it 'uses the #attributes override' do
45-
permission = create(:permission)
90+
describe 'with a from-scratch serializer (no wraps)' do
91+
let(:serializer) do
92+
Class.new(described_class) do
93+
def attributes(record) = { 'id' => record.id, 'kind' => 'probe' }
94+
end
95+
end
4696

47-
serialized = McpServer::Serializers::Permission.serialize(permission)
97+
it 'uses the #attributes override and symbolizes its top-level keys' do
98+
serialized = serializer.serialize(record)
4899

49-
expect(serialized.keys).to eq(
50-
%i[action permitted_by group_ids demographic_questions verification_expiry access_denied_explanation_multiloc]
51-
)
100+
expect(serialized).to match(id: record.id, kind: 'probe')
101+
end
102+
103+
it 'exposes `params` to the #attributes override' do
104+
serializer = Class.new(described_class) do
105+
def attributes(_record) = { echo: params[:probe] }
106+
end
107+
108+
serialized = serializer.serialize(record, params: { probe: 'x' })
109+
110+
expect(serialized).to eq(echo: 'x')
52111
end
53112

54113
it 'raises when neither wraps nor #attributes is provided' do
55114
serializer = Class.new(described_class)
56115

57-
expect { serializer.serialize(create(:area)) }
116+
expect { serializer.serialize(record) }
58117
.to raise_error(NotImplementedError, /wraps/)
59118
end
60119
end
61120

62-
describe 'collection semantics' do
63-
it 'returns an array for a relation and a hash for a single record' do
64-
create_list(:area, 2)
65-
params = { current_user: }
121+
describe '#urls' do
122+
it 'returns the admin and public URLs of the record' do
123+
project = create(:project)
124+
serializer = Class.new(described_class) do
125+
def attributes(record) = urls(record)
126+
end
66127

67-
expect(McpServer::Serializers::Area.serialize(Area.all, params:)).to be_an(Array)
68-
expect(McpServer::Serializers::Area.serialize(Area.first, params:)).to be_a(Hash)
69-
end
128+
serialized = serializer.serialize(project)
70129

71-
it 'returns an empty array for an empty relation' do
72-
expect(McpServer::Serializers::Area.serialize(Area.none)).to eq([])
130+
expect(serialized).to match(
131+
admin_url: end_with("/admin/projects/#{project.id}"),
132+
public_url: end_with("/projects/#{project.slug}")
133+
)
73134
end
74135
end
75136
end

0 commit comments

Comments
 (0)