From 8073aec68860e64d0275bd44e7d67bf827e848d7 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 15:44:07 +0200 Subject: [PATCH 1/2] fix(workflow-executor): follow a reference smart field whose value is 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 --- .../src/adapters/agent-client-agent-port.ts | 20 ++++- .../adapters/agent-client-agent-port.test.ts | 87 ++++++++++++++++++- .../load-related-record-step-executor.test.ts | 47 ++++++++++ 3 files changed, 149 insertions(+), 5 deletions(-) diff --git a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts index f492d3615c..de7829dfc4 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -244,10 +244,22 @@ export default class AgentClientAgentPort implements AgentPort { user, ); - const linkage = parent.values[toCamelCase(relation)] as - | Record - | null - | undefined; + const raw = parent.values[toCamelCase(relation)]; + + // Rails/Express reference smart fields are serialized as plain attributes, so the value IS + // the related id, with no linkage object to unpack. + if (raw != null && typeof raw !== 'object') { + return this.getRecord( + { + collection: relatedSchema.collectionName, + id: String(raw).split('|'), + ...(fields?.length ? { fields } : {}), + }, + user, + ); + } + + const linkage = raw as Record | null | undefined; const packedId = linkage?.id as string | undefined; if (!linkage || !packedId) return null; diff --git a/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts b/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts index 81d62f4aaa..74eabeb655 100644 --- a/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts +++ b/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts @@ -112,6 +112,7 @@ describe('AgentClientAgentPort', () => { let mockCollection: ReturnType['mockCollection']; let mockRelation: ReturnType['mockRelation']; let mockAction: ReturnType['mockAction']; + let mockClient: ReturnType['client']; let user: StepUser; let port: AgentClientAgentPort; @@ -119,7 +120,7 @@ describe('AgentClientAgentPort', () => { jest.clearAllMocks(); const mocks = createMockClient(); - ({ mockCollection, mockRelation, mockAction } = mocks); + ({ mockCollection, mockRelation, mockAction, client: mockClient } = mocks); mockedCreateRemoteAgentClient.mockReturnValue(mocks.client as any); const schemaCache = new SchemaCache(); @@ -846,6 +847,90 @@ describe('AgentClientAgentPort', () => { expect(result?.recordId).toEqual(['acme', '7']); }); + // Rails/Express reference smart fields are serialized as plain attributes: the projected value + // is the related id itself, with no linkage object to unpack. + it('reads the target by id when the projected value is a scalar', async () => { + mockCollection.getOne + .mockResolvedValueOnce({ card: 'uuid-1' }) + .mockResolvedValueOnce({ id: 'uuid-1', reference: 'CARD-1' }); + + const result = await port.getSingleRelatedData( + { + collection: 'claims', + id: [42], + relation: 'card', + relatedSchema: { ...ordersSchema, collectionName: 'cards' }, + }, + user, + ); + + expect(mockCollection.getOne).toHaveBeenNthCalledWith(1, [42], { fields: ['card@@@id'] }); + expect(mockCollection.getOne).toHaveBeenNthCalledWith(2, ['uuid-1'], {}); + expect(mockClient.collection).toHaveBeenCalledWith('cards'); + expect(result).toEqual({ + collectionName: 'cards', + recordId: ['uuid-1'], + values: { id: 'uuid-1', reference: 'CARD-1' }, + }); + }); + + it('passes the caller fields to the scalar target read', async () => { + mockCollection.getOne + .mockResolvedValueOnce({ card: 'uuid-1' }) + .mockResolvedValueOnce({ reference: 'CARD-1' }); + + await port.getSingleRelatedData( + { + collection: 'claims', + id: [42], + relation: 'card', + relatedSchema: { ...ordersSchema, collectionName: 'cards' }, + fields: ['reference'], + }, + user, + ); + + expect(mockCollection.getOne).toHaveBeenNthCalledWith(2, ['uuid-1'], { + fields: ['reference'], + }); + }); + + it('splits a composite packed id carried by a scalar attribute', async () => { + mockCollection.getOne + .mockResolvedValueOnce({ card: 'acme|7' }) + .mockResolvedValueOnce({ id: 'acme|7' }); + + const result = await port.getSingleRelatedData( + { + collection: 'claims', + id: [42], + relation: 'card', + relatedSchema: { ...ordersSchema, collectionName: 'cards' }, + }, + user, + ); + + expect(mockCollection.getOne).toHaveBeenNthCalledWith(2, ['acme', '7'], {}); + expect(result?.recordId).toEqual(['acme', '7']); + }); + + it('returns null when the scalar attribute is empty, without reading the target', async () => { + mockCollection.getOne.mockResolvedValue({ card: null }); + + const result = await port.getSingleRelatedData( + { + collection: 'claims', + id: [42], + relation: 'card', + relatedSchema: { ...ordersSchema, collectionName: 'cards' }, + }, + user, + ); + + expect(result).toBeNull(); + expect(mockCollection.getOne).toHaveBeenCalledTimes(1); + }); + it('returns null when the parent has no linkage to the xToOne relation', async () => { mockCollection.getOne.mockResolvedValue({ order: null }); diff --git a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts index c5f9c99f57..6923dd1e8a 100644 --- a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts @@ -4344,6 +4344,53 @@ describe('LoadRelatedRecordStepExecutor', () => { ); }); + // Regression: a Rails/Express reference smart field reaches the executor as a plain BelongsTo + // now that the server derives the relation from `reference` alone. Pinning it must follow it, + // where it used to fail as an invalid pre-recorded arg because the field was not a relation. + it('follows a pinned reference-only BelongsTo', async () => { + const { model, bindTools } = makeMockModel(); + const runStore = makeMockRunStore(); + const agentPort = makeMockAgentPort([ + makeRelatedRecordData({ collectionName: 'cards', recordId: ['card-1'], values: {} }), + ]); + const context = makeContext({ + model, + runStore, + agentPort, + workflowPort: makeMockWorkflowPort({ + customers: makeCollectionSchema({ + fields: [ + { fieldName: 'email', displayName: 'Email', isRelationship: false }, + { + fieldName: 'card', + displayName: 'Card', + isRelationship: true, + relationType: 'BelongsTo', + relatedCollectionName: 'cards', + }, + ], + }), + }), + stepDefinition: makeStep({ + executionType: StepExecutionMode.FullyAutomated, + preRecordedArgs: { relationName: 'card' }, + }), + }); + + const result = await new LoadRelatedRecordStepExecutor(context).execute(); + + expect(result.stepOutcome.status).toBe('success'); + expect(bindTools).not.toHaveBeenCalled(); + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.objectContaining({ relation: 'card' }), + expect.anything(), + ); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ executionParams: { displayName: 'Card', name: 'card' } }), + ); + }); + it('pins the source record via selectedRecordStepId (among several records)', async () => { // Base customers #42 (step 0) + a loaded order #99 (step 1) are both available; // pinning step 1 must make the relation follow the ORDER, not the base customer. From 9a5110d8a24c13f7d7e34d7e775d017a855d8827 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 7 Sep 2026 16:33:34 +0200 Subject: [PATCH 2/2] fix(workflow-executor): guard the scalar related-id against empty and single-key values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../src/adapters/agent-client-agent-port.ts | 15 ++++-- .../adapters/agent-client-agent-port.test.ts | 51 +++++++++++++++++-- .../load-related-record-step-executor.test.ts | 12 +++-- 3 files changed, 65 insertions(+), 13 deletions(-) diff --git a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts index de7829dfc4..8ea5f57d60 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -246,13 +246,20 @@ export default class AgentClientAgentPort implements AgentPort { const raw = parent.values[toCamelCase(relation)]; - // Rails/Express reference smart fields are serialized as plain attributes, so the value IS - // the related id, with no linkage object to unpack. - if (raw != null && typeof raw !== 'object') { + // A forest-rails smart field declared with `field ... reference:` serializes as a plain + // attribute, so the value IS the related id. The same apimap shape coming from the + // `belongs_to` DSL, and every forest-express reference field, still arrives as a linkage — + // hence a check on the runtime shape rather than on the schema. + // An empty string is the idiomatic Ruby answer for an unset association (`&.id.to_s`), and it + // would serialize to an id-less by-id URL, which agents route to the index action instead. + if (raw != null && raw !== '' && typeof raw !== 'object') { return this.getRecord( { collection: relatedSchema.collectionName, - id: String(raw).split('|'), + // Only a composite key is pipe-packed. The value of a smart field is written by the + // client, so splitting a single-key id would tear a legitimate "a|b" in two. + id: + relatedSchema.primaryKeyFields.length > 1 ? String(raw).split('|') : [raw as string], ...(fields?.length ? { fields } : {}), }, user, diff --git a/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts b/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts index 74eabeb655..366b26d412 100644 --- a/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts +++ b/packages/workflow-executor/test/adapters/agent-client-agent-port.test.ts @@ -847,8 +847,6 @@ describe('AgentClientAgentPort', () => { expect(result?.recordId).toEqual(['acme', '7']); }); - // Rails/Express reference smart fields are serialized as plain attributes: the projected value - // is the related id itself, with no linkage object to unpack. it('reads the target by id when the projected value is a scalar', async () => { mockCollection.getOne .mockResolvedValueOnce({ card: 'uuid-1' }) @@ -895,7 +893,7 @@ describe('AgentClientAgentPort', () => { }); }); - it('splits a composite packed id carried by a scalar attribute', async () => { + it('splits a scalar attribute only when the target key is composite', async () => { mockCollection.getOne .mockResolvedValueOnce({ card: 'acme|7' }) .mockResolvedValueOnce({ id: 'acme|7' }); @@ -905,7 +903,11 @@ describe('AgentClientAgentPort', () => { collection: 'claims', id: [42], relation: 'card', - relatedSchema: { ...ordersSchema, collectionName: 'cards' }, + relatedSchema: { + ...ordersSchema, + collectionName: 'cards', + primaryKeyFields: ['tenantId', 'cardId'], + }, }, user, ); @@ -914,7 +916,27 @@ describe('AgentClientAgentPort', () => { expect(result?.recordId).toEqual(['acme', '7']); }); - it('returns null when the scalar attribute is empty, without reading the target', async () => { + // The value is produced by client-written code, so a pipe in it is data, not packing. + it('keeps a pipe in a scalar attribute intact when the target key is single', async () => { + mockCollection.getOne + .mockResolvedValueOnce({ card: 'acme|corp' }) + .mockResolvedValueOnce({ id: 'acme|corp' }); + + const result = await port.getSingleRelatedData( + { + collection: 'claims', + id: [42], + relation: 'card', + relatedSchema: { ...ordersSchema, collectionName: 'cards' }, + }, + user, + ); + + expect(mockCollection.getOne).toHaveBeenNthCalledWith(2, ['acme|corp'], {}); + expect(result?.recordId).toEqual(['acme|corp']); + }); + + it('returns null when the scalar attribute is null, without reading the target', async () => { mockCollection.getOne.mockResolvedValue({ card: null }); const result = await port.getSingleRelatedData( @@ -931,6 +953,25 @@ describe('AgentClientAgentPort', () => { expect(mockCollection.getOne).toHaveBeenCalledTimes(1); }); + // `object.card&.id.to_s` is the idiomatic Ruby getter, and it answers "" for an unset + // association. Reading it as an id would build an id-less URL that agents route to the index. + it('returns null when the scalar attribute is an empty string, without reading the target', async () => { + mockCollection.getOne.mockResolvedValue({ card: '' }); + + const result = await port.getSingleRelatedData( + { + collection: 'claims', + id: [42], + relation: 'card', + relatedSchema: { ...ordersSchema, collectionName: 'cards' }, + }, + user, + ); + + expect(result).toBeNull(); + expect(mockCollection.getOne).toHaveBeenCalledTimes(1); + }); + it('returns null when the parent has no linkage to the xToOne relation', async () => { mockCollection.getOne.mockResolvedValue({ order: null }); diff --git a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts index 6923dd1e8a..b4ff5d5daf 100644 --- a/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/load-related-record-step-executor.test.ts @@ -4344,9 +4344,9 @@ describe('LoadRelatedRecordStepExecutor', () => { ); }); - // Regression: a Rails/Express reference smart field reaches the executor as a plain BelongsTo - // now that the server derives the relation from `reference` alone. Pinning it must follow it, - // where it used to fail as an invalid pre-recorded arg because the field was not a relation. + // Regression: a forest-rails smart field reaches the executor as a plain BelongsTo now that the + // server derives the relation from a `reference` whose target is in the apimap. Pinning it must + // follow it, where it used to fail as an invalid pre-recorded arg — the field was not a relation. it('follows a pinned reference-only BelongsTo', async () => { const { model, bindTools } = makeMockModel(); const runStore = makeMockRunStore(); @@ -4382,7 +4382,11 @@ describe('LoadRelatedRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); expect(bindTools).not.toHaveBeenCalled(); expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( - expect.objectContaining({ relation: 'card' }), + expect.objectContaining({ + collection: 'customers', + relation: 'card', + relatedSchema: expect.objectContaining({ collectionName: 'cards' }), + }), expect.anything(), ); expect(runStore.saveStepExecution).toHaveBeenCalledWith(