From a1a6c02de3fb03208d9555665cf648384c466651 Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Thu, 28 May 2026 15:49:51 +0200 Subject: [PATCH 01/11] fix(workflow): related record step executor --- .../src/adapters/agent-client-agent-port.ts | 27 +- .../load-related-record-step-executor.ts | 307 ++++++-- .../src/http/pending-data-validators.ts | 38 +- .../workflow-executor/src/ports/agent-port.ts | 4 +- .../src/types/step-execution-data.ts | 25 +- .../src/types/validated/collection.ts | 3 + .../adapters/agent-client-agent-port.test.ts | 113 +-- .../load-related-record-step-executor.test.ts | 666 ++++++++++++++---- .../step-execution-formatters.test.ts | 7 +- .../executors/step-summary-builder.test.ts | 14 +- .../test/http/pending-data-validators.test.ts | 38 +- .../integration/workflow-execution.test.ts | 15 +- 12 files changed, 949 insertions(+), 308 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 24bec25231..4ee0f8cfa9 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -25,7 +25,7 @@ import { // The agent-client HTTP layer deserializes JSON:API responses with camelCase keys. // 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( values: Record, originalFieldNames: string[] | undefined, ): Record { @@ -110,37 +110,24 @@ export default class AgentClientAgentPort implements AgentPort { }); } + // Returns raw rows as deserialized by agent-client (camelCase keys, no PK extraction). + // The caller resolves the related collection's schema and maps rows → RecordData; keeping + // schema-aware mapping out of the port avoids the silent fallback when the related + // collection isn't in the cache. async getRelatedData( { collection, id, relation, limit, fields }: GetRelatedDataQuery, user: StepUser, - ): Promise { + ): Promise[]> { return this.callAgent('getRelatedData', async () => { const client = this.createClient(user); - const parentSchema = this.resolveSchema(collection); - const relationField = parentSchema.fields.find(f => f.fieldName === relation); - const relatedCollectionName = relationField?.relatedCollectionName ?? relation; - const relatedSchema = this.resolveSchema(relatedCollectionName); - const records = await client + return client .collection(collection) .relation(relation, id) .list>({ ...(limit !== null && { pagination: { size: limit, number: 1 } }), ...(fields?.length && { fields }), }); - - return records.map(record => { - const restored = restoreFieldNames(record, [ - ...relatedSchema.primaryKeyFields, - ...(fields ?? []), - ]); - - return { - collectionName: relatedSchema.collectionName, - recordId: relatedSchema.primaryKeyFields.map(f => restored[f] as string | number), - values: restored, - }; - }); }); } diff --git a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts index 2d985faf9c..788c9a9723 100644 --- a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts @@ -1,12 +1,17 @@ import type { CreateActivityLogArgs } from '../ports/activity-log-port'; import type { StepExecutionResult } from '../types/execution-context'; -import type { LoadRelatedRecordStepExecutionData, RelationRef } from '../types/step-execution-data'; +import type { + LoadRelatedRecordCandidate, + LoadRelatedRecordStepExecutionData, + RelationRef, +} from '../types/step-execution-data'; import type { CollectionSchema, RecordData, RecordRef } from '../types/validated/collection'; import type { LoadRelatedRecordStepDefinition } from '../types/validated/step-definition'; import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; import { z } from 'zod'; +import { restoreFieldNames } from '../adapters/agent-client-agent-port'; import { InvalidAIResponseError, InvalidPreRecordedArgsError, @@ -35,6 +40,10 @@ Choose the record that best matches the user request based on the provided field interface RelationTarget extends RelationRef { selectedRecordRef: RecordRef; relationType?: 'BelongsTo' | 'HasMany' | 'HasOne' | 'BelongsToMany'; + relatedCollectionName: string; + // Primary key field name on the related collection — supplied by the orchestrator's + // schema. Required for the xToOne projection syntax ('@@@'). + relatedPrimaryKey?: string; } export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { @@ -55,6 +64,15 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor(pending, async exec => this.resolveFromSelection(exec), ); @@ -64,6 +82,36 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { + if (!execution.pendingData) { + throw new StepStateError(`Step at index ${this.context.stepIndex} has no pending data`); + } + + const schema = await this.getCollectionSchema(execution.selectedRecordRef.collectionName); + const target = this.buildTarget(schema, fieldDisplayName, execution.selectedRecordRef); + const { availableRecordIds, suggestedRecord } = await this.collectCandidateIds(target); + + await this.context.runStore.saveStepExecution(this.context.runId, { + ...execution, + userConfirmation: undefined, + pendingData: { + ...execution.pendingData, + suggestedField: { name: target.name, displayName: target.displayName }, + availableRecordIds, + suggestedRecord, + }, + }); + + return this.buildOutcomeResult({ status: 'awaiting-input' }); + } + private async handleFirstCall(): Promise { const { stepDefinition: step } = this.context; const { preRecordedArgs } = step; @@ -85,7 +133,7 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { + private async saveAndAwaitInput( + target: RelationTarget, + sourceSchema: CollectionSchema, + ): Promise { const { selectedRecordRef, name, displayName } = target; - const { relatedData, bestIndex, suggestedFields } = await this.selectBestFromRelatedData( - target, - 50, - ); + const { availableRecordIds, suggestedRecord } = await this.collectCandidateIds(target); - const selectedRecordId = relatedData[bestIndex].recordId; + const availableFields: RelationRef[] = sourceSchema.fields + .filter(f => f.isRelationship) + .map(f => ({ name: f.fieldName, displayName: f.displayName })); await this.context.runStore.saveStepExecution(this.context.runId, { type: 'load-related-record', stepIndex: this.context.stepIndex, - pendingData: { displayName, name, suggestedFields, selectedRecordId }, + pendingData: { + availableFields, + suggestedField: { name, displayName }, + availableRecordIds, + suggestedRecord, + }, selectedRecordRef, }); return this.buildOutcomeResult({ status: 'awaiting-input' }); } + // Branch C: collects the recordIds the frontend can present to the user, plus the + // AI's suggested pick. xToOne has exactly one candidate (no AI ranking needed); + // to-many goes through getRelatedData + the existing field/record AI selection. + // When the related collection has a layout-level `referenceField` configured, the + // candidates also carry its value so the frontend can display human-readable labels + // (e.g. "John Doe") instead of raw record ids. + private async collectCandidateIds(target: RelationTarget): Promise<{ + availableRecordIds: LoadRelatedRecordCandidate[]; + suggestedRecord: LoadRelatedRecordCandidate; + }> { + if (target.relationType === 'BelongsTo' || target.relationType === 'HasOne') { + const candidate = await this.fetchXToOneCandidate(target); + + return { availableRecordIds: [candidate], suggestedRecord: candidate }; + } + + const { relatedData, bestIndex, relatedSchema } = await this.selectBestFromRelatedData( + target, + 50, + ); + const referenceField = relatedSchema.referenceField ?? null; + const toCandidate = (r: RecordData): LoadRelatedRecordCandidate => ({ + recordId: r.recordId, + // `referenceField` is a fieldName on the related collection. fetchRelatedData + // has already restored field names from camelCase, so reading r.values[field] + // works for both `name` and `full_name` style identifiers. Coerce to string — + // the agent may return numbers/dates/etc. for display fields. + referenceFieldValue: referenceField + ? this.extractReferenceFieldValue(r.values, referenceField) + : null, + }); + + return { + availableRecordIds: relatedData.map(toCandidate), + suggestedRecord: toCandidate(relatedData[bestIndex]), + }; + } + + private extractReferenceFieldValue( + values: Record, + referenceField: string, + ): string | null { + const v = values[referenceField]; + + return v === undefined || v === null ? null : String(v); + } + /** Branch B: automatic execution. HasMany uses 2 AI calls; others take the first result. */ private async resolveAndLoadAutomatic(target: RelationTarget): Promise { - const record = - target.relationType === 'HasMany' - ? await this.selectBestRelatedRecord(target) - : await this.fetchFirstCandidate(target); + const record = await this.fetchRecordForRelation(target); return this.persistAndReturn(record, target, undefined); } + // Dispatches by relation type: xToOne reads the FK from the parent (no /relationships + // route exists on the agent for ManyToOne/OneToOne); HasMany uses AI selection; the + // remaining to-many shapes take the first /relationships result. + private async fetchRecordForRelation(target: RelationTarget): Promise { + if (target.relationType === 'BelongsTo' || target.relationType === 'HasOne') { + return this.fetchXToOneRecordRef(target); + } + + if (target.relationType === 'HasMany') { + return this.selectBestRelatedRecord(target); + } + + return this.fetchFirstCandidate(target); + } + + // For ManyToOne/OneToOne: project the relation on the parent record. Forest projection + // syntax requires at least one related field per relation, so we project the related + // collection's primary key (`@@@`) and, when configured, also the + // reference field for display. The orchestrator supplies both names alongside the + // schema — no extra getCollectionSchema fetch needed here. The agent's JSON:API + // serializer fills the relationship's `data.id` with the *full* related primary key + // (packed with "|" for composite keys) regardless of which fields we project, so + // split('|') gives back the complete recordId. + private async fetchXToOneRecordRef(target: RelationTarget): Promise { + const candidate = await this.fetchXToOneCandidate(target); + + return { + collectionName: target.relatedCollectionName, + recordId: candidate.recordId, + stepIndex: this.context.stepIndex, + }; + } + + // Same projection logic as fetchXToOneRecordRef, but also extracts the related + // collection's reference-field value (when configured) so the frontend can render + // a human-readable label in the awaiting-input dropdown. + private async fetchXToOneCandidate(target: RelationTarget): Promise { + if (!target.relatedPrimaryKey) { + throw new StepStateError( + `Cannot load xToOne relation "${target.name}" on collection ` + + `"${target.selectedRecordRef.collectionName}": missing relatedPrimaryKey in schema.`, + ); + } + + // Resolve the related schema for the optional referenceField. Cached after the first + // 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}`]; + + if (referenceField && referenceField !== target.relatedPrimaryKey) { + fields.push(`${target.name}@@@${referenceField}`); + } + + const parent = await this.agentPort.getRecord( + { + collection: target.selectedRecordRef.collectionName, + id: target.selectedRecordRef.recordId, + fields, + }, + this.context.user, + ); + + const relation = parent.values[target.name] as Record | null | undefined; + const packedId = relation?.id as string | undefined; + + if (!packedId) { + throw new RelatedRecordNotFoundError(target.selectedRecordRef.collectionName, target.name); + } + + return { + recordId: packedId.split('|'), + referenceFieldValue: referenceField + ? this.extractReferenceFieldValue(relation as Record, referenceField) + : null, + }; + } + // Branch A: builds RecordRef from the user-confirmed selection without a new getRelatedData call. private async resolveFromSelection( execution: LoadRelatedRecordStepExecutionData, @@ -149,10 +328,26 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor f.displayName === userConfirmation.fieldDisplayName) + : pendingData.suggestedField; - // Re-derive relatedCollectionName and displayName because the user may have swapped the relation. + if (!relationRef) { + throw new StepStateError( + `Step at index ${this.context.stepIndex} could not resolve relation "${userConfirmation?.fieldDisplayName}" from available fields`, + ); + } + + const { name, displayName } = relationRef; + // suggestedRecord is a LoadRelatedRecordCandidate; only the recordId is needed here. + // The reference-field value is purely for display in awaiting-input and never persisted + // on the final RecordRef. + const selectedRecordId = + userConfirmation?.selectedRecordId ?? pendingData.suggestedRecord.recordId; + + // Re-derive relatedCollectionName from the live schema — frontend never sends it. const schema = await this.getCollectionSchema(selectedRecordRef.collectionName); const field = schema.fields.find(f => f.fieldName === name); @@ -162,10 +357,8 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor, + target: Pick, limit: number, - ): Promise<{ relatedData: RecordData[]; bestIndex: number; suggestedFields: string[] }> { + ): Promise<{ + relatedData: RecordData[]; + bestIndex: number; + suggestedFields: string[]; + relatedSchema: CollectionSchema; + }> { const { selectedRecordRef, name } = target; - - const relatedData = await this.agentPort.getRelatedData( - { - collection: selectedRecordRef.collectionName, - id: selectedRecordRef.recordId, - relation: name, - limit, - }, - this.context.user, - ); + const relatedSchema = await this.getCollectionSchema(target.relatedCollectionName); + const relatedData = await this.fetchRelatedData(target, relatedSchema, limit); if (relatedData.length === 0) { throw new RelatedRecordNotFoundError(selectedRecordRef.collectionName, name); } if (relatedData.length === 1) { - return { relatedData, bestIndex: 0, suggestedFields: [] }; + return { relatedData, bestIndex: 0, suggestedFields: [], relatedSchema }; } const { preRecordedArgs } = this.context.stepDefinition; @@ -212,10 +402,14 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor, + target: Pick, limit: number, ): Promise { const { selectedRecordRef, name } = target; - const relatedData = await this.agentPort.getRelatedData( + const relatedSchema = await this.getCollectionSchema(target.relatedCollectionName); + const relatedData = await this.fetchRelatedData(target, relatedSchema, limit); + + if (relatedData.length === 0) { + throw new RelatedRecordNotFoundError(selectedRecordRef.collectionName, name); + } + + return relatedData.map(r => this.toRecordRef(r)); + } + + // Calls the agent port and maps raw rows → RecordData using the related collection's schema. + // Schema is resolved by the caller so the cache is warmed via getCollectionSchema (which + // falls back to workflowPort), avoiding the silent ["id"] PK fallback the port used to do. + private async fetchRelatedData( + target: Pick, + relatedSchema: CollectionSchema, + limit: number, + ): Promise { + const rows = await this.agentPort.getRelatedData( { - collection: selectedRecordRef.collectionName, - id: selectedRecordRef.recordId, - relation: name, + collection: target.selectedRecordRef.collectionName, + id: target.selectedRecordRef.recordId, + relation: target.name, limit, }, this.context.user, ); - if (relatedData.length === 0) { - throw new RelatedRecordNotFoundError(selectedRecordRef.collectionName, name); - } + return rows.map(row => { + const restored = restoreFieldNames( + row, + relatedSchema.fields.map(f => f.fieldName), + ); - return relatedData.map(r => this.toRecordRef(r)); + return { + collectionName: relatedSchema.collectionName, + recordId: relatedSchema.primaryKeyFields.map(f => restored[f] as string | number), + values: restored, + }; + }); } /** Persists the loaded record ref and returns a success outcome. */ diff --git a/packages/workflow-executor/src/http/pending-data-validators.ts b/packages/workflow-executor/src/http/pending-data-validators.ts index d739e1b40d..0c9413c913 100644 --- a/packages/workflow-executor/src/http/pending-data-validators.ts +++ b/packages/workflow-executor/src/http/pending-data-validators.ts @@ -23,25 +23,45 @@ const triggerActionPatchSchema = z const mcpPatchSchema = z.object({ userConfirmed: z.boolean() }).strict(); +// Accepts two shapes: +// 1. Confirmation patch: `userConfirmed: boolean` (+ optional overrides) — finalizes +// the step or skips it. +// 2. Field-preview patch: `fieldDisplayName: string` alone, with `userConfirmed` +// omitted — asks the executor to re-list candidates for a different relation +// WITHOUT finalizing. The executor refreshes pendingData and stays awaiting-input. +// Required when the frontend lets the user switch relations: the IDs originally +// stored under `availableRecordIds` belong to the AI-suggested relation only. const loadRelatedRecordPatchSchema = z .object({ - userConfirmed: z.boolean(), + userConfirmed: z.boolean().optional(), // User may intentionally switch to a different relation than the one the AI selected. - // The executor re-derives relatedCollectionName and displayName from FieldSchema when - // processing the confirmation. - 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(), // User may override the AI-selected record; must be non-empty when provided. - // Required when overriding the relation name — the original record ID belongs to a - // different collection and cannot be reused for the new relation. + // Required when confirming with a relation override — the original record ID + // belongs to a different collection and cannot be reused for the new relation. selectedRecordId: z .array(z.union([z.string(), z.number()])) .min(1) .optional(), }) .strict() - .refine(data => data.name === undefined || data.selectedRecordId !== undefined, { - message: 'selectedRecordId is required when overriding the relation name', - }); + .refine( + data => { + // Preview patch (no confirm): fieldDisplayName alone is sufficient. + if (data.userConfirmed === undefined) return data.fieldDisplayName !== undefined; + // Confirm patch with relation override: selectedRecordId required. + if (data.fieldDisplayName !== undefined) return data.selectedRecordId !== undefined; + + return true; + }, + { + message: + 'selectedRecordId is required when confirming with a relation override, ' + + 'or omit userConfirmed to preview candidates for a different relation', + }, + ); const guidancePatchSchema = z .object({ userInput: z.string().optional(), diff --git a/packages/workflow-executor/src/ports/agent-port.ts b/packages/workflow-executor/src/ports/agent-port.ts index 4ccb652409..aee401493a 100644 --- a/packages/workflow-executor/src/ports/agent-port.ts +++ b/packages/workflow-executor/src/ports/agent-port.ts @@ -25,7 +25,9 @@ export type GetActionFormInfoQuery = { collection: string; action: string; id: I export interface AgentPort { getRecord(query: GetRecordQuery, user: StepUser): Promise; updateRecord(query: UpdateRecordQuery, user: StepUser): Promise; - getRelatedData(query: GetRelatedDataQuery, user: StepUser): Promise; + // 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[]>; executeAction(query: ExecuteActionQuery, user: StepUser): Promise; // Old Ruby agents with hooks.load=false return 404; agent-client falls back to the fields // passed via ActionEndpointsByCollection (populated from the orchestrator's schema). diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 1c37b1460d..b3f2d2fb9c 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -133,11 +133,26 @@ export interface RecordStepExecutionData extends BaseStepExecutionData { // -- Load Related Record -- -export interface LoadRelatedRecordPendingData extends RelationRef { - // undefined when not computed (record has no non-relation fields). - suggestedFields?: string[]; - // AI-selected initially; frontend can override via userConfirmation.selectedRecordId. - selectedRecordId: Array; +// One row of `availableRecordIds`. `referenceFieldValue` is the layout-level reference +// field (collection.displayFieldName on the orchestrator side) — when the related +// collection has one configured and the agent returns a value, the frontend shows it +// instead of the raw id. Both `null` (no reference field configured) and `undefined` +// (configured but value missing) collapse to the same "fall back to id" rendering. +export interface LoadRelatedRecordCandidate { + recordId: Array; + referenceFieldValue?: string | null; +} + +export interface LoadRelatedRecordPendingData { + // All relation fields available on the source collection (name + displayName each). + availableFields: RelationRef[]; + // AI-selected relation from `availableFields`. + suggestedField: RelationRef; + // First 50 records of `suggestedField` paired with their reference-field value. + availableRecordIds: LoadRelatedRecordCandidate[]; + // AI-selected record from `availableRecordIds`; frontend can override via + // userConfirmation.selectedRecordId. + suggestedRecord: LoadRelatedRecordCandidate; } export interface LoadRelatedRecordStepExecutionData diff --git a/packages/workflow-executor/src/types/validated/collection.ts b/packages/workflow-executor/src/types/validated/collection.ts index 6d5b73d2de..fe1776196b 100644 --- a/packages/workflow-executor/src/types/validated/collection.ts +++ b/packages/workflow-executor/src/types/validated/collection.ts @@ -77,6 +77,9 @@ export const CollectionSchemaSchema = z // null when the rendering has no explicit displayName configured — normalized to collectionName. collectionDisplayName: z.string().nullable(), primaryKeyFields: z.array(z.string().min(1)).min(1), + // Layout-level "reference field" used to display a record (e.g. "name", "title"). + // Null when the team didn't configure one; callers fall back to the primary key. + referenceField: z.string().nullable().optional(), fields: z.array(FieldSchemaSchema), actions: z.array(ActionSchemaSchema).optional().default([]), }) 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 aacaff0652..840ece3bf9 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 @@ -247,38 +247,27 @@ describe('AgentClientAgentPort', () => { }); describe('getRelatedData', () => { - it('should return RecordData[] with recordId extracted from PK fields', async () => { - mockRelation.list.mockResolvedValue([ + // Contract: this port is now a thin HTTP adapter for related-data lists. It returns the + // raw rows from agent-client (camelCase keys, no PK extraction, no collectionName). + // Schema resolution and the row → RecordData mapping happen in the executor — that's + // where the related collection's schema can be fetched on demand via the workflow port. + it('returns raw rows from agent-client untouched', async () => { + const rows = [ { id: 10, title: 'Post A' }, { id: 11, title: 'Post B' }, - ]); + ]; + mockRelation.list.mockResolvedValue(rows); const result = await port.getRelatedData( - { - collection: 'users', - id: [42], - relation: 'posts', - limit: null, - }, + { collection: 'users', id: [42], relation: 'posts', limit: null }, user, ); expect(mockCollection.relation).toHaveBeenCalledWith('posts', [42]); - expect(result).toEqual([ - { - collectionName: 'posts', - recordId: [10], - values: { id: 10, title: 'Post A' }, - }, - { - collectionName: 'posts', - recordId: [11], - values: { id: 11, title: 'Post B' }, - }, - ]); + expect(result).toEqual(rows); }); - it('should apply pagination when limit is a number', async () => { + it('applies pagination when limit is a number', async () => { mockRelation.list.mockResolvedValue([{ id: 10, title: 'Post A' }]); await port.getRelatedData( @@ -291,7 +280,7 @@ describe('AgentClientAgentPort', () => { ); }); - it('should not apply pagination when limit is null', async () => { + it('does not apply pagination when limit is null', async () => { mockRelation.list.mockResolvedValue([]); await port.getRelatedData( @@ -302,50 +291,22 @@ describe('AgentClientAgentPort', () => { expect(mockRelation.list).toHaveBeenCalledWith({}); }); - it('should fallback to relationName when no CollectionSchema exists', async () => { - mockRelation.list.mockResolvedValue([{ id: 1 }]); - - const result = await port.getRelatedData( - { - collection: 'users', - id: [42], - relation: 'unknownRelation', - limit: null, - }, - user, - ); - - expect(result[0].collectionName).toBe('unknownRelation'); - expect(result[0].recordId).toEqual([1]); - }); - - it('should return an empty array when no related data exists', async () => { + it('returns an empty array when no related data exists', async () => { mockRelation.list.mockResolvedValue([]); expect( await port.getRelatedData( - { - collection: 'users', - id: [42], - relation: 'posts', - limit: null, - }, + { collection: 'users', id: [42], relation: 'posts', limit: null }, user, ), ).toEqual([]); }); - it('should forward fields to the list call when provided', async () => { + it('forwards fields to the list call when provided', async () => { mockRelation.list.mockResolvedValue([{ id: 10, title: 'Post A' }]); await port.getRelatedData( - { - collection: 'users', - id: [42], - relation: 'posts', - limit: null, - fields: ['title'], - }, + { collection: 'users', id: [42], relation: 'posts', limit: null, fields: ['title'] }, user, ); @@ -354,7 +315,7 @@ describe('AgentClientAgentPort', () => { ); }); - it('should omit fields from the list call when not provided', async () => { + it('omits fields from the list call when not provided', async () => { mockRelation.list.mockResolvedValue([{ id: 10 }]); await port.getRelatedData( @@ -367,49 +328,27 @@ describe('AgentClientAgentPort', () => { ); }); - it('should restore snake_case field names in recordId and values when agent returns camelCase keys', async () => { + it('does not consult the schema cache for the related collection', async () => { + // No 'posts' entry in the cache — the port must still succeed. The fallback + // warn-and-default-to-["id"] used to live here; that logic now belongs to the + // executor, which routes through getCollectionSchema (cache + workflow port). const cache = new SchemaCache(); - cache.set('users', { - collectionName: 'users', - collectionDisplayName: 'Users', - primaryKeyFields: ['id'], - fields: [ - { - fieldName: 'posts', - displayName: 'Posts', - isRelationship: true, - relatedCollectionName: 'posts', - }, - ], - actions: [], - }); - cache.set('posts', { - collectionName: 'posts', - collectionDisplayName: 'Posts', - primaryKeyFields: ['post_id'], - fields: [], - actions: [], - }); const localPort = new AgentClientAgentPort({ agentUrl: 'http://agent', authSecret: 'secret', schemaCache: cache, }); + const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); mockRelation.list.mockResolvedValue([{ postId: 99, createdAt: '2024-01-01' }]); const result = await localPort.getRelatedData( - { - collection: 'users', - id: [42], - relation: 'posts', - limit: null, - fields: ['post_id', 'created_at'], - }, + { collection: 'users', id: [42], relation: 'posts', limit: null }, user, ); - expect(result[0].recordId).toEqual([99]); - expect(result[0].values).toEqual({ post_id: 99, created_at: '2024-01-01' }); + expect(result).toEqual([{ postId: 99, createdAt: '2024-01-01' }]); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); }); }); 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 f7f7a25883..d65498f19a 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 @@ -22,6 +22,17 @@ function makeStep( }; } +// Wraps a raw recordId array into a LoadRelatedRecordCandidate for pendingData +// fixtures and assertions. Tests that care about referenceFieldValue pass the +// second argument; everything else gets `null`, which matches the executor's +// behavior when the related collection has no referenceField configured. +function cand( + recordId: Array, + referenceFieldValue: string | null = null, +): { recordId: Array; referenceFieldValue: string | null } { + return { recordId, referenceFieldValue }; +} + function makeRecordRef(overrides: Partial = {}): RecordRef { return { collectionName: 'customers', @@ -40,11 +51,45 @@ function makeRelatedRecordData(overrides: Partial = {}): RecordData }; } -function makeMockAgentPort(relatedData: RecordData[] = [makeRelatedRecordData()]): AgentPort { +/** + * The agent port now returns raw rows (no PK extraction). For test ergonomics we keep + * the RecordData[] shape in fixtures and translate here: each RecordData becomes a raw + * row by inlining the recordId under the PK field name (default 'id', or first + * primaryKeyField when provided) alongside its values. + */ +function toAgentRows( + records: RecordData[], + primaryKeyFields: string[] = ['id'], +): Record[] { + return records.map(r => { + const pkValues = Object.fromEntries(primaryKeyFields.map((field, i) => [field, r.recordId[i]])); + + return { ...pkValues, ...r.values }; + }); +} + +function makeMockAgentPort( + relatedData: RecordData[] = [makeRelatedRecordData()], + primaryKeyFields: string[] = ['id'], +): AgentPort { + // xToOne path uses getRecord(parent, fields: ['@@@']) and reads + // parent.values[].id (jsonapi-serializer materializes the relationship + // linkage as a nested object on the parent). The mock reflects this contract by + // exposing the first relatedData[0]'s recordId joined with "|" under the relation + // name extracted from the @@@ projection. Tests can override per-call. + const getRecord = jest.fn(async ({ fields }: { fields?: string[] }) => { + const projection = fields?.[0]; + const relationName = projection?.split('@@@')[0]; + const packedId = relatedData[0]?.recordId.map(String).join('|'); + const values = relationName && packedId ? { [relationName]: { id: packedId } } : {}; + + return { collectionName: 'parent', recordId: [], values }; + }); + return { - getRecord: jest.fn(), + getRecord, updateRecord: jest.fn(), - getRelatedData: jest.fn().mockResolvedValue(relatedData), + getRelatedData: jest.fn().mockResolvedValue(toAgentRows(relatedData, primaryKeyFields)), executeAction: jest.fn(), } as unknown as AgentPort; } @@ -63,6 +108,7 @@ function makeCollectionSchema(overrides: Partial = {}): Collec isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'orders', + relatedPrimaryKey: 'id', }, { fieldName: 'address', @@ -164,10 +210,13 @@ function makePendingExecution( type: 'load-related-record', stepIndex: 0, pendingData: { - displayName: 'Order', - name: 'order', - selectedRecordId: [99], - suggestedFields: ['status', 'amount'], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, selectedRecordRef: makeRecordRef(), ...overrides, @@ -191,10 +240,12 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('success'); - expect(agentPort.getRelatedData).toHaveBeenCalledWith( - { collection: 'customers', id: [42], relation: 'order', limit: 1 }, + // BelongsTo: project the relation on the parent record; no /relationships call. + expect(agentPort.getRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], fields: ['order@@@id'] }, expect.objectContaining({ id: 1 }), ); + expect(agentPort.getRelatedData).not.toHaveBeenCalled(); expect(runStore.saveStepExecution).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ @@ -204,7 +255,7 @@ describe('LoadRelatedRecordStepExecutor', () => { executionResult: expect.objectContaining({ record: expect.objectContaining({ collectionName: 'orders', - recordId: [99], + recordId: ['99'], stepIndex: 0, }), }), @@ -227,6 +278,7 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Address', isRelationship: true, relationType: 'HasMany', + relatedCollectionName: 'addresses', }, ], }); @@ -319,6 +371,7 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Address', isRelationship: true, relationType: 'HasMany', + relatedCollectionName: 'addresses', }, ], }); @@ -552,6 +605,8 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Profile', isRelationship: true, relationType: 'HasOne', + relatedCollectionName: 'profiles', + relatedPrimaryKey: 'id', }, ], }); @@ -572,16 +627,18 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('success'); - // HasOne uses the same fetchFirstCandidate path as BelongsTo — limit: 1 - expect(agentPort.getRelatedData).toHaveBeenCalledWith( - { collection: 'customers', id: [42], relation: 'profile', limit: 1 }, + // HasOne uses the same xToOne path as BelongsTo: project the relation on the + // parent and split the packed id. No /relationships call. + expect(agentPort.getRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], fields: ['profile@@@id'] }, expect.objectContaining({ id: 1 }), ); + expect(agentPort.getRelatedData).not.toHaveBeenCalled(); expect(runStore.saveStepExecution).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ executionResult: expect.objectContaining({ - record: expect.objectContaining({ collectionName: 'profiles', recordId: [5] }), + record: expect.objectContaining({ collectionName: 'profiles', recordId: ['5'] }), }), }), ); @@ -599,11 +656,13 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('awaiting-input'); - expect(agentPort.getRelatedData).toHaveBeenCalledWith( - { collection: 'customers', id: [42], relation: 'order', limit: 50 }, + // BelongsTo → xToOne path: project the relation on the parent. No /relationships call. + expect(agentPort.getRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], fields: ['order@@@id'] }, expect.objectContaining({ id: 1 }), ); - // Single record → only select-relation AI call + expect(agentPort.getRelatedData).not.toHaveBeenCalled(); + // xToOne has exactly one candidate → only select-relation AI call (no field/record selection) expect(mockModel.bindTools).toHaveBeenCalledTimes(1); expect(runStore.saveStepExecution).toHaveBeenCalledWith( 'run-1', @@ -611,10 +670,13 @@ describe('LoadRelatedRecordStepExecutor', () => { type: 'load-related-record', stepIndex: 0, pendingData: { - displayName: 'Order', - name: 'order', - selectedRecordId: [99], - suggestedFields: [], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand(['99'])], + suggestedRecord: cand(['99']), }, selectedRecordRef: expect.objectContaining({ collectionName: 'customers', @@ -624,17 +686,19 @@ describe('LoadRelatedRecordStepExecutor', () => { ); }); + // Uses HasMany ('Address') because BelongsTo/HasOne now short-circuit to a single + // xToOne candidate (no select-fields/select-record-by-content AI calls). it('runs field-selection + record-selection AI calls when multiple related records exist', async () => { const relatedData: RecordData[] = [ - { collectionName: 'orders', recordId: [1], values: { status: 'pending' } }, - { collectionName: 'orders', recordId: [2], values: { status: 'completed' } }, + { collectionName: 'addresses', recordId: [1], values: { city: 'Paris' } }, + { collectionName: 'addresses', recordId: [2], values: { city: 'Lyon' } }, ]; const agentPort = makeMockAgentPort(relatedData); - const ordersSchema = makeCollectionSchema({ - collectionName: 'orders', - collectionDisplayName: 'Orders', - fields: [{ fieldName: 'status', displayName: 'Status', isRelationship: false }], + const addressesSchema = makeCollectionSchema({ + collectionName: 'addresses', + collectionDisplayName: 'Addresses', + fields: [{ fieldName: 'city', displayName: 'City', isRelationship: false }], }); const invoke = jest @@ -643,19 +707,19 @@ describe('LoadRelatedRecordStepExecutor', () => { tool_calls: [ { name: 'select-relation', - args: { relationName: 'Order', reasoning: 'Load order' }, + args: { relationName: 'Address', reasoning: 'Load address' }, id: 'c1', }, ], }) .mockResolvedValueOnce({ - tool_calls: [{ name: 'select-fields', args: { fieldNames: ['Status'] }, id: 'c2' }], + tool_calls: [{ name: 'select-fields', args: { fieldNames: ['City'] }, id: 'c2' }], }) .mockResolvedValueOnce({ tool_calls: [ { name: 'select-record-by-content', - args: { recordIndex: 1, reasoning: 'Completed is best' }, + args: { recordIndex: 1, reasoning: 'Lyon is best' }, id: 'c3', }, ], @@ -670,7 +734,7 @@ describe('LoadRelatedRecordStepExecutor', () => { runStore, workflowPort: makeMockWorkflowPort({ customers: makeCollectionSchema(), - orders: ordersSchema, + addresses: addressesSchema, }), }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -683,25 +747,30 @@ describe('LoadRelatedRecordStepExecutor', () => { 'run-1', expect.objectContaining({ pendingData: { - displayName: 'Order', - name: 'order', - selectedRecordId: [2], // record at index 1 - suggestedFields: ['status'], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'address', displayName: 'Address' }, + availableRecordIds: [cand([1]), cand([2])], + suggestedRecord: cand([2]), // record at index 1 }, }), ); }); + // Uses HasMany ('Address') because BelongsTo/HasOne now short-circuit to a single + // xToOne candidate (no field/record AI selection). it('skips field-selection AI call when related collection has no non-relation fields', async () => { const relatedData: RecordData[] = [ - { collectionName: 'orders', recordId: [1], values: {} }, - { collectionName: 'orders', recordId: [2], values: {} }, + { collectionName: 'addresses', recordId: [1], values: {} }, + { collectionName: 'addresses', recordId: [2], values: {} }, ]; const agentPort = makeMockAgentPort(relatedData); - const ordersSchema = makeCollectionSchema({ - collectionName: 'orders', - collectionDisplayName: 'Orders', + const addressesSchema = makeCollectionSchema({ + collectionName: 'addresses', + collectionDisplayName: 'Addresses', fields: [], }); @@ -711,7 +780,7 @@ describe('LoadRelatedRecordStepExecutor', () => { tool_calls: [ { name: 'select-relation', - args: { relationName: 'Order', reasoning: 'Load order' }, + args: { relationName: 'Address', reasoning: 'Load address' }, id: 'c1', }, ], @@ -735,7 +804,7 @@ describe('LoadRelatedRecordStepExecutor', () => { runStore, workflowPort: makeMockWorkflowPort({ customers: makeCollectionSchema(), - orders: ordersSchema, + addresses: addressesSchema, }), }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -749,8 +818,8 @@ describe('LoadRelatedRecordStepExecutor', () => { 'run-1', expect.objectContaining({ pendingData: expect.objectContaining({ - selectedRecordId: [1], - suggestedFields: [], + suggestedRecord: cand([1]), + availableRecordIds: [cand([1]), cand([2])], }), }), ); @@ -758,14 +827,17 @@ describe('LoadRelatedRecordStepExecutor', () => { }); describe('confirmation accepted (Branch A)', () => { - it('uses selectedRecordId from pendingData, no getRelatedData call', async () => { + it('uses suggestedRecord from pendingData, no getRelatedData call', async () => { const agentPort = makeMockAgentPort(); const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: ['status', 'amount'], - selectedRecordId: [99], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, userConfirmation: { userConfirmed: true }, }); @@ -788,22 +860,24 @@ describe('LoadRelatedRecordStepExecutor', () => { record: expect.objectContaining({ collectionName: 'orders', recordId: [99] }), }), pendingData: expect.objectContaining({ - displayName: 'Order', - name: 'order', - selectedRecordId: [99], + suggestedField: { name: 'order', displayName: 'Order' }, + suggestedRecord: cand([99]), }), }), ); }); - it('uses selectedRecordId when the user overrides the AI suggestion', async () => { + it('uses suggestedRecord when the user does not override the AI suggestion', async () => { const agentPort = makeMockAgentPort(); const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: ['status', 'amount'], - selectedRecordId: [42], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([42])], + suggestedRecord: cand([42]), }, userConfirmation: { userConfirmed: true }, }); @@ -829,14 +903,17 @@ describe('LoadRelatedRecordStepExecutor', () => { }); describe('confirmation with user override of selectedRecordId (Branch A)', () => { - it('preserves AI suggestion in pendingData and writes user choice to executionParams', async () => { + it('preserves AI suggestion in pendingData and writes user choice to executionResult', async () => { // Persisted state: AI suggested record [99], awaiting confirmation. const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - selectedRecordId: [99], - suggestedFields: ['status', 'amount'], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99]), cand([42])], + suggestedRecord: cand([99]), }, }); const agentPort = makeMockAgentPort(); @@ -862,9 +939,8 @@ describe('LoadRelatedRecordStepExecutor', () => { expect.objectContaining({ type: 'load-related-record', pendingData: expect.objectContaining({ - displayName: 'Order', - name: 'order', - selectedRecordId: [99], // AI suggestion preserved + suggestedField: { name: 'order', displayName: 'Order' }, + suggestedRecord: cand([99]), // AI suggestion preserved }), executionResult: expect.objectContaining({ record: expect.objectContaining({ collectionName: 'orders', recordId: [42] }), @@ -874,15 +950,18 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); - describe('confirmation with user override of relation name (Branch A)', () => { + describe('confirmation with user override of relation (Branch A)', () => { it('re-derives relatedCollectionName when the user switches to a different relation', async () => { - // AI suggested "order" (→ orders collection). User switches to "address" (→ addresses). + // AI suggested "order" (→ orders collection). User switches to "Address" (→ addresses). const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - selectedRecordId: [99], - suggestedFields: [], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, }); const runStore = makeMockRunStore({ @@ -892,7 +971,7 @@ describe('LoadRelatedRecordStepExecutor', () => { runStore, incomingPendingData: { userConfirmed: true, - name: 'address', + fieldDisplayName: 'Address', selectedRecordId: [7], }, }); @@ -906,9 +985,8 @@ describe('LoadRelatedRecordStepExecutor', () => { expect.objectContaining({ // AI suggestion preserved on pendingData pendingData: expect.objectContaining({ - name: 'order', - displayName: 'Order', - selectedRecordId: [99], + suggestedField: { name: 'order', displayName: 'Order' }, + suggestedRecord: cand([99]), }), // User-overridden relation resolves to the addresses collection executionParams: { name: 'address', displayName: 'Address' }, @@ -936,10 +1014,10 @@ describe('LoadRelatedRecordStepExecutor', () => { }); const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: [], - selectedRecordId: [99], + availableFields: [{ name: 'order', displayName: 'Order' }], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, userConfirmation: { userConfirmed: true }, }); @@ -977,10 +1055,10 @@ describe('LoadRelatedRecordStepExecutor', () => { }); const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: [], - selectedRecordId: [99], + availableFields: [{ name: 'order', displayName: 'Order' }], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, userConfirmation: { userConfirmed: true }, }); @@ -1000,7 +1078,7 @@ describe('LoadRelatedRecordStepExecutor', () => { expect(runStore.saveStepExecution).not.toHaveBeenCalled(); }); - it('uses overridden relation name from pendingData to derive relatedCollectionName', async () => { + it('uses overridden suggestedField from pendingData to derive relatedCollectionName', async () => { const schema = makeCollectionSchema({ fields: [ { @@ -1019,13 +1097,17 @@ describe('LoadRelatedRecordStepExecutor', () => { }, ], }); - // User overrode AI's suggestion of 'order' to 'address' via PATCH + // Pending data already reflects 'address' as the suggested relation (e.g. user override + // was previously persisted, or the AI picked it directly). const execution = makePendingExecution({ pendingData: { - displayName: 'Address', - name: 'address', - suggestedFields: [], - selectedRecordId: [77], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'address', displayName: 'Address' }, + availableRecordIds: [cand([77])], + suggestedRecord: cand([77]), }, userConfirmation: { userConfirmed: true }, }); @@ -1053,10 +1135,10 @@ describe('LoadRelatedRecordStepExecutor', () => { const execution = makePendingExecution({ selectedRecordRef: { collectionName: 'customers', recordId: [42], stepIndex: 0 }, pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: [], - selectedRecordId: [99], + availableFields: [{ name: 'order', displayName: 'Order' }], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, userConfirmation: { userConfirmed: true }, }); @@ -1081,10 +1163,13 @@ describe('LoadRelatedRecordStepExecutor', () => { const agentPort = makeMockAgentPort(); const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: ['status', 'amount'], - selectedRecordId: [99], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, userConfirmation: { userConfirmed: false }, }); @@ -1102,12 +1187,319 @@ describe('LoadRelatedRecordStepExecutor', () => { 'run-1', expect.objectContaining({ executionResult: { skipped: true }, - pendingData: expect.objectContaining({ displayName: 'Order', name: 'order' }), + pendingData: expect.objectContaining({ + suggestedField: { name: 'order', displayName: 'Order' }, + }), }), ); }); }); + // The frontend lets the user switch to a different relation before confirming. To + // populate the new relation's `availableRecordIds`, it POSTs a "preview" patch: + // `{ fieldDisplayName }` with no `userConfirmed`. The executor re-lists candidates, + // refreshes pendingData, clears userConfirmation, and stays awaiting-input. + describe('field-preview patch (Branch A — no confirm)', () => { + it('re-lists candidates for the new relation and stays awaiting-input', async () => { + const execution = makePendingExecution({ + pendingData: { + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand(['99'])], + suggestedRecord: cand(['99']), + }, + }); + // User switched to Address (HasMany). The default mock returns the order fixture; + // override with address candidates so we can verify the new IDs land in pendingData. + const agentPort = makeMockAgentPort([ + { collectionName: 'addresses', recordId: [1], values: { city: 'Paris' } }, + { collectionName: 'addresses', recordId: [2], values: { city: 'Lyon' } }, + ]); + const addressesSchema = makeCollectionSchema({ + collectionName: 'addresses', + collectionDisplayName: 'Addresses', + fields: [{ fieldName: 'city', displayName: 'City', isRelationship: false }], + }); + // The schema-cache fetch for 'addresses' goes through the workflow port. + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema(), + addresses: addressesSchema, + }); + // With 2 candidates, selectBestFromRelatedData calls the AI for field + record + // selection. Wire those up so the preview can pick a suggestedRecord. + const invoke = jest + .fn() + .mockResolvedValueOnce({ + tool_calls: [{ name: 'select-fields', args: { fieldNames: ['City'] }, id: 'c1' }], + }) + .mockResolvedValueOnce({ + tool_calls: [ + { + name: 'select-record-by-content', + args: { recordIndex: 1, reasoning: 'Lyon' }, + id: 'c2', + }, + ], + }); + const model = { + bindTools: jest.fn().mockReturnValue({ invoke }), + } as unknown as ExecutionContext['model']; + + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ + model, + agentPort, + runStore, + workflowPort, + incomingPendingData: { fieldDisplayName: 'Address' }, + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('awaiting-input'); + + // Two saves: one from patchAndReloadPendingData persisting userConfirmation, + // one from refreshCandidatesForField writing the new pendingData. The latter + // is the one the frontend reads. + const finalSave = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(finalSave).toEqual( + expect.objectContaining({ + type: 'load-related-record', + // userConfirmation cleared so the next bodyless trigger re-emits awaiting-input + // cleanly via handleConfirmationFlow (no stale fieldDisplayName ghost-confirms). + userConfirmation: undefined, + pendingData: expect.objectContaining({ + // availableFields is immutable — only suggestedField + candidates change. + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'address', displayName: 'Address' }, + availableRecordIds: [cand([1]), cand([2])], + suggestedRecord: cand([2]), // AI's select-record-by-content pick + }), + }), + ); + }); + + it('reruns xToOne candidate lookup when previewing a BelongsTo relation', async () => { + // Same setup but switching to Order (BelongsTo). Verifies the xToOne path is + // used inside refreshCandidatesForField — no AI calls, single candidate from + // the parent's projected relation linkage. + const execution = makePendingExecution({ + pendingData: { + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'address', displayName: 'Address' }, + availableRecordIds: [cand([1]), cand([2])], + suggestedRecord: cand([2]), + }, + }); + const agentPort = makeMockAgentPort(); // default: order recordId [99] + const workflowPort = makeMockWorkflowPort({ customers: makeCollectionSchema() }); + const mockModel = makeMockModel({}); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort, + incomingPendingData: { fieldDisplayName: 'Order' }, + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('awaiting-input'); + // xToOne path goes through getRecord with `@@@` projection. + expect(agentPort.getRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], fields: ['order@@@id'] }, + expect.objectContaining({ id: 1 }), + ); + expect(agentPort.getRelatedData).not.toHaveBeenCalled(); + + const finalSave = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(finalSave).toEqual( + expect.objectContaining({ + userConfirmation: undefined, + pendingData: expect.objectContaining({ + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand(['99'])], + suggestedRecord: cand(['99']), + }), + }), + ); + }); + + it('returns error when the previewed relation does not exist on the source collection', async () => { + const execution = makePendingExecution(); + const agentPort = makeMockAgentPort(); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ + agentPort, + runStore, + incomingPendingData: { fieldDisplayName: 'NotAField' }, + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + }); + }); + + // The related collection may have a layout-level `referenceField` (e.g. `name`, + // `title`) used to display records in the UI. When configured, candidate records + // in pendingData carry the resolved value so the awaiting-input dropdown can show + // human-readable labels instead of raw ids. + describe('referenceField propagation in pendingData (Branch C)', () => { + it('exposes referenceFieldValue from the related collection on each HasMany candidate', async () => { + // HasMany path: fetchRelatedData returns full rows; the executor reads + // values[referenceField] for each candidate. + const relatedData: RecordData[] = [ + { collectionName: 'addresses', recordId: [1], values: { city: 'Paris' } }, + { collectionName: 'addresses', recordId: [2], values: { city: 'Lyon' } }, + ]; + const agentPort = makeMockAgentPort(relatedData); + const addressesSchema = makeCollectionSchema({ + collectionName: 'addresses', + collectionDisplayName: 'Addresses', + referenceField: 'city', + fields: [{ fieldName: 'city', displayName: 'City', isRelationship: false }], + }); + const invoke = jest + .fn() + .mockResolvedValueOnce({ + tool_calls: [ + { + name: 'select-relation', + args: { relationName: 'Address', reasoning: 'Load address' }, + id: 'c1', + }, + ], + }) + .mockResolvedValueOnce({ + tool_calls: [{ name: 'select-fields', args: { fieldNames: ['City'] }, id: 'c2' }], + }) + .mockResolvedValueOnce({ + tool_calls: [ + { + name: 'select-record-by-content', + args: { recordIndex: 0, reasoning: 'Paris' }, + id: 'c3', + }, + ], + }); + const model = { + bindTools: jest.fn().mockReturnValue({ invoke }), + } as unknown as ExecutionContext['model']; + const runStore = makeMockRunStore(); + const context = makeContext({ + model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ + customers: makeCollectionSchema(), + addresses: addressesSchema, + }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + await executor.execute(); + + const saved = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(saved.pendingData.availableRecordIds).toEqual([ + { recordId: [1], referenceFieldValue: 'Paris' }, + { recordId: [2], referenceFieldValue: 'Lyon' }, + ]); + expect(saved.pendingData.suggestedRecord).toEqual({ + recordId: [1], + referenceFieldValue: 'Paris', + }); + }); + + it('projects and extracts the referenceField on the xToOne path', async () => { + // xToOne path: getRecord projects `@@@` AND `@@@`; + // the executor reads relation[referenceField] from the parent's nested relation linkage. + const ordersSchema = makeCollectionSchema({ + collectionName: 'orders', + collectionDisplayName: 'Orders', + referenceField: 'reference', + fields: [{ fieldName: 'reference', displayName: 'Reference', isRelationship: false }], + }); + + const agentPort = makeMockAgentPort(); + // Override getRecord to return the projected reference field alongside the id. + (agentPort.getRecord as jest.Mock).mockResolvedValue({ + collectionName: 'customers', + recordId: [42], + values: { order: { id: '99', reference: 'ORD-2026-001' } }, + }); + const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'Load order' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ + customers: makeCollectionSchema(), + orders: ordersSchema, + }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + await executor.execute(); + + // Verify the projection includes the reference field. + expect(agentPort.getRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], fields: ['order@@@id', 'order@@@reference'] }, + expect.objectContaining({ id: 1 }), + ); + + const saved = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(saved.pendingData.suggestedRecord).toEqual({ + recordId: ['99'], + referenceFieldValue: 'ORD-2026-001', + }); + }); + + it('falls back to null referenceFieldValue when the related collection has no referenceField configured', async () => { + // Default makeCollectionSchema doesn't set referenceField → executor skips the + // extra projection and writes null on every candidate. + const agentPort = makeMockAgentPort(); + const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'Load order' }); + const runStore = makeMockRunStore(); + const context = makeContext({ model: mockModel.model, agentPort, runStore }); + const executor = new LoadRelatedRecordStepExecutor(context); + + await executor.execute(); + + // No reference-field projection — only the PK. + expect(agentPort.getRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], fields: ['order@@@id'] }, + expect.objectContaining({ id: 1 }), + ); + + const saved = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(saved.pendingData.suggestedRecord).toEqual({ + recordId: ['99'], + referenceFieldValue: null, + }); + }); + }); + describe('trigger before PATCH (Branch A)', () => { it('re-emits awaiting-input when userConfirmation is not yet set', async () => { const agentPort = makeMockAgentPort(); @@ -1269,10 +1661,13 @@ describe('LoadRelatedRecordStepExecutor', () => { it('returns error outcome when saveStepExecution fails after load (Branch A confirmed)', async () => { const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: ['status', 'amount'], - selectedRecordId: [99], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, userConfirmation: { userConfirmed: true }, }); @@ -1303,6 +1698,8 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Order', isRelationship: true, relationType: 'BelongsTo', + relatedCollectionName: 'orders', + relatedPrimaryKey: 'id', }, ], }); @@ -1379,10 +1776,12 @@ describe('LoadRelatedRecordStepExecutor', () => { }); describe('infra error propagation', () => { + // Uses HasMany ('Address') because xToOne reads from the parent record via getRecord, + // not getRelatedData. The infra-error contract is the same for both port methods. it('returns error outcome for getRelatedData infrastructure errors (Branch B)', async () => { const agentPort = makeMockAgentPort(); (agentPort.getRelatedData as jest.Mock).mockRejectedValue(new Error('Connection refused')); - const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'test' }); + const mockModel = makeMockModel({ relationName: 'Address', reasoning: 'test' }); const context = makeContext({ model: mockModel.model, agentPort, @@ -1397,7 +1796,7 @@ describe('LoadRelatedRecordStepExecutor', () => { it('returns error outcome for getRelatedData infrastructure errors (Branch C)', async () => { const agentPort = makeMockAgentPort(); (agentPort.getRelatedData as jest.Mock).mockRejectedValue(new Error('Connection refused')); - const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'test' }); + const mockModel = makeMockModel({ relationName: 'Address', reasoning: 'test' }); const context = makeContext({ model: mockModel.model, agentPort }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -1411,7 +1810,7 @@ describe('LoadRelatedRecordStepExecutor', () => { (agentPort.getRelatedData as jest.Mock).mockRejectedValue( new AgentPortError('getRelatedData', new Error('DB connection lost')), ); - const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'test' }); + const mockModel = makeMockModel({ relationName: 'Address', reasoning: 'test' }); const context = makeContext({ model: mockModel.model, agentPort, @@ -1451,6 +1850,8 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Invoice', isRelationship: true, relationType: 'BelongsTo', + relatedCollectionName: 'invoices', + relatedPrimaryKey: 'id', }, ], }); @@ -1518,9 +1919,10 @@ describe('LoadRelatedRecordStepExecutor', () => { 'run-1', expect.objectContaining({ pendingData: expect.objectContaining({ - displayName: 'Invoice', - name: 'invoice', - selectedRecordId: [55], + suggestedField: { name: 'invoice', displayName: 'Invoice' }, + // xToOne path packs the related PK as a string via split('|') of the + // agent's serialized relation id. + suggestedRecord: cand(['55']), }), selectedRecordRef: expect.objectContaining({ recordId: [99], collectionName: 'orders' }), }), @@ -1639,10 +2041,13 @@ describe('LoadRelatedRecordStepExecutor', () => { it('returns error outcome when saveStepExecution fails on user reject (Branch A)', async () => { const execution = makePendingExecution({ pendingData: { - displayName: 'Order', - name: 'order', - suggestedFields: ['status', 'amount'], - selectedRecordId: [99], + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), }, userConfirmation: { userConfirmed: false }, }); @@ -1670,6 +2075,8 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Order', isRelationship: true, relationType: 'BelongsTo', + relatedCollectionName: 'orders', + relatedPrimaryKey: 'id', }, ], }); @@ -1685,17 +2092,30 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('success'); - expect(agentPort.getRelatedData).toHaveBeenCalledWith( - { collection: 'customers', id: [42], relation: 'order', limit: 1 }, + // BelongsTo → xToOne path: project the relation on the parent record. + expect(agentPort.getRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], fields: ['order@@@id'] }, expect.objectContaining({ id: 1 }), ); + expect(agentPort.getRelatedData).not.toHaveBeenCalled(); }); }); describe('schema caching', () => { - it('fetches getCollectionSchema once per collection even when called twice (Branch B)', async () => { - const workflowPort = makeMockWorkflowPort(); + // Both xToOne and HasMany now fetch the related schema (xToOne reads + // relatedSchema.referenceField for the dropdown label projection). The test + // asserts each schema is fetched at most once per run. + it('fetches getCollectionSchema once per collection (parent + related, no duplicate fetches)', async () => { + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema(), + addresses: makeCollectionSchema({ + collectionName: 'addresses', + collectionDisplayName: 'Addresses', + }), + }); + const mockModel = makeMockModel({ relationName: 'Address', reasoning: 'Load address' }); const context = makeContext({ + model: mockModel.model, workflowPort, stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), }); @@ -1703,7 +2123,10 @@ describe('LoadRelatedRecordStepExecutor', () => { await executor.execute(); - expect(workflowPort.getCollectionSchema).toHaveBeenCalledTimes(1); + // Parent (customers) and related (addresses) — fetched once each, no duplicates. + expect(workflowPort.getCollectionSchema).toHaveBeenCalledTimes(2); + expect(workflowPort.getCollectionSchema).toHaveBeenCalledWith('customers', 'run-1'); + expect(workflowPort.getCollectionSchema).toHaveBeenCalledWith('addresses', 'run-1'); }); }); @@ -1722,9 +2145,10 @@ describe('LoadRelatedRecordStepExecutor', () => { stepIndex: 3, selectedRecordRef: makeRecordRef(), pendingData: { - displayName: 'Invoice', - name: 'invoice', - selectedRecordId: [55], + availableFields: [{ name: 'invoice', displayName: 'Invoice' }], + suggestedField: { name: 'invoice', displayName: 'Invoice' }, + availableRecordIds: [cand([55])], + suggestedRecord: cand([55]), }, }; @@ -1737,6 +2161,8 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Order', isRelationship: true, relationType: 'BelongsTo', + relatedCollectionName: 'orders', + relatedPrimaryKey: 'id', }, ], }); diff --git a/packages/workflow-executor/test/executors/step-execution-formatters.test.ts b/packages/workflow-executor/test/executors/step-execution-formatters.test.ts index 9a79847e02..a11f34029f 100644 --- a/packages/workflow-executor/test/executors/step-execution-formatters.test.ts +++ b/packages/workflow-executor/test/executors/step-execution-formatters.test.ts @@ -38,9 +38,10 @@ describe('StepExecutionFormatters', () => { stepIndex: 1, selectedRecordRef: { collectionName: 'customers', recordId: [42], stepIndex: 0 }, pendingData: { - displayName: 'Address', - name: 'address', - selectedRecordId: [1], + availableFields: [{ name: 'address', displayName: 'Address' }], + suggestedField: { name: 'address', displayName: 'Address' }, + availableRecordIds: [{ recordId: [1], referenceFieldValue: null }], + suggestedRecord: { recordId: [1], referenceFieldValue: null }, }, }; diff --git a/packages/workflow-executor/test/executors/step-summary-builder.test.ts b/packages/workflow-executor/test/executors/step-summary-builder.test.ts index 10dc86fea6..371111da20 100644 --- a/packages/workflow-executor/test/executors/step-summary-builder.test.ts +++ b/packages/workflow-executor/test/executors/step-summary-builder.test.ts @@ -275,9 +275,10 @@ describe('StepSummaryBuilder', () => { stepIndex: 1, selectedRecordRef: { collectionName: 'customers', recordId: [42], stepIndex: 0 }, pendingData: { - displayName: 'Address', - name: 'address', - selectedRecordId: [1], + availableFields: [{ name: 'address', displayName: 'Address' }], + suggestedField: { name: 'address', displayName: 'Address' }, + availableRecordIds: [{ recordId: [1], referenceFieldValue: null }], + suggestedRecord: { recordId: [1], referenceFieldValue: null }, }, }; @@ -462,7 +463,12 @@ describe('StepSummaryBuilder', () => { type: 'load-related-record', stepIndex: 1, selectedRecordRef: { collectionName: 'customers', recordId: [42], stepIndex: 0 }, - pendingData: { displayName: 'Address', name: 'address', selectedRecordId: [1] }, + pendingData: { + availableFields: [{ name: 'address', displayName: 'Address' }], + suggestedField: { name: 'address', displayName: 'Address' }, + availableRecordIds: [{ recordId: [1], referenceFieldValue: null }], + suggestedRecord: { recordId: [1], referenceFieldValue: null }, + }, executionResult: { relation: { name: 'address', displayName: 'Address' }, record: { collectionName: 'addresses', recordId: [1], stepIndex: 1 }, diff --git a/packages/workflow-executor/test/http/pending-data-validators.test.ts b/packages/workflow-executor/test/http/pending-data-validators.test.ts index 72738d04c7..a06549f303 100644 --- a/packages/workflow-executor/test/http/pending-data-validators.test.ts +++ b/packages/workflow-executor/test/http/pending-data-validators.test.ts @@ -155,24 +155,40 @@ describe('patchBodySchemas', () => { }); }); - it('accepts confirmation with both name and selectedRecordId (relation override)', () => { - expect(schema.parse({ userConfirmed: true, name: 'address', selectedRecordId: [7] })).toEqual( - { userConfirmed: true, name: 'address', selectedRecordId: [7] }, + it('accepts confirmation with both fieldDisplayName and selectedRecordId (relation override)', () => { + expect( + schema.parse({ + userConfirmed: true, + fieldDisplayName: 'Address', + selectedRecordId: [7], + }), + ).toEqual({ userConfirmed: true, fieldDisplayName: 'Address', selectedRecordId: [7] }); + }); + + it('rejects fieldDisplayName override on confirm without selectedRecordId — original record ID belongs to a different collection', () => { + expect(() => schema.parse({ userConfirmed: true, fieldDisplayName: 'Address' })).toThrow( + 'selectedRecordId is required when confirming with a relation override', ); }); - it('rejects name override without selectedRecordId — original record ID belongs to a different collection', () => { - expect(() => schema.parse({ userConfirmed: true, name: 'address' })).toThrow( - 'selectedRecordId is required when overriding the relation name', - ); - }); - - it('rejects empty string name — empty string is not a valid relation name', () => { - expect(() => schema.parse({ userConfirmed: true, name: '' })).toThrow(); + it('rejects empty string fieldDisplayName — empty string is not a valid display name', () => { + expect(() => schema.parse({ userConfirmed: true, fieldDisplayName: '' })).toThrow(); }); it('rejects unknown fields (strict schema)', () => { expect(() => schema.parse({ userConfirmed: true, extra: 'leak' })).toThrow(); }); + + // Preview patch: fieldDisplayName alone, no userConfirmed. The executor uses this + // to re-list candidates for a different relation without finalizing the step. + it('accepts a preview patch — fieldDisplayName alone, no userConfirmed', () => { + expect(schema.parse({ fieldDisplayName: 'Address' })).toEqual({ + fieldDisplayName: 'Address', + }); + }); + + it('rejects an empty patch — must carry either userConfirmed or a fieldDisplayName preview', () => { + expect(() => schema.parse({})).toThrow(); + }); }); }); diff --git a/packages/workflow-executor/test/integration/workflow-execution.test.ts b/packages/workflow-executor/test/integration/workflow-execution.test.ts index 9e57a2dabc..5b053d9596 100644 --- a/packages/workflow-executor/test/integration/workflow-execution.test.ts +++ b/packages/workflow-executor/test/integration/workflow-execution.test.ts @@ -79,6 +79,7 @@ const COLLECTION_SCHEMA_WITH_RELATION: CollectionSchema = { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'orders', + relatedPrimaryKey: 'id', }, ], actions: [], @@ -503,9 +504,13 @@ describe('workflow execution (integration)', () => { }); const agentPort = createMockAgentPort(); - agentPort.getRelatedData.mockResolvedValue([ - { collectionName: 'orders', recordId: [99], values: { id: 99, total: 100 } }, - ]); + // BelongsTo → xToOne path: executor reads parent.values..id from getRecord. + // The agent serializes the relation linkage's `id` from the foreign collection's PK. + agentPort.getRecord.mockResolvedValue({ + collectionName: 'customers', + recordId: [42], + values: { order: { id: '99' } }, + }); const { server, runStore } = createIntegrationSetup({ workflowPort, @@ -547,7 +552,9 @@ describe('workflow execution (integration)', () => { type: 'load-related-record', executionResult: { relation: { name: 'order', displayName: 'Order' }, - record: { collectionName: 'orders', recordId: [99], stepIndex: 0 }, + // xToOne packs the related PK as a string via split('|') of the agent's + // serialized relation id. + record: { collectionName: 'orders', recordId: ['99'], stepIndex: 0 }, }, }), ); From d308ca068abc1e2b3a69d9b5af99a4ab980f13f7 Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Fri, 29 May 2026 10:17:43 +0200 Subject: [PATCH 02/11] fix: add coverage --- .../load-related-record-step-executor.test.ts | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) 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 d65498f19a..54edfa0222 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 @@ -266,6 +266,43 @@ describe('LoadRelatedRecordStepExecutor', () => { }), ); }); + + // Guards against a schema-shape bug: the orchestrator always supplies + // `relatedPrimaryKey` for relationship fields, but if it ever lands missing, + // the xToOne path has no way to project `@@@` and must fail loud + // rather than silently mis-project. + it('returns error when relatedPrimaryKey is missing on the relation field', async () => { + const schemaWithoutPk = makeCollectionSchema({ + fields: [ + { + fieldName: 'order', + displayName: 'Order', + isRelationship: true, + relationType: 'BelongsTo', + relatedCollectionName: 'orders', + // relatedPrimaryKey intentionally omitted + }, + ], + }); + const agentPort = makeMockAgentPort(); + const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'test' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ customers: schemaWithoutPk }), + stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + // No agent call should reach the projection — the guard fires before getRecord. + expect(agentPort.getRecord).not.toHaveBeenCalled(); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); }); describe('executionType=FullyAutomated: HasMany — 2 AI calls (Branch B)', () => { @@ -645,6 +682,93 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); + // BelongsToMany falls through to the same to-many candidate path as the default + // branch (neither xToOne nor HasMany). Routes through fetchFirstCandidate -> + // fetchCandidates -> getRelatedData with limit: 1, then picks the first row. + describe('executionType=FullyAutomated: BelongsToMany — load direct (Branch B)', () => { + it('fetches 1 related record via /relationships and returns success', async () => { + const belongsToManySchema = makeCollectionSchema({ + fields: [ + { + fieldName: 'tags', + displayName: 'Tags', + isRelationship: true, + relationType: 'BelongsToMany', + relatedCollectionName: 'tags', + relatedPrimaryKey: 'id', + }, + ], + }); + const agentPort = makeMockAgentPort([ + { collectionName: 'tags', recordId: [7], values: {} }, + ]); + const mockModel = makeMockModel({ relationName: 'Tags', reasoning: 'Load tags' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ customers: belongsToManySchema }), + stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('success'); + // To-many path: /relationships call with limit: 1, no parent-record projection. + expect(agentPort.getRelatedData).toHaveBeenCalledWith( + { collection: 'customers', id: [42], relation: 'tags', limit: 1 }, + expect.objectContaining({ id: 1 }), + ); + expect(agentPort.getRecord).not.toHaveBeenCalled(); + expect(runStore.saveStepExecution).toHaveBeenCalledWith( + 'run-1', + expect.objectContaining({ + executionResult: expect.objectContaining({ + record: expect.objectContaining({ collectionName: 'tags', recordId: [7] }), + }), + }), + ); + }); + + // fetchCandidates throws RelatedRecordNotFoundError when the agent returns an + // empty list. Same user-facing message as the other empty-result paths. + it('returns error when getRelatedData returns an empty array', async () => { + const belongsToManySchema = makeCollectionSchema({ + fields: [ + { + fieldName: 'tags', + displayName: 'Tags', + isRelationship: true, + relationType: 'BelongsToMany', + relatedCollectionName: 'tags', + relatedPrimaryKey: 'id', + }, + ], + }); + const agentPort = makeMockAgentPort([]); + const mockModel = makeMockModel({ relationName: 'Tags', reasoning: 'Load tags' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ customers: belongsToManySchema }), + stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + 'The related record could not be found. It may have been deleted.', + ); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); + }); + describe('without executionType=FullyAutomated: awaiting-input (Branch C)', () => { it('saves AI suggestion in pendingData and returns awaiting-input (single record — no field/record AI calls)', async () => { const agentPort = makeMockAgentPort(); // returns 1 record: orders #99 @@ -1358,6 +1482,33 @@ describe('LoadRelatedRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('error'); }); + + // refreshCandidatesForField guards against a corrupted/partial execution where + // a preview patch lands but the persisted execution carries no pendingData. + // Twin of the "no pending data in confirmation flow" test for the resolve path. + it('returns error when execution exists but pendingData is absent', async () => { + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([ + { + type: 'load-related-record', + stepIndex: 0, + selectedRecordRef: makeRecordRef(), + }, + ]), + }); + const context = makeContext({ + runStore, + incomingPendingData: { fieldDisplayName: 'Address' }, + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + 'An unexpected error occurred while processing this step.', + ); + }); }); // The related collection may have a layout-level `referenceField` (e.g. `name`, From 4dde00d62e6bbc933a90eadf2637545c9a21f4cf Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Fri, 29 May 2026 11:22:13 +0200 Subject: [PATCH 03/11] fix: lint --- .../test/executors/load-related-record-step-executor.test.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) 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 54edfa0222..d0630769cf 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 @@ -699,9 +699,7 @@ describe('LoadRelatedRecordStepExecutor', () => { }, ], }); - const agentPort = makeMockAgentPort([ - { collectionName: 'tags', recordId: [7], values: {} }, - ]); + const agentPort = makeMockAgentPort([{ collectionName: 'tags', recordId: [7], values: {} }]); const mockModel = makeMockModel({ relationName: 'Tags', reasoning: 'Load tags' }); const runStore = makeMockRunStore(); const context = makeContext({ From 9cebc7718dcc49401f22390a2bd1f7372bee5bff Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Mon, 1 Jun 2026 12:12:33 +0200 Subject: [PATCH 04/11] fix: review --- .../src/adapters/agent-client-agent-port.ts | 24 ++- .../load-related-record-step-executor.ts | 89 ++------- .../src/executors/record-step-executor.ts | 8 +- .../src/http/pending-data-validators.ts | 19 +- .../workflow-executor/src/ports/agent-port.ts | 9 +- .../src/types/step-execution-data.ts | 2 +- .../adapters/agent-client-agent-port.test.ts | 174 ++++++++++++++---- .../load-related-record-step-executor.test.ts | 95 ++++++---- .../test/http/pending-data-validators.test.ts | 26 +-- 9 files changed, 268 insertions(+), 178 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 4ee0f8cfa9..2a86641f89 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -110,24 +110,32 @@ export default class AgentClientAgentPort implements AgentPort { }); } - // Returns raw rows as deserialized by agent-client (camelCase keys, no PK extraction). - // The caller resolves the related collection's schema and maps rows → RecordData; keeping - // schema-aware mapping out of the port avoids the silent fallback when the related - // collection isn't in the cache. async getRelatedData( - { collection, id, relation, limit, fields }: GetRelatedDataQuery, + { collection, id, relation, relatedSchema, limit, fields }: GetRelatedDataQuery, user: StepUser, - ): Promise[]> { + ): Promise { return this.callAgent('getRelatedData', async () => { const client = this.createClient(user); - - return client + const rows = await client .collection(collection) .relation(relation, id) .list>({ ...(limit !== null && { pagination: { size: limit, number: 1 } }), ...(fields?.length && { fields }), }); + + return rows.map(row => { + const restored = restoreFieldNames( + row, + relatedSchema.fields.map(f => f.fieldName), + ); + + return { + collectionName: relatedSchema.collectionName, + recordId: relatedSchema.primaryKeyFields.map(f => restored[f] as string | number), + values: restored, + }; + }); }); } diff --git a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts index 788c9a9723..9e91fdbd5f 100644 --- a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts @@ -43,7 +43,7 @@ interface RelationTarget extends RelationRef { relatedCollectionName: string; // Primary key field name on the related collection — supplied by the orchestrator's // schema. Required for the xToOne projection syntax ('@@@'). - relatedPrimaryKey?: string; + computedKey?: string; } export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { @@ -64,13 +64,10 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor(pending, async exec => @@ -82,20 +79,16 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { if (!execution.pendingData) { throw new StepStateError(`Step at index ${this.context.stepIndex} has no pending data`); } const schema = await this.getCollectionSchema(execution.selectedRecordRef.collectionName); - const target = this.buildTarget(schema, fieldDisplayName, execution.selectedRecordRef); + const target = this.buildTarget(schema, fieldName, execution.selectedRecordRef); const { availableRecordIds, suggestedRecord } = await this.collectCandidateIds(target); await this.context.runStore.saveStepExecution(this.context.runId, { @@ -153,7 +146,7 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor ({ recordId: r.recordId, - // `referenceField` is a fieldName on the related collection. fetchRelatedData - // has already restored field names from camelCase, so reading r.values[field] - // works for both `name` and `full_name` style identifiers. Coerce to string — - // the agent may return numbers/dates/etc. for display fields. referenceFieldValue: referenceField ? this.extractReferenceFieldValue(r.values, referenceField) : null, @@ -240,9 +223,6 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { if (target.relationType === 'BelongsTo' || target.relationType === 'HasOne') { return this.fetchXToOneRecordRef(target); @@ -255,14 +235,6 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor@@@`) and, when configured, also the - // reference field for display. The orchestrator supplies both names alongside the - // schema — no extra getCollectionSchema fetch needed here. The agent's JSON:API - // serializer fills the relationship's `data.id` with the *full* related primary key - // (packed with "|" for composite keys) regardless of which fields we project, so - // split('|') gives back the complete recordId. private async fetchXToOneRecordRef(target: RelationTarget): Promise { const candidate = await this.fetchXToOneCandidate(target); @@ -273,24 +245,19 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { - if (!target.relatedPrimaryKey) { + if (!target.computedKey) { throw new StepStateError( `Cannot load xToOne relation "${target.name}" on collection ` + `"${target.selectedRecordRef.collectionName}": missing relatedPrimaryKey in schema.`, ); } - // Resolve the related schema for the optional referenceField. Cached after the first - // 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}`]; + const fields = [`${target.name}@@@${target.computedKey}`]; - if (referenceField && referenceField !== target.relatedPrimaryKey) { + if (referenceField && referenceField !== target.computedKey) { fields.push(`${target.name}@@@${referenceField}`); } @@ -310,10 +277,14 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor, [target.computedKey, referenceField]) + : (relation as Record); + return { recordId: packedId.split('|'), referenceFieldValue: referenceField - ? this.extractReferenceFieldValue(relation as Record, referenceField) + ? this.extractReferenceFieldValue(restoredRelation, referenceField) : null, }; } @@ -328,22 +299,17 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor f.displayName === userConfirmation.fieldDisplayName) + const relationRef = userConfirmation?.fieldName + ? pendingData.availableFields.find(f => f.name === userConfirmation.fieldName) : pendingData.suggestedField; if (!relationRef) { throw new StepStateError( - `Step at index ${this.context.stepIndex} could not resolve relation "${userConfirmation?.fieldDisplayName}" from available fields`, + `Step at index ${this.context.stepIndex} could not resolve relation "${userConfirmation?.fieldName}" from available fields`, ); } const { name, displayName } = relationRef; - // suggestedRecord is a LoadRelatedRecordCandidate; only the recordId is needed here. - // The reference-field value is purely for display in awaiting-input and never persisted - // on the final RecordRef. const selectedRecordId = userConfirmation?.selectedRecordId ?? pendingData.suggestedRecord.recordId; @@ -430,14 +396,12 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { const candidates = await this.fetchCandidates(target, 1); return candidates[0]; } - // Throws RelatedRecordNotFoundError when the result is empty. private async fetchCandidates( target: Pick, limit: number, @@ -453,36 +417,21 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor this.toRecordRef(r)); } - // Calls the agent port and maps raw rows → RecordData using the related collection's schema. - // Schema is resolved by the caller so the cache is warmed via getCollectionSchema (which - // falls back to workflowPort), avoiding the silent ["id"] PK fallback the port used to do. private async fetchRelatedData( target: Pick, relatedSchema: CollectionSchema, limit: number, ): Promise { - const rows = await this.agentPort.getRelatedData( + return this.agentPort.getRelatedData( { collection: target.selectedRecordRef.collectionName, id: target.selectedRecordRef.recordId, relation: target.name, + relatedSchema, limit, }, this.context.user, ); - - return rows.map(row => { - const restored = restoreFieldNames( - row, - relatedSchema.fields.map(f => f.fieldName), - ); - - return { - collectionName: relatedSchema.collectionName, - recordId: relatedSchema.primaryKeyFields.map(f => restored[f] as string | number), - values: restored, - }; - }); } /** Persists the loaded record ref and returns a success outcome. */ diff --git a/packages/workflow-executor/src/executors/record-step-executor.ts b/packages/workflow-executor/src/executors/record-step-executor.ts index 4430dfacc4..987fd8c708 100644 --- a/packages/workflow-executor/src/executors/record-step-executor.ts +++ b/packages/workflow-executor/src/executors/record-step-executor.ts @@ -76,17 +76,17 @@ export default abstract class RecordStepExecutor< return schema; } - protected findField(schema: CollectionSchema, name: string): FieldSchema | undefined { + protected findField(schema: CollectionSchema, displayName: string): FieldSchema | undefined { // LLMs occasionally return formatting variants of field names (e.g. "first_name" for // "firstname", "full-name" for "Full Name") even though the tool schema declares them // as literals. Fall back to a normalized comparison so a cosmetic variation doesn't // fail an otherwise correct step. const normalizeFieldName = (s: string) => s.toLowerCase().replace(/[\s_-]/g, ''); - const normalized = normalizeFieldName(name); + const normalized = normalizeFieldName(displayName); const exact = - schema.fields.find(f => f.displayName === name) ?? - schema.fields.find(f => f.fieldName === name); + schema.fields.find(f => f.displayName === displayName) ?? + schema.fields.find(f => f.fieldName === displayName); if (exact) return exact; const fuzzy = schema.fields.filter( diff --git a/packages/workflow-executor/src/http/pending-data-validators.ts b/packages/workflow-executor/src/http/pending-data-validators.ts index 0c9413c913..2c4f849b45 100644 --- a/packages/workflow-executor/src/http/pending-data-validators.ts +++ b/packages/workflow-executor/src/http/pending-data-validators.ts @@ -26,18 +26,19 @@ const mcpPatchSchema = z.object({ userConfirmed: z.boolean() }).strict(); // Accepts two shapes: // 1. Confirmation patch: `userConfirmed: boolean` (+ optional overrides) — finalizes // the step or skips it. -// 2. Field-preview patch: `fieldDisplayName: string` alone, with `userConfirmed` -// omitted — asks the executor to re-list candidates for a different relation -// WITHOUT finalizing. The executor refreshes pendingData and stays awaiting-input. +// 2. Field-preview patch: `fieldName: string` alone, with `userConfirmed` omitted — +// asks the executor to re-list candidates for a different relation WITHOUT +// finalizing. The executor refreshes pendingData and stays awaiting-input. // Required when the frontend lets the user switch relations: the IDs originally // stored under `availableRecordIds` belong to the AI-suggested relation only. const loadRelatedRecordPatchSchema = z .object({ userConfirmed: z.boolean().optional(), // User may intentionally switch to a different relation than the one the AI selected. - // 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(), + // Sent as the technical fieldName (matches CollectionSchemaField.fieldName from the + // orchestrator); the executor re-derives displayName + relatedCollectionName from + // the live schema when processing the confirmation. + fieldName: z.string().min(1).optional(), // User may override the AI-selected record; must be non-empty when provided. // Required when confirming with a relation override — the original record ID // belongs to a different collection and cannot be reused for the new relation. @@ -49,10 +50,10 @@ const loadRelatedRecordPatchSchema = z .strict() .refine( data => { - // Preview patch (no confirm): fieldDisplayName alone is sufficient. - if (data.userConfirmed === undefined) return data.fieldDisplayName !== undefined; + // 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.fieldDisplayName !== undefined) return data.selectedRecordId !== undefined; + if (data.fieldName !== undefined) return data.selectedRecordId !== undefined; return true; }, diff --git a/packages/workflow-executor/src/ports/agent-port.ts b/packages/workflow-executor/src/ports/agent-port.ts index aee401493a..866a580cf2 100644 --- a/packages/workflow-executor/src/ports/agent-port.ts +++ b/packages/workflow-executor/src/ports/agent-port.ts @@ -1,7 +1,7 @@ /** @draft Types derived from the workflow-executor spec -- subject to change. */ import type { StepUser } from '../types/execution-context'; -import type { RecordData } from '../types/validated/collection'; +import type { CollectionSchema, RecordData } from '../types/validated/collection'; export type Id = string | number; @@ -15,6 +15,9 @@ export type GetRelatedDataQuery = { collection: string; id: Id[]; relation: string; + // Schema of the RELATED collection — supplied by the caller so the port can extract the + // record ID and restore original field names without consulting any cache. + relatedSchema: CollectionSchema; fields?: string[]; } & Limit; @@ -25,9 +28,7 @@ export type GetActionFormInfoQuery = { collection: string; action: string; id: I export interface AgentPort { getRecord(query: GetRecordQuery, user: StepUser): Promise; updateRecord(query: UpdateRecordQuery, user: StepUser): Promise; - // 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[]>; + getRelatedData(query: GetRelatedDataQuery, user: StepUser): Promise; executeAction(query: ExecuteActionQuery, user: StepUser): Promise; // Old Ruby agents with hooks.load=false return 404; agent-client falls back to the fields // passed via ActionEndpointsByCollection (populated from the orchestrator's schema). diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index b3f2d2fb9c..084470bd1e 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -140,7 +140,7 @@ export interface RecordStepExecutionData extends BaseStepExecutionData { // (configured but value missing) collapse to the same "fall back to id" rendering. export interface LoadRelatedRecordCandidate { recordId: Array; - referenceFieldValue?: string | null; + referenceFieldValue: string | null; } export interface LoadRelatedRecordPendingData { 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 840ece3bf9..dc16cd539b 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 @@ -247,31 +247,127 @@ describe('AgentClientAgentPort', () => { }); describe('getRelatedData', () => { - // Contract: this port is now a thin HTTP adapter for related-data lists. It returns the - // raw rows from agent-client (camelCase keys, no PK extraction, no collectionName). - // Schema resolution and the row → RecordData mapping happen in the executor — that's - // where the related collection's schema can be fetched on demand via the workflow port. - it('returns raw rows from agent-client untouched', async () => { - const rows = [ + const postsSchema = { + collectionName: 'posts', + collectionDisplayName: 'Posts', + primaryKeyFields: ['id'], + fields: [ + { fieldName: 'id', displayName: 'id', isRelationship: false, type: 'Number' as const }, + { + fieldName: 'title', + displayName: 'title', + isRelationship: false, + type: 'String' as const, + }, + ], + actions: [], + }; + + it('maps raw rows to RecordData using the supplied related schema', async () => { + mockRelation.list.mockResolvedValue([ { id: 10, title: 'Post A' }, { id: 11, title: 'Post B' }, - ]; - mockRelation.list.mockResolvedValue(rows); + ]); const result = await port.getRelatedData( - { collection: 'users', id: [42], relation: 'posts', limit: null }, + { + collection: 'users', + id: [42], + relation: 'posts', + relatedSchema: postsSchema, + limit: null, + }, user, ); expect(mockCollection.relation).toHaveBeenCalledWith('posts', [42]); - expect(result).toEqual(rows); + expect(result).toEqual([ + { collectionName: 'posts', recordId: [10], values: { id: 10, title: 'Post A' } }, + { collectionName: 'posts', recordId: [11], values: { id: 11, title: 'Post B' } }, + ]); + }); + + it('restores snake_case field names from camelCase deserialized rows', async () => { + const snakeSchema = { + ...postsSchema, + primaryKeyFields: ['post_id'], + fields: [ + { + fieldName: 'post_id', + displayName: 'Post id', + isRelationship: false, + type: 'Number' as const, + }, + { + fieldName: 'created_at', + displayName: 'Created at', + isRelationship: false, + type: 'Date' as const, + }, + ], + }; + mockRelation.list.mockResolvedValue([{ postId: 99, createdAt: '2024-01-01' }]); + + const result = await port.getRelatedData( + { + collection: 'users', + id: [42], + relation: 'posts', + relatedSchema: snakeSchema, + limit: null, + }, + user, + ); + + expect(result).toEqual([ + { + collectionName: 'posts', + recordId: [99], + values: { post_id: 99, created_at: '2024-01-01' }, + }, + ]); + }); + + it('extracts composite primary keys in the order declared by the schema', async () => { + const compositeSchema = { + ...postsSchema, + primaryKeyFields: ['tenantId', 'postId'], + fields: [ + { + fieldName: 'tenantId', + displayName: 'Tenant', + isRelationship: false, + type: 'String' as const, + }, + { + fieldName: 'postId', + displayName: 'Post', + isRelationship: false, + type: 'Number' as const, + }, + ], + }; + mockRelation.list.mockResolvedValue([{ tenantId: 'acme', postId: 7 }]); + + const result = await port.getRelatedData( + { + collection: 'users', + id: [42], + relation: 'posts', + relatedSchema: compositeSchema, + limit: null, + }, + user, + ); + + expect(result[0].recordId).toEqual(['acme', 7]); }); it('applies pagination when limit is a number', async () => { mockRelation.list.mockResolvedValue([{ id: 10, title: 'Post A' }]); await port.getRelatedData( - { collection: 'users', id: [42], relation: 'posts', limit: 5 }, + { collection: 'users', id: [42], relation: 'posts', relatedSchema: postsSchema, limit: 5 }, user, ); @@ -284,7 +380,13 @@ describe('AgentClientAgentPort', () => { mockRelation.list.mockResolvedValue([]); await port.getRelatedData( - { collection: 'users', id: [42], relation: 'posts', limit: null }, + { + collection: 'users', + id: [42], + relation: 'posts', + relatedSchema: postsSchema, + limit: null, + }, user, ); @@ -296,7 +398,13 @@ describe('AgentClientAgentPort', () => { expect( await port.getRelatedData( - { collection: 'users', id: [42], relation: 'posts', limit: null }, + { + collection: 'users', + id: [42], + relation: 'posts', + relatedSchema: postsSchema, + limit: null, + }, user, ), ).toEqual([]); @@ -306,7 +414,14 @@ describe('AgentClientAgentPort', () => { mockRelation.list.mockResolvedValue([{ id: 10, title: 'Post A' }]); await port.getRelatedData( - { collection: 'users', id: [42], relation: 'posts', limit: null, fields: ['title'] }, + { + collection: 'users', + id: [42], + relation: 'posts', + relatedSchema: postsSchema, + limit: null, + fields: ['title'], + }, user, ); @@ -319,7 +434,13 @@ describe('AgentClientAgentPort', () => { mockRelation.list.mockResolvedValue([{ id: 10 }]); await port.getRelatedData( - { collection: 'users', id: [42], relation: 'posts', limit: null }, + { + collection: 'users', + id: [42], + relation: 'posts', + relatedSchema: postsSchema, + limit: null, + }, user, ); @@ -327,29 +448,6 @@ describe('AgentClientAgentPort', () => { expect.not.objectContaining({ fields: expect.anything() }), ); }); - - it('does not consult the schema cache for the related collection', async () => { - // No 'posts' entry in the cache — the port must still succeed. The fallback - // warn-and-default-to-["id"] used to live here; that logic now belongs to the - // executor, which routes through getCollectionSchema (cache + workflow port). - const cache = new SchemaCache(); - const localPort = new AgentClientAgentPort({ - agentUrl: 'http://agent', - authSecret: 'secret', - schemaCache: cache, - }); - const warn = jest.spyOn(console, 'warn').mockImplementation(() => {}); - mockRelation.list.mockResolvedValue([{ postId: 99, createdAt: '2024-01-01' }]); - - const result = await localPort.getRelatedData( - { collection: 'users', id: [42], relation: 'posts', limit: null }, - user, - ); - - expect(result).toEqual([{ postId: 99, createdAt: '2024-01-01' }]); - expect(warn).not.toHaveBeenCalled(); - warn.mockRestore(); - }); }); describe('executeAction', () => { 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 d0630769cf..ee78629228 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 @@ -51,27 +51,7 @@ function makeRelatedRecordData(overrides: Partial = {}): RecordData }; } -/** - * The agent port now returns raw rows (no PK extraction). For test ergonomics we keep - * the RecordData[] shape in fixtures and translate here: each RecordData becomes a raw - * row by inlining the recordId under the PK field name (default 'id', or first - * primaryKeyField when provided) alongside its values. - */ -function toAgentRows( - records: RecordData[], - primaryKeyFields: string[] = ['id'], -): Record[] { - return records.map(r => { - const pkValues = Object.fromEntries(primaryKeyFields.map((field, i) => [field, r.recordId[i]])); - - return { ...pkValues, ...r.values }; - }); -} - -function makeMockAgentPort( - relatedData: RecordData[] = [makeRelatedRecordData()], - primaryKeyFields: string[] = ['id'], -): AgentPort { +function makeMockAgentPort(relatedData: RecordData[] = [makeRelatedRecordData()]): AgentPort { // xToOne path uses getRecord(parent, fields: ['@@@']) and reads // parent.values[].id (jsonapi-serializer materializes the relationship // linkage as a nested object on the parent). The mock reflects this contract by @@ -89,7 +69,7 @@ function makeMockAgentPort( return { getRecord, updateRecord: jest.fn(), - getRelatedData: jest.fn().mockResolvedValue(toAgentRows(relatedData, primaryKeyFields)), + getRelatedData: jest.fn().mockResolvedValue(relatedData), executeAction: jest.fn(), } as unknown as AgentPort; } @@ -386,7 +366,13 @@ describe('LoadRelatedRecordStepExecutor', () => { // Fetches 50 candidates (HasMany) expect(agentPort.getRelatedData).toHaveBeenCalledWith( - { collection: 'customers', id: [42], relation: 'address', limit: 50 }, + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'address', + limit: 50, + relatedSchema: expect.objectContaining({ collectionName: 'addresses' }), + }), expect.objectContaining({ id: 1 }), ); @@ -716,7 +702,13 @@ describe('LoadRelatedRecordStepExecutor', () => { expect(result.stepOutcome.status).toBe('success'); // To-many path: /relationships call with limit: 1, no parent-record projection. expect(agentPort.getRelatedData).toHaveBeenCalledWith( - { collection: 'customers', id: [42], relation: 'tags', limit: 1 }, + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'tags', + limit: 1, + relatedSchema: expect.objectContaining({ collectionName: 'tags' }), + }), expect.objectContaining({ id: 1 }), ); expect(agentPort.getRecord).not.toHaveBeenCalled(); @@ -1093,7 +1085,7 @@ describe('LoadRelatedRecordStepExecutor', () => { runStore, incomingPendingData: { userConfirmed: true, - fieldDisplayName: 'Address', + fieldName: 'address', selectedRecordId: [7], }, }); @@ -1319,7 +1311,7 @@ describe('LoadRelatedRecordStepExecutor', () => { // The frontend lets the user switch to a different relation before confirming. To // populate the new relation's `availableRecordIds`, it POSTs a "preview" patch: - // `{ fieldDisplayName }` with no `userConfirmed`. The executor re-lists candidates, + // `{ fieldName }` with no `userConfirmed`. The executor re-lists candidates, // refreshes pendingData, clears userConfirmation, and stays awaiting-input. describe('field-preview patch (Branch A — no confirm)', () => { it('re-lists candidates for the new relation and stays awaiting-input', async () => { @@ -1378,7 +1370,7 @@ describe('LoadRelatedRecordStepExecutor', () => { agentPort, runStore, workflowPort, - incomingPendingData: { fieldDisplayName: 'Address' }, + incomingPendingData: { fieldName: 'address' }, }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -1394,7 +1386,7 @@ describe('LoadRelatedRecordStepExecutor', () => { expect.objectContaining({ type: 'load-related-record', // userConfirmation cleared so the next bodyless trigger re-emits awaiting-input - // cleanly via handleConfirmationFlow (no stale fieldDisplayName ghost-confirms). + // cleanly via handleConfirmationFlow (no stale fieldName ghost-confirms). userConfirmation: undefined, pendingData: expect.objectContaining({ // availableFields is immutable — only suggestedField + candidates change. @@ -1436,7 +1428,7 @@ describe('LoadRelatedRecordStepExecutor', () => { agentPort, runStore, workflowPort, - incomingPendingData: { fieldDisplayName: 'Order' }, + incomingPendingData: { fieldName: 'order' }, }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -1472,7 +1464,7 @@ describe('LoadRelatedRecordStepExecutor', () => { const context = makeContext({ agentPort, runStore, - incomingPendingData: { fieldDisplayName: 'NotAField' }, + incomingPendingData: { fieldName: 'notAField' }, }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -1496,7 +1488,7 @@ describe('LoadRelatedRecordStepExecutor', () => { }); const context = makeContext({ runStore, - incomingPendingData: { fieldDisplayName: 'Address' }, + incomingPendingData: { fieldName: 'address' }, }); const executor = new LoadRelatedRecordStepExecutor(context); @@ -1624,6 +1616,47 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); + // Regression: jsonapi-serializer emits the nested relation linkage with camelCased + // attribute keys (full_name → fullName). The executor must restore those keys + // before reading values[referenceField], otherwise snake_case referenceFields + // silently resolve to undefined and the dropdown loses its label. + it('restores camelCased keys on the nested relation linkage when referenceField is snake_case', async () => { + const ordersSchema = makeCollectionSchema({ + collectionName: 'orders', + collectionDisplayName: 'Orders', + referenceField: 'full_name', + fields: [{ fieldName: 'full_name', displayName: 'Full Name', isRelationship: false }], + }); + + const agentPort = makeMockAgentPort(); + // Mock the real agent-client deserialization shape: camelCased keys. + (agentPort.getRecord as jest.Mock).mockResolvedValue({ + collectionName: 'customers', + recordId: [42], + values: { order: { id: '99', fullName: 'John Doe' } }, + }); + const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'Load order' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ + customers: makeCollectionSchema(), + orders: ordersSchema, + }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + await executor.execute(); + + const saved = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(saved.pendingData.suggestedRecord).toEqual({ + recordId: ['99'], + referenceFieldValue: 'John Doe', + }); + }); + it('falls back to null referenceFieldValue when the related collection has no referenceField configured', async () => { // Default makeCollectionSchema doesn't set referenceField → executor skips the // extra projection and writes null on every candidate. diff --git a/packages/workflow-executor/test/http/pending-data-validators.test.ts b/packages/workflow-executor/test/http/pending-data-validators.test.ts index a06549f303..33ab88a942 100644 --- a/packages/workflow-executor/test/http/pending-data-validators.test.ts +++ b/packages/workflow-executor/test/http/pending-data-validators.test.ts @@ -155,39 +155,39 @@ describe('patchBodySchemas', () => { }); }); - it('accepts confirmation with both fieldDisplayName and selectedRecordId (relation override)', () => { + it('accepts confirmation with both fieldName and selectedRecordId (relation override)', () => { expect( schema.parse({ userConfirmed: true, - fieldDisplayName: 'Address', + fieldName: 'address', selectedRecordId: [7], }), - ).toEqual({ userConfirmed: true, fieldDisplayName: 'Address', selectedRecordId: [7] }); + ).toEqual({ userConfirmed: true, fieldName: 'address', selectedRecordId: [7] }); }); - it('rejects fieldDisplayName override on confirm without selectedRecordId — original record ID belongs to a different collection', () => { - expect(() => schema.parse({ userConfirmed: true, fieldDisplayName: 'Address' })).toThrow( + it('rejects fieldName override on confirm without selectedRecordId — original record ID belongs to a different collection', () => { + expect(() => schema.parse({ userConfirmed: true, fieldName: 'address' })).toThrow( 'selectedRecordId is required when confirming with a relation override', ); }); - it('rejects empty string fieldDisplayName — empty string is not a valid display name', () => { - expect(() => schema.parse({ userConfirmed: true, fieldDisplayName: '' })).toThrow(); + it('rejects empty string fieldName — empty string is not a valid field name', () => { + expect(() => schema.parse({ userConfirmed: true, fieldName: '' })).toThrow(); }); it('rejects unknown fields (strict schema)', () => { expect(() => schema.parse({ userConfirmed: true, extra: 'leak' })).toThrow(); }); - // Preview patch: fieldDisplayName alone, no userConfirmed. The executor uses this - // to re-list candidates for a different relation without finalizing the step. - it('accepts a preview patch — fieldDisplayName alone, no userConfirmed', () => { - expect(schema.parse({ fieldDisplayName: 'Address' })).toEqual({ - fieldDisplayName: 'Address', + // Preview patch: fieldName alone, no userConfirmed. The executor uses this to + // re-list candidates for a different relation without finalizing the step. + it('accepts a preview patch — fieldName alone, no userConfirmed', () => { + expect(schema.parse({ fieldName: 'address' })).toEqual({ + fieldName: 'address', }); }); - it('rejects an empty patch — must carry either userConfirmed or a fieldDisplayName preview', () => { + it('rejects an empty patch — must carry either userConfirmed or a fieldName preview', () => { expect(() => schema.parse({})).toThrow(); }); }); From d5b0329a59b3cac74a274f196d1db98357d20ef7 Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Mon, 1 Jun 2026 13:21:55 +0200 Subject: [PATCH 05/11] fix: add getSingleRelatedData to agent port --- .../src/adapters/agent-client-agent-port.ts | 37 ++++ .../load-related-record-step-executor.ts | 36 +--- .../workflow-executor/src/ports/agent-port.ts | 21 ++ .../adapters/agent-client-agent-port.test.ts | 181 +++++++++++++++++ .../load-related-record-step-executor.test.ts | 189 ++++++------------ .../integration/workflow-execution.test.ts | 13 +- 6 files changed, 317 insertions(+), 160 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 2a86641f89..51b9d4360b 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -4,6 +4,7 @@ import type { GetActionFormInfoQuery, GetRecordQuery, GetRelatedDataQuery, + GetSingleRelatedDataQuery, UpdateRecordQuery, } from '../ports/agent-port'; import type SchemaCache from '../schema-cache'; @@ -139,6 +140,42 @@ export default class AgentClientAgentPort implements AgentPort { }); } + // xToOne relations have no /relationships/ route on the agent. We read the + // parent record with a `@@@` projection and unpack the relation linkage + // jsonapi-serializer emits as a nested object on the parent (with the related PK packed + // under "id" when composite). + async getSingleRelatedData( + { collection, id, relation, relatedSchema, fields }: GetSingleRelatedDataQuery, + user: StepUser, + ): Promise { + return this.callAgent('getSingleRelatedData', async () => { + const projectedFields = Array.from( + new Set([...relatedSchema.primaryKeyFields, ...(fields ?? [])]), + ); + const parent = await this.getRecord( + { + collection, + id, + fields: projectedFields.map(f => `${relation}@@@${f}`), + }, + user, + ); + + const linkage = parent.values[relation] as Record | null | undefined; + const packedId = linkage?.id as string | undefined; + + if (!linkage || !packedId) return null; + + const restored = restoreFieldNames(linkage, projectedFields); + + return { + collectionName: relatedSchema.collectionName, + recordId: packedId.split('|'), + values: restored, + }; + }); + } + async executeAction( { collection, action, id }: ExecuteActionQuery, user: StepUser, diff --git a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts index 9e91fdbd5f..39c73736e1 100644 --- a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts @@ -11,7 +11,6 @@ import type { LoadRelatedRecordStepDefinition } from '../types/validated/step-de import { DynamicStructuredTool, HumanMessage, SystemMessage } from '@forestadmin/ai-proxy'; import { z } from 'zod'; -import { restoreFieldNames } from '../adapters/agent-client-agent-port'; import { InvalidAIResponseError, InvalidPreRecordedArgsError, @@ -41,9 +40,6 @@ interface RelationTarget extends RelationRef { selectedRecordRef: RecordRef; relationType?: 'BelongsTo' | 'HasMany' | 'HasOne' | 'BelongsToMany'; relatedCollectionName: string; - // Primary key field name on the related collection — supplied by the orchestrator's - // schema. Required for the xToOne projection syntax ('@@@'). - computedKey?: string; } export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { @@ -146,7 +142,6 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { - if (!target.computedKey) { - throw new StepStateError( - `Cannot load xToOne relation "${target.name}" on collection ` + - `"${target.selectedRecordRef.collectionName}": missing relatedPrimaryKey in schema.`, - ); - } - const relatedSchema = await this.getCollectionSchema(target.relatedCollectionName); const referenceField = relatedSchema.referenceField ?? null; - const fields = [`${target.name}@@@${target.computedKey}`]; - - if (referenceField && referenceField !== target.computedKey) { - fields.push(`${target.name}@@@${referenceField}`); - } - const parent = await this.agentPort.getRecord( + const candidate = await this.agentPort.getSingleRelatedData( { collection: target.selectedRecordRef.collectionName, id: target.selectedRecordRef.recordId, - fields, + relation: target.name, + relatedSchema, + ...(referenceField && { fields: [referenceField] }), }, this.context.user, ); - const relation = parent.values[target.name] as Record | null | undefined; - const packedId = relation?.id as string | undefined; - - if (!packedId) { + if (!candidate) { throw new RelatedRecordNotFoundError(target.selectedRecordRef.collectionName, target.name); } - const restoredRelation = referenceField - ? restoreFieldNames(relation as Record, [target.computedKey, referenceField]) - : (relation as Record); - return { - recordId: packedId.split('|'), + recordId: candidate.recordId, referenceFieldValue: referenceField - ? this.extractReferenceFieldValue(restoredRelation, referenceField) + ? this.extractReferenceFieldValue(candidate.values, referenceField) : null, }; } diff --git a/packages/workflow-executor/src/ports/agent-port.ts b/packages/workflow-executor/src/ports/agent-port.ts index 866a580cf2..5fa71a042e 100644 --- a/packages/workflow-executor/src/ports/agent-port.ts +++ b/packages/workflow-executor/src/ports/agent-port.ts @@ -21,6 +21,22 @@ export type GetRelatedDataQuery = { fields?: string[]; } & Limit; +// xToOne relations (BelongsTo / HasOne) — the agent does not serve +// /forest///relationships/ for these; the port instead reads +// the parent record with a `@@@` projection, then unpacks the relation +// linkage embedded on the parent. +export type GetSingleRelatedDataQuery = { + collection: string; + id: Id[]; + relation: string; + // Schema of the RELATED collection — needed to extract the record ID and (when set) + // include the referenceField in the projection. + relatedSchema: CollectionSchema; + // Extra fields to project on the related side, beyond the related collection's PK. + // Pass referenceField here when the caller wants to read it from the linkage. + fields?: string[]; +}; + export type ExecuteActionQuery = { collection: string; action: string; id?: Id[] }; export type GetActionFormInfoQuery = { collection: string; action: string; id: Id[] }; @@ -29,6 +45,11 @@ export interface AgentPort { getRecord(query: GetRecordQuery, user: StepUser): Promise; updateRecord(query: UpdateRecordQuery, user: StepUser): Promise; getRelatedData(query: GetRelatedDataQuery, user: StepUser): Promise; + // Returns null when the parent has no related record (xToOne with no linkage). + getSingleRelatedData( + query: GetSingleRelatedDataQuery, + user: StepUser, + ): Promise; executeAction(query: ExecuteActionQuery, user: StepUser): Promise; // Old Ruby agents with hooks.load=false return 404; agent-client falls back to the fields // passed via ActionEndpointsByCollection (populated from the orchestrator's schema). 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 dc16cd539b..2b2d7d246b 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 @@ -450,6 +450,187 @@ describe('AgentClientAgentPort', () => { }); }); + describe('getSingleRelatedData', () => { + // xToOne relations don't expose /relationships/ on the agent. The port reads + // the parent record with a `@@@` projection and unpacks the linkage + // that jsonapi-serializer emits as a nested object on the parent. + const ordersSchema = { + collectionName: 'orders', + collectionDisplayName: 'Orders', + primaryKeyFields: ['id'], + fields: [ + { fieldName: 'id', displayName: 'id', isRelationship: false, type: 'Number' as const }, + { + fieldName: 'reference', + displayName: 'Reference', + isRelationship: false, + type: 'String' as const, + }, + ], + actions: [], + }; + + it('projects the related PK on the parent and unpacks the linkage as RecordData', async () => { + mockCollection.list.mockResolvedValue([{ order: { id: '99' } }]); + + const result = await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'order', + relatedSchema: ordersSchema, + }, + user, + ); + + expect(mockCollection.list).toHaveBeenCalledWith( + expect.objectContaining({ fields: ['order@@@id'] }), + ); + expect(result).toEqual({ + collectionName: 'orders', + recordId: ['99'], + values: { id: '99' }, + }); + }); + + it('adds extra projections for caller-supplied fields (e.g. referenceField)', async () => { + mockCollection.list.mockResolvedValue([{ order: { id: '99', reference: 'ORD-2026-001' } }]); + + const result = await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'order', + relatedSchema: ordersSchema, + fields: ['reference'], + }, + user, + ); + + expect(mockCollection.list).toHaveBeenCalledWith( + expect.objectContaining({ fields: ['order@@@id', 'order@@@reference'] }), + ); + expect(result?.values).toEqual({ id: '99', reference: 'ORD-2026-001' }); + }); + + it('deduplicates the PK if the caller passes it again in fields', async () => { + mockCollection.list.mockResolvedValue([{ order: { id: '99' } }]); + + await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'order', + relatedSchema: ordersSchema, + fields: ['id'], + }, + user, + ); + + expect(mockCollection.list).toHaveBeenCalledWith( + expect.objectContaining({ fields: ['order@@@id'] }), + ); + }); + + // Regression: jsonapi-serializer emits the nested linkage with camelCased attribute + // keys (full_name → fullName). The adapter must restore those keys before returning + // values, otherwise snake_case referenceFields silently resolve to undefined upstream. + it('restores camelCased keys on the linkage when fields are snake_case', async () => { + const snakeSchema = { + ...ordersSchema, + fields: [ + { fieldName: 'id', displayName: 'id', isRelationship: false, type: 'Number' as const }, + { + fieldName: 'full_name', + displayName: 'Full name', + isRelationship: false, + type: 'String' as const, + }, + ], + }; + mockCollection.list.mockResolvedValue([{ order: { id: '99', fullName: 'John Doe' } }]); + + const result = await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'order', + relatedSchema: snakeSchema, + fields: ['full_name'], + }, + user, + ); + + expect(result?.values).toEqual({ id: '99', full_name: 'John Doe' }); + }); + + it('splits composite PKs from the packed "id" linkage', async () => { + const compositeSchema = { + ...ordersSchema, + primaryKeyFields: ['tenantId', 'orderId'], + fields: [ + { + fieldName: 'tenantId', + displayName: 'Tenant', + isRelationship: false, + type: 'String' as const, + }, + { + fieldName: 'orderId', + displayName: 'Order', + isRelationship: false, + type: 'Number' as const, + }, + ], + }; + mockCollection.list.mockResolvedValue([{ order: { id: 'acme|7' } }]); + + const result = await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'order', + relatedSchema: compositeSchema, + }, + user, + ); + + expect(result?.recordId).toEqual(['acme', '7']); + }); + + it('returns null when the parent has no linkage to the xToOne relation', async () => { + mockCollection.list.mockResolvedValue([{ order: null }]); + + const result = await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'order', + relatedSchema: ordersSchema, + }, + user, + ); + + expect(result).toBeNull(); + }); + + it('returns null when the linkage object is present but has no id', async () => { + mockCollection.list.mockResolvedValue([{ order: {} }]); + + const result = await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'order', + relatedSchema: ordersSchema, + }, + user, + ); + + expect(result).toBeNull(); + }); + }); + describe('executeAction', () => { it('should forward the RecordId array to agent-client and call execute', async () => { mockAction.execute.mockResolvedValue({ success: 'done' }); 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 ee78629228..d104636fb2 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 @@ -52,24 +52,18 @@ function makeRelatedRecordData(overrides: Partial = {}): RecordData } function makeMockAgentPort(relatedData: RecordData[] = [makeRelatedRecordData()]): AgentPort { - // xToOne path uses getRecord(parent, fields: ['@@@']) and reads - // parent.values[].id (jsonapi-serializer materializes the relationship - // linkage as a nested object on the parent). The mock reflects this contract by - // exposing the first relatedData[0]'s recordId joined with "|" under the relation - // name extracted from the @@@ projection. Tests can override per-call. - const getRecord = jest.fn(async ({ fields }: { fields?: string[] }) => { - const projection = fields?.[0]; - const relationName = projection?.split('@@@')[0]; - const packedId = relatedData[0]?.recordId.map(String).join('|'); - const values = relationName && packedId ? { [relationName]: { id: packedId } } : {}; - - return { collectionName: 'parent', recordId: [], values }; - }); + // xToOne path uses getSingleRelatedData; mock returns the first relatedData entry shaped + // as a RecordData with the recordId stringified, mirroring the real adapter which packs + // composite ids via "|" and splits them back into strings. Tests can override per-call. + const first = relatedData[0]; + const xToOneCandidate = first ? { ...first, recordId: first.recordId.map(String) } : null; + const getSingleRelatedData = jest.fn(async () => xToOneCandidate); return { - getRecord, + getRecord: jest.fn(), updateRecord: jest.fn(), getRelatedData: jest.fn().mockResolvedValue(relatedData), + getSingleRelatedData, executeAction: jest.fn(), } as unknown as AgentPort; } @@ -220,9 +214,14 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('success'); - // BelongsTo: project the relation on the parent record; no /relationships call. - expect(agentPort.getRecord).toHaveBeenCalledWith( - { collection: 'customers', id: [42], fields: ['order@@@id'] }, + // BelongsTo: port's xToOne method; no /relationships call. + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'order', + relatedSchema: expect.objectContaining({ collectionName: 'orders' }), + }), expect.objectContaining({ id: 1 }), ); expect(agentPort.getRelatedData).not.toHaveBeenCalled(); @@ -246,43 +245,6 @@ describe('LoadRelatedRecordStepExecutor', () => { }), ); }); - - // Guards against a schema-shape bug: the orchestrator always supplies - // `relatedPrimaryKey` for relationship fields, but if it ever lands missing, - // the xToOne path has no way to project `@@@` and must fail loud - // rather than silently mis-project. - it('returns error when relatedPrimaryKey is missing on the relation field', async () => { - const schemaWithoutPk = makeCollectionSchema({ - fields: [ - { - fieldName: 'order', - displayName: 'Order', - isRelationship: true, - relationType: 'BelongsTo', - relatedCollectionName: 'orders', - // relatedPrimaryKey intentionally omitted - }, - ], - }); - const agentPort = makeMockAgentPort(); - const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'test' }); - const runStore = makeMockRunStore(); - const context = makeContext({ - model: mockModel.model, - agentPort, - runStore, - workflowPort: makeMockWorkflowPort({ customers: schemaWithoutPk }), - stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), - }); - const executor = new LoadRelatedRecordStepExecutor(context); - - const result = await executor.execute(); - - expect(result.stepOutcome.status).toBe('error'); - // No agent call should reach the projection — the guard fires before getRecord. - expect(agentPort.getRecord).not.toHaveBeenCalled(); - expect(runStore.saveStepExecution).not.toHaveBeenCalled(); - }); }); describe('executionType=FullyAutomated: HasMany — 2 AI calls (Branch B)', () => { @@ -650,10 +612,14 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('success'); - // HasOne uses the same xToOne path as BelongsTo: project the relation on the - // parent and split the packed id. No /relationships call. - expect(agentPort.getRecord).toHaveBeenCalledWith( - { collection: 'customers', id: [42], fields: ['profile@@@id'] }, + // HasOne uses the same xToOne path as BelongsTo. No /relationships call. + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'profile', + relatedSchema: expect.objectContaining({ collectionName: 'profiles' }), + }), expect.objectContaining({ id: 1 }), ); expect(agentPort.getRelatedData).not.toHaveBeenCalled(); @@ -711,7 +677,7 @@ describe('LoadRelatedRecordStepExecutor', () => { }), expect.objectContaining({ id: 1 }), ); - expect(agentPort.getRecord).not.toHaveBeenCalled(); + expect(agentPort.getSingleRelatedData).not.toHaveBeenCalled(); expect(runStore.saveStepExecution).toHaveBeenCalledWith( 'run-1', expect.objectContaining({ @@ -770,9 +736,14 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('awaiting-input'); - // BelongsTo → xToOne path: project the relation on the parent. No /relationships call. - expect(agentPort.getRecord).toHaveBeenCalledWith( - { collection: 'customers', id: [42], fields: ['order@@@id'] }, + // BelongsTo → xToOne path. No /relationships call. + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'order', + relatedSchema: expect.objectContaining({ collectionName: 'orders' }), + }), expect.objectContaining({ id: 1 }), ); expect(agentPort.getRelatedData).not.toHaveBeenCalled(); @@ -1435,9 +1406,14 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('awaiting-input'); - // xToOne path goes through getRecord with `@@@` projection. - expect(agentPort.getRecord).toHaveBeenCalledWith( - { collection: 'customers', id: [42], fields: ['order@@@id'] }, + // xToOne path goes through the port's getSingleRelatedData method. + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'order', + relatedSchema: expect.objectContaining({ collectionName: 'orders' }), + }), expect.objectContaining({ id: 1 }), ); expect(agentPort.getRelatedData).not.toHaveBeenCalled(); @@ -1571,9 +1547,7 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); - it('projects and extracts the referenceField on the xToOne path', async () => { - // xToOne path: getRecord projects `@@@` AND `@@@`; - // the executor reads relation[referenceField] from the parent's nested relation linkage. + it('passes the referenceField to getSingleRelatedData and extracts its value from the result', async () => { const ordersSchema = makeCollectionSchema({ collectionName: 'orders', collectionDisplayName: 'Orders', @@ -1582,11 +1556,10 @@ describe('LoadRelatedRecordStepExecutor', () => { }); const agentPort = makeMockAgentPort(); - // Override getRecord to return the projected reference field alongside the id. - (agentPort.getRecord as jest.Mock).mockResolvedValue({ - collectionName: 'customers', - recordId: [42], - values: { order: { id: '99', reference: 'ORD-2026-001' } }, + (agentPort.getSingleRelatedData as jest.Mock).mockResolvedValue({ + collectionName: 'orders', + recordId: ['99'], + values: { id: '99', reference: 'ORD-2026-001' }, }); const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'Load order' }); const runStore = makeMockRunStore(); @@ -1603,9 +1576,13 @@ describe('LoadRelatedRecordStepExecutor', () => { await executor.execute(); - // Verify the projection includes the reference field. - expect(agentPort.getRecord).toHaveBeenCalledWith( - { collection: 'customers', id: [42], fields: ['order@@@id', 'order@@@reference'] }, + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'order', + fields: ['reference'], + }), expect.objectContaining({ id: 1 }), ); @@ -1616,50 +1593,9 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); - // Regression: jsonapi-serializer emits the nested relation linkage with camelCased - // attribute keys (full_name → fullName). The executor must restore those keys - // before reading values[referenceField], otherwise snake_case referenceFields - // silently resolve to undefined and the dropdown loses its label. - it('restores camelCased keys on the nested relation linkage when referenceField is snake_case', async () => { - const ordersSchema = makeCollectionSchema({ - collectionName: 'orders', - collectionDisplayName: 'Orders', - referenceField: 'full_name', - fields: [{ fieldName: 'full_name', displayName: 'Full Name', isRelationship: false }], - }); - - const agentPort = makeMockAgentPort(); - // Mock the real agent-client deserialization shape: camelCased keys. - (agentPort.getRecord as jest.Mock).mockResolvedValue({ - collectionName: 'customers', - recordId: [42], - values: { order: { id: '99', fullName: 'John Doe' } }, - }); - const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'Load order' }); - const runStore = makeMockRunStore(); - const context = makeContext({ - model: mockModel.model, - agentPort, - runStore, - workflowPort: makeMockWorkflowPort({ - customers: makeCollectionSchema(), - orders: ordersSchema, - }), - }); - const executor = new LoadRelatedRecordStepExecutor(context); - - await executor.execute(); - - const saved = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; - expect(saved.pendingData.suggestedRecord).toEqual({ - recordId: ['99'], - referenceFieldValue: 'John Doe', - }); - }); - it('falls back to null referenceFieldValue when the related collection has no referenceField configured', async () => { - // Default makeCollectionSchema doesn't set referenceField → executor skips the - // extra projection and writes null on every candidate. + // Default makeCollectionSchema doesn't set referenceField → executor omits `fields` + // when calling getSingleRelatedData and writes null on every candidate. const agentPort = makeMockAgentPort(); const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'Load order' }); const runStore = makeMockRunStore(); @@ -1668,9 +1604,8 @@ describe('LoadRelatedRecordStepExecutor', () => { await executor.execute(); - // No reference-field projection — only the PK. - expect(agentPort.getRecord).toHaveBeenCalledWith( - { collection: 'customers', id: [42], fields: ['order@@@id'] }, + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.not.objectContaining({ fields: expect.anything() }), expect.objectContaining({ id: 1 }), ); @@ -2274,9 +2209,13 @@ describe('LoadRelatedRecordStepExecutor', () => { const result = await executor.execute(); expect(result.stepOutcome.status).toBe('success'); - // BelongsTo → xToOne path: project the relation on the parent record. - expect(agentPort.getRecord).toHaveBeenCalledWith( - { collection: 'customers', id: [42], fields: ['order@@@id'] }, + // BelongsTo → xToOne path: port's getSingleRelatedData method. + expect(agentPort.getSingleRelatedData).toHaveBeenCalledWith( + expect.objectContaining({ + collection: 'customers', + id: [42], + relation: 'order', + }), expect.objectContaining({ id: 1 }), ); expect(agentPort.getRelatedData).not.toHaveBeenCalled(); diff --git a/packages/workflow-executor/test/integration/workflow-execution.test.ts b/packages/workflow-executor/test/integration/workflow-execution.test.ts index 5b053d9596..afd797a9bc 100644 --- a/packages/workflow-executor/test/integration/workflow-execution.test.ts +++ b/packages/workflow-executor/test/integration/workflow-execution.test.ts @@ -159,6 +159,7 @@ function createMockAgentPort(): jest.Mocked { values: { id: 42, status: 'active' }, }), getRelatedData: jest.fn().mockResolvedValue([]), + getSingleRelatedData: jest.fn().mockResolvedValue(null), executeAction: jest.fn().mockResolvedValue(undefined), getActionFormInfo: jest.fn().mockResolvedValue({ hasForm: false }), probe: jest.fn().mockResolvedValue(undefined), @@ -504,12 +505,12 @@ describe('workflow execution (integration)', () => { }); const agentPort = createMockAgentPort(); - // BelongsTo → xToOne path: executor reads parent.values..id from getRecord. - // The agent serializes the relation linkage's `id` from the foreign collection's PK. - agentPort.getRecord.mockResolvedValue({ - collectionName: 'customers', - recordId: [42], - values: { order: { id: '99' } }, + // BelongsTo → xToOne path: the port resolves the related record via getSingleRelatedData, + // which the adapter implements by projecting on the parent's record endpoint. + agentPort.getSingleRelatedData.mockResolvedValue({ + collectionName: 'orders', + recordId: ['99'], + values: { id: '99' }, }); const { server, runStore } = createIntegrationSetup({ From 040d585d5b5ff74147a574ef7c62e6e4c22003aa Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Mon, 1 Jun 2026 16:19:06 +0200 Subject: [PATCH 06/11] fix: review --- .../src/types/step-execution-data.ts | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 084470bd1e..0be6f2c5da 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -132,26 +132,15 @@ export interface RecordStepExecutionData extends BaseStepExecutionData { } // -- Load Related Record -- - -// One row of `availableRecordIds`. `referenceFieldValue` is the layout-level reference -// field (collection.displayFieldName on the orchestrator side) — when the related -// collection has one configured and the agent returns a value, the frontend shows it -// instead of the raw id. Both `null` (no reference field configured) and `undefined` -// (configured but value missing) collapse to the same "fall back to id" rendering. export interface LoadRelatedRecordCandidate { recordId: Array; referenceFieldValue: string | null; } export interface LoadRelatedRecordPendingData { - // All relation fields available on the source collection (name + displayName each). availableFields: RelationRef[]; - // AI-selected relation from `availableFields`. suggestedField: RelationRef; - // First 50 records of `suggestedField` paired with their reference-field value. availableRecordIds: LoadRelatedRecordCandidate[]; - // AI-selected record from `availableRecordIds`; frontend can override via - // userConfirmation.selectedRecordId. suggestedRecord: LoadRelatedRecordCandidate; } @@ -162,7 +151,6 @@ export interface LoadRelatedRecordStepExecutionData pendingData?: LoadRelatedRecordPendingData; selectedRecordRef: RecordRef; executionParams?: RelationRef; - // Source is always selectedRecordRef, not repeated here (consistent with other step types). executionResult?: { relation: RelationRef; record: RecordRef } | { skipped: true }; } From 794a98ac6d41f8ededa55bbf4dee252303e18511 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 2 Jun 2026 14:15:49 +0200 Subject: [PATCH 07/11] test(workflow-executor): cover confirm-with-override stale relation; drop unused export MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add a test for resolveFromSelection when the confirmed relation is not in availableFields (stale/renamed) — asserts the step errors and neither getRelatedData nor saveStepExecution is called. - restoreFieldNames is only used inside the agent-client adapter now; drop its unused export. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/adapters/agent-client-agent-port.ts | 2 +- .../load-related-record-step-executor.test.ts | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) 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 51b9d4360b..0530b879e5 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -26,7 +26,7 @@ import { // The agent-client HTTP layer deserializes JSON:API responses with camelCase keys. // 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. -export function restoreFieldNames( +function restoreFieldNames( values: Record, originalFieldNames: string[] | undefined, ): Record { 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 d104636fb2..38ba1b6193 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 @@ -1163,6 +1163,37 @@ describe('LoadRelatedRecordStepExecutor', () => { expect(runStore.saveStepExecution).not.toHaveBeenCalled(); }); + it('returns error when the confirmed relation is not in availableFields (stale/renamed)', async () => { + const agentPort = makeMockAgentPort(); + const execution = makePendingExecution({ + pendingData: { + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [cand([99])], + suggestedRecord: cand([99]), + }, + // Frontend confirms a relation that no longer exists in availableFields. + userConfirmation: { userConfirmed: true, fieldName: 'ghost', selectedRecordId: [7] }, + }); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ agentPort, runStore }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + 'An unexpected error occurred while processing this step.', + ); + expect(agentPort.getRelatedData).not.toHaveBeenCalled(); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); + it('uses overridden suggestedField from pendingData to derive relatedCollectionName', async () => { const schema = makeCollectionSchema({ fields: [ From 4b81cd4b0d36d0df6f77e22ca0f905120f9fb509 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 2 Jun 2026 14:31:54 +0200 Subject: [PATCH 08/11] fix(workflow-executor): project a single sub-field for xToOne relations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getSingleRelatedData projected both the PK and the reference field on the relation (fields[store]=id,name), which the agent's parseProjection can't split into two sub-fields — it 400s with "Invalid projection: '.id,name'". The JSON:API serializer fills the linkage `id` regardless of projection, so we only need ONE sub-field: the requested reference field (for display), else a single PK field just to pull the relation into the response. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/adapters/agent-client-agent-port.ts | 13 ++++++++----- .../test/adapters/agent-client-agent-port.test.ts | 13 +++++++------ 2 files changed, 15 insertions(+), 11 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 0530b879e5..73faa0fe84 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -149,14 +149,17 @@ export default class AgentClientAgentPort implements AgentPort { user: StepUser, ): Promise { return this.callAgent('getSingleRelatedData', async () => { - const projectedFields = Array.from( - new Set([...relatedSchema.primaryKeyFields, ...(fields ?? [])]), - ); + // The agent can't parse multiple sub-fields on one relation in a single projection + // (`fields[store]=id,name` is read as a single field name → ValidationError). The linkage + // `id` carries the (packed) related PK regardless of projection, so project at most ONE + // field: the requested reference field for display, else a single PK field just to pull the + // relation into the response. + const projectedField = fields?.[0] ?? relatedSchema.primaryKeyFields[0]; const parent = await this.getRecord( { collection, id, - fields: projectedFields.map(f => `${relation}@@@${f}`), + fields: [`${relation}@@@${projectedField}`], }, user, ); @@ -166,7 +169,7 @@ export default class AgentClientAgentPort implements AgentPort { if (!linkage || !packedId) return null; - const restored = restoreFieldNames(linkage, projectedFields); + const restored = restoreFieldNames(linkage, [projectedField]); return { collectionName: relatedSchema.collectionName, 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 2b2d7d246b..dbd9c05fc0 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 @@ -493,7 +493,7 @@ describe('AgentClientAgentPort', () => { }); }); - it('adds extra projections for caller-supplied fields (e.g. referenceField)', async () => { + it('projects only the caller field (e.g. referenceField), not the PK — the linkage id comes free', async () => { mockCollection.list.mockResolvedValue([{ order: { id: '99', reference: 'ORD-2026-001' } }]); const result = await port.getSingleRelatedData( @@ -507,14 +507,15 @@ describe('AgentClientAgentPort', () => { user, ); + // Single sub-field only: the agent can't parse `fields[order]=id,reference`. expect(mockCollection.list).toHaveBeenCalledWith( - expect.objectContaining({ fields: ['order@@@id', 'order@@@reference'] }), + expect.objectContaining({ fields: ['order@@@reference'] }), ); expect(result?.values).toEqual({ id: '99', reference: 'ORD-2026-001' }); }); - it('deduplicates the PK if the caller passes it again in fields', async () => { - mockCollection.list.mockResolvedValue([{ order: { id: '99' } }]); + it('projects at most one sub-field even when the caller passes several', async () => { + mockCollection.list.mockResolvedValue([{ order: { id: '99', reference: 'ORD-2026-001' } }]); await port.getSingleRelatedData( { @@ -522,13 +523,13 @@ describe('AgentClientAgentPort', () => { id: [42], relation: 'order', relatedSchema: ordersSchema, - fields: ['id'], + fields: ['reference', 'label'], }, user, ); expect(mockCollection.list).toHaveBeenCalledWith( - expect.objectContaining({ fields: ['order@@@id'] }), + expect.objectContaining({ fields: ['order@@@reference'] }), ); }); From 989dddf88a9ecc45dd46a960d442ed0a5b05a802 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 2 Jun 2026 14:57:07 +0200 Subject: [PATCH 09/11] fix(workflow-executor): normalize relatedCollectionName reference + plainer port error - The orchestrator sends relatedCollectionName as Forest's `collection.targetKey` reference (e.g. "store.id"), which made getCollectionSchema("store.id") 404 and broke xToOne relation loading. Strip it to a plain collection name in the forest-server-workflow-port adapter (transport concern stays in the adapter; the zod schema stays a clean domain contract). The target key lives in relatedPrimaryKey. - WorkflowPortError.userMessage no longer leaks internal jargon ("orchestrator") nor claims a connection cause (it also wraps 4xx like "collection not found"): "This step couldn't be completed. Please try again, and contact your administrator if the problem continues." Co-Authored-By: Claude Opus 4.8 (1M context) --- .../adapters/forest-server-workflow-port.ts | 16 ++++++++++++- packages/workflow-executor/src/errors.ts | 2 +- .../forest-server-workflow-port.test.ts | 24 +++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts b/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts index 78ee0d027b..507268a826 100644 --- a/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts +++ b/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts @@ -43,6 +43,12 @@ const ROUTES = { mcpServerConfigs: '/liana/mcp-server-configs-with-details', }; +// Forest sends relatedCollectionName as a `collection.targetKey` reference (e.g. "store.id"); +// normalize it to a plain collection name (the target key lives in relatedPrimaryKey). +function stripReferenceKey(name: string | undefined): string | undefined { + return name?.includes('.') ? name.slice(0, name.lastIndexOf('.')) : name; +} + export default class ForestServerWorkflowPort implements WorkflowPort { private readonly options: HttpOptions; private readonly logger: Logger; @@ -188,7 +194,15 @@ export default class ForestServerWorkflowPort implements WorkflowPort { ); try { - return CollectionSchemaSchema.parse(response); + const schema = CollectionSchemaSchema.parse(response); + + return { + ...schema, + fields: schema.fields.map(field => ({ + ...field, + relatedCollectionName: stripReferenceKey(field.relatedCollectionName), + })), + }; } catch (err) { if (err instanceof z.ZodError) { // runId is passed for observability — the schema call is scoped to a run. diff --git a/packages/workflow-executor/src/errors.ts b/packages/workflow-executor/src/errors.ts index 8e0ba5d919..21f30c25ab 100644 --- a/packages/workflow-executor/src/errors.ts +++ b/packages/workflow-executor/src/errors.ts @@ -253,7 +253,7 @@ export class WorkflowPortError extends WorkflowExecutorError { `Workflow port "${operation}" failed: ${ cause instanceof Error ? cause.message : String(cause) }`, - 'Failed to communicate with the workflow orchestrator. Please try again.', + "This step couldn't be completed. Please try again, and contact your administrator if the problem continues.", ); this.cause = cause; } diff --git a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts index 5759b395f1..30ea03e379 100644 --- a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts +++ b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts @@ -732,6 +732,30 @@ describe('ForestServerWorkflowPort', () => { }); }); + it("strips the target key from relatedCollectionName (Forest 'collection.key' reference)", async () => { + mockQuery.mockResolvedValue({ + collectionName: 'accounts', + collectionDisplayName: 'Accounts', + primaryKeyFields: ['id'], + fields: [ + { + fieldName: 'store', + displayName: 'Store', + isRelationship: true, + relationType: 'BelongsTo', + relatedCollectionName: 'store.id', + relatedPrimaryKey: 'id', + type: null, + }, + ], + actions: [], + }); + + const result = await port.getCollectionSchema('accounts', '42'); + + expect(result.fields[0].relatedCollectionName).toBe('store'); + }); + it('accepts type File (Forest Admin extension)', async () => { mockQuery.mockResolvedValue({ collectionName: 'users', From 7b222ded47c99a585e556ada7c859e753c45fa15 Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 2 Jun 2026 15:42:29 +0200 Subject: [PATCH 10/11] fix(workflow): load-related-record snake_case relations + empty-list handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - adapter: look up xToOne linkage under the camelCased relation key (agent-client camelCases JSON:API keys); snake_case relations like billing_address no longer resolve to null / false "not found" - awaiting-input (Branch C): an empty related-record list now returns an empty candidate list with no suggestedRecord instead of erroring; the user can switch relation. Fully-automated (Branch B) still errors — it cannot load nothing - buildTarget throws instead of masking a missing relatedCollectionName - reword stale Branch B dispatch comment Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/adapters/agent-client-agent-port.ts | 13 +- .../load-related-record-step-executor.ts | 51 +++-- .../src/executors/record-step-executor.ts | 8 +- .../src/types/step-execution-data.ts | 3 +- .../adapters/agent-client-agent-port.test.ts | 26 +++ .../forest-server-workflow-port.test.ts | 29 ++- .../load-related-record-step-executor.test.ts | 182 +++++++++++++++--- .../integration/workflow-execution.test.ts | 1 - 8 files changed, 264 insertions(+), 49 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 73faa0fe84..4ec3b9e3b9 100644 --- a/packages/workflow-executor/src/adapters/agent-client-agent-port.ts +++ b/packages/workflow-executor/src/adapters/agent-client-agent-port.ts @@ -23,6 +23,10 @@ import { extractErrorMessage, } from '../errors'; +function toCamelCase(name: string): string { + return name.replace(/_([a-zA-Z0-9])/g, (_, c: string) => c.toUpperCase()); +} + // The agent-client HTTP layer deserializes JSON:API responses with camelCase keys. // 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. @@ -35,8 +39,7 @@ function restoreFieldNames( const camelToOriginal: Record = {}; for (const name of originalFieldNames) { - const camelName = name.replace(/_([a-zA-Z0-9])/g, (_, c: string) => c.toUpperCase()); - camelToOriginal[camelName] = name; + camelToOriginal[toCamelCase(name)] = name; } return Object.fromEntries(Object.entries(values).map(([k, v]) => [camelToOriginal[k] ?? k, v])); @@ -164,7 +167,11 @@ export default class AgentClientAgentPort implements AgentPort { user, ); - const linkage = parent.values[relation] as Record | null | undefined; + // agent-client camelCases relation keys; look the linkage up under the camelCased name. + const linkage = parent.values[toCamelCase(relation)] as + | Record + | null + | undefined; const packedId = linkage?.id as string | undefined; if (!linkage || !packedId) return null; diff --git a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts index 39c73736e1..d327d3dd24 100644 --- a/packages/workflow-executor/src/executors/load-related-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/load-related-record-step-executor.ts @@ -136,12 +136,18 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { if (target.relationType === 'BelongsTo' || target.relationType === 'HasOne') { const candidate = await this.fetchXToOneCandidate(target); - return { availableRecordIds: [candidate], suggestedRecord: candidate }; + return candidate + ? { availableRecordIds: [candidate], suggestedRecord: candidate } + : { availableRecordIds: [] }; } const { relatedData, bestIndex, relatedSchema } = await this.selectBestFromRelatedData( target, 50, ); + + if (relatedData.length === 0) { + return { availableRecordIds: [] }; + } + const referenceField = relatedSchema.referenceField ?? null; const toCandidate = (r: RecordData): LoadRelatedRecordCandidate => ({ recordId: r.recordId, @@ -211,7 +224,7 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { const record = await this.fetchRecordForRelation(target); @@ -233,6 +246,10 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { const candidate = await this.fetchXToOneCandidate(target); + if (!candidate) { + throw new RelatedRecordNotFoundError(target.selectedRecordRef.collectionName, target.name); + } + return { collectionName: target.relatedCollectionName, recordId: candidate.recordId, @@ -240,7 +257,9 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { + private async fetchXToOneCandidate( + target: RelationTarget, + ): Promise { const relatedSchema = await this.getCollectionSchema(target.relatedCollectionName); const referenceField = relatedSchema.referenceField ?? null; @@ -255,9 +274,7 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { - const { selectedRecordRef, name } = target; const relatedSchema = await this.getCollectionSchema(target.relatedCollectionName); const relatedData = await this.fetchRelatedData(target, relatedSchema, limit); - if (relatedData.length === 0) { - throw new RelatedRecordNotFoundError(selectedRecordRef.collectionName, name); - } - - if (relatedData.length === 1) { + // Empty (bestIndex unused — callers guard on length) or single → no ranking needed. + if (relatedData.length <= 1) { return { relatedData, bestIndex: 0, suggestedFields: [], relatedSchema }; } @@ -371,6 +388,10 @@ export default class LoadRelatedRecordStepExecutor extends RecordStepExecutor { const { relatedData, bestIndex } = await this.selectBestFromRelatedData(target, 50); + if (relatedData.length === 0) { + throw new RelatedRecordNotFoundError(target.selectedRecordRef.collectionName, target.name); + } + return this.toRecordRef(relatedData[bestIndex]); } diff --git a/packages/workflow-executor/src/executors/record-step-executor.ts b/packages/workflow-executor/src/executors/record-step-executor.ts index 987fd8c708..4430dfacc4 100644 --- a/packages/workflow-executor/src/executors/record-step-executor.ts +++ b/packages/workflow-executor/src/executors/record-step-executor.ts @@ -76,17 +76,17 @@ export default abstract class RecordStepExecutor< return schema; } - protected findField(schema: CollectionSchema, displayName: string): FieldSchema | undefined { + protected findField(schema: CollectionSchema, name: string): FieldSchema | undefined { // LLMs occasionally return formatting variants of field names (e.g. "first_name" for // "firstname", "full-name" for "Full Name") even though the tool schema declares them // as literals. Fall back to a normalized comparison so a cosmetic variation doesn't // fail an otherwise correct step. const normalizeFieldName = (s: string) => s.toLowerCase().replace(/[\s_-]/g, ''); - const normalized = normalizeFieldName(displayName); + const normalized = normalizeFieldName(name); const exact = - schema.fields.find(f => f.displayName === displayName) ?? - schema.fields.find(f => f.fieldName === displayName); + schema.fields.find(f => f.displayName === name) ?? + schema.fields.find(f => f.fieldName === name); if (exact) return exact; const fuzzy = schema.fields.filter( diff --git a/packages/workflow-executor/src/types/step-execution-data.ts b/packages/workflow-executor/src/types/step-execution-data.ts index 0be6f2c5da..06bc3e9b12 100644 --- a/packages/workflow-executor/src/types/step-execution-data.ts +++ b/packages/workflow-executor/src/types/step-execution-data.ts @@ -141,7 +141,8 @@ export interface LoadRelatedRecordPendingData { availableFields: RelationRef[]; suggestedField: RelationRef; availableRecordIds: LoadRelatedRecordCandidate[]; - suggestedRecord: LoadRelatedRecordCandidate; + // Absent when the relation has no linked record(s): the list is empty and there's nothing to suggest. + suggestedRecord?: LoadRelatedRecordCandidate; } export interface LoadRelatedRecordStepExecutionData 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 dbd9c05fc0..6fa0c9e8de 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 @@ -565,6 +565,32 @@ describe('AgentClientAgentPort', () => { expect(result?.values).toEqual({ id: '99', full_name: 'John Doe' }); }); + // Regression: the relation NAME itself can be snake_case (billing_address). jsonapi-serializer + // emits the linkage under the camelCased key (billingAddress), so looking it up by the raw + // name returned null and the relation never loaded. + it('finds the linkage when the relation name is snake_case', async () => { + mockCollection.list.mockResolvedValue([{ billingAddress: { id: '7|2' } }]); + + const result = await port.getSingleRelatedData( + { + collection: 'users', + id: [42], + relation: 'billing_address', + relatedSchema: { ...ordersSchema, collectionName: 'addresses' }, + }, + user, + ); + + expect(mockCollection.list).toHaveBeenCalledWith( + expect.objectContaining({ fields: ['billing_address@@@id'] }), + ); + expect(result).toEqual({ + collectionName: 'addresses', + recordId: ['7', '2'], + values: { id: '7|2' }, + }); + }); + it('splits composite PKs from the packed "id" linkage', async () => { const compositeSchema = { ...ordersSchema, diff --git a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts index 30ea03e379..66bcff1825 100644 --- a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts +++ b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts @@ -641,6 +641,7 @@ describe('ForestServerWorkflowPort', () => { collectionDisplayName: 'Users', primaryKeyFields: ['id'], referenceField: 'name', + futureUnknownField: 'ignored', fields: [ { fieldName: 'store', @@ -657,8 +658,10 @@ describe('ForestServerWorkflowPort', () => { const result = await port.getCollectionSchema('users', '42'); - expect(result).not.toHaveProperty('referenceField'); + // Genuinely-unknown keys are stripped; declared fields (referenceField) are preserved. + expect(result).not.toHaveProperty('futureUnknownField'); expect(result.fields[0]).not.toHaveProperty('relatedPrimaryKey'); + expect(result.referenceField).toBe('name'); expect(result.fields[0]).toMatchObject({ fieldName: 'store', relatedCollectionName: 'stores', @@ -756,6 +759,30 @@ describe('ForestServerWorkflowPort', () => { expect(result.fields[0].relatedCollectionName).toBe('store'); }); + it('leaves relatedCollectionName unchanged when it carries no target key (no dot)', async () => { + mockQuery.mockResolvedValue({ + collectionName: 'accounts', + collectionDisplayName: 'Accounts', + primaryKeyFields: ['id'], + fields: [ + { + fieldName: 'store', + displayName: 'Store', + isRelationship: true, + relationType: 'BelongsTo', + relatedCollectionName: 'store', + relatedPrimaryKey: 'id', + type: null, + }, + ], + actions: [], + }); + + const result = await port.getCollectionSchema('accounts', '42'); + + expect(result.fields[0].relatedCollectionName).toBe('store'); + }); + it('accepts type File (Forest Admin extension)', async () => { mockQuery.mockResolvedValue({ collectionName: 'users', 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 38ba1b6193..67217b290c 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 @@ -82,7 +82,6 @@ function makeCollectionSchema(overrides: Partial = {}): Collec isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'orders', - relatedPrimaryKey: 'id', }, { fieldName: 'address', @@ -419,6 +418,7 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Address', isRelationship: true, relationType: 'HasMany', + relatedCollectionName: 'addresses', }, ], }); @@ -461,6 +461,7 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Address', isRelationship: true, relationType: 'HasMany', + relatedCollectionName: 'addresses', }, ], }); @@ -529,6 +530,7 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Address', isRelationship: true, relationType: 'HasMany', + relatedCollectionName: 'addresses', }, ], }); @@ -591,7 +593,6 @@ describe('LoadRelatedRecordStepExecutor', () => { isRelationship: true, relationType: 'HasOne', relatedCollectionName: 'profiles', - relatedPrimaryKey: 'id', }, ], }); @@ -647,7 +648,6 @@ describe('LoadRelatedRecordStepExecutor', () => { isRelationship: true, relationType: 'BelongsToMany', relatedCollectionName: 'tags', - relatedPrimaryKey: 'id', }, ], }); @@ -699,7 +699,6 @@ describe('LoadRelatedRecordStepExecutor', () => { isRelationship: true, relationType: 'BelongsToMany', relatedCollectionName: 'tags', - relatedPrimaryKey: 'id', }, ], }); @@ -909,6 +908,56 @@ describe('LoadRelatedRecordStepExecutor', () => { }), ); }); + + // When the xToOne relation has no linked record, the step still awaits input with an empty + // candidate list (no suggestedRecord) — the user can switch relation. It is NOT an error. + it('returns awaiting-input with an empty candidate list when the xToOne relation has no linked record', async () => { + const agentPort = makeMockAgentPort([]); // getSingleRelatedData → null + const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'Load order' }); + const runStore = makeMockRunStore(); + const context = makeContext({ model: mockModel.model, agentPort, runStore }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('awaiting-input'); + const saved = (runStore.saveStepExecution as jest.Mock).mock.calls[0][1]; + expect(saved.pendingData.availableRecordIds).toEqual([]); + expect(saved.pendingData.suggestedRecord).toBeUndefined(); + }); + + it('returns awaiting-input with an empty candidate list when the to-many relation has no records', async () => { + const agentPort = makeMockAgentPort([]); // getRelatedData → [] + const mockModel = makeMockModel({ relationName: 'Address', reasoning: 'Load address' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ + customers: makeCollectionSchema({ + fields: [ + { + fieldName: 'address', + displayName: 'Address', + isRelationship: true, + relationType: 'HasMany', + relatedCollectionName: 'addresses', + }, + ], + }), + }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('awaiting-input'); + const saved = (runStore.saveStepExecution as jest.Mock).mock.calls[0][1]; + expect(saved.pendingData.availableRecordIds).toEqual([]); + expect(saved.pendingData.suggestedRecord).toBeUndefined(); + expect(agentPort.getRelatedData).toHaveBeenCalled(); + }); }); describe('confirmation accepted (Branch A)', () => { @@ -985,6 +1034,34 @@ describe('LoadRelatedRecordStepExecutor', () => { }), ); }); + + // Confirming while the candidate list is empty (no suggestedRecord, no override) leaves + // nothing to load → error. The front is expected to prevent this by offering a relation switch. + it('returns error when confirming an empty candidate list with no selection', async () => { + const execution = makePendingExecution({ + pendingData: { + availableFields: [ + { name: 'order', displayName: 'Order' }, + { name: 'address', displayName: 'Address' }, + ], + suggestedField: { name: 'order', displayName: 'Order' }, + availableRecordIds: [], + }, + userConfirmation: { userConfirmed: true }, + }); + const runStore = makeMockRunStore({ + getStepExecutions: jest.fn().mockResolvedValue([execution]), + }); + const context = makeContext({ agentPort: makeMockAgentPort(), runStore }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + 'The related record could not be found. It may have been deleted.', + ); + }); }); describe('confirmation with user override of selectedRecordId (Branch A)', () => { @@ -1578,6 +1655,48 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); + it('exposes a null referenceFieldValue when a HasMany candidate has no value for the reference field', async () => { + const relatedData: RecordData[] = [ + { collectionName: 'addresses', recordId: [1], values: { city: null } }, + ]; + const agentPort = makeMockAgentPort(relatedData); + const addressesSchema = makeCollectionSchema({ + collectionName: 'addresses', + collectionDisplayName: 'Addresses', + referenceField: 'city', + fields: [{ fieldName: 'city', displayName: 'City', isRelationship: false }], + }); + const mockModel = makeMockModel({ relationName: 'Address', reasoning: 'Load address' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + agentPort, + runStore, + workflowPort: makeMockWorkflowPort({ + customers: makeCollectionSchema({ + fields: [ + { + fieldName: 'address', + displayName: 'Address', + isRelationship: true, + relationType: 'HasMany', + relatedCollectionName: 'addresses', + }, + ], + }), + addresses: addressesSchema, + }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + await executor.execute(); + + const saved = (runStore.saveStepExecution as jest.Mock).mock.calls.at(-1)?.[1]; + expect(saved.pendingData.availableRecordIds).toEqual([ + { recordId: [1], referenceFieldValue: null }, + ]); + }); + it('passes the referenceField to getSingleRelatedData and extracts its value from the result', async () => { const ordersSchema = makeCollectionSchema({ collectionName: 'orders', @@ -1714,6 +1833,40 @@ describe('LoadRelatedRecordStepExecutor', () => { }); }); + describe('StepStateError on malformed schema', () => { + // A relationship field with no relatedCollectionName can't be followed — throw rather than + // silently falling back to the field name (which would 404 later as a bogus collection). + it('returns error when the selected relation has no relatedCollectionName', async () => { + const schema = makeCollectionSchema({ + fields: [ + { + fieldName: 'order', + displayName: 'Order', + isRelationship: true, + relationType: 'BelongsTo', + }, + ], + }); + const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'test' }); + const runStore = makeMockRunStore(); + const context = makeContext({ + model: mockModel.model, + runStore, + workflowPort: makeMockWorkflowPort({ customers: schema }), + stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), + }); + const executor = new LoadRelatedRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + 'An unexpected error occurred while processing this step.', + ); + expect(runStore.saveStepExecution).not.toHaveBeenCalled(); + }); + }); + describe('RelatedRecordNotFoundError', () => { it('returns error when BelongsTo getRelatedData returns empty array (Branch B)', async () => { const agentPort = makeMockAgentPort([]); @@ -1744,6 +1897,7 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Address', isRelationship: true, relationType: 'HasMany', + relatedCollectionName: 'addresses', }, ], }); @@ -1767,22 +1921,6 @@ describe('LoadRelatedRecordStepExecutor', () => { ); expect(runStore.saveStepExecution).not.toHaveBeenCalled(); }); - - it('returns error when getRelatedData returns empty array (Branch C)', async () => { - const agentPort = makeMockAgentPort([]); - const mockModel = makeMockModel({ relationName: 'Order', reasoning: 'test' }); - const runStore = makeMockRunStore(); - const context = makeContext({ model: mockModel.model, agentPort, runStore }); - const executor = new LoadRelatedRecordStepExecutor(context); - - const result = await executor.execute(); - - expect(result.stepOutcome.status).toBe('error'); - expect(result.stepOutcome.error).toBe( - 'The related record could not be found. It may have been deleted.', - ); - expect(runStore.saveStepExecution).not.toHaveBeenCalled(); - }); }); describe('RunStorePortError post-load', () => { @@ -1847,7 +1985,6 @@ describe('LoadRelatedRecordStepExecutor', () => { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'orders', - relatedPrimaryKey: 'id', }, ], }); @@ -1999,7 +2136,6 @@ describe('LoadRelatedRecordStepExecutor', () => { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'invoices', - relatedPrimaryKey: 'id', }, ], }); @@ -2224,7 +2360,6 @@ describe('LoadRelatedRecordStepExecutor', () => { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'orders', - relatedPrimaryKey: 'id', }, ], }); @@ -2314,7 +2449,6 @@ describe('LoadRelatedRecordStepExecutor', () => { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'orders', - relatedPrimaryKey: 'id', }, ], }); diff --git a/packages/workflow-executor/test/integration/workflow-execution.test.ts b/packages/workflow-executor/test/integration/workflow-execution.test.ts index afd797a9bc..2089b18bb4 100644 --- a/packages/workflow-executor/test/integration/workflow-execution.test.ts +++ b/packages/workflow-executor/test/integration/workflow-execution.test.ts @@ -79,7 +79,6 @@ const COLLECTION_SCHEMA_WITH_RELATION: CollectionSchema = { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'orders', - relatedPrimaryKey: 'id', }, ], actions: [], From 88bbefa31b8d738b83342e9c12f1b7349c153d3b Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Tue, 2 Jun 2026 17:55:36 +0200 Subject: [PATCH 11/11] chore(workflow-executor): drop stale relatedPrimaryKey references The relatedPrimaryKey field was removed from the schema/RelationTarget (dead field; the related PK comes from primaryKeyFields[]). Clean up the leftover mentions: a stale comment in forest-server-workflow-port and raw-input occurrences in tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/adapters/forest-server-workflow-port.ts | 2 +- .../test/adapters/forest-server-workflow-port.test.ts | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts b/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts index 507268a826..ab38d77e52 100644 --- a/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts +++ b/packages/workflow-executor/src/adapters/forest-server-workflow-port.ts @@ -44,7 +44,7 @@ const ROUTES = { }; // Forest sends relatedCollectionName as a `collection.targetKey` reference (e.g. "store.id"); -// normalize it to a plain collection name (the target key lives in relatedPrimaryKey). +// normalize it to a plain collection name (the related PK comes from the schema's primaryKeyFields). function stripReferenceKey(name: string | undefined): string | undefined { return name?.includes('.') ? name.slice(0, name.lastIndexOf('.')) : name; } diff --git a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts index 66bcff1825..30c8a4eaca 100644 --- a/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts +++ b/packages/workflow-executor/test/adapters/forest-server-workflow-port.test.ts @@ -649,7 +649,7 @@ describe('ForestServerWorkflowPort', () => { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'stores', - relatedPrimaryKey: 'id', + futureFieldKey: 'ignored', type: null, }, ], @@ -660,7 +660,7 @@ describe('ForestServerWorkflowPort', () => { // Genuinely-unknown keys are stripped; declared fields (referenceField) are preserved. expect(result).not.toHaveProperty('futureUnknownField'); - expect(result.fields[0]).not.toHaveProperty('relatedPrimaryKey'); + expect(result.fields[0]).not.toHaveProperty('futureFieldKey'); expect(result.referenceField).toBe('name'); expect(result.fields[0]).toMatchObject({ fieldName: 'store', @@ -747,7 +747,6 @@ describe('ForestServerWorkflowPort', () => { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'store.id', - relatedPrimaryKey: 'id', type: null, }, ], @@ -771,7 +770,6 @@ describe('ForestServerWorkflowPort', () => { isRelationship: true, relationType: 'BelongsTo', relatedCollectionName: 'store', - relatedPrimaryKey: 'id', type: null, }, ],