Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,6 @@ The response includes a `gitBranchName` field with a suggested branch name conta
Decisions live in `docs/adr/`. Run `/adr` to record one.

- [A case-insensitive Decision follows Postgres's ILIKE, whatever the datasource](docs/adr/2026-09-09-a-case-insensitive-decision-follows-postgres-s-ilike-whatever-the-data.md) — 2026-09-09-a-case-insensitive-decision-follows-postgres-s-ilike-whatever-the-data [accepted]
- [The liana name the schema already carries decides a v1 agent's capabilities](docs/adr/2026-09-11-the-liana-name-the-schema-already-carries-decides-a-v1-agent-s-capabi.md) — 2026-09-11-the-liana-name-the-schema-already-carries-decides-a-v1-agent-s-capabi [accepted]
- [A schema declaring no primary key gets one derived from its `id` field](docs/adr/2026-09-11-a-schema-declaring-no-primary-key-gets-one-derived-from-its-id-field.md) — 2026-09-11-a-schema-declaring-no-primary-key-gets-one-derived-from-its-id-field [accepted]
<!-- ADR:INDEX:END -->
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
status: accepted
date: 2026-09-11
tags: [agent-bff, primary-key, v1-compatibility]
affected_components: [agent-bff]
---

# A schema declaring no primary key gets one derived from its `id` field

`forest_liana` only publishes `isPrimaryKey` from 9.17.6 (2026-06-04), so every collection of an older one reaches the BFF keyless, `unpackPrimaryKey` rejects every record the agent returns and a plain list answers `500 mapping_error` — on 125 `forest-rails` environments active in the last 120 days, the twenty oldest run between 2.14.6 and 8.3.2, several in production. The read-model therefore derives a key when the schema declares none: a field literally named `id`, typed as the schema declares it, `String` when no such field exists. The trigger is the shape of the schema, not the liana version, so a collection that declares a key keeps it and every v2 agent is untouched; the fallback only fires where the alternative is a 500. Two things are guessed and both are published as guesses rather than hidden. The arity: the schema names no key, so a collection whose real one is composite packs `tenant|42` behind the same silence, and splitting that against one derived key would 500 the whole list — a derived key is flagged `derived` and `unpackPrimaryKey` passes it through whole, typed only when the declared `id` field says `Number`. The name: on the schema this was measured against, 12 of 269 collections declare no `id` field at all, so `__forest.primaryKey` carries the opaque agent id under a name that is not a column, which `ForestRecordMeta` states as the one exception to its per-column promise. Where an `id` field does exist, the context marks it `isPrimaryKey` together with `isPrimaryKeyDerived`, because nothing confirms it is the column the records are really keyed on and a filter on it can answer 200 with no row; where it does not, the context marks no field at all. The cost is bounded: every route takes the record id packed and opaque, so nothing in the request path breaks, and the invented name only misses for a consumer filtering ON the key — which a keyless collection could not do either way, since nothing in the schema names that column. Rejected: deriving only when an `id` field is declared, which leaves those 12 ordinary listable collections on the 500 this exists to remove; and reading the key from the agent's capabilities, which the v1 lianas this targets do not serve.
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
status: accepted
date: 2026-09-11
tags: [agent-bff, capabilities, v1-compatibility]
affected_components: [agent-bff, forestadmin-client]
---

# The liana name the schema already carries decides a v1 agent's capabilities

`forest-express-sequelize`, `forest-express-mongoose` and `forest-rails` never served `POST /forest/_internal/capabilities`, so every constrained read the BFF fronts died on that 404 and the collection was unusable. On that 404 the BFF now matches the schema's `meta.liana` against those three names and synthesizes the answer from the apimap the liana already pushed: the operator set that generation implements per column type, plus the fields the schema marks filterable and sortable. Any other liana keeps throwing, so a new v2 agent is never silently downgraded to a legacy operator set — the legacy list is closed in practice while the v2 family grows. The published operators are the measured intersection of what `forest-express-sequelize 9.6.10` and `forest_liana 9.21.0` honour, not the column type's whole table: an operator only one of them implements maps to `422 unprocessable_entity`, where an operator neither advertises is the `400 invalid_filter_operator` a caller can act on — `includes_all` answers 500 with a leaking SQL fragment in Express and 422 in Rails, `i_contains` works in Rails only, so neither is published. An array column publishes no operator at all, matching v2, where `allowedOperatorsForColumnType` is keyed by primitive names only. The name is self-reported by the agent, and that is accepted: it reaches the BFF inside the schema the agent pushed and the SaaS served back, so it is exactly as trustworthy as the field list read beside it, and a wrong name can only narrow the operator set of the very agent that declared it. Rejected: an operator-set environment flag (`BFF_LEGACY_CAPABILITIES`), which asked a human to restate per deployment what the schema already carries and went stale on the next agent upgrade; and probing the agent's behaviour to infer its generation, which turns every cold capabilities read into a series of requests whose failures are indistinguishable from an agent that is merely down.
9 changes: 9 additions & 0 deletions packages/agent-bff/src/action/action-routes-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,15 @@ async function handleExecute({
}

if (error instanceof ActionFormValidationError) {
const { unstructuredCause } = error;

if (unstructuredCause) {
logger('Warn', 'Agent action 4xx carried no structured error; client message is generic', {
status: unstructuredCause.status,
cause: unstructuredCause.responseText ?? unstructuredCause.body,
});
}

const html = sanitizeActionHtml(error.html, logger);

throw actionError(error.message, html === null ? undefined : { html });
Expand Down
23 changes: 21 additions & 2 deletions packages/agent-bff/src/context/build-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface ContextField {
inverseOf?: string;
polymorphicTargets?: string[];
isPrimaryKey?: boolean;
isPrimaryKeyDerived?: boolean;
isRequired?: boolean;
isReadOnly?: boolean;
enums?: string[];
Expand Down Expand Up @@ -113,6 +114,7 @@ function toContextValidations(validations: unknown[] | null | undefined): Contex
function toContextField(
field: FieldWithWireEnums,
ambiguousKeys: ReadonlySet<string>,
derivedPrimaryKeys: ReadonlySet<string>,
): ContextField {
const serialized: ContextField = { field: field.field, type: field.type };

Expand All @@ -126,7 +128,21 @@ function toContextField(
const polymorphicTargets = toArray(field.polymorphicReferencedModels);
if (polymorphicTargets.length > 0) serialized.polymorphicTargets = [...polymorphicTargets];

if (field.isPrimaryKey) serialized.isPrimaryKey = true;
// The read-model derives a key when the schema declares none, and the BFF builds record
// identifiers from it. Publishing only the schema's flag would leave a client unable to name the
// key the BFF is actually using.
//
// A derived key is flagged as such, because it is a GUESS: the schema published nothing, so this
// field named `id` may not be the real key. Publishing it as a plain `isPrimaryKey` would send a
// client to filter on it, and a filter against a column that is not the key answers 200 with no
// row — a silence far worse than the 422 it would get on a field the collection does not expose.
if (!field.isPrimaryKey && derivedPrimaryKeys.has(field.field)) {
serialized.isPrimaryKey = true;
serialized.isPrimaryKeyDerived = true;
} else if (field.isPrimaryKey) {
serialized.isPrimaryKey = true;
}

if (field.isRequired) serialized.isRequired = true;
if (field.isReadOnly) serialized.isReadOnly = true;

Expand Down Expand Up @@ -168,10 +184,13 @@ function toContextCollection(
field => typeof field === 'object' && field !== null,
);
const ambiguousKeys = ambiguousRecordKeys(fields);
const derivedPrimaryKeys = new Set(
readModel.getPrimaryKeys(collection.name).map(key => key.name),
);

return {
name: collection.name,
fields: fields.map(field => toContextField(field, ambiguousKeys)),
fields: fields.map(field => toContextField(field, ambiguousKeys, derivedPrimaryKeys)),
actions: toArray(collection.actions)
.filter(action => {
const allowed = allowedActions[action?.name];
Expand Down
29 changes: 27 additions & 2 deletions packages/agent-bff/src/data/agent-query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import type { PageInput, SortClauseInput } from './request-schemas';
import type { Logger } from '../ports/logger-port';
import type { ZodType, z } from 'zod';

import { toWireFilter } from '@forestadmin/agent-client';

import {
CountFlatInputs,
ListFlatInputs,
Expand Down Expand Up @@ -37,6 +39,7 @@ function isPlainObject(value: unknown): value is Record<string, unknown> {

const LEAF_KEYS = ['field', 'operator', 'value'];
const BRANCH_KEYS = ['aggregator', 'conditions'];
const AGGREGATORS = ['And', 'Or'];

function loggingRejections<T>(logger: Logger, parse: () => T): T {
try {
Expand All @@ -61,6 +64,27 @@ function assertNoStrayKey(node: Record<string, unknown>, allowed: string[]): voi
}
}

// An absent aggregator stays allowed and is forwarded, as the document says. A present one must name
// an aggregator that exists: anything else -- `5`, or `xor` -- passes every shape check, reaches the
// agent and comes back as a 503 agent_unavailable, where the caller can act on a 400.
//
// The comparison is case-insensitive although the document enumerates PascalCase only:
// `toWireFilter` lowercases the aggregator on the way out, so `and` already reaches the agent as the
// `and` it parses, and rejecting it here would break a caller nothing else refuses.
function assertAggregator(node: Record<string, unknown>): void {
const { aggregator } = node;

if (aggregator === undefined) return;

const named =
typeof aggregator === 'string' &&
AGGREGATORS.some(candidate => candidate.toLowerCase() === aggregator.toLowerCase());

if (!named) {
throw invalidRequest(`A filter branch aggregator must be one of: ${AGGREGATORS.join(', ')}`);
}
}

function assertFilterNode(node: unknown, depth = 0): void {
if (depth > MAX_FILTER_DEPTH) throw filterTooDeep(MAX_FILTER_DEPTH);
if (!isPlainObject(node)) return;
Expand All @@ -73,6 +97,7 @@ function assertFilterNode(node: unknown, depth = 0): void {

if (readableAsBranch) {
assertNoStrayKey(node, BRANCH_KEYS);
assertAggregator(node);
node.conditions.forEach(condition => assertFilterNode(condition, depth + 1));

return;
Expand Down Expand Up @@ -181,7 +206,7 @@ export function buildListAgentQuery(
): AgentQuery {
const query: AgentQuery = { timezone };

if (body.filter !== undefined) query.filters = JSON.stringify(body.filter);
if (body.filter !== undefined) query.filters = JSON.stringify(toWireFilter(body.filter));
if (body.projection?.length) query[`fields[${collection}]`] = body.projection.join(',');
if (body.sort?.length) query.sort = serializeSort(body.sort);
if (body.page) Object.assign(query, serializePage(body.page));
Expand All @@ -193,7 +218,7 @@ export function buildListAgentQuery(
export function buildCountAgentQuery(timezone: string, body: CountRequestBody): AgentQuery {
const query: AgentQuery = { timezone };

if (body.filter !== undefined) query.filters = JSON.stringify(body.filter);
if (body.filter !== undefined) query.filters = JSON.stringify(toWireFilter(body.filter));
applySearch(query, body);

return query;
Expand Down
7 changes: 6 additions & 1 deletion packages/agent-bff/src/data/data-routes-middleware.ts
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ function resolveCapabilities(
() =>
deps.store.getCapabilities(
collection,
createAgentCapabilitiesFetcher({ transport: deps.transport, token: deps.token }),
createAgentCapabilitiesFetcher({
transport: deps.transport,
token: deps.token,
store: deps.store,
logger: deps.logger,
}),
),
deps.logger,
);
Expand Down
107 changes: 106 additions & 1 deletion packages/agent-bff/src/data/pack-id.ts
Original file line number Diff line number Diff line change
@@ -1,26 +1,130 @@
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 = '|';

// The only column type unpacked to a number, mirroring the agent's `IdUtils.unpackId`.
const NUMBER_COLUMN_TYPE = 'Number';

/**
* The numeric form of an id, but only when it round-trips back to the exact same characters. A
* derived key carries the agent id opaque, so a cast that loses anything defeats its whole purpose:
* `9007199254740993` casts to `...992` and would name a different record, `Infinity` and `NaN`
* serialize as `null`, and `1e3` or `042` come back spelled differently from what the record's `id`
* holds. In every one of those the string is kept, which the contract already allows.
*/
function toNumberIfLossless(value: string): string | number {
const numeric = Number(value);

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
* `{ pkField: value }` map for `__forest.primaryKey`. Throws a mapping error rather than emitting a
* malformed key when the schema lacks key metadata or the packed id shape does not match it.
*
* A DERIVED key is the exception: the schema published none, so its arity is a guess and the real
* key may be composite. Splitting `tenant|42` against one guessed key would 500 every list of that
* collection, so a derived key takes the packed id whole — typed when the declared `id` field says
* `Number` and the id is numeric, left a string otherwise, and never a throw.
*/
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');
}

const [first] = primaryKeys;

if (first.derived) {
return {
[first.name]: first.type === NUMBER_COLUMN_TYPE ? toNumberIfLossless(packedId) : packedId,
};
}

const values = packedId.split(PACKED_ID_SEPARATOR);

if (values.length !== primaryKeys.length) {
Expand All @@ -29,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;
Expand Down
2 changes: 1 addition & 1 deletion packages/agent-bff/src/data/response-mappers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function mapListResponse(
...record,
__forest: {
collection,
primaryKey: unpackPrimaryKey(String(record.id), primaryKeys),
primaryKey: unpackPrimaryKey(String(record.id), primaryKeys, record),
},
};
});
Expand Down
Loading
Loading