fix(agent-bff): order composite key segments by the record, not by the apimap

packages/agent-bff/src/data/pack-id.ts

@@ -1,5 +1,6 @@
 import type { PrimaryKeyField } from '../read-model/read-model';
 
+import recordKey from './record-key';
 import { mappingError } from '../http/bff-local-errors';
 
 export const PACKED_ID_SEPARATOR = '|';
@@ -20,6 +21,82 @@ function toNumberIfLossless(value: string): string | number {
   return Number.isSafeInteger(numeric) && String(numeric) === value ? numeric : value;
 }
 
+/**
+ * The value a record carries for a key field, when it can stand for a packed segment. The field is
+ * read under its own name first, then under the camelCase key the deserializer emits
+ * (`agent-client/src/http-requester.ts`). Anything that is not a string or a finite number is
+ * ignored: a record attribute can also be `null`, a boolean, a relation object written over the
+ * same key, or the JSON of a Buffer, and none of those names a segment.
+ *
+ * A key whose response key is shared with another field is refused outright. The record then holds
+ * a single value under that key and which field wrote it is not knowable here, so reading it could
+ * claim the segment of a sibling key — worse than not matching at all.
+ */
+function comparableValue(record: Record<string, unknown>, key: PrimaryKeyField): string | null {
+  if (key.ambiguousRecordKey) return null;
+
+  const { name } = key;
+  const declared = record[name];
+  const value = declared === undefined || declared === null ? record[recordKey(name)] : declared;
+
+  if (typeof value === 'string') return value;
+
+  return typeof value === 'number' && Number.isFinite(value) ? String(value) : null;
+}
+
+/**
+ * The packed segments, reordered onto the keys they belong to.
+ *
+ * The pairing cannot come from the order the keys arrive in: the apimap sorts its fields
+ * alphabetically (`agent/src/utils/forest-schema/generator-collection.ts`) while the agent packs in
+ * declaration order (`agent/src/utils/id.ts`), and the two agree only by accident — a
+ * `tenant_id`/`seq` collection packs `acme|42` and gets `"acme"` cast as `seq`, so every list of it
+ * answers 500.
+ *
+ * The record settles it. Its attributes hold the key values, so a segment equal to one of them
+ * belongs to that key whatever the published order. The record is used for THAT and nothing else —
+ * the value emitted is always the packed segment, never the record's own. The case that forces it
+ * is a key named `id`: `jsonapi-serializer` overwrites that attribute with the resource id
+ * (`deserializer-utils.js`), so the record would hand back `acme|42` for it. Reading the segment
+ * instead keeps that key correct, and it matches nothing, so it takes what its siblings left. A
+ * `Date` or a Buffer key is the same story from the other side: their attribute form differs from
+ * the packed one (ISO versus `String(date)`), they match nothing, and they keep their position.
+ *
+ * One key the record cannot answer for is still placed: the single segment left over is necessarily
+ * its own, whatever the published order. Two are not. Which of the remaining segments is whose is
+ * exactly the question the record failed to answer, and placing them anyway hands back a wrong key
+ * under a 200 where the positional pairing would have raised its numeric-cast error. So the whole
+ * reordering is dropped as soon as a second key goes unread, errors included. Keeping an unread key
+ * on its own segment instead is not enough: with four keys, two read and two unread, both unread
+ * ones can sit on their published segment and both still be wrong.
+ *
+ * With no record, the values are returned whole and the pairing is the positional one.
+ */
+function segmentsByKey(
+  values: string[],
+  primaryKeys: PrimaryKeyField[],
+  record?: Record<string, unknown>,
+): string[] {
+  if (!record) return values;
+
+  const claimed = values.map(() => false);
+  const matched = primaryKeys.map(key => {
+    const wanted = comparableValue(record, key);
+    const index = wanted === null ? -1 : values.findIndex((v, i) => !claimed[i] && v === wanted);
+
+    if (index === -1) return null;
+    claimed[index] = true;
+
+    return values[index];
+  });
+
+  if (matched.filter(value => value === null).length > 1) return values;
+
+  const leftovers = values.filter((_, index) => !claimed[index]);
+
+  return matched.map(value => value ?? (leftovers.shift() as string));
+}
+
 /**
  * Rebuild the structured primary key of a record from its opaque packed id, mirroring the agent's
  * `IdUtils.packId`/`unpackId` (`|`-joined values, `Number` columns cast back to numbers). Returns a
@@ -34,6 +111,7 @@ function toNumberIfLossless(value: string): string | number {
 export default function unpackPrimaryKey(
   packedId: string,
   primaryKeys: PrimaryKeyField[],
+  record?: Record<string, unknown>,
 ): Record<string, string | number> {
   if (primaryKeys.length === 0) {
     throw mappingError('Cannot build primary key: the collection exposes no key metadata');
@@ -55,10 +133,11 @@ export default function unpackPrimaryKey(
     );
   }
 
+  const segments = segmentsByKey(values, primaryKeys, record);
   const result: Record<string, string | number> = {};
 
   primaryKeys.forEach(({ name, type }, index) => {
-    const value = values[index];
+    const value = segments[index];
 
     if (type !== NUMBER_COLUMN_TYPE) {
       result[name] = value;

packages/agent-bff/src/data/response-mappers.ts

@@ -34,7 +34,7 @@ export function mapListResponse(
       ...record,
       __forest: {
         collection,
-        primaryKey: unpackPrimaryKey(String(record.id), primaryKeys),
+        primaryKey: unpackPrimaryKey(String(record.id), primaryKeys, record),
       },
     };
   });

packages/agent-bff/src/openapi/unfolded-paths.ts

@@ -457,10 +457,15 @@ function parentIdSchema(
       // A composite id only works as its packed string: a number could never carry the separator.
       type: 'string',
       pattern: NON_BLANK_PATTERN,
+      // The order the keys are published in is not the order the agent packs them in, so naming one
+      // would send a client to build an id the agent unpacks onto the wrong columns.
       description:
-        `The composite id of the parent ${quoted(parent)} record: the values of ` +
-        `${primaryKeys.map(key => key.name).join(', ')} joined by ` +
-        `${quoted(PACKED_ID_SEPARATOR)}, in that order.`,
+        `The composite id of the parent ${quoted(parent)} record, taken verbatim from that ` +
+        `record's own id: the values of ${primaryKeys.map(key => key.name).join(', ')} joined by ` +
+        `${quoted(
+          PACKED_ID_SEPARATOR,
+        )}, in the order the agent packs them. Copy it from a listed ` +
+        `record rather than assembling it.`,
     };
   }
 

packages/agent-bff/src/read-model/read-model.ts

@@ -1,6 +1,8 @@
 import type { ActionEndpointsByCollection } from '@forestadmin/agent-client';
 import type { ForestSchemaCollection, ForestSchemaField } from '@forestadmin/forestadmin-client';
 
+import recordKey, { groupByRecordKey } from '../data/record-key';
+
 export const RELATIONSHIP_TYPES = [
   'BelongsTo',
   'HasOne',
@@ -14,7 +16,17 @@ export type RelationTarget =
   | { type: RelationshipType; polymorphic: false; target: string }
   | { type: RelationshipType; polymorphic: true; targets: string[] };
 
-export type PrimaryKeyField = { name: string; type: string; derived?: true };
+export type PrimaryKeyField = {
+  name: string;
+  type: string;
+  derived?: true;
+  /**
+   * Set when this key shares its response key with another field of the collection. The record then
+   * holds one value under that key and there is no telling whose, so the key cannot be read back
+   * from a record — see `unpackPrimaryKey`.
+   */
+  ambiguousRecordKey?: true;
+};
 
 export type ListableRelation = { name: string; foreignCollection: string };
 
