docs(agent): document the embedded BFF and the mount order it needs

[nbouliol]

Stacked on #1876, last of the stack.

What

• agent-bff/README.md gains an "Embedded in an agent" section: what addBff() inherits and why it cannot be overridden, and a table of the four things that differ from the standalone deployment (configuration source, no AGENT_URL, no HTTP_PORT, openapiEnabled default inverted, /health always 200). The /health section now documents features as the chain it is — tokenEncryptionKey → oauth → ai — rather than four independent switches.
• agent/README.md, which was empty, now covers the two optional in-process components: addBff() and addWorkflowExecutor().
• packages/_example wires addBff() alongside mountAiMcpServer(), on the seven frameworks it already mounts on. It is the only place where the ordering constraint below is visible in real code.

The ordering constraint, verified rather than assumed

The review of PRD-1076 predicted that an express.json() registered before the mount would empty the BFF's request bodies. Measured, it is worse and better at once: the request fails with 500 stream.not.readable. Worse because a POST cannot work at all; better because it is loud — no silently dropped filter, projection or search.

Both directions are now pinned in the e2e suite, so the contract stops being folklore:

• agent mounted before the host's parser → 200, one record
• host's parser first → 500 stream.not.readable

Tests

agent 85 suites / 1535 tests.

Production guidance

Recorded in the BFF README: embedded for a single deployment, which is most of them; standalone when several agents share one BFF, or when the BFF and the agent have to scale separately.

Fixes PRD-1076

🤖 Generated with Claude Code

Note

Document embedded BFF and add mount-order tests to agent and agent-bff

• Expands the agent-bff and agent READMEs with embedded-agent usage, inherited configuration, middleware ordering, and standalone-vs-embedded comparison
• Updates the example agent in agent.ts to read BFF_ALLOWED_ORIGINS and BFF_TOKEN_ENCRYPTION_KEY, then register an in-process BFF in the builder chain
• Adds e2e tests in embedded-bff.e2e.test.ts covering BFF mounted before/after host body parser and host CORS middleware, pinning success, stream.not.readable failure, and preflight header behavior

Changes since #1877 opened

• Changed CORS Layer 1 to reject requests from non-allowlisted origins with 403 origin_not_allowed status instead of serving responses without CORS headers [8e07e4f]
• Renamed the health endpoint response field from features to configured and clarified that it reflects configured optional surfaces rather than upstream Forest server reachability [8e07e4f]
• Added support for mounting the BFF under arbitrary sub-paths with dynamic prefix derivation [8e07e4f]
• Expanded integration guidance for body parsers and CORS middleware mounting order [8e07e4f]

^{Macroscope summarized 3c34a4a.}

[linear-code]

PRD-1076

[qltysh]

Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

🚦 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]

Spec (PRD-1076): steps 18, 20 and 21 are delivered. Step 19 is delivered on its documentation half and half of its test half — and the half that got the test is the less dangerous one. Step 17's documentation requirement is not met at all.

Step 19 asked to "Document the host-registration contract and test it", naming two hazards. The body-parser hazard is done properly: documented with the measured failure mode, and the new test asserts both directions with an exact status and an exact error type, on isolated agents and per-instance schema paths. The cors() hazard is documented well (agent-bff/README.md:273-274) and has no test — and it is the security-relevant one, since a permissive host cors() silently neutralises the BFF's strict origin allow-list.

The ticket's prediction was wrong and this PR is right to correct it. Step 19 predicted an emptied request body; the measured behaviour is 500 stream.not.readable, and I traced the chain to confirm it: express.json() leaves req.readable === false, @koa/bodyparser reaches raw-body, which throws createError(500, 'stream is not readable', { type: 'stream.not.readable' }), and isSerializableError duck-types on status/type/message so the type is echoed verbatim rather than flattened to internal_error. Replacing a guess with a measurement is the right call and worth saying.

Also verified accurate, so it does not get re-checked: the inherited secrets/urls/logger really cannot be overridden (BffEmbedOptions exposes none of them); openapiEnabled really defaults off embedded and on standalone; the features block's four flags match their stated sources, including the openapi conjunction with a mounted agent edge; /bff really is registered on all five mount targets plus the standalone callback; and the /health disclosure note draws the right distinction — it says which capabilities are configured without naming config keys, which is what the code does.

Seven findings inline.

[Tonours]

My eight findings are all addressed on 7b79c5a: both quick-starts and the addBff() JSDoc mount before start(), the registration-order sample starts the agent, the served/exported OpenAPI split is documented, agentTimeoutMs bounds the wait, not the work is there, the example dependency is "*", the CORS preflight pair is tested in the same shape as the body-parser one, the fixed /bff prefix and the sub-router limitation are stated, and the four items on the agent-bff README are in.

Holding the approval on one thing that is still open — Macroscope's last comment, which got no reply:

packages/_example/.env.example:23 tells the reader to generate BFF_TOKEN_ENCRYPTION_KEY with openssl rand -hex 32. That is 64 hex characters; parseEncryptionKey (env-config.ts:72-86) accepts the charset but decodes it to 48 bytes, not 32, so the agent throws Invalid configuration: BFF_TOKEN_ENCRYPTION_KEY must be base64-encoded and exactly 32 bytes (AES-256). at startup. Anyone following the example file cannot boot it.

openssl rand -base64 32 is the right command. The line right below (FOREST_EXECUTOR_ENCRYPTION_KEY, line 34) is correct as -hex 32 — that value is not validated — so only line 23 changes. Both READMEs already say "base64-encoded 32-byte", so this is the example file alone.

Approving as soon as that line is fixed.

[nbouliol]

Fixed on 49a7010. .env.example:23 is now openssl rand -base64 32, with the line stating what parseEncryptionKey wants (base64, decoding to exactly 32 bytes) rather than only how to generate it. FOREST_EXECUTOR_ENCRYPTION_KEY stays -hex 32, as you noted.

Two more from Macroscope in the same commit: the lifecycle paragraph now distinguishes bff_not_started from bff_stopped, and the MCP collision is documented as throwing from the builder call rather than from start() (also widened to any basePath landing inside /bff, matching collidesWithBff).

agent: 63/63 on test/bff, eslint src test 0 errors.

[Tonours]

Delta since my last review (7b79c5a → 0c6f3df): rebase onto #1876 plus three doc commits and 8e84f1f. Three statements no longer match the code they describe — ironic given 2340409's own subject.

Should fix

1. /health still documents features; the code now serializes configured — agent-bff/README.md:215 and :218 show "features": { … }, and :221 says features says what this deployment actually serves. 55875e0 (#1876) renamed the key to configured and inverted the claim on purpose ("Deliberately not a statement that they work", health-route.ts:6-10). A reader following this README branches on a key that no longer exists. Same for the closing sentence at :230 ("It reports what is served, not how it was configured") — that is now precisely backwards. This is macroscopeapp's open thread; sample, table intro and closing sentence all need the rename.

2. The sub-router paragraph describes the pre-90001a0 behavior — agent/README.md:46-48: "an agent mounted on an express router at /api answers at /api/bff while the OpenAPI document still advertises /bff". Since 90001a0 (#1873) the document advertises /api/bff: the e2e suite added in 55875e0 asserts servers: [{ url: '/api/bff' }] and the prefixed docs-page urls for exactly this app.use('/api', mounted) shape. Either drop the warning or reword it around whatever residual reason remains to prefer a root mount.

3. The cors paragraph describes the pre-deny behavior — agent/README.md:41-44: a permissive host cors() "answers the preflight with its own policy, and the BFF's exact-origin allow-list never gets a say." Since 55875e0 the allow-list does get a say: the request that follows the hijacked preflight is refused with 403 origin_not_allowed (e2e-asserted). Reword to say the host preflight is lost but the followed request is still refused — and keep it in sync with whatever #1876 decides for same-origin Origin (see my review there: same-origin POSTs currently 403 unless the own origin is listed).

Verified good

8e84f1f (supertest import name) and 0c6f3df (IP-whitelist exemption paragraph) check out against the code; the mountless quick-starts now chain mountOnStandaloneServer before start() as reviewed.

[nbouliol]

All three fixed on 8e07e4f.

1. /health features → configured — sample, table intro and header, plus the agent README one-liner. Carried the meaning over rather than just the key: the block says which surfaces were set up, deliberately not that they work. The closing sentence ("what is served, not how it was configured") was backwards and now says the body names the surfaces, never the values behind them.

2. Sub-router — warning dropped. app.use("/api", mounted) is now documented as supported, with the emitted prefix derived per request from originalUrl (base-path.ts), so servers and the docs page carry /api/bff. Confirmed against the e2e assertions at embedded-bff.e2e.test.ts:492-495.

3. CORS — reworded to what the code does: the host preflight is lost and unrecoverable, but the request that follows still reaches the app and is refused with 403 origin_not_allowed. Also fixed the Layer 1 bullet two sections up, which still said "anything else gets no CORS headers (the browser blocks)" — same stale claim, and it was not in this PR's diff so nothing else was going to catch it. Both mention that a request with no Origin passes untouched, which is what keeps server-to-server api-key calls working.

On the same-origin question you raised on #1876: I have not documented anything about it here, since it is that PR's call. If it changes, the two paragraphs above are the ones to revisit.

agent: test/bff 69/69, eslint src test 0 errors.

[Tonours]

Verified at 8e07e4f: all three paragraphs now match the code. /health documents configured with the set-up-not-working semantics and the closing sentence no longer says the opposite; the sub-router warning became a supported-shape statement matching resolveEmittedBase (e2e-asserted servers: /api/bff); the cors paragraph states the 403 deny and keeps the no-Origin case that server-to-server calls rely on. The env-table degraded claim was resynced in 55bc6fa. Approving.

One non-blocking note for the next rebase: once aaaaf58's same-origin host exemption lands under this branch, Layer 1's "any other Origin is refused outright" wants the exemption named — same-origin callers are served without being listed.

[Tonours]

Verified at 8e07e4f: all three paragraphs now match the code — /health documents configured with the set-up-not-working semantics, the sub-router warning became a supported-shape statement matching resolveEmittedBase (e2e-asserted servers: /api/bff), and the cors paragraph states the 403 deny while keeping the no-Origin case server-to-server calls rely on. Env-table degraded claim resynced in 55bc6fa. Approving.

One non-blocking note for the next rebase: once aaaaf58's same-origin host exemption lands under this branch, Layer 1's "any other Origin is refused outright" wants the exemption named — same-origin callers are served without being listed.