From f73aeb3541d4a3d3c7143ffb462fbc74b5623186 Mon Sep 17 00:00:00 2001 From: Brun Christophe Date: Wed, 3 Jun 2026 10:40:28 +0200 Subject: [PATCH] fix(agent-client): serialize fields[] projection as comma-separated string (PRD-437) QuerySerializer.formatFields returned an array for the sparse fieldset projection (e.g. { 'fields[Customer]': ['first_name','last_name','email'] }). superagent serializes arrays via qs.stringify(obj, { indices: false }) as repeated keys without brackets: fields[Customer]=first_name&fields[Customer]=last_name&fields[Customer]=email Rack-based (Ruby) agents collapse duplicate scalar keys to the LAST value, so parse_projection (which does fields.split(',')) only ever saw the last field. A getData/read-record reading first_name, last_name, email returned only email (plus the primary key). The TS agent was unaffected by luck: it does fields.toString().split(','), and Array.toString() rejoins with commas. Join each projection into a single comma-separated value (JSON:API sparse fieldset standard), which both Ruby (split) and Node agents parse correctly. Also fixes the same class of issue for update-record and getRelatedData, which share this serialization. Sibling of PRD-273 #8 (filter operator casing). Update the mcp-server consumer tests that asserted the old array shape, and tighten the field assertions to token-based matching (no substring matches). fixes PRD-437 Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/agent-client/src/query-serializer.ts | 8 +- .../test/query-serializer.test.ts | 88 ++++++++----------- packages/mcp-server/test/server.test.ts | 15 ++-- 3 files changed, 50 insertions(+), 61 deletions(-) diff --git a/packages/agent-client/src/query-serializer.ts b/packages/agent-client/src/query-serializer.ts index a89fb48e54..55eb5b16ae 100644 --- a/packages/agent-client/src/query-serializer.ts +++ b/packages/agent-client/src/query-serializer.ts @@ -91,7 +91,7 @@ export default class QuerySerializer { .toLowerCase(); } - private static formatFields(collectionName: string, fields: string[]): Record { + private static formatFields(collectionName: string, fields: string[]): Record { if (!fields) return {}; const projectionName = `fields[${HttpRequester.escapeUrlSlug(collectionName)}]`; @@ -138,6 +138,10 @@ export default class QuerySerializer { } }); - return projection; + // Join per type (`fields[users]=id,name`): an array would serialize as repeated params, + // which Ruby (Rack) agents collapse to the last value only — dropping every field but one. + return Object.fromEntries( + Object.entries(projection).map(([key, values]) => [key, values.join(',')]), + ); } } diff --git a/packages/agent-client/test/query-serializer.test.ts b/packages/agent-client/test/query-serializer.test.ts index 61b2b243cd..e9261d7a9d 100644 --- a/packages/agent-client/test/query-serializer.test.ts +++ b/packages/agent-client/test/query-serializer.test.ts @@ -81,13 +81,23 @@ describe('QuerySerializer', () => { expect(result.search).toBe('john'); expect(result['page[size]']).toBe(15); expect(result['page[number]']).toBe(3); - expect(result['fields[users]']).toEqual(['id', 'name']); + expect(result['fields[users]']).toBe('id,name'); }); }); it('should serialize fields with escaped collection name', () => { const result = QuerySerializer.serialize({ fields: ['id', 'name', 'email'] }, 'users'); - expect(result['fields[users]']).toEqual(['id', 'name', 'email']); + expect(result['fields[users]']).toBe('id,name,email'); + }); + + it('should serialize fields as a single comma-separated value, not an array', () => { + // Regression (PRD-437): an array serializes as repeated params that Ruby agents collapse to one. + const result = QuerySerializer.serialize( + { fields: ['first_name', 'last_name', 'email'] }, + 'users', + ); + expect(result['fields[users]']).toBe('first_name,last_name,email'); + expect(Array.isArray(result['fields[users]'])).toBe(false); }); it('should serialize ascending sort', () => { @@ -188,7 +198,7 @@ describe('QuerySerializer', () => { expect(result.search).toBe('john'); expect(result['page[size]']).toBe(20); expect(result['page[number]']).toBe(1); - expect(result['fields[users]']).toEqual(['id', 'name']); + expect(result['fields[users]']).toBe('id,name'); expect(result.sort).toBe('-createdAt'); const parsed = JSON.parse(result.filters as string); expect(parsed.conditions[0].operator).toBe('equal'); @@ -197,7 +207,7 @@ describe('QuerySerializer', () => { it('should handle collection names with special characters', () => { const result = QuerySerializer.serialize({ fields: ['id'] }, 'user+data'); - expect(result['fields[user\\+data]']).toEqual(['id']); + expect(result['fields[user\\+data]']).toBe('id'); }); it('should handle undefined sort', () => { @@ -214,11 +224,8 @@ describe('QuerySerializer', () => { it('should handle single relation field with @@@ separator', () => { const result = QuerySerializer.serialize({ fields: ['id', 'customer@@@name'] }, 'orders'); - // Main collection should have id and the relation name (customer) - expect(result['fields[orders]']).toContain('id'); - expect(result['fields[orders]']).toContain('customer'); - // Related collection should have the field name - expect(result['fields[customer]']).toContain('name'); + expect(result['fields[orders]']).toBe('id,customer'); + expect(result['fields[customer]']).toBe('name'); }); it('should handle multiple relation fields from the same relation', () => { @@ -227,12 +234,8 @@ describe('QuerySerializer', () => { 'orders', ); - // Main collection should have id and the relation name (customer) - may appear twice - expect(result['fields[orders]']).toContain('id'); - expect(result['fields[orders]']).toContain('customer'); - // Related collection should have both field names - expect(result['fields[customer]']).toContain('name'); - expect(result['fields[customer]']).toContain('email'); + expect(result['fields[orders]']).toBe('id,customer'); + expect(result['fields[customer]']).toBe('name,email'); }); it('should handle multiple different relations', () => { @@ -241,13 +244,9 @@ describe('QuerySerializer', () => { 'orders', ); - // Main collection should have id and both relation names - expect(result['fields[orders]']).toContain('id'); - expect(result['fields[orders]']).toContain('customer'); - expect(result['fields[orders]']).toContain('product'); - // Each related collection should have its field - expect(result['fields[customer]']).toContain('name'); - expect(result['fields[product]']).toContain('title'); + expect(result['fields[orders]']).toBe('id,customer,product'); + expect(result['fields[customer]']).toBe('name'); + expect(result['fields[product]']).toBe('title'); }); it('should only process first level of relation with multiple @@@ separators', () => { @@ -256,12 +255,9 @@ describe('QuerySerializer', () => { 'orders', ); - // Main collection should have id and customer relation - expect(result['fields[orders]']).toContain('id'); - expect(result['fields[orders]']).toContain('customer'); - // Only first level is processed - "address@@@city" becomes the field name - expect(result['fields[customer]']).toContain('address@@@city'); - // No further nesting - address collection should not exist + // "address@@@city" stays the field name — no nesting beyond the first level + expect(result['fields[orders]']).toBe('id,customer'); + expect(result['fields[customer]']).toBe('address@@@city'); expect(result['fields[address]']).toBeUndefined(); }); @@ -271,21 +267,15 @@ describe('QuerySerializer', () => { 'orders', ); - // Main collection should only have the relation name - expect(result['fields[orders]']).toContain('customer'); - expect(result['fields[orders]']).not.toContain('name'); - expect(result['fields[orders]']).not.toContain('email'); - // Related collection should have both fields - expect(result['fields[customer]']).toContain('name'); - expect(result['fields[customer]']).toContain('email'); + expect(result['fields[orders]']).toBe('customer'); + expect(result['fields[customer]']).toBe('name,email'); }); it('should escape special characters in relation names', () => { const result = QuerySerializer.serialize({ fields: ['id', 'user+data@@@name'] }, 'orders'); - expect(result['fields[orders]']).toContain('id'); - expect(result['fields[orders]']).toContain('user+data'); - expect(result['fields[user\\+data]']).toContain('name'); + expect(result['fields[orders]']).toBe('id,user+data'); + expect(result['fields[user\\+data]']).toBe('name'); }); it('should handle empty fields array', () => { @@ -298,28 +288,28 @@ describe('QuerySerializer', () => { it('should skip empty strings in fields', () => { const result = QuerySerializer.serialize({ fields: ['', 'name', ''] }, 'users'); - expect(result['fields[users]']).toEqual(['name']); + expect(result['fields[users]']).toBe('name'); }); it('should skip malformed @@@ separator with missing relation name', () => { const result = QuerySerializer.serialize({ fields: ['id', '@@@fieldName'] }, 'orders'); // Should only include 'id', skip the malformed '@@@fieldName' - expect(result['fields[orders]']).toEqual(['id']); + expect(result['fields[orders]']).toBe('id'); }); it('should skip malformed @@@ separator with missing field name', () => { const result = QuerySerializer.serialize({ fields: ['id', 'customer@@@'] }, 'orders'); // Should only include 'id', skip the malformed 'customer@@@' - expect(result['fields[orders]']).toEqual(['id']); + expect(result['fields[orders]']).toBe('id'); }); it('should skip standalone @@@ separator', () => { const result = QuerySerializer.serialize({ fields: ['id', '@@@'] }, 'orders'); // Should only include 'id', skip the malformed '@@@' - expect(result['fields[orders]']).toEqual(['id']); + expect(result['fields[orders]']).toBe('id'); }); it('should not include duplicate relation names', () => { @@ -328,29 +318,27 @@ describe('QuerySerializer', () => { 'orders', ); - // customer should only appear once in the orders fields - const ordersFields = result['fields[orders]'] as string[]; - const customerCount = ordersFields.filter(f => f === 'customer').length; - expect(customerCount).toBe(1); + // customer appears once even though two of its fields were requested + expect(result['fields[orders]']).toBe('id,customer'); }); it('should not include duplicate field names', () => { const result = QuerySerializer.serialize({ fields: ['id', 'id', 'name', 'name'] }, 'users'); - expect(result['fields[users]']).toEqual(['id', 'name']); + expect(result['fields[users]']).toBe('id,name'); }); it('should handle whitespace in field names', () => { const result = QuerySerializer.serialize({ fields: [' id ', ' name '] }, 'users'); - expect(result['fields[users]']).toEqual(['id', 'name']); + expect(result['fields[users]']).toBe('id,name'); }); it('should handle whitespace around @@@ separator', () => { const result = QuerySerializer.serialize({ fields: [' customer @@@ name '] }, 'orders'); - expect(result['fields[orders]']).toContain('customer'); - expect(result['fields[customer]']).toContain('name'); + expect(result['fields[orders]']).toBe('customer'); + expect(result['fields[customer]']).toBe('name'); }); }); }); diff --git a/packages/mcp-server/test/server.test.ts b/packages/mcp-server/test/server.test.ts index dd59443e09..f1d3c8b9f7 100644 --- a/packages/mcp-server/test/server.test.ts +++ b/packages/mcp-server/test/server.test.ts @@ -1700,7 +1700,7 @@ describe('ForestMCPServer Instance', () => { }); expect(response.status).toBe(200); - expect(capturedQueryParams['fields[users]']).toEqual(['id', 'name', 'email']); + expect(capturedQueryParams['fields[users]']).toBe('id,name,email'); }); it('should pass relation fields with @@@ separator to the agent', async () => { @@ -1724,13 +1724,10 @@ describe('ForestMCPServer Instance', () => { }); expect(response.status).toBe(200); - // Main collection should have the relation name - expect(capturedQueryParams['fields[users]']).toContain('id'); - expect(capturedQueryParams['fields[users]']).toContain('name'); - expect(capturedQueryParams['fields[users]']).toContain('company'); - // Related collection should have the field names - expect(capturedQueryParams['fields[company]']).toContain('name'); - expect(capturedQueryParams['fields[company]']).toContain('address'); + const usersFields = (capturedQueryParams['fields[users]'] as string).split(','); + const companyFields = (capturedQueryParams['fields[company]'] as string).split(','); + expect(usersFields).toEqual(expect.arrayContaining(['id', 'name', 'company'])); + expect(companyFields).toEqual(expect.arrayContaining(['name', 'address'])); }); }); @@ -1970,7 +1967,7 @@ describe('ForestMCPServer Instance', () => { expect(capturedQueryParams.search).toBe('john'); expect(capturedQueryParams.searchExtended).toBe(true); expect(capturedQueryParams.sort).toBe('name'); - expect(capturedQueryParams['fields[users]']).toEqual(['id', 'name', 'email']); + expect(capturedQueryParams['fields[users]']).toBe('id,name,email'); expect(capturedQueryParams['page[size]']).toBe(20); expect(capturedQueryParams['page[number]']).toBe(2);