Skip to content

fix(agent-bff): wrap action errors in the standard error envelope - #1799

Open
Tonours wants to merge 1 commit into
mainfrom
feature/prd-867-wrap-action-errors-in-the-standard-error-envelope
Open

fix(agent-bff): wrap action errors in the standard error envelope#1799
Tonours wants to merge 1 commit into
mainfrom
feature/prd-867-wrap-action-errors-in-the-standard-error-envelope

Conversation

@Tonours

@Tonours Tonours commented Aug 4, 2026

Copy link
Copy Markdown
Member

fixes PRD-867

Problem

Every BFF failure goes out in the shared envelope. Except one. A native action error
(resultBuilder.error(...)) came out flat, no wrapper:

{ "type": "error", "status": 400, "message": "SAR already filed", "html": "<strong>…</strong>" }

Everything else:

{ "error": { "type": "unknown_field", "status": 422, "message": "..." } }

PRD-641 says the Zendesk client branches on error.type. That branch never fires here.
response.error is undefined, so the client skips it and reads a business failure as a success. The
agent's message and its html are dropped. Nothing crashes, the result is just wrong.

Found during the PRD-674 manual QA, against a real agent.

Root cause

action-routes-middleware.ts assigned ctx.status / ctx.body by hand for
ActionFormValidationError. That bypasses toErrorBody, the function that owns the envelope shape.
That is how the two shapes drifted apart.

Fix

Throw a BffHttpError instead of writing the body. The error middleware runs it through
toErrorBody, so this response cannot drift again:

{ "error": { "type": "action_error", "status": 400, "message": "...", "details": { "html": "..." } } }

The agent's html rides in details, the envelope's only extension point.

toErrorBody already omits details when undefined, and ActionFormValidationError.html is already
optional. So "key absent, not null" needed no extra code. The old ?? null was what forced the
explicit null.

Scope

Unchanged: the four other action result shapes. success, webhook, redirect stay flat on
body.type. unsupported_action_result (501) was already enveloped. A new test pins a success as
flat, so nobody wraps the rest by accident.

Breaking change, taken on purpose. type: "error" was one of five action result shapes and two tests
asserted it. Uniformising costs that symmetry: a failure now reads body.error.type while the other
outcomes read body.type. What it buys is one error path in the client instead of two. Free today
because no consumer exists. After PRD-678 writes the Zendesk client it breaks a shipped integration.

How to test

yarn workspace @forestadmin/agent-bff test

On an action whose execute returns resultBuilder.error(msg, { html }):

Body
before { type: "error", status: 400, message, html }
after { error: { type: "action_error", status: 400, message, details: { html } } }

Manually, through the PRD-674 QA suite (qa-execute-e2e.sh section 8, updated to assert the envelope
and no top-level type): an already-filed SAR returns the enveloped 400, and the same action on an
unfiled SAR still returns 200. So the error branch is what gets exercised, not a permanently broken
action.

Validation

  • 706 tests / 54 suites pass, lint clean.
  • Reverted the fix, re-ran the three new tests: 2 fail. They discriminate instead of passing both ways.
  • macroscope codereview against origin/main: completed, zero findings.

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 4, 2026

Copy link
Copy Markdown

PRD-867

@qltysh

qltysh Bot commented Aug 4, 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/action/action-routes-middleware.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/http/bff-local-errors.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.

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.

1 participant