Skip to content

fix(workflow-executor): follow a reference smart field whose value is the related id - #1884

Open
Scra3 wants to merge 2 commits into
mainfrom
fix/reference-only-relation-scalar-linkage
Open

fix(workflow-executor): follow a reference smart field whose value is the related id#1884
Scra3 wants to merge 2 commits into
mainfrom
fix/reference-only-relation-scalar-linkage

Conversation

@Scra3

@Scra3 Scra3 commented Sep 7, 2026

Copy link
Copy Markdown
Member

Context

A client's load-related-record step could not follow the card relation of their CardClaim__Claim collection. Their agent is forest-rails, where card is a reference smart field: it carries a reference and no relationship.

This PR fixes the executor half. The server half is forestadmin-server#8482, and this one ships first — see Rollout.

The problem

getSingleRelatedData follows an xToOne relation by reading the parent with a <relation>@@@<field> projection and unpacking the JSON:API linkage. But a forest-rails smart field declared with field ... reference: registers as a plain attribute (lib/forest_liana/collection.rbattribute(name, &compute_value)), so the projected value is the related id itself, a bare string. linkage?.id was undefined, the method returned null, and the step concluded "no related record" instead of loading it.

That silence is why this ships before the server change: once the server serves these fields as BelongsTo, an executor without this fix turns a loud configuration error into a quiet false negative.

Why a runtime shape check rather than a schema branch

The same apimap shape can serialize either way, so the schema cannot tell them apart:

Declaration apimap Serialized as
forest-rails field ... reference: type: 'String', no relationship plain attribute → new branch
forest-rails belongs_to ... reference: identical has_one(include_data: true) → linkage
forest-express, any reference field same JSON:API relationship → linkage

Branching on typeof raw !== 'object' sorts them automatically, which is why the fix is small and why forest-express needs no special case.

The change

One branch in the adapter, behind the same port method, so the step executor stays unaware of which shape the agent emitted:

  • a scalar value is read as the target record id via getRecord on the related collection;
  • null, undefined and '' keep the linkage path and still return null. The empty string matters: object.card&.id.to_s is the idiomatic Ruby getter and answers "" for an unset association, which would have serialized to an id-less by-id URL that agents route to the index action;
  • the pipe split only applies when the target key is composite. That packing is the agent's own convention, and a smart field's value is written by client code, so an id legitimately containing a|b must survive intact;
  • an orphan id lets RecordNotFoundError surface. Deliberate: the value is present, so the relation is set and the target simply cannot be read — that is a data inconsistency, not an absent relation.

Tests

7 in getSingleRelatedData: scalar read, caller fields forwarding, composite split, single-key pipe preserved, empty string, null, plus the untouched linkage cases. One step-level test that a pinned reference-only BelongsTo is followed, asserting the target collection and not just the relation name.

Full package suite: 1703 passed, 0 failed. Diff coverage 100%.

Rollout

Ship and release this before forestadmin-server#8482. In the reverse order, an executor without this fix sees a BelongsTo whose value is a scalar, reads no linkage, and silently skips: for a fully-automated step the run goes green with no record.

Node clients that embed the executor stay in that window until they upgrade — worth a line in the agent release note.

Definition of Done

General

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes — pending the paired server PR; verifiable end-to-end only once both are deployed
  • Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • Consider the security impact of the changes made — the extra read goes through the same authenticated agent client and the same getRecord path as every other record read, with the caller's own scopes; no new surface

🤖 Generated with Claude Code

… the related id

forest-rails and forest-express serialize a reference smart field as a plain
JSON:API attribute, so the value is the related record id and there is no
linkage object to unpack. getSingleRelatedData returned null on those, which
reads as "no related record" instead of loading it.

The scalar branch reads the target by that id through the same port method, so
the step executor stays unaware of which shape the agent emitted. An orphan id
still raises RecordNotFoundError: that is a data inconsistency, not an absent
relation.

Ships before the server change that serves these fields as BelongsTo, so no
executor ever sees a relation it cannot follow.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@qltysh

qltysh Bot commented Sep 7, 2026

Copy link
Copy Markdown

Qlty


Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (1)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: A
...ages/workflow-executor/src/adapters/agent-client-agent-port.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.

… single-key values

Review of the paired server change surfaced two holes in the scalar branch.

An empty string passed the guard. `object.card&.id.to_s` is the idiomatic Ruby
getter and answers "" for an unset association, which serialized to an id-less
by-id URL that agents route to the index action instead — a list where a record
was expected. It now takes the same path as a missing linkage and returns null.

The pipe split was unconditional. That packing is the agent's own convention for
a composite key, but a smart field's value is written by client code, so an id
that legitimately contains "a|b" was torn in two against a single-key target.
It now splits only when the target key is composite, mirroring getRelatedData.

Also corrects the comment: forest-express serializes every reference field as a
JSON:API relationship, and so does forest-rails' `belongs_to` DSL from the very
same apimap shape. Only `field ... reference:` yields an attribute, which is why
the branch reads the runtime shape rather than the schema.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@qltysh

qltysh Bot commented Sep 7, 2026

Copy link
Copy Markdown

1 new issue

Tool Category Rule Count
qlty Structure Function with high complexity (count = 12): getSingleRelatedData 1

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