feat(agent-bff): document permissions and openapi routes - #1862
Conversation
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (2)
🛟 Help
|
41a3301 to
0ac7297
Compare
nbouliol
left a comment
There was a problem hiding this comment.
Reviewed the documented contract against the runtime. Two descriptions state something the code does not do (the agent-side cache, and the timezone middleware covering every /agent route) — those are worth fixing before merge since the whole value of the PR is that a consumer can trust the document. The rest are smaller accuracy points.
|
Correction — my previous push got the cache claim backwards, and the review pass caught it.
What it says now:
Both halves are pinned by tests now, the default and the non-default one, so neither can be flipped again without a red suite. Two more published claims corrected on the same push:
Also: the HEAD description said a 404 "carries the same |

What
Registers the two live routes the served OpenAPI document was missing, so a consumer building from the document alone can fetch permission hints and discover the document endpoint. Both are registered before the generic/unfolded fork, so both document variants carry them.
GET /agent/v1/permissions— the fullcrud+actionsresponse schema (PermissionHints,ActionHints,CrudHints), thecollectionsquery filter, the sharedX-Forest-Timezoneheader parameter, both auth modes, and the display-hint contract: hints gray out buttons, they never authorize.GET/HEAD /agent/openapi.json— self-documenting, with a typed404 openapi_disabled.fixes PRD-1107
What the document now says that it did not
Three claims were wrong or misleading before review and are corrected here — the whole value of this PR is that a consumer can trust the document, so they matter more than the routes themselves:
The hints lag; the agent does not. The BFF caches hints for 15 minutes. The agent runs with
instantCacheRefreshby default (options-validator.ts,?? true) and invalidates on Forest events, so it enforces a new permission while these hints are still stale. Only an agent explicitly configuredinstantCacheRefresh: falsecaches 15 minutes of its own, independently — and then the two can disagree in either direction until both expire. Both cases are stated, and both are pinned by tests.A 503 here does not always carry
Retry-After. Onlypermissions_unavailabledoes. The route also answersschema_unavailableandkey_resolution_unavailable, which do not — a client backing off on a header that is not there would have looped immediately.The timezone middleware does not cover every
/agentroute.createOpenApiRoutes, the context routes and the AI relay are all mounted before it. The permissions route is behind it, which is why it 400smissing_timezoneon a deployment with no default — the description now says "unlike the context and document routes" instead of "like every route under/agent".crudis not a role fact on a development environment.canRolePerformshort-circuits there and all six flags come back true whatever the role.ActionHintsalready said this;CrudHintsnow does too.Smaller accuracy fixes:
collectionsis published asstring | string[]because a repeated parameter is joined and re-split, so both forms are one request; and the sharedError503/Retry-Afterdescriptions name the failure class (the BFF could not reach the Forest server) rather than the single instance they listed.Why
/healthstays outPRD-1107 accepts either outcome — "documented, or a deliberate decision is recorded that they are intentionally out of the agent contract" — and its regression check is "at minimum
/agent/v1/permissions".The decision is the boundary, not tooling: this document describes the auth-gated
/agentsurface, and/healthis answered inbff-http-serverbefore CORS and before the auth edge. A consumer generating a client from this document should not get a/healthmethod that plays by different rules than every other one it holds./oauth/*is out for the same reason, and so isGET /docs— the unauthenticated viewer that renders this very document, which was neither described nor recorded before.info.descriptionrecords all three.Scope and safety
Documentation only. No middleware, mounting condition, status or body change. A drift guard in
test/permissions/build-permission-hints.test.tsparses a realbuildPermissionHintsoutput with the documented schema, so the documented shape cannot silently diverge from the runtime one.How to test
Definition of Done
General
Security