@@ -160,9 +172,20 @@ export default class ReadModel {
    */
   private buildPrimaryKeys(collection: ForestSchemaCollection): void {
     const keys: PrimaryKeyField[] = [];
+    const fields = collection.fields ?? [];
+    const ambiguous = new Set<string>();
 
-    for (const field of collection.fields ?? []) {
-      if (field.isPrimaryKey) keys.push({ name: field.field, type: field.type });
+    groupByRecordKey(fields, field => field.field).forEach((group, key) => {
+      if (group.length > 1) ambiguous.add(key);
+    });
+
+    for (const field of fields) {
+      if (field.isPrimaryKey) {
+        const key: PrimaryKeyField = { name: field.field, type: field.type };
+        if (ambiguous.has(recordKey(field.field))) key.ambiguousRecordKey = true;
+
+        keys.push(key);
+      }
     }
 
     if (keys.length === 0) {

packages/agent-bff/test/data/pack-id.test.ts

@@ -39,6 +39,190 @@ describe('unpackPrimaryKey', () => {
     ).toThrow(expect.objectContaining({ type: 'mapping_error', status: 500 }));
   });
 
+  describe('when the record can say which segment belongs to which key', () => {
+    it('should place the segments by value rather than by the order the apimap published', () => {
+      expect(
+        unpackPrimaryKey(
+          'acme|42',
+          [
+            { name: 'seq', type: 'Number' },
+            { name: 'tenant_id', type: 'String' },
+          ],
+          { tenantId: 'acme', seq: 42, id: 'acme|42' },
+        ),
+      ).toEqual({ seq: 42, tenant_id: 'acme' });
+    });
+
+    it('should read a key under its exact name when the record carries it unchanged', () => {
+      expect(
+        unpackPrimaryKey(
+          'ab|7',
+          [
+            { name: 'orderId', type: 'Number' },
+            { name: 'sku', type: 'String' },
+          ],
+          { orderId: 7, sku: 'ab' },
+        ),
+      ).toEqual({ orderId: 7, sku: 'ab' });
+    });
+
+    it('should give a key named id the leftover segment, since its attribute holds the packed id', () => {
+      expect(
+        unpackPrimaryKey(
+          'acme|42',
+          [
+            { name: 'id', type: 'Number' },
+            { name: 'tenant', type: 'String' },
+          ],
+          { tenant: 'acme', id: 'acme|42' },
+        ),
+      ).toEqual({ id: 42, tenant: 'acme' });
+    });
+
+    it('should keep a date key positional, its attribute being ISO where the id is not', () => {
+      expect(
+        unpackPrimaryKey(
+          'Thu Jan 01 2026 00:00:00 GMT+0100|acme',
+          [
+            { name: 'day', type: 'Date' },
+            { name: 'tenant', type: 'String' },
+          ],
+          { day: '2026-01-01T00:00:00.000Z', tenant: 'acme' },
+        ),
+      ).toEqual({ day: 'Thu Jan 01 2026 00:00:00 GMT+0100', tenant: 'acme' });
+    });
+
+    it.each([
+      ['null', null],
+      ['a boolean', false],
+      ['a relation object', { id: 3 }],
+      ['a buffer payload', { type: 'Buffer', data: [1, 2] }],
+    ])('should not read %s as a key value, leaving that key its position', (_, value) => {
+      expect(
+        unpackPrimaryKey(
+          '7|ab',
+          [
+            { name: 'orderId', type: 'Number' },
+            { name: 'sku', type: 'String' },
+          ],
+          { orderId: value, sku: 'ab' },
+        ),
+      ).toEqual({ orderId: 7, sku: 'ab' });
+    });
+
+    it('should refuse to read a key whose response key another field shares', () => {
+      expect(
+        unpackPrimaryKey(
+          'A|B|C',
+          [
+            { name: 'ownerId', type: 'String', ambiguousRecordKey: true },
+            { name: 'owner_id', type: 'String', ambiguousRecordKey: true },
+            { name: 'sku', type: 'String' },
+          ],
+          { ownerId: 'B', sku: 'C' },
+        ),
+      ).toEqual({ ownerId: 'A', owner_id: 'B', sku: 'C' });
+    });
+
+    it('should leave an unmatched key on its own segment when no match claimed it', () => {
+      expect(
+        unpackPrimaryKey(
+          '42|Thu Jan 01 2026 00:00:00 GMT+0100|7',
+          [
+            { name: 'ref', type: 'String' },
+            { name: 'day', type: 'String' },
+            { name: 'seq', type: 'Number' },
+          ],
+          { seq: 7, day: '2026-01-01T00:00:00.000Z', id: '42|Thu Jan 01 2026 00:00:00 GMT+0100|7' },
+        ),
+      ).toEqual({ ref: '42', day: 'Thu Jan 01 2026 00:00:00 GMT+0100', seq: 7 });
+    });
+
+    it('should move an unmatched key only when a matched key took its segment', () => {
+      expect(
+        unpackPrimaryKey(
+          'g|b|a',
+          [
+            { name: 'alpha', type: 'String' },
+            { name: 'beta', type: 'String' },
+            { name: 'gamma', type: 'String' },
+          ],
+          { alpha: 'a', beta: 'b' },
+        ),
+      ).toEqual({ alpha: 'a', beta: 'b', gamma: 'g' });
+    });
+
+    it('should keep the positional pairing when two unread keys each sit on a published segment', () => {
+      expect(() =>
+        unpackPrimaryKey(
+          '9|5|ab|7',
+          [
+            { name: 'id', type: 'Number' },
+            { name: 'owner_id', type: 'String', ambiguousRecordKey: true },
+            { name: 'seq', type: 'Number' },
+            { name: 'sku', type: 'String' },
+          ],
+          { seq: 7, sku: 'ab', ownerId: '5', id: '9|5|ab|7' },
+        ),
+      ).toThrow(expect.objectContaining({ type: 'mapping_error', status: 500 }));
+    });
+
+    it('should keep the positional pairing, error included, when two displaced keys stay unread', () => {
+      expect(() =>
+        unpackPrimaryKey(
+          'ab|9|5',
+          [
+            { name: 'id', type: 'Number' },
+            { name: 'owner_id', type: 'String', ambiguousRecordKey: true },
+            { name: 'sku', type: 'String' },
+          ],
+          { sku: 'ab', ownerId: '9', id: 'ab|9|5' },
+        ),
+      ).toThrow(expect.objectContaining({ type: 'mapping_error', status: 500 }));
+    });
+
+    it('should fall back to the positional segments when no key matches', () => {
+      expect(
+        unpackPrimaryKey(
+          '7|ab',
+          [
+            { name: 'orderId', type: 'Number' },
+            { name: 'sku', type: 'String' },
+          ],
+          { createdAt: '2026-09-14T00:00:00.000Z' },
+        ),
+      ).toEqual({ orderId: 7, sku: 'ab' });
+    });
+
+    it('should still throw when a positionally assigned Number segment is not numeric', () => {
+      expect(() =>
+        unpackPrimaryKey(
+          'acme|42',
+          [
+            { name: 'seq', type: 'Number' },
+            { name: 'tenant_id', type: 'String' },
+          ],
+          {},
+        ),
+      ).toThrow(expect.objectContaining({ type: 'mapping_error', status: 500 }));
+    });
+
+    it('should leave a derived key whole, since its column was never declared', () => {
+      expect(
+        unpackPrimaryKey('tenant|42', [{ name: 'id', type: 'String', derived: true }], {
+          id: 'tenant|42',
+          tenant: 'tenant',
+        }),
+      ).toEqual({ id: 'tenant|42' });
+    });
+
+    it('should keep a single numeric key typed', () => {
+      expect(unpackPrimaryKey('42', [{ name: 'id', type: 'Number' }], { id: '42' })).toEqual({
+        id: 42,
+      });
+    });
+  });
+
   describe('when the key was derived, so its arity is a guess', () => {
     it('should keep a packed composite id whole rather than 500 on the segment count', () => {
       expect(