feat(agent-bff): expose relation list and count with foreign-collection validation

[nbouliol]

What

Exposes the relation read surface on the BFF: POST /v1/:collection/relations/:relation/list and /count. The relation route resolves authorization/allow-list on the parent, but projection, filter, sort, and count run against the foreign collection — the BFF owns the correct agent query shape so the agent's non-uniform relation precedent (parent-vs-foreign projection serialization) never leaks.

Builds on PRD-671's shared helpers (flat mapping, packed-id, count-deactivation).

How

• Route: extends data-routes-middleware with a second regex \/agent\/v1\/:collection\/relations\/:relation\/(list|count), reusing the existing token/read-model/error plumbing.
• Relation resolution: to-many only (HasMany/BelongsToMany). To-one, polymorphic (multi-target), and absent relations map to unknown_relation (404). Verified against both the Node agent (per-type route generation) and the Ruby agent (only PolymorphicManyToOne carries multiple targets, and it is to-one).
• Parent vs foreign: the parent name + parentId + relation name go only into the agent path; fields[FOREIGN], filters, sort, and count run on the foreign collection. A test proves the parent is used solely for route resolution.
• parentId: opaque parent record id (int/UUID/composite packed). Presence + primitive check only, forwarded unchanged (URL-encoded in the path). Missing/empty/malformed → invalid_request (400) with no agent call.
• Mapping/errors: reuses mapListResponse/mapCountResponse (foreign-side) and PRD-670 error mapping. The nested-relation guard is deliberately NOT wired (relation routes legitimately accept foreign fields).

No agent or agent-client package changes; all changes are agent-bff-local.

Tests

• Relation list flat mapping (data[] + __forest foreign + countStatus: not_requested)
• Foreign-projection proof (fields[posts], not fields[users])
• Relation count available/deactivated on the foreign collection
• 404 for unknown/to-one/polymorphic relation and unknown parent collection
• 400 for missing/empty/malformed parentId (agent not called)
• Composite parentId passthrough unchanged
• Agent failure → PRD-670 envelope
• parseParentId unit cases

yarn workspace @forestadmin/agent-bff lint test build all green (541 tests).

Fixes PRD-672

🤖 Generated with Claude Code

Note

Add relation list and count endpoints to agent-bff data routes middleware

• Adds POST /agent/v1/:collection/relations/:relation/list and .../count endpoints to data-routes-middleware.ts, routing through a new handleRelation orchestrator that validates relation existence, allow-list status, and foreign collection before dispatching.
• Adds listRelation() and countRelationRaw() to the AgentDataClient interface and factory in agent-data-client.ts; count requests bypass deserialization via skipDeserialization: true.
• Adds parseRelationListRequest and parseRelationCountRequest in agent-query.ts with a parseParentId validator that accepts strings and finite numbers, rejecting empty/whitespace/non-finite values with a 400 invalid_request.
• Only to-many relations (HasMany, BelongsToMany) are listable; to-one and polymorphic relations return 404. Nested relation field paths are rejected with 400.
• Risk: Relation routes share the existing collection allow-list check but add a second foreign-collection allow-list check; disallowed foreign collections return 404 unknown_collection rather than a data error.

^{Macroscope summarized 3ab6908.}

[linear-code]

PRD-672

[qltysh]

3 new issues

Tool	Category	Rule	Count
qlty	Structure	Function with many parameters (count = 4): listRelation	3

[qltysh]

Coverage Impact

⬆️ Merging this pull request will increase total coverage on main by 0.04%.

Modified Files with Diff Coverage (3)

Rating	File	% Diff	Uncovered Line #s
	packages/agent-bff/src/data/agent-data-client.ts	100.0%
	packages/agent-bff/src/data/agent-query.ts	100.0%
	packages/agent-bff/src/data/data-routes-middleware.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.

[ShohanRahman]

Review — important findings

Multi-perspective review (correctness, security, tests, types). No Critical bugs; the error pipeline and the parent/foreign authorization design are solid. The items below are worth resolving before merge.

🔴 Nested-relation field-path guard is not wired on relation routes → foreign-of-foreign authorization gap

src/data/data-routes-middleware.ts:121-149 (handleRelationList / handleRelationCount)

