Skip to content

fix(agent-bff): derive a primary key when the schema declares none - #1895

Merged
Tonours merged 10 commits into
fix/agent-bff-4xx-body-leakfrom
fix/agent-bff-derive-primary-key
Sep 14, 2026
Merged

fix(agent-bff): derive a primary key when the schema declares none#1895
Tonours merged 10 commits into
fix/agent-bff-4xx-body-leakfrom
fix/agent-bff-derive-primary-key

Conversation

@Tonours

@Tonours Tonours commented Sep 10, 2026

Copy link
Copy Markdown
Member

Stacked on #1894, which merges first. Review only the last commit; the parent's diff belongs to that PR.

What

A collection whose schema declares no primary key gets one derived from its id field.

Why

forest_liana publishes isPrimaryKey only from 9.17.6 (2026-06-04). Below that line every collection is keyless, so unpackPrimaryKey rejects every record the agent returns and a plain list answers 500 mapping_error. Of the 125 forest-rails environments active in the last 120 days, the twenty oldest run between 2.14.6 and 8.3.2, several of them production.

How

buildPrimaryKeys falls back to a field literally named id, typed as the schema declares it, String when no such field exists. Keyed on the shape of the schema rather than the liana version: a collection that declares a key keeps it, and every v2 agent declares one, so this only fires where the alternative is a 500.

How to test

yarn workspace @forestadmin/agent-bff test. Three unit tests cover the declared-key, derived-key and no-id-field cases.

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

@qltysh

qltysh Bot commented Sep 10, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (19)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
packages/agent-bff/src/read-model/forest-schema-client.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/read-model/read-model.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/openapi/unfolded-paths.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/context/build-context.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/read-model/schema-cache.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/read-model/agent-capabilities-fetcher.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/openapi/collect-unfolding.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/validation/operator-normalizer.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/data/pack-id.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/data/agent-query.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/validation/validation-errors.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/read-model/read-model-store.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-bff/src/validation/capabilities-validator.ts100.0%
New file Coverage rating: A
packages/agent-client/src/filter-wire-format.ts100.0%
New file Coverage rating: A
packages/agent-bff/src/read-model/synthesize-capabilities.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-client/src/query-serializer.ts100.0%
Coverage rating: A Coverage rating: A
packages/forestadmin-client/src/schema/index.ts100.0%
Coverage rating: A Coverage rating: A
packages/agent-client/src/index.ts100.0%
Coverage rating: A Coverage rating: A
packages/forestadmin-client/src/permissions/forest-http-api.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.

Comment thread packages/agent-bff/src/read-model/read-model.ts
Comment thread packages/agent-bff/src/read-model/read-model.ts Outdated
@Tonours
Tonours force-pushed the fix/agent-bff-4xx-body-leak branch from a3c826a to 77f255d Compare September 10, 2026 17:04
@Tonours
Tonours force-pushed the fix/agent-bff-derive-primary-key branch from 3be7bf2 to 990d725 Compare September 10, 2026 17:04
@Tonours
Tonours force-pushed the fix/agent-bff-derive-primary-key branch from 990d725 to 64e7bf7 Compare September 10, 2026 18:37
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6Q1hDksLxX4pCp6Rw2PrB

@nbouliol nbouliol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec (no ticket): no Linear ticket is linked to this PR, so the functional check was limited to the PR description, which the diff matches.

Comment thread packages/agent-bff/src/read-model/read-model.ts Outdated
Tonours and others added 2 commits September 11, 2026 11:46
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6Q1hDksLxX4pCp6Rw2PrB
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6Q1hDksLxX4pCp6Rw2PrB
Comment thread packages/agent-bff/src/read-model/read-model.ts Outdated
@qltysh

qltysh Bot commented Sep 11, 2026

Copy link
Copy Markdown

2 new issues

Tool Category Rule Count
qlty Structure Function with many returns (count = 4): filterSchema 1
qlty Structure Function with high complexity (count = 11): createAgentCapabilitiesFetcher 1

Comment thread packages/agent-bff/src/openapi/unfolded-paths.ts
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6Q1hDksLxX4pCp6Rw2PrB
Comment thread packages/agent-bff/src/read-model/agent-capabilities-fetcher.ts
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01F6Q1hDksLxX4pCp6Rw2PrB

@nbouliol nbouliol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spec (no ticket): no Linear ticket is linked to this PR, so the functional check was limited to the PR description, which the diff matches.

Comment thread packages/agent-bff/src/context/build-context.ts Outdated
Comment thread packages/agent-bff/src/data/pack-id.ts
Comment thread packages/agent-bff/src/openapi/unfolded-paths.ts Outdated
Comment thread packages/agent-bff/src/openapi/unfolded-paths.ts Outdated
Comment thread packages/agent-bff/src/read-model/read-model.ts
Comment thread packages/agent-bff/test/context/build-context.test.ts
Comment thread packages/agent-bff/test/openapi/openapi-unfolded.test.ts
Tonours and others added 2 commits September 11, 2026 16:18

@nbouliol nbouliol left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving on the review side: the seven findings from the validator run at 59ef8a1a are all closed and verified at 3797d1ee, each with the property that closes its class and a regression pinned where a test applies.

Two things this approval does not clear:

  • #1894 is still open, and this PR is based on its branch, so it cannot merge first.
  • LLM Integration Tests (ai-proxy) is red. Unrelated to this diff, but it is red.

@Tonours
Tonours merged commit d96316b into fix/agent-bff-4xx-body-leak Sep 14, 2026
32 of 33 checks passed
@Tonours
Tonours deleted the fix/agent-bff-derive-primary-key branch September 14, 2026 08:14
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.

2 participants