Skip to content

feat(agent-bff): document permissions and openapi routes - #1862

Open
Tonours wants to merge 4 commits into
mainfrom
fix/prd-1107-openapi-missing-routes
Open

feat(agent-bff): document permissions and openapi routes#1862
Tonours wants to merge 4 commits into
mainfrom
fix/prd-1107-openapi-missing-routes

Conversation

@Tonours

@Tonours Tonours commented Aug 31, 2026

Copy link
Copy Markdown
Member

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 full crud + actions response schema (PermissionHints, ActionHints, CrudHints), the collections query filter, the shared X-Forest-Timezone header 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 typed 404 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 instantCacheRefresh by 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 configured instantCacheRefresh: false caches 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. Only permissions_unavailable does. The route also answers schema_unavailable and key_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 /agent route. createOpenApiRoutes, the context routes and the AI relay are all mounted before it. The permissions route is behind it, which is why it 400s missing_timezone on a deployment with no default — the description now says "unlike the context and document routes" instead of "like every route under /agent".

  • crud is not a role fact on a development environment. canRolePerform short-circuits there and all six flags come back true whatever the role. ActionHints already said this; CrudHints now does too.

Smaller accuracy fixes: collections is published as string | string[] because a repeated parameter is joined and re-split, so both forms are one request; and the shared Error503 / Retry-After descriptions name the failure class (the BFF could not reach the Forest server) rather than the single instance they listed.

Why /health stays out

PRD-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 /agent surface, and /health is answered in bff-http-server before CORS and before the auth edge. A consumer generating a client from this document should not get a /health method that plays by different rules than every other one it holds.

/oauth/* is out for the same reason, and so is GET /docs — the unauthenticated viewer that renders this very document, which was neither described nor recorded before. info.description records 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.ts parses a real buildPermissionHints output with the documented schema, so the documented shape cannot silently diverge from the runtime one.

How to test

yarn workspace @forestadmin/agent-bff test   # redocly lint included
yarn workspace @forestadmin/agent-bff lint && yarn workspace @forestadmin/agent-bff build

Definition of Done

General

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes
  • Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • Consider the security impact of the changes made

@linear-code

linear-code Bot commented Aug 31, 2026

Copy link
Copy Markdown

PRD-1107

@qltysh

qltysh Bot commented Aug 31, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (2)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
packages/agent-bff/src/openapi/openapi-document.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/openapi/schemas.ts100.0%
Total100.0%
🚦 See full report on Qlty Cloud »

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@Tonours
Tonours force-pushed the fix/prd-1107-openapi-missing-routes branch from 41a3301 to 0ac7297 Compare August 31, 2026 14:00
Comment thread packages/agent-bff/src/openapi/openapi-document.ts

@nbouliol nbouliol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread packages/agent-bff/src/openapi/openapi-document.ts
Comment thread packages/agent-bff/src/openapi/openapi-document.ts Outdated
Comment thread packages/agent-bff/src/openapi/openapi-document.ts Outdated
Comment thread packages/agent-bff/src/openapi/openapi-document.ts Outdated
Comment thread packages/agent-bff/src/openapi/openapi-document.ts
Comment thread packages/agent-bff/src/openapi/openapi-document.ts Outdated
Comment thread packages/agent-bff/src/openapi/schemas.ts
Comment thread packages/agent-bff/src/openapi/openapi-document.ts
@Tonours

Tonours commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

Correction — my previous push got the cache claim backwards, and the review pass caught it.

instantCacheRefresh defaults to true (options-validator.ts:44, ?? true), and when it is on the agent takes DEFAULT_CACHE_DURATION_WITH_EVENTS and invalidates on Forest events. The 15-minute fallback only applies to an agent explicitly configured instantCacheRefresh: false. So the original text was right by default, and "the agent caches its own copy for the same 15 minutes by default" — which I wrote in response to the thread above — was wrong.

What it says now:

…cached for 15 minutes, so they lag a change made in Forest. The agent does not: it runs with instantCacheRefresh by default and invalidates on Forest events, so it enforces the new permission while these hints are still stale. An agent explicitly configured with instantCacheRefresh: false caches for 15 minutes of its own, independently, and the two can then disagree in either direction until both expire.

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:

  • The 503 does not always carry Retry-After. Only permissions_unavailable does. The same route also answers schema_unavailable (via agent-route-helpers.ts) and key_resolution_unavailable from the API-key step, neither of which sets the header. A client backing off on a header that is not there would have looped immediately. All three types are named now.
  • /docs was missing from the surface statement. cli-core.ts mounts createDocsRoutes, which serves GET /docs and its bundle unauthenticated — a live public route the document neither described nor recorded as excluded, which is the gap PRD-1107 is about. It is recorded with /health and /oauth/*.

Also: the HEAD description said a 404 "carries the same openapi_disabled type as the GET", two sentences after saying HEAD answers with no body — koa ends a HEAD response without one, so the type is unobservable there. It now says to read the status, not the type. And the comment added to build-permission-hints.test.ts is gone; the test name already carried it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants