feat(agent-bff): proxy the ai query on post /agent/v1/ai/query - #1845
feat(agent-bff): proxy the ai query on post /agent/v1/ai/query#1845Tonours wants to merge 15 commits into
Conversation
|
Coverage Impact This PR will not change total coverage. Modified Files with Diff Coverage (16) 🤖 Increase coverage with AI coding...🚦 See full report on Qlty Cloud » 🛟 Help
|
6 new issues
|
| const app = new Koa(); | ||
|
|
||
| app.use(createErrorMiddleware({ logger: () => undefined })); | ||
| app.use(bodyParser({ jsonLimit: '1mb', enableTypes: ['json'] })); |
There was a problem hiding this comment.
Importing AI_BODY_LIMIT here instead of the literal would make the fixture track the real limit.
| .post('/agent/v1/ai/query') | ||
| .set('Authorization', `Bearer ${sessionToken()}`) | ||
| .set('Content-Type', 'application/json') | ||
| .set('Content-Length', String(2 * 1024 * 1024)) |
There was a problem hiding this comment.
This declares a length it never sends, so raw-body short-circuits on the header and the byte-counting path is never exercised; with the pass case at 900 KB, neither test sits at the boundary — a real ~1.05 MB body would cover both sides.
| it('should target the fixed ai-query path with the ai-name of this deployment', async () => { | ||
| await makeClient().query(params()); | ||
|
|
||
| expect(firstCall()[0]).toBe(`${FOREST_SERVER_URL}/api/ai-proxy/ai-query?ai-name=zendesk`); |
There was a problem hiding this comment.
What about a POST /agent/v1/ai/query?ai-name=other case asserting the outgoing URL still carries zendesk? Both ctx.path comparisons would break silently on a refactor to ctx.url.
| export const AI_QUERY_ROUTE = '/agent/v1/ai/query'; | ||
|
|
||
| const JSON_CONTENT_TYPE = 'application/json'; | ||
| const MAX_CAUSE_DEPTH = 3; |
There was a problem hiding this comment.
No test walks a cause chain deeper than 3, so nothing proves the recursion actually stops.
| 'deployment runs without the OAuth configuration.', | ||
| }); | ||
|
|
||
| export const AiQueryRequestSchema = z |
There was a problem hiding this comment.
Nothing validates against this, yet the served contract now carries a BFF-authored shape nobody will keep in sync with routeArgsSchema — z.unknown(), as used for the 200, would match the passthrough decision.
|
|
||
| describe('BFF_AI_TIMEOUT_MS', () => { | ||
| it('should default to 2 minutes when unset, since an AI generation is slow', () => { | ||
| expect(parseConfig({ ...VALID_ENV }).aiTimeoutMs).toBe(120_000); |
There was a problem hiding this comment.
DEFAULT_AI_TIMEOUT_MS is already imported and used three cases below — using it here too keeps the default in one place.
| }); | ||
|
|
||
| it('should not mount the AI query route without the OAuth configuration, since it has no session', async () => { | ||
| const token = jsonwebtoken.sign( |
There was a problem hiding this comment.
A sessionToken() helper already exists in the sibling describe — reusing it drops three copies of this block.
Co-authored-by: macroscopeapp[bot] <170038800+macroscopeapp[bot]@users.noreply.github.com>
ad383e2 to
55fce99
Compare

fixes PRD-1033
What
POST /agent/v1/ai/queryrelays the body unchanged to/api/ai-proxy/ai-query, authenticated with the OAuth session the BFF already holds. Second half of PRD-944;GET /agent/v1/contextshipped in #1838.Why
After the OAuth switch the Zendesk app has no SaaS token left — it sends its
bff_accesstoapi.forestadmin.com, which rejects a JWT it never issued. The provider keys live on the SaaS, so this one route is a relay rather than a contract.How
Authorization(sessionsaasAccessTokenviaensureFreshServerAccess),Content-Type,forest-environment-id,forest-rendering-id. Nothing incoming is copied — the SaaS prefers a session cookie over the Bearer (make-fetch-token.ts:26-30), andx-mcp-oauth-tokensis injected asAuthorizationtoward an MCP server.:route.403 oauth_required: it produces nosid, so there is nosaasAccessTokento forward. A third-party app authenticating via OAuth will work with no code change.@koa/bodyparserskips whenctx.request.bodyis set. Data routes keep 16 KB,/oauth/tokenkeeps parsing. The global 16 KB breaks AI prompts past ~30 collections (measured).403/429; status is preserved, body replaced. Timeout →504, bounded byBFF_AI_TIMEOUT_MS(default 120s) — nothing else on the chain bounds an AI generation today.ai-nameis a constant, not an env var: it selects the SaaS billing line and model, so a caller must not choose it.forest-bff openapiexport therefore omits it: it cannot know the runtime configuration.Scope and safety
ALLOWED_HEADERSis deliberately unchanged. Theforest-*headers the app sends today are exactly what the BFF now derives server-side; letting them through the preflight would mean proving we ignore them. The app drops them instead.No body validation (the SaaS validates, and its schema is deliberately loose), no rate limiting, no session store change.
How to test
Two tests exist only to catch a mount-order inversion, and both were verified to fail when the lines are swapped: "900 KB passes" (a
2 MB → 413test does not catch it — both orders give 413), and therunClitest reaching the route withoutX-Forest-Timezone.Known limitation
Not reachable from the Zendesk app until its own switch lands:
fetchAIProxystill targets the SaaS and still sendsforest-*headers, which this preflight rejects. Separate PR, same ticket.Behind the BFF every user exits on one IP, so the SaaS per-IP anti-brute-force becomes a shared quota. Degrades cleanly (the
429is relayed), worth watching in production.Definition of Done
General
Security