feat(agent-bff): serve the agent schema contract on GET /agent/v1/context - #1838
feat(agent-bff): serve the agent schema contract on GET /agent/v1/context#1838Tonours wants to merge 17 commits into
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.
| return Array.isArray(value) ? value : []; | ||
| } | ||
|
|
||
| type FieldWithWireEnums = ForestSchemaField & { enums?: string[] }; |
There was a problem hiding this comment.
ForestSchemaField declares enum, the agent emits enums — fixing the type upstream in forestadmin-client would drop this intersection and the as unknown as cast in the fixture.
There was a problem hiding this comment.
Agreed on the diagnosis: ForestSchemaField declares enum, the agent emits enums (generator-fields.ts:71, generator-actions.ts:45), and unfolding.ts:80 already works around the same gap.
Not doing it in this PR though — fixing the declared type in forestadmin-client changes a published type for every consumer of that package, which is a wider blast radius than a BFF route deserves to carry. Worth its own ticket; I'll open one. Until then the intersection type is named FieldWithWireEnums so the reason is visible at the use site rather than hidden in a cast.
| 'rule with no operand carries no `value`.', | ||
| }); | ||
|
|
||
| const ContextFieldSchema = z.object({ |
There was a problem hiding this comment.
ContextField in build-context.ts restates this shape with nothing tying the two together — z.infer<typeof ContextFieldSchema> would keep them from drifting.
There was a problem hiding this comment.
Tempting, and I went back and forth on it. Not taking it: z.infer<typeof ContextFieldSchema> would make the serializer depend on the OpenAPI schema, so the documentation layer becomes the source of truth for the runtime shape. I'd rather have the dependency the other way round, or neither.
The drift risk you're pointing at is real though. A conformance test — validate a built context against ContextResponseSchema — would catch it without inverting the dependency. Happy to add that here if you'd prefer it over leaving the two declarations side by side.
There was a problem hiding this comment.
Added the conformance test in b1d4c50, since you did not come back on the offer and the drift risk you raised is real either way.
build-context.test.ts now validates a built context against ContextResponseSchema with safeParse, using the fixture that covers every wire edge. That catches a divergence between the serializer and the document without making the OpenAPI layer the source of truth for the runtime shape — the dependency stays test-only, neither module imports the other.
I checked it can actually fail rather than just pass: typing meta.schemaRevision as a string makes it fail with Invalid input: expected number, received string, and reverting makes it green again.
Still not taking z.infer<typeof ContextFieldSchema> for the reason above, but the two declarations are now pinned to each other by a test instead of by nothing.
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.
…and relationship union
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.
| if (field.reference) serialized.reference = field.reference; | ||
| if (field.inverseOf) serialized.inverseOf = field.inverseOf; | ||
|
|
||
| const polymorphicTargets = toArray(field.polymorphicReferencedModels); | ||
| if (polymorphicTargets.length > 0) serialized.polymorphicTargets = [...polymorphicTargets]; |
There was a problem hiding this comment.
collect-unfolding.ts:157 drops relations whose foreign collection is outside the allow-list so the document doesn't "promise a dead path" — here reference and polymorphicTargets go out unfiltered, so the contract can name a target absent from collections[].
There was a problem hiding this comment.
Real gap — the fixture already exhibits it: it references orders and teams while serving only users, User.address, My Coll and collectionWithoutFieldsNorActions. Documented rather than filtered, in c917ac3.
The two cases are not symmetric. collect-unfolding emits routes, so documenting orders while it is hidden promises an endpoint that 404s — dropping it is right there. /context emits a schema: reference: "orders.customerId" claims the field points at orders, which stays true whether or not this document serves it.
Filtering would also undo your round-1 point. Strip reference and polymorphicTargets and ownerPolymorphic goes back to {"field":"ownerPolymorphic","type":"String","relationship":"BelongsTo"} — a relation indistinguishable from a text column, which is exactly the bug those two fields were added to fix. I would rather not trade a blocking finding for a smaller one.
So the contract now states it: the description says a target named by reference or polymorphicTargets is not guaranteed to appear in collections[], and the consumer must cross-check. A test pins the behaviour so it reads as a decision rather than an oversight. If you would rather the document never name an unserved target, that is a contract change worth its own ticket — say so and I will open it.

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