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.
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
19 changes: 19 additions & 0 deletions packages/agent-bff/src/data/pack-id.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,22 @@ export const PACKED_ID_SEPARATOR = '|';
// The only column type unpacked to a number, mirroring the agent's `IdUtils.unpackId`.
const NUMBER_COLUMN_TYPE = 'Number';

function toNumberIfWhole(value: string): string | number {
const numeric = Number(value);

return Number.isNaN(numeric) ? value : numeric;
}

/**
* 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,
Expand All @@ -21,6 +32,14 @@ export default function unpackPrimaryKey(
throw mappingError('Cannot build primary key: the collection exposes no key metadata');
}

const [first] = primaryKeys;

if (first.derived) {
Comment thread
nbouliol marked this conversation as resolved.
return {
[first.name]: first.type === NUMBER_COLUMN_TYPE ? toNumberIfWhole(packedId) : packedId,
};
}

const values = packedId.split(PACKED_ID_SEPARATOR);

if (values.length !== primaryKeys.length) {
Expand Down
43 changes: 30 additions & 13 deletions packages/agent-bff/src/openapi/collect-unfolding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,23 +98,35 @@ function documentedOperators(
return toCanonicalOperatorSet(normalized);
}

/**
* The filterable subset, plus whether a field was dropped as undocumentable rather than as genuinely
* not filterable. The caller needs the two apart: a field with no operator is an ANSWER the document
* can state, while a field dropped for operator skew leaves the filterable set incomplete, and an
* incomplete set that ends up empty must not be documented as "nothing is filterable".
*/
function collectFilterableFields(
collection: string,
capabilities: CapabilitiesResult,
logger: Logger,
): FilterableField[] {
return capabilities.fields.flatMap(field => {
if ((field.operators?.length ?? 0) === 0) return [];
): { fields: FilterableField[]; undocumentable: boolean } {
const fields: FilterableField[] = [];
let undocumentable = false;

const operators = documentedOperators(
collection,
field.name,
field.operators as string[],
logger,
);
for (const field of capabilities.fields) {
if ((field.operators?.length ?? 0) > 0) {
const operators = documentedOperators(
collection,
field.name,
field.operators as string[],
logger,
);

return operators === null ? [] : [{ name: field.name, operators }];
});
if (operators === null) undocumentable = true;
else fields.push({ name: field.name, operators });
}
}

return { fields, undocumentable };
}

async function collectFields(
Expand All @@ -132,10 +144,15 @@ async function collectFields(
return UNTYPED('no_fields');
}

const filterable = collectFilterableFields(collection, capabilities, logger);

return {
projectable: capabilities.fields.map(({ name, type }) => ({ name, type })),
filterable: collectFilterableFields(collection, capabilities, logger),
projectable: capabilities.fields.map(({ name, type, sortable }) =>
sortable === false ? { name, type, sortable } : { name, type },
),
filterable: filterable.fields,
degraded: null,
...(filterable.undocumentable ? { undocumentableFilter: true as const } : {}),
};
} catch (error) {
// A single unreachable collection must not cost the whole document: the collection keeps its
Expand Down
24 changes: 22 additions & 2 deletions packages/agent-bff/src/openapi/schemas.ts
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,14 @@ export const ForestRecordMetaSchema = z
'The record identity, unpacked from the agent id. A composite primary key carries one ' +
'entry per column. The values are TYPED here — a Number key column is a number — whereas ' +
'the record carries the same id as a string under `id`, so comparing the two forms ' +
'without coercion fails.',
'without coercion fails. One exception to the per-column promise: against a schema that ' +
'declares no primary key at all, this carries the agent id whole under the name `id`, ' +
'never split per column — the schema published no key, so its real shape is unknown and a ' +
'composite one would stay packed, separator included. That `id` is a real column only when ' +
'the schema declares a field of that name; otherwise it is not a column at all and the ' +
'context marks no field as the key. When it does declare one, the context flags it ' +
'`isPrimaryKeyDerived`, meaning the key is a guess and a filter on it may match nothing. ' +
'Filter on a field the context lists, never on a key name read back here.',
});

export const ListResponseSchema = z
Expand All @@ -297,7 +304,9 @@ export const ListResponseSchema = z
description:
'Records are flat, each carrying a `__forest` envelope. A record always holds `id`, the ' +
`agent id as a string — a composite key is its values joined by \`${PACKED_ID_SEPARATOR}\` — ` +
'while `__forest.primaryKey` holds that same id typed and split per column. The list never ' +
'while `__forest.primaryKey` holds that same id typed and split per column — with the one ' +
'exception `ForestRecordMeta` describes, where the name it carries is not a column. ' +
'The list never ' +
'carries a total: call the count endpoint for that, which is why `countStatus` is always ' +
'`not_requested`. ' +
'It is always one page, not guaranteed to be the whole collection: a request that omitted ' +
Expand Down Expand Up @@ -366,6 +375,17 @@ const ContextFieldSchema = z.object({
inverseOf: z.string().optional(),
polymorphicTargets: z.array(z.string()).optional(),
isPrimaryKey: z.boolean().optional(),
isPrimaryKeyDerived: z
.boolean()
.optional()
.openapi({
description:
'Set only alongside `isPrimaryKey`, when the schema declared no primary key at all and ' +
'the BFF derived one from a field named `id`. The key is then a GUESS: it is what ' +
'`__forest.primaryKey` carries, but nothing confirms it is the column the records are ' +
'really keyed on, so a filter on it can answer 200 with no row. Use it to read record ' +
'identities, not to filter by identity.',
}),
isRequired: z.boolean().optional(),
isReadOnly: z.boolean().optional(),
enums: z.array(z.string()).optional(),
Expand Down
7 changes: 6 additions & 1 deletion packages/agent-bff/src/openapi/unfolded-document.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,12 @@ export default async function buildUnfoldedDocument(
const unfolding = await collectUnfolding({
readModel,
store: source.store,
capabilitiesFetcher: createAgentCapabilitiesFetcher({ transport: source.transport, token }),
capabilitiesFetcher: createAgentCapabilitiesFetcher({
transport: source.transport,
token,
store: source.store,
logger: source.logger,
}),
logger: source.logger,
});

Expand Down
Loading
Loading