fix(agent-bff): report an agent timeout as 504 agent_timeout

[Tonours]

What

An upstream agent timeout answers 504 agent_timeout instead of 502 network_error. Every data and action operation of the OpenAPI document declares 504 through a shared Error504 component, distinct from the AI relay's Error504AiQuery.

502 network_error now means what it says: the transport failed outright — connection refused, host unresolvable.

fixes PRD-1108

Why / decisions

• New type agent_timeout rather than reusing the AI relay's upstream_timeout, which names the Forest server hop, not the agent.
• The timeout is typed at the transport boundary, not sniffed at the mapper. create-agent-http-requester.ts translates superagent's code === 'ECONNABORTED' plus numeric timeout into an exported AgentTimeoutError; mapAgentError branches on instanceof, like it already does for AgentHttpError. The BFF imports superagent nowhere else, and the mapper had no business knowing its private error shape.
• Both query and stream are wrapped unconditionally. The requester used to return the bare HttpRequester when no default timeout was configured, so a caller passing its own maxTimeAllowed got an untranslated superagent error and a 502.
• A transport-level regression test drives a real HttpRequester against a server that accepts and never answers within a short maxTimeAllowed, so the 504 path stays bound to superagent's real error shape. If that shape changes, the test fails.

502 vs 504, precisely

The deadline is armed when the request starts, and the default is 10 s (BFF_AGENT_TIMEOUT_MS). So:

• Agent accepts and does not answer in time → 504.
• Host blackholes SYNs, never accepts, never resets → 504 as well, at the default. The kernel's own connect timeout (~75 s) cannot win at a 10 s deadline. Raise BFF_AGENT_TIMEOUT_MS past it and that case reverts to 502: the kernel gives up first and reports ETIMEDOUT, which superagent passes through raw.
• Connection refused, DNS failure → 502, whenever it happens.

Both descriptions say this now, and the README carries it in the error-contract paragraph.

Scope and safety

• Unit tests pin every other mapping branch; none moved.
• The AI relay keeps answering 504 upstream_timeout through its own fetch + AbortSignal.timeout path, untouched.
• HttpRequester.stream still never settles on a timeout. Pre-existing, no BFF call site uses it, left as a known defect for agent-client.
• A client branching on 502 network_error for slow agents now sees 504 agent_timeout. The Zendesk client is the known consumer, tracked in the ticket's satellite scope — and a timeout is exactly the case that should be retryable there, so it is a real behaviour gap rather than a cosmetic one.

Documented

BFF_AGENT_TIMEOUT_MS had no row in the README env table despite BFF_AI_TIMEOUT_MS having a detailed one. Both the row and the agent_timeout / network_error split are in now.

How to test

• yarn workspace @forestadmin/agent-bff test
• 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]

PRD-1108

[qltysh]

1 new issue

Tool	Category	Rule	Count
qlty	Structure	Function with many returns (count = 7): mapAgentError	1

[qltysh]

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (2)

Rating	File	% Diff	Uncovered Line #s
	packages/agent-bff/src/http/agent-error-mapper.ts	100.0%
	packages/agent-bff/src/agent/create-agent-http-requester.ts	100.0%
	Total	100.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]

Spec (PRD-1108): conforms. A slow agent now answers 504 agent_timeout (real-transport regression test), a refused connection still answers 502 network_error with The agent could not be reached, and the OpenAPI document declares 504 on every data and action operation through the shared Error504 component, distinct from Error504AiQuery.

Applies to: the PR as a whole.

[Preferential] This PR embeds a decision that meets the ADR bar: it is hard to reverse (the agent_timeout type is a public contract that cross-repo consumers branch on, forest-for-zendesk must learn it), surprising without context (timeout detection keys on superagent's raw error shape, code === 'ECONNABORTED' plus a numeric timeout, inside the BFF mapper), and a real trade-off (coupling to superagent's shape, guarded by the transport regression test, versus throwing a typed error from the separately-published agent-client package). No ADR in the org covers it (docs/adr search on timeout, error, openapi, bff, http, status: no relevant match). Consider recording it with /adr so the rationale survives outside this PR's plan.

[nbouliol]

Two follow-ups outside the diff: the README env table documents BFF_AI_TIMEOUT_MS and its 504 in detail but has no row for BFF_AGENT_TIMEOUT_MS, and the error-contract section doesn't mention agent_timeout. And per the ticket's Zendesk scope, forest-client.ts will bucket agent_timeout as unknown/non-retryable until a matching change ships.

[Tonours]

Both fixed on this push.

BFF_AGENT_TIMEOUT_MS now has its row in the env table, with the default, the 504 agent_timeout it produces, and the 502 it does not.

The error-contract paragraph in Request edge now names the split:

An agent that accepts the connection and then does not answer within BFF_AGENT_TIMEOUT_MS answers 504 agent_timeout; an agent that refuses the connection or whose host does not resolve answers 502 network_error. The deadline is armed when the request starts, so a host that accepts nothing and never resets is a 504 too — 502 means the transport failed outright, not that it ran out of time.

On the Zendesk side: agreed, forest-client.ts buckets agent_timeout as unknown and non-retryable until a matching change ships. That is the ticket's satellite scope and stays out of this PR — worth saying that a timeout is exactly the case that should be retryable there, so it is a real behaviour gap rather than a cosmetic one.

[Tonours]

Third push. The review pass caught that my own correction was itself overstated.

The blackhole → 504 claim needed a bound. It holds only while BFF_AGENT_TIMEOUT_MS is below the OS connect timeout (~75 s macOS, ~127 s Linux). Set it to 300000 — allowed, the cap is 2147483647 — and against a blackholed host the kernel gives up first: Node emits connect ETIMEDOUT, superagent passes it through raw with no timeout property, isSuperagentTimeout is false, and the mapper answers 502. This PR's own test asserts exactly that (agent-error-mapper.test.ts, ETIMEDOUT → 502), so the prose contradicted a test sitting next to it.

Both the OpenAPI 504 description and the README now say "at the default", and name where it stops holding. A test asserts that clause, so the qualification cannot be dropped silently.

Dropped the withTypedTimeout around stream. It could never fire: superagent's pipe() calls end() with no callback, so on deadline _timeoutError invokes a noop and aborts, the destination stream emits neither finish nor error, and the promise never settles — which is the "never settles on a timeout" defect the PR already documents. Wrapping a promise that never rejects buys nothing. maxTimeAllowed is still defaulted on stream; only the translation is gone.

Fixed a garbled README clause ("…answers 502 network_error instead, whenever it fails").

Two commit titles on this branch are 73–74 chars. The repo squashes on the PR title, so neither reaches main.

[nbouliol]

The typed AgentTimeoutError at the transport seam is the right fix — the mapper no longer knows superagent exists, and the 502/504 split is now documented in the OpenAPI descriptions and the README. Both regression cases are pinned end-to-end against real sockets.

One thing worth a follow-up: the narrowed 502 description now names only refused-connection and DNS, but that branch is still the catch-all for ECONNRESET, socket hang up and TLS failures.