The top-level handlers call assertNoRelationFieldPaths(...) before building the query; the relation handlers do not, and the test "should not wire the nested-relation guard on relation routes" locks the gap in. The PR frames this as intentional ("relation routes legitimately accept foreign fields"), but the guard rejects only field paths containing : — it does not block plain foreign fields. On /users/relations/posts/list, projection: ['id','title'] passes; only filter: { field: 'author:name' } (a posts → author traversal) is blocked.

The underlying authority gap is real: the agent's list-related.ts asserts browse only on the immediate foreign collection (assertCanBrowse(context, this.foreignCollection.name)), while parseProjectionWithPks / the filter factory accept nested :-paths. So a caller with access to users.posts can project/filter/sort by fields of a third collection whose browse permission is never checked — the exact class of bug #1736 added this guard to close for top-level lists.

Fix: call assertNoRelationFieldPaths(collectListFieldPaths(body)) / collectCountFieldPaths(body) in the two relation handlers and invert the locking test. Legitimate foreign fields keep working. (If nested foreign traversal is genuinely wanted here, it needs an explicit browse check on the nested target rather than skipping the guard.)

🟡 RelationListRequestBody / RelationCountRequestBody are dead for their distinguishing field

src/data/agent-query.ts:24,26 + cast at data-routes-middleware.ts:179

Both types add parentId?: unknown, but parentId is never read through them — it's extracted via a separate cast (rawBody as { parentId?: unknown }).parentId before parsing, and the handlers that receive the typed body never touch .parentId. The types are therefore indistinguishable from their bases in every consumer, and the cast duplicates their shape informally. Either delete the two types, or fold parentId validation into the parse step (e.g. parseRelationListRequest returning { parentId: string } & ListRequestBody) so there's one validated path. (parseParentId itself is well-designed — the right pattern to align to.)

🟡 Test gaps

• BelongsToMany never exercised. resolveForeignCollection treats HasMany || BelongsToMany as listable, but every fixture uses only HasMany — narrowing it to HasMany alone would pass all tests. The relation() fixture helper already supports BelongsToMany; add a 200 list/count case.
• Relation guards untested on the count path. unknown/to-one/polymorphic-relation and disallowed-foreign-collection 404s are covered for list only. Shared code path (low risk), but a per-operation refactor could silently drop count-side protection — add one count-side 404 case.
• Foreign PK resolution not proven distinct from the parent's. All fixtures use id for both parent and foreign, so a bug swapping deps.collection/deps.foreignCollection in getPrimaryKeys(foreignCollection) wouldn't be caught. Add a foreign collection with a differently-named/composite PK and assert __forest.primaryKey uses the foreign one.

_{🤖 Generated with Claude Code}

[nbouliol]

Review addressed in 4e43287

Thanks — all three resolved.

🔴 Nested-relation guard on relation routes — fixed

You're right, and it corrects my own reasoning: the guard rejects only :-paths, not plain foreign fields, so wiring it closes the foreign-of-foreign browse gap (agent list-related asserts browse only on the immediate foreign collection) without breaking legitimate foreign projections. assertNoRelationFieldPaths is now called in both handleRelationList/handleRelationCount, and the locking test is inverted: projection:['id','title'] → 200, filter:{field:'author:name'} → 422 (list and count).

⚠️ Note: PRD-672 lists this guard as an explicit non-goal. I deviated deliberately because it is a real authz gap. @AnthonyGuimard flagging for awareness — shout if you want it reverted to strict PRD scope + a follow-up ticket instead.

🟡 Dead relation-body types — fixed

RelationListRequestBody/RelationCountRequestBody now carry a required parentId: string, populated by new parseRelationListRequest/parseRelationCountRequest (validate body + parseParentId in one pass). The informal cast is gone; handlers read body.parentId.

🟡 Test gaps — fixed

• BelongsToMany now exercised via a tags relation (200 list + count).
• Foreign PK proven distinct: tags PK is slug (not id), asserting __forest.primaryKey = { slug } — catches a parent/foreign getPrimaryKeys swap.
• Count-path 404s added (unknown / to-one / disallowed-foreign) + nested-path 422 on count.

589 tests green, lint + build clean.