fix: load related record by EnkiP · Pull Request #1610 · ForestAdmin/agent-nodejs

EnkiP · 2026-05-29T07:29:35Z

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

Note

Fix load-related-record step executor to support x-to-one relations and richer pending data

Adds getSingleRelatedData to AgentPort and AgentClientAgentPort for BelongsTo/HasOne relations, fetching a single related record via parent projections instead of a list query.
Replaces the flat pendingData shape (displayName, selectedRecordId) with a structured shape: availableFields, suggestedField, availableRecordIds (candidate objects), and suggestedRecord.
Adds support for preview-only PATCH requests that change the selected relation field without confirming, triggering candidate refresh and returning awaiting-input.
Propagates relatedSchema to all getRelatedData calls so field name restoration and ID extraction are schema-driven rather than inferred from the parent relation.
Risk: LoadRelatedRecordPendingData shape is a breaking change for any client or persisted state relying on the old selectedRecordId/suggestedFields fields.

Changes since #1610 opened

Removed inline documentation comments from workflow-executor type definitions [386cce1]

^{Macroscope summarized 6224c76.}

qltysh · 2026-05-29T07:30:40Z

All good ✅

qltysh · 2026-05-29T07:36:38Z

Coverage Impact

Unable to calculate total coverage change because base branch coverage was not found.

Modified Files with Diff Coverage (4)

File	% Diff	Uncovered Line #s
...ow-executor/src/executors/load-related-record-step-executor.ts	98.1%	285
packages/workflow-executor/src/executors/record-step-executor.ts	100.0%
packages/workflow-executor/src/http/pending-data-validators.ts	100.0%
...ages/workflow-executor/src/adapters/agent-client-agent-port.ts	100.0%
Total	98.6%

🤖 Increase coverage with AI coding...

In the `fix/load-related-record` branch, add test coverage for this new code:

- `packages/workflow-executor/src/executors/load-related-record-step-executor.ts` -- Line 285

🚦 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.

hercemer42 · 2026-05-29T13:48:22Z

Claude has added a lot of comments, but your code is readable as is, so in my opinion they aren't necessary. I'd recommend to keep them to a minimum.

hercemer42 · 2026-05-29T13:36:06Z

+
+    return {
+      recordId: packedId.split('|'),
+      referenceFieldValue: referenceField


[Claude Opus 4.8] — Should fix

extractReferenceFieldValue(relation, referenceField) reads the reference field off the nested relation object, but that object is the raw deserialized relationship — its attribute keys are camelCased (full_name → fullName) and are never passed through restoreFieldNames. So a snake_case referenceField always yields undefined → null, and the human-readable label silently never renders on the xToOne path.

The comment at lines 212-215 ("fetchRelatedData has already restored field names … so reading r.values[field] works") holds for the to-many path but not for this one. Camel-case referenceField before reading, or run the nested object through restoreFieldNames. Same root cause as the relation-key Must-fix above. The test at line ~1594 hand-builds the getRecord result already in restored form, so it can't catch this.

hercemer42 · 2026-05-29T13:36:06Z

+      : pendingData.suggestedField;

-    // Re-derive relatedCollectionName and displayName because the user may have swapped the relation.
+    if (!relationRef) {


[Claude Opus 4.8] — Should fix (coverage)

This reachable error path is untested. The validator refine in pending-data-validators.ts accepts { userConfirmed: true, fieldDisplayName: <stale/renamed>, selectedRecordId } without checking the name is a real entry in availableFields, so a stale relation from the frontend lands here and throws StepStateError. The preview twin is tested ("returns error when the previewed relation does not exist") but this confirm-with-override branch is not — it's the one genuinely reachable uncovered line in the file. Per CLAUDE.md (changed files ≥90% covered; cover error edge cases), add a test asserting status === 'error' and that saveStepExecution is not called.

hercemer42 · 2026-05-29T13:36:06Z

+// (configured but value missing) collapse to the same "fall back to id" rendering.
+export interface LoadRelatedRecordCandidate {
+  recordId: Array<string | number>;
+  referenceFieldValue?: string | null;


[Claude Opus 4.8] — Preferential

referenceFieldValue?: string | null is a three-state type, but every producer (toCandidate, fetchXToOneCandidate) and extractReferenceFieldValue only ever emit string | null — undefined is never produced, and the doc comment says null/undefined collapse to the same rendering anyway. Tightening to a required referenceFieldValue: string | null removes a representable-but-never-produced state with zero behavior change.

Scra3 · 2026-05-29T14:41:01Z

-    name: z.string().min(1).optional(),
+    // Sent as the displayName; the executor re-derives the technical fieldName and
+    // relatedCollectionName from the live schema when processing the confirmation.
+    fieldDisplayName: z.string().min(1).optional(),


Why don't send directly the technical name?

The frontend needs to display the displayName, we don't really need the tech name for anything

Prefer to use fieldName, it's the unique key. DisplayName can be not unique. Also it can change during the process. This field is used to display nicely the field, it's a cosmetic field

Scra3 · 2026-05-29T14:42:34Z

-  getRelatedData(query: GetRelatedDataQuery, user: StepUser): Promise<RecordData[]>;
+  // Returns raw rows from the agent (camelCase keys, no PK extraction). The caller is
+  // responsible for resolving the related collection's schema and mapping rows → RecordData.
+  getRelatedData(query: GetRelatedDataQuery, user: StepUser): Promise<Record<string, unknown>[]>;


Why it's not RecordData[] to return ?

Because I wanted to avoid having to pass the schema to the agent-port, but it's ok, I change

Scra3 · 2026-05-29T14:51:55Z

-      throw new RelatedRecordNotFoundError(selectedRecordRef.collectionName, name);
-    }
+    return rows.map(row => {
+      const restored = restoreFieldNames(


Don't use something from adapter inside business domain. If want to format data, you must to do it inside adapter/port. It's not the job of the step business :)

Scra3 · 2026-05-29T14:53:33Z

+    // lookup, so the extra fetch only pays once per related collection per run.
+    const relatedSchema = await this.getCollectionSchema(target.relatedCollectionName);
+    const referenceField = relatedSchema.referenceField ?? null;
+    const fields = [`${target.name}@@@${target.relatedPrimaryKey}`];


Same than my previous comment, this syntaxe is related to HTTP transport. It must be move to the port.

I don't see how to move this logic to the agentPort. Here we are fetching some fields in the relation from the parent record, to get the id and the referenceField. At this step we don't have the id so we can't fetch the child record directly.
This logic is heavily related to the loadRelatedRecord step so I think it's ok to leave it there

Yes that why adapter and port exists ! We don't want to be linked to the agent format. Adapter and port encapsulate how we communicate with the outside. This logic must be in the adapter :/

Scra3 · 2026-05-29T14:56:38Z

    { collection, id, relation, limit, fields }: GetRelatedDataQuery,
    user: StepUser,
-  ): Promise<RecordData[]> {
+  ): Promise<Record<string, unknown>[]> {


It returns RecordData[], why it's moved to an other type ?

It wasn't, the restoreFieldNames was working only for the already fetched collection schema, which wasn't the case for relationships

I fixed it, & restored the RecordData[] type

Scra3 · 2026-05-29T15:02:37Z

+      // Branch A-preview -- user switched relation without confirming: re-list candidates
+      // for the new relation, refresh pendingData, stay awaiting-input. Detected by a
+      // patch carrying `fieldDisplayName` but no `userConfirmed`.
+      const conf = pending.userConfirmation;


This preview-vs-confirm discrimination duplicates the refine in http/pending-data-validators.ts (loadRelatedRecordPatchSchema) — the two encode the same rule and can
drift if it changes; consider a single source of truth (tagged union at parse time, or a shared predicate).

I don't understand, nothing seems repeated, we simply do different things according to the user input

Scra3 · 2026-06-01T07:53:12Z

+  relatedCollectionName: string;
+  // Primary key field name on the related collection — supplied by the orchestrator's
+  // schema. Required for the xToOne projection syntax ('<relation>@@@<pk>').
+  relatedPrimaryKey?: string;


Is it possible to be computedKey?

Scra3 · 2026-06-01T11:01:45Z

+  relatedCollectionName: string;
+  // Primary key field name on the related collection — supplied by the orchestrator's
+  // schema. Required for the xToOne projection syntax ('<relation>@@@<pk>').
+  computedKey?: string;


change the name: Type is recordId I think

macroscopeapp · 2026-06-01T11:31:45Z

+      // Preview patch (no confirm): fieldName alone is sufficient.
+      if (data.userConfirmed === undefined) return data.fieldName !== undefined;
+      // Confirm patch with relation override: selectedRecordId required.
+      if (data.fieldName !== undefined) return data.selectedRecordId !== undefined;


🟡 Medium http/pending-data-validators.ts:56

The .refine() validation on line 56 requires selectedRecordId whenever fieldName is provided, even when userConfirmed is false. This means sending { userConfirmed: false, fieldName: "someField" } incorrectly fails validation with a message about "confirming with a relation override" — but declining a step is not confirming. The check should include data.userConfirmed === true to match the documented intent.

Suggested change

if (data.fieldName !== undefined) return data.selectedRecordId !== undefined;

if (data.userConfirmed === true && data.fieldName !== undefined) return data.selectedRecordId !== undefined;

🚀 Reply "fix it for me" or copy this AI Prompt for your agent:

In file packages/workflow-executor/src/http/pending-data-validators.ts around line 56: The `.refine()` validation on line 56 requires `selectedRecordId` whenever `fieldName` is provided, even when `userConfirmed` is `false`. This means sending `{ userConfirmed: false, fieldName: "someField" }` incorrectly fails validation with a message about "confirming with a relation override" — but declining a step is not confirming. The check should include `data.userConfirmed === true` to match the documented intent. Evidence trail: packages/workflow-executor/src/http/pending-data-validators.ts lines 34-65 at REVIEWED_COMMIT. Line 54 only catches `userConfirmed === undefined`, line 56 applies to any defined `userConfirmed` (including `false`). The inline comment on line 55 says 'Confirm patch' but code doesn't check `userConfirmed === true`.

Scra3 · 2026-06-01T11:32:20Z

 // Field names in the schema and in GetRecordQuery.fields use the original format (e.g. snake_case).
 // This function restores the original field names so callers can look up values by schema fieldName.
-function restoreFieldNames(
+export function restoreFieldNames(


Export to remove

Scra3 · 2026-06-01T11:34:09Z

+  availableFields: RelationRef[];
+  // AI-selected relation from `availableFields`.
+  suggestedField: RelationRef;
+  // First 50 records of `suggestedField` paired with their reference-field value.


If we change to 40 or whatever, the comment will be outdated. Remove it plz :)

Scra3 · 2026-06-01T11:35:45Z

-  suggestedFields?: string[];
-  // AI-selected initially; frontend can override via userConfirmation.selectedRecordId.
-  selectedRecordId: Array<string | number>;
+// One row of `availableRecordIds`. `referenceFieldValue` is the layout-level reference


We can remove all the comment.

Scra3 · 2026-06-01T11:40:14Z

    return relatedData.map(r => this.toRecordRef(r));
  }

+  private async fetchRelatedData(


I think we can remove this method. It does nothing

it is used at 2 places, it's not big but it simplifies a little the call to the agentPort

Scra3

Found a blocking bug in the xToOne relation projection (getSingleRelatedData).

Scra3 · 2026-06-01T14:39:09Z

+  ): Promise<RecordData | null> {
+    return this.callAgent('getSingleRelatedData', async () => {
+      const projectedFields = Array.from(
+        new Set([...relatedSchema.primaryKeyFields, ...(fields ?? [])]),


issue (blocking): projecting the PK and the reference field on one relation produces fields[<relation>]=id,reference, which the agent's parseProjection can't split into two sub-fields — it 400s with Invalid projection: The '<relation>.id,name' field was not found; project a single sub-field instead (the linkage id is returned by the JSON:API serializer regardless of projection).

fix(workflow): related record step executor

36bf3c2

EnkiP changed the base branch from main to feat/prd-214-server-step-mapper May 29, 2026 07:29

macroscopeapp Bot reviewed May 29, 2026

View reviewed changes

Comment thread packages/workflow-executor/src/executors/load-related-record-step-executor.ts Outdated

macroscopeapp Bot reviewed May 29, 2026

View reviewed changes

Comment thread packages/workflow-executor/src/adapters/agent-client-agent-port.ts

Comment thread packages/workflow-executor/src/adapters/agent-client-agent-port.ts

Enki Pontvianne added 2 commits May 29, 2026 10:17

fix: add coverage

86e3f78

fix: add coverage

255da34

macroscopeapp Bot mentioned this pull request May 29, 2026

(via EnkiP): fix(workflow-executor): restore field names in fetchXToOneCandidate befo #1611

Open

Enki Pontvianne added 2 commits May 29, 2026 11:22

fix: lint

ca08d64

fix: lint

48995ff

hercemer42 requested changes May 29, 2026

View reviewed changes

Scra3 reviewed May 29, 2026

View reviewed changes

Scra3 requested changes Jun 1, 2026

View reviewed changes

Scra3 reviewed Jun 1, 2026

View reviewed changes

fix: review

a47f15f

EnkiP force-pushed the fix/load-related-record branch from 23d4ee8 to a47f15f Compare June 1, 2026 10:16

Scra3 reviewed Jun 1, 2026

View reviewed changes

fix: add getSingleRelatedData to agent port

6224c76

EnkiP force-pushed the fix/load-related-record branch from 9d4e394 to 6224c76 Compare June 1, 2026 11:27

macroscopeapp Bot reviewed Jun 1, 2026

View reviewed changes

Scra3 reviewed Jun 1, 2026

View reviewed changes

fix: review

386cce1

Scra3 reviewed Jun 1, 2026

View reviewed changes

	if (data.fieldName !== undefined) return data.selectedRecordId !== undefined;
	if (data.userConfirmed === true && data.fieldName !== undefined) return data.selectedRecordId !== undefined;

Conversation

EnkiP commented May 29, 2026 • edited by macroscopeapp Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Definition of Done

General

Security

Fix load-related-record step executor to support x-to-one relations and richer pending data

Changes since #1610 opened

Uh oh!

qltysh Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

qltysh Bot commented May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hercemer42 commented May 29, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scra3 Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scra3 May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scra3 May 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macroscopeapp Bot Jun 1, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scra3 Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Scra3 Jun 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

EnkiP commented May 29, 2026 •

edited by macroscopeapp Bot

Loading

qltysh Bot commented May 29, 2026 •

edited

Loading

qltysh Bot commented May 29, 2026 •

edited

Loading

Scra3 Jun 1, 2026 •

edited

Loading

Scra3 May 29, 2026 •

edited

Loading

Scra3 May 29, 2026 •

edited

Loading

Scra3 Jun 1, 2026 •

edited

Loading

Scra3 Jun 1, 2026 •

edited

Loading