feat(agent-bff): serve the agent schema contract on GET /agent/v1/context - #1838
Conversation
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (8)
🛟 Help
|
nbouliol
left a comment
There was a problem hiding this comment.
Solid work — the snapshot seam is the right call, and the fixture pins a genuinely wide set of wire edges. One blocking point on the relation metadata, plus a few smaller things.
nbouliol
left a comment
There was a problem hiding this comment.
Second pass. Relation metadata and the helper extraction look good.
One thing to flag up front: my previous comment on the declared status codes was wrong in both halves, and you acted on it — details in the thread on openapi-document.ts. Net effect is that 403 should come back and 413/415 should go.
On the API-key reversal: the reasoning holds — the document reads no principal, and it is meant to be crossed with /agent/v1/permissions. But the ticket still carries Given le mode API-key, when la route est appelée, then elle est refusée — mode OAuth exigé, and the widening is real (a rendering-scoped key can now enumerate the whole environment schema). Worth updating PRD-944 rather than leaving the AC contradicted by the PR body.
Still open from the first pass: the allow-list filter in build-context.ts is now provably dead — your own new read-model-store.test.ts:32-34 asserts the two can never diverge.
nbouliol
left a comment
There was a problem hiding this comment.
Dropping the filter is right — buildContext has one non-test caller, isn't exported from the package index, getSchemaSnapshot has no await between the collections and the revision read, and the deleted test's invariant is covered directly at read-model-store.test.ts:30-37.
Ran locally on this head: 1123 tests / 72 suites pass, tsc --noEmit clean, prettier --check clean. The only red CI job is the ai-proxy LLM integration suite, which is a live OpenAI call unrelated to this PR.
One knock-on from the new conformance test, in the thread below: it also pins fields: [] for the fields-less fixture collection as contract-valid, which hardens the gap against jamais de fields: [].
Still outstanding at the ticket level, not the code: PRD-944 still carries Given le mode API-key, then elle est refusée — mode OAuth exigé, and there is now a green test asserting the opposite. Worth updating the AC rather than leaving it contradicted.
nbouliol
left a comment
There was a problem hiding this comment.
All three of the last round's points landed. The round-trip assertion is exactly the fix, and documenting the unserved-target behaviour rather than filtering it is the better call — I'm not reopening that.
Ran locally on this head: 1124 tests / 72 suites pass, tsc --noEmit clean, prettier --check clean. The only red CI job is still the ai-proxy LLM integration suite, a live OpenAI call unrelated to this PR.
What's left is small. Nothing here blocks.
Still outstanding at the ticket level rather than in the code: PRD-944 carries Given le mode API-key, then elle est refusée — mode OAuth exigé, and there is a green test asserting the opposite. Worth updating the AC.
nbouliol
left a comment
There was a problem hiding this comment.
Good batch. Every point from the last round is closed, and the ticket amendment settles the API-key question I'd been carrying since pass 2 — I'm treating that as resolved.
Verified on this head: tsc --noEmit clean, prettier --check clean, CI green on agent-bff. (My local jest run showed failures, but all of them are the same supertest .send() resolution error in timezone-middleware.test.ts, which this PR never touches — the worktree has no installed node_modules. Environmental, ignore it.)
The recursive ContextFieldType renders correctly: components.schemas.ContextFieldType emits anyOf: [string, array→$ref self, object{fields:[…$ref self]}], and the spec-validity and unfolded tests pass over it.
Two things below. Neither blocks.
One last doc gap with no diff line to hang it on: packages/agent-bff/README.md never mentions /agent/v1/context, and its Timezone section still states None → 400 missing_timezone unconditionally, which is no longer true for this route.
nbouliol
left a comment
There was a problem hiding this comment.
Both points from last round are properly closed. The composite enums fix matches ForestServerColumnType and has real fixture coverage (nested array, array-of-composite, sub-field enums: ['FR','BE']), and the round-trip test is load-bearing — deleting enums from the zod schema does make it fail. CI fully green this time, tsc and prettier clean.
On the guard removal: you were right about null — it throws in ReadModel (read-model.ts:29 for fields, :105 for actions) before buildContext ever runs, so those two guards were dead, and keeping the action.fields one is exactly right since read-model.ts:117 spreads null into {} without complaint. I said as much before checking primitives, which behave differently — thread below.
Also below: the warm-cache assertion, which I should have raised several rounds ago and didn't.
One AC-wording note, not code: the ticket line says the document is "filtered by nothing", but actions genuinely are filtered. The served OpenAPI already says it correctly ("minus the endpoint-less actions"), so it is the AC sentence that is stale.
| ): ContextCollection { | ||
| return { | ||
| name: collection.name, | ||
| fields: toArray(collection.fields).map(toContextField), |
There was a problem hiding this comment.
A primitive entry survives ReadModel (toRelationTarget reads .relationship off a string and returns null), so it reaches here and serializes as {} — no field, no type, against the published ContextFieldSchema. null really was dead code; primitives weren't.
There was a problem hiding this comment.
Right, and the guard is back at build-context.ts:129. toRelationTarget reads .relationship off a string and returns null, buildPrimaryKeys reads .isPrimaryKey and gets undefined, so a primitive walks through ReadModel untouched and used to serialize as {}. Fixture now carries a bare 'garbage' entry in users.fields, and the round-trip test against ContextResponseSchema fails without the filter — so does the new explicit one.
| const enums = toArray(field.enums); | ||
| if (enums.length > 0) serialized.enums = [...enums]; |
There was a problem hiding this comment.
enums: [] is dropped here but kept on action fields at :107, so an enum column with no values is indistinguishable from enums: null — build-context.test.ts:178 argues empty lists "carry meaning".
There was a problem hiding this comment.
Aligned on the action-field behaviour: fields now use the same Array.isArray check, so enums: [] survives and enums: null still drops. Fixture has statusWithEmptyEnums, and the test asserting the old omit-on-empty behaviour is replaced by one pinning the empty list.
| await request(app.callback()).get(ROUTE); | ||
| await request(app.callback()).get(ROUTE); | ||
|
|
||
| expect(fetchSchema).toHaveBeenCalledTimes(1); |
There was a problem hiding this comment.
The warm response is never read, so a warm hit returning 500 or an empty collections still passes — the TTL test just below captures its second response and asserts on it.
There was a problem hiding this comment.
Fixed: the warm request is captured and asserted — status 200 and body equal to the cold one, so a warm 500 or an emptied collections now fails the test rather than passing on the fetch count alone.
…and relationship union
72f4c41 to
abd9d43
Compare
|
Rebased on
The three review points are addressed in On the AC wording: updated PRD-944 — "aucune collection n'est filtrée", with the action filter spelled out explicitly. Local run on this head: 1179 tests pass. The 10 failures are all missing local deps and reproduce on plain |
abd9d43 to
4369061
Compare
# @forestadmin/agent-bff [1.20.0](https://github.com/ForestAdmin/agent-nodejs/compare/@forestadmin/agent-bff@1.19.1...@forestadmin/agent-bff@1.20.0) (2026-08-24) ### Features * **agent-bff:** serve the agent schema contract on GET /agent/v1/context ([#1838](#1838)) ([7b5d48a](7b5d48a))

fixes PRD-944
What
New route
GET /agent/v1/context, declared in the served OpenAPI document. Serves the allow-listed agent schema so a trusted UI can build its own requests.{ "collections": [ { "name": "articles", "fields": [ { "field": "id", "type": "Uuid", "isPrimaryKey": true }, { "field": "status", "type": "Enum", "enums": ["DRAFT","PUBLISHED"] }, { "field": "thumbnail", "type": "String", "validations": [{ "type": "is like", "value": "/^data:.*;base64,.*/" }] }, { "field": "orders", "type": "String", "relationship": "HasMany", "reference": "orders.customerId", "inverseOf": "customer" } ], "actions": [ { "id": "…", "name": "Ban user", "type": "single", "fields": [ … ] } ] } ], "meta": { "schemaRevision": 3, "environmentId": 42 } }Both auth modes reach it — an OAuth session and a BFF API key get the same document, since the schema is not caller-scoped. No timezone required.
Why
The client needs the schema to know which collections exist, which fields are filterable text, which actions take records. Today it gets that from the Forest SaaS directly, which means a SaaS token in the browser. The BFF already caches this schema for its own data routes.
How
SchemaCache(raw schema), notReadModel(a projection that drops most field metadata)ReadModelStore.getSchemaSnapshot()returns{collections, readModel, revision}in one step, so collections and the read-model filtering them always belong to the same generationcreateTimezoneMiddleware, which otherwise rejects any/agentrequest without a timezoneField types are passed through verbatim. A
Binarycolumn is advertised asString— that is what it is on the wire, bytes travel as a data uri or hex — sovalidationsis the only reliable way to tell an encoded field from plain text.relationshipandpolymorphicTargetstell a to-one from a to-many, whichreferencealone cannot.Scope
No AI relay (
POST /agent/v1/ai/query) — separate PR. No rendering/project/team identity. Per-field operators stay out (PRD-685).Known limitation
meta.schemaRevisionis a process-local counter, reset on restart: a redeploy shipping a schema change can reproduce a previous value. Matters only if a consumer uses it to invalidate a cache.Test
yarn workspace @forestadmin/agent-bff test1121 tests. One fixture pins every edge the wire produces: array and composite types, relations with and without
inverseOf, polymorphic, the three action types, an endpoint-less action,enums: null, falsy defaults, malformed validations, dotted and spaced collection names.Definition of Done
General
Security