From 36bf3c2f117f7cfca9cdfbaabadb643835aeeb67 Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Thu, 28 May 2026 15:49:51 +0200 Subject: [PATCH 1/4] 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 | 5 + .../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, 951 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 df0515016f..b0711af28c 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 e488f599cf..dd562abc93 100644 --- a/packages/workflow-executor/src/types/validated/collection.ts +++ b/packages/workflow-executor/src/types/validated/collection.ts @@ -42,6 +42,8 @@ export const FieldSchemaSchema = z relationType: z.enum(['BelongsTo', 'HasMany', 'HasOne', 'BelongsToMany']).optional(), /** Target collection name; only meaningful for relationship fields. */ relatedCollectionName: z.string().optional(), + /** Primary key field name on the related collection; only meaningful for relationship fields. */ + relatedPrimaryKey: z.string().optional(), /** Column type — null for relationship fields. */ type: ColumnTypeSchema.nullable(), /** Allowed values for Enum fields. */ @@ -81,6 +83,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), }) 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 3972918435..348fdeff35 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' }), }), @@ -1640,10 +2042,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 }, }); @@ -1671,6 +2076,8 @@ describe('LoadRelatedRecordStepExecutor', () => { displayName: 'Order', isRelationship: true, relationType: 'BelongsTo', + relatedCollectionName: 'orders', + relatedPrimaryKey: 'id', }, ], }); @@ -1686,17 +2093,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 }), }); @@ -1704,7 +2124,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'); }); }); @@ -1723,9 +2146,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]), }, }; @@ -1738,6 +2162,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 09504bb910..85e1e97845 100644 --- a/packages/workflow-executor/test/http/pending-data-validators.test.ts +++ b/packages/workflow-executor/test/http/pending-data-validators.test.ts @@ -80,24 +80,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 80f86192e4..f5ba5fada2 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 86e3f78914dd74ec4fd000e5aeaafe9e532e207c Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Fri, 29 May 2026 10:17:43 +0200 Subject: [PATCH 2/4] 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 348fdeff35..1fd5c6cc78 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 255da34ecede9850084818961094e8346917cf40 Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Fri, 29 May 2026 10:17:43 +0200 Subject: [PATCH 3/4] 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 348fdeff35..1fd5c6cc78 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 ca08d64c0c52227bea2b1cb5cd84594a63140874 Mon Sep 17 00:00:00 2001 From: Enki Pontvianne Date: Fri, 29 May 2026 11:22:13 +0200 Subject: [PATCH 4/4] 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 1fd5c6cc78..85eb05ab1a 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({