From 2a58c1f53d410751efd3f9f010398488cd2bbe9e Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 1 Jun 2026 15:55:58 +0200 Subject: [PATCH 1/3] fix(workflow-executor): tolerate orchestrator collection-schema drift MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The orchestrator deploys independently and evolves the collection schema it returns (adds fields like relatedPrimaryKey/referenceField, may omit step-specific props). With .strict(), any additive change made the whole step fail at parse — even for fields the step never uses. Strip unknown keys on the orchestrator collection schema (CollectionSchema + nested FieldSchema/ActionSchema) and keep only structural fields required. Demote step-specific props that have a use-time guard: actions -> optional (default []), guarded by NoActionsError in trigger-action; field type -> optional, guarded by buildZodSchemaForField's string fallback in update-record. We now fail only when a step genuinely lacks what it needs, at execution. Produced/envelope schemas and frontend HTTP bodies keep .strict(). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/workflow-executor/CLAUDE.md | 2 +- .../src/types/validated/collection.ts | 50 ++++++++----------- .../forest-server-workflow-port.test.ts | 50 +++++++++++++++++-- 3 files changed, 69 insertions(+), 33 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index c6f8b581e6..401002af08 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -93,7 +93,7 @@ src/ - **Auto-chain from `/update-step` response** — `WorkflowPort.updateStepExecution` returns `AvailableRunDispatch | null`: when non-null, the `Runner` executes the next step inline instead of waiting for the next poll. The chain exits on `null` (awaiting-input / finished / error), on a non-progressing `stepIndex` (server bug defense), at `maxChainDepth` (config, default 50), or when `stop()` is called. Each chained step uses the `forestServerToken` from its own dispatch — token freshness is preserved across the chain. The port retries `POST /update-step` on transient failures (network, 5xx) — this relies on server-side idempotency: the orchestrator MUST deduplicate identical outcomes for a given `(runId, stepIndex)` to prevent double side-effects on retry. - **Pre-recorded AI decisions** — Record step executors support `preRecordedArgs` in the step definition to bypass AI calls. When provided, executors use the pre-recorded values (display names) directly instead of invoking the AI. Each record step type has its own typed `preRecordedArgs` shape. Validation happens via schema resolution — invalid display names throw `InvalidPreRecordedArgsError`. Partial args are supported: only the provided fields skip AI, the rest still use AI. - **Graceful shutdown** — `stop()` drains in-flight steps before closing resources. The `state` getter exposes the lifecycle: `idle → running → draining → stopped`. `stopTimeoutMs` (default 30s) prevents `stop()` from hanging forever if a step is stuck. The HTTP server stays up during drain so the frontend can still query run status. Signal handling (`SIGTERM`/`SIGINT`) is the consumer's responsibility — the Runner is a library class. -- **Boundary validation** — Types that cross a trust boundary (wire from the orchestrator, or mapper output) live under `src/types/validated/` and are declared as zod schemas with TS types inferred via `z.infer<>`. Every schema uses `.strict()` by default. Validation runs at the boundary where the data enters the executor (`forest-server-workflow-port.getCollectionSchema` → `CollectionSchemaSchema.parse`, `run-to-available-step-mapper.toAvailableStepExecution` → `AvailableStepExecutionSchema.parse`). On parse failure: throw `DomainValidationError` (extends `WorkflowExecutorError`) → bucketized as malformed → reported to the orchestrator. Types outside `validated/` (`execution-context.ts`, `step-execution-data.ts`) are internal runtime state and are not zod-validated. Note: `StepOutcome` is validated when it arrives as input via `previousSteps`; outputs produced by executors are trusted by construction. +- **Boundary validation** — Types that cross a trust boundary (wire from the orchestrator, or mapper output) live under `src/types/validated/` and are declared as zod schemas with TS types inferred via `z.infer<>`. Strictness depends on origin: schemas the executor **produces** (mapper output `AvailableStepExecutionSchema`, the reconstructed envelope `StepUserSchema`/`StepSchema`/`RecordRefSchema`) and **frontend** HTTP bodies (`pending-data-validators.ts`) use `.strict()` — they catch our own bugs / input hygiene. The **orchestrator collection schema** (`CollectionSchemaSchema` & nested `FieldSchema`/`ActionSchema`) instead **strips** unknown keys (no `.strict()`) and keeps only structural fields required (`collectionName`, `primaryKeyFields`, `fields`, and per-field `fieldName`/`displayName`/`isRelationship`); step-specific props (`actions`, field `type`, …) are optional and asserted at use-time by the consuming executor (`findField` → `FieldNotFoundError`/`RelationNotFoundError`, `NoActionsError`, `buildZodSchemaForField` fallback). This keeps the executor resilient to independent orchestrator drift: we fail at step execution, only when a step genuinely lacks what it needs — never in bulk up front for an unrelated add/remove. Validation runs at the boundary where data enters (`forest-server-workflow-port.getCollectionSchema` → `CollectionSchemaSchema.parse`, `run-to-available-step-mapper.toAvailableStepExecution` → `AvailableStepExecutionSchema.parse`). On parse failure: throw `DomainValidationError` (extends `WorkflowExecutorError`) → bucketized as malformed (dispatch) or surfaced as a step error (execution). Types outside `validated/` (`execution-context.ts`, `step-execution-data.ts`) are internal runtime state and are not zod-validated. Note: `StepOutcome` is validated when it arrives as input via `previousSteps`; outputs produced by executors are trusted by construction. ## Commands diff --git a/packages/workflow-executor/src/types/validated/collection.ts b/packages/workflow-executor/src/types/validated/collection.ts index e488f599cf..c624dd578a 100644 --- a/packages/workflow-executor/src/types/validated/collection.ts +++ b/packages/workflow-executor/src/types/validated/collection.ts @@ -33,21 +33,17 @@ const ColumnTypeSchema: z.ZodType = z.lazy(() => ]), ); -export const FieldSchemaSchema = z - .object({ - fieldName: z.string().min(1), - displayName: z.string().min(1), - isRelationship: z.boolean(), - /** Cardinality of the relation. Absent for non-relationship fields. */ - relationType: z.enum(['BelongsTo', 'HasMany', 'HasOne', 'BelongsToMany']).optional(), - /** Target collection name; only meaningful for relationship fields. */ - relatedCollectionName: z.string().optional(), - /** Column type — null for relationship fields. */ - type: ColumnTypeSchema.nullable(), - /** Allowed values for Enum fields. */ - enumValues: z.array(z.string()).min(1).optional(), - }) - .strict(); +// Orchestrator collection schema: strip unknown keys (no `.strict()`) and keep only structural +// fields required, so additive/step-specific drift doesn't fail steps that don't use it. +export const FieldSchemaSchema = z.object({ + fieldName: z.string().min(1), + displayName: z.string().min(1), + isRelationship: z.boolean(), + relationType: z.enum(['BelongsTo', 'HasMany', 'HasOne', 'BelongsToMany']).optional(), + relatedCollectionName: z.string().optional(), + type: ColumnTypeSchema.nullable().optional(), + enumValues: z.array(z.string()).min(1).optional(), +}); export type FieldSchema = z.infer; // ActionSchema.fields / hooks content is a discriminated union owned by the upstream @@ -59,20 +55,17 @@ const ActionHooksSchema = z load: z.boolean(), change: z.array(z.unknown()), }) - .strict() .optional(); -export const ActionSchemaSchema = z - .object({ - name: z.string().min(1), - displayName: z.string().min(1), - endpoint: z.string().min(1), - /** Static form fields. Used as fallback when the agent's /hooks/load route 404s (old Ruby agents). */ - fields: ActionFieldsSchema, - /** Action lifecycle hooks. Drives agent-client's dynamic form loading. */ - hooks: ActionHooksSchema, - }) - .strict(); +export const ActionSchemaSchema = z.object({ + name: z.string().min(1), + displayName: z.string().min(1), + endpoint: z.string().min(1), + /** Static form fields. Used as fallback when the agent's /hooks/load route 404s (old Ruby agents). */ + fields: ActionFieldsSchema, + /** Action lifecycle hooks. Drives agent-client's dynamic form loading. */ + hooks: ActionHooksSchema, +}); export type ActionSchema = z.infer; export const CollectionSchemaSchema = z @@ -82,9 +75,8 @@ export const CollectionSchemaSchema = z collectionDisplayName: z.string().nullable(), primaryKeyFields: z.array(z.string().min(1)).min(1), fields: z.array(FieldSchemaSchema), - actions: z.array(ActionSchemaSchema), + actions: z.array(ActionSchemaSchema).optional().default([]), }) - .strict() .transform(data => ({ ...data, collectionDisplayName: data.collectionDisplayName || data.collectionName, 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 c51c04837d..5759b395f1 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 @@ -635,17 +635,61 @@ describe('ForestServerWorkflowPort', () => { await expect(port.getCollectionSchema('users', '42')).rejects.toThrow(/invalid/i); }); - it('rejects unknown extra fields on the wire (strict mode)', async () => { + it('strips unknown extra fields on the wire (orchestrator drift tolerance)', async () => { + mockQuery.mockResolvedValue({ + collectionName: 'users', + collectionDisplayName: 'Users', + primaryKeyFields: ['id'], + referenceField: 'name', + fields: [ + { + fieldName: 'store', + displayName: 'Store', + isRelationship: true, + relationType: 'BelongsTo', + relatedCollectionName: 'stores', + relatedPrimaryKey: 'id', + type: null, + }, + ], + actions: [], + }); + + const result = await port.getCollectionSchema('users', '42'); + + expect(result).not.toHaveProperty('referenceField'); + expect(result.fields[0]).not.toHaveProperty('relatedPrimaryKey'); + expect(result.fields[0]).toMatchObject({ + fieldName: 'store', + relatedCollectionName: 'stores', + }); + }); + + it('defaults actions to [] when the orchestrator omits it', async () => { mockQuery.mockResolvedValue({ collectionName: 'users', collectionDisplayName: 'Users', primaryKeyFields: ['id'], fields: [], + }); + + const result = await port.getCollectionSchema('users', '42'); + + expect(result.actions).toEqual([]); + }); + + it('accepts a field without type (omitted by the orchestrator)', async () => { + mockQuery.mockResolvedValue({ + collectionName: 'users', + collectionDisplayName: 'Users', + primaryKeyFields: ['id'], + fields: [{ fieldName: 'email', displayName: 'Email', isRelationship: false }], actions: [], - unexpectedNewField: 'oops', }); - await expect(port.getCollectionSchema('users', '42')).rejects.toThrow(); + const result = await port.getCollectionSchema('users', '42'); + + expect(result.fields[0].type).toBeUndefined(); }); it.each([null, ''])( From 57bf3a16dd06bfb11de0d98a3a7b9cf661e988af Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 1 Jun 2026 16:18:40 +0200 Subject: [PATCH 2/3] fix(workflow-executor): fail loudly when updating a field with no column type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. Making FieldSchema.type optional (resilience) introduced a silent mis-coercion: a non-relationship field whose type the orchestrator omits fell through buildZodSchemaForField to z.string(), and coerceFieldValue skipped coercion (type == null) — so in fully-automated mode a wrong-typed value could be written to the record with no error. update-record now throws FieldTypeMissingError use-time when a writable field has no type (the step genuinely lacks what it needs), instead of the string fallback. Relationship fields (type intentionally null) still pass through. The collection schema stays tolerant (type optional) so other step types remain drift-resilient. Also: scope the strip-policy comment to the whole collection schema section and trim the verbose CLAUDE.md boundary-validation bullet (review nits). Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/workflow-executor/CLAUDE.md | 2 +- packages/workflow-executor/src/errors.ts | 10 ++++ .../executors/update-record-step-executor.ts | 25 +++++++--- .../src/types/validated/collection.ts | 7 ++- .../update-record-step-executor.test.ts | 47 +++++++++++++++++++ .../integration/workflow-execution.test.ts | 18 +++---- 6 files changed, 90 insertions(+), 19 deletions(-) diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index 401002af08..5ee61b96ef 100644 --- a/packages/workflow-executor/CLAUDE.md +++ b/packages/workflow-executor/CLAUDE.md @@ -93,7 +93,7 @@ src/ - **Auto-chain from `/update-step` response** — `WorkflowPort.updateStepExecution` returns `AvailableRunDispatch | null`: when non-null, the `Runner` executes the next step inline instead of waiting for the next poll. The chain exits on `null` (awaiting-input / finished / error), on a non-progressing `stepIndex` (server bug defense), at `maxChainDepth` (config, default 50), or when `stop()` is called. Each chained step uses the `forestServerToken` from its own dispatch — token freshness is preserved across the chain. The port retries `POST /update-step` on transient failures (network, 5xx) — this relies on server-side idempotency: the orchestrator MUST deduplicate identical outcomes for a given `(runId, stepIndex)` to prevent double side-effects on retry. - **Pre-recorded AI decisions** — Record step executors support `preRecordedArgs` in the step definition to bypass AI calls. When provided, executors use the pre-recorded values (display names) directly instead of invoking the AI. Each record step type has its own typed `preRecordedArgs` shape. Validation happens via schema resolution — invalid display names throw `InvalidPreRecordedArgsError`. Partial args are supported: only the provided fields skip AI, the rest still use AI. - **Graceful shutdown** — `stop()` drains in-flight steps before closing resources. The `state` getter exposes the lifecycle: `idle → running → draining → stopped`. `stopTimeoutMs` (default 30s) prevents `stop()` from hanging forever if a step is stuck. The HTTP server stays up during drain so the frontend can still query run status. Signal handling (`SIGTERM`/`SIGINT`) is the consumer's responsibility — the Runner is a library class. -- **Boundary validation** — Types that cross a trust boundary (wire from the orchestrator, or mapper output) live under `src/types/validated/` and are declared as zod schemas with TS types inferred via `z.infer<>`. Strictness depends on origin: schemas the executor **produces** (mapper output `AvailableStepExecutionSchema`, the reconstructed envelope `StepUserSchema`/`StepSchema`/`RecordRefSchema`) and **frontend** HTTP bodies (`pending-data-validators.ts`) use `.strict()` — they catch our own bugs / input hygiene. The **orchestrator collection schema** (`CollectionSchemaSchema` & nested `FieldSchema`/`ActionSchema`) instead **strips** unknown keys (no `.strict()`) and keeps only structural fields required (`collectionName`, `primaryKeyFields`, `fields`, and per-field `fieldName`/`displayName`/`isRelationship`); step-specific props (`actions`, field `type`, …) are optional and asserted at use-time by the consuming executor (`findField` → `FieldNotFoundError`/`RelationNotFoundError`, `NoActionsError`, `buildZodSchemaForField` fallback). This keeps the executor resilient to independent orchestrator drift: we fail at step execution, only when a step genuinely lacks what it needs — never in bulk up front for an unrelated add/remove. Validation runs at the boundary where data enters (`forest-server-workflow-port.getCollectionSchema` → `CollectionSchemaSchema.parse`, `run-to-available-step-mapper.toAvailableStepExecution` → `AvailableStepExecutionSchema.parse`). On parse failure: throw `DomainValidationError` (extends `WorkflowExecutorError`) → bucketized as malformed (dispatch) or surfaced as a step error (execution). Types outside `validated/` (`execution-context.ts`, `step-execution-data.ts`) are internal runtime state and are not zod-validated. Note: `StepOutcome` is validated when it arrives as input via `previousSteps`; outputs produced by executors are trusted by construction. +- **Boundary validation** — Types that cross a trust boundary (wire from the orchestrator, or mapper output) live under `src/types/validated/` as zod schemas with TS types inferred via `z.infer<>`. Strictness depends on origin: schemas the executor **produces** (mapper output) and **frontend** HTTP bodies use `.strict()` (catch our own bugs / input hygiene); the **orchestrator collection schema** instead **strips** unknown keys and requires only structural fields, with step-specific props optional and asserted at use-time by the consuming executor. This keeps the executor resilient to independent orchestrator drift — we fail at step execution, only when a step genuinely lacks what it needs, never in bulk up front for an unrelated add/remove. Validation runs where data enters (`forest-server-workflow-port.getCollectionSchema`, `run-to-available-step-mapper.toAvailableStepExecution`). On parse failure: throw `DomainValidationError` (extends `WorkflowExecutorError`) → bucketized as malformed (dispatch) or surfaced as a step error (execution). Types outside `validated/` are internal runtime state and not zod-validated. Note: `StepOutcome` is validated when it arrives as input via `previousSteps`; executor outputs are trusted by construction. ## Commands diff --git a/packages/workflow-executor/src/errors.ts b/packages/workflow-executor/src/errors.ts index 828d2758fb..8e0ba5d919 100644 --- a/packages/workflow-executor/src/errors.ts +++ b/packages/workflow-executor/src/errors.ts @@ -174,6 +174,16 @@ export class FieldNotFoundError extends WorkflowExecutorError { } } +export class FieldTypeMissingError extends WorkflowExecutorError { + constructor(name: string, collectionName: string) { + super( + `Field "${name}" in collection "${collectionName}" has no column type`, + "This field can't be updated because its type is missing from the schema. " + + 'Contact your administrator if the problem persists.', + ); + } +} + export class ActionNotFoundError extends WorkflowExecutorError { constructor(name: string, collectionName: string) { super( diff --git a/packages/workflow-executor/src/executors/update-record-step-executor.ts b/packages/workflow-executor/src/executors/update-record-step-executor.ts index 4fc70ecdcd..4c41a5a600 100644 --- a/packages/workflow-executor/src/executors/update-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/update-record-step-executor.ts @@ -9,6 +9,7 @@ import { z } from 'zod'; import { FieldNotFoundError, + FieldTypeMissingError, InvalidPreRecordedArgsError, NoWritableFieldsError, StepStateError, @@ -74,9 +75,15 @@ function buildZodSchemaForPrimitive(type: string, enumValues?: string[]): z.ZodT } } -function buildZodSchemaForField(field: FieldSchema): z.ZodTypeAny { +function buildZodSchemaForField(field: FieldSchema, collectionName: string): z.ZodTypeAny { const { type, enumValues } = field; + // A writable (non-relationship) field with no column type is a malformed schema for this update: + // we'd otherwise fall through to z.string() and silently write the wrong type. Fail visibly. + if (type == null) { + throw new FieldTypeMissingError(field.displayName, collectionName); + } + if (Array.isArray(type)) { // Nested array (e.g. [['String']]) → treat as opaque JSON. if (Array.isArray(type[0])) return z.array(jsonStringSchema); @@ -94,11 +101,15 @@ function buildZodSchemaForField(field: FieldSchema): z.ZodTypeAny { // Coerce a user-overridden value to the field's native type before updating the record. // The HTTP schema accepts `unknown`, so the override may be a boolean or an array; this turns // it into the type the datasource expects, and throws a StepStateError on mismatch. -function coerceFieldValue(fieldSchema: FieldSchema | undefined, value: unknown): unknown { - // No coercible primitive schema (field not found or relationship) → leave it as-is. - if (!fieldSchema || fieldSchema.type == null || value === null) return value; +function coerceFieldValue( + fieldSchema: FieldSchema | undefined, + value: unknown, + collectionName: string, +): unknown { + // Field not found, relationship (type intentionally null), or explicit null → nothing to coerce. + if (!fieldSchema || fieldSchema.isRelationship || value === null) return value; - const parsed = buildZodSchemaForField(fieldSchema).safeParse(value); + const parsed = buildZodSchemaForField(fieldSchema, collectionName).safeParse(value); if (!parsed.success) { throw new StepStateError( @@ -182,7 +193,7 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor { @@ -316,7 +327,7 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor z.object({ fieldName: z.literal(f.displayName), - value: buildZodSchemaForField(f).nullable(), + value: buildZodSchemaForField(f, schema.collectionName).nullable(), reasoning: z.string().describe('Why this field and value were chosen'), }), ) as FieldObject[]; diff --git a/packages/workflow-executor/src/types/validated/collection.ts b/packages/workflow-executor/src/types/validated/collection.ts index c624dd578a..6d5b73d2de 100644 --- a/packages/workflow-executor/src/types/validated/collection.ts +++ b/packages/workflow-executor/src/types/validated/collection.ts @@ -3,6 +3,11 @@ import { z } from 'zod'; // -- Schema types (structure of a collection — source: WorkflowPort) -- +// +// These schemas (Field/Action/Collection) come from the orchestrator, which deploys independently +// and evolves this contract over time. They intentionally omit `.strict()` so unknown keys are +// stripped, not rejected, and require only structural fields — step-specific props are optional and +// asserted at use-time by the consuming executor. Don't re-add `.strict()` here. // Mirrors PrimitiveTypes from @forestadmin/datasource-toolkit — kept local to avoid // adding a hard dependency on datasource-toolkit from the executor package. @@ -33,8 +38,6 @@ const ColumnTypeSchema: z.ZodType = z.lazy(() => ]), ); -// Orchestrator collection schema: strip unknown keys (no `.strict()`) and keep only structural -// fields required, so additive/step-specific drift doesn't fail steps that don't use it. export const FieldSchemaSchema = z.object({ fieldName: z.string().min(1), displayName: z.string().min(1), diff --git a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts index 993f68bb63..bab19d382f 100644 --- a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts @@ -600,6 +600,36 @@ describe('UpdateRecordStepExecutor', () => { }); }); + describe('FieldTypeMissingError — writable field without a column type', () => { + it('fails the step instead of silently coercing the value to string (AI path)', async () => { + const agentPort = makeMockAgentPort(); + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema({ + // Non-relationship field with no `type` (orchestrator drift). Schema accepts it (resilience), + // but update-record must not build a string schema and write the wrong type silently. + fields: [{ fieldName: 'age', displayName: 'Age', isRelationship: false }], + }), + }); + const mockModel = makeMockModel({ input: { fieldName: 'Age', value: '42', reasoning: 'r' } }); + const context = makeContext({ + model: mockModel.model, + agentPort, + workflowPort, + stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), + }); + const executor = new UpdateRecordStepExecutor(context); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + "This field can't be updated because its type is missing from the schema. " + + 'Contact your administrator if the problem persists.', + ); + expect(agentPort.updateRecord).not.toHaveBeenCalled(); + }); + }); + describe('resolveFieldName failure', () => { it('returns error when field is not found during executionType=FullyAutomated (Branch B)', async () => { // AI returns a display name that doesn't match any field in the schema @@ -1846,5 +1876,22 @@ describe('UpdateRecordStepExecutor', () => { expect.objectContaining({ id: 1 }), ); }); + + it('fails the step when overriding a non-relationship field that has no type', async () => { + const typelessField = { fieldName: 'age', displayName: 'Age', isRelationship: false }; + const { executor, agentPort } = makeCoercionContext(typelessField, null, { + userConfirmed: true, + value: '42', + }); + + const result = await executor.execute(); + + expect(result.stepOutcome.status).toBe('error'); + expect(result.stepOutcome.error).toBe( + "This field can't be updated because its type is missing from the schema. " + + 'Contact your administrator if the problem persists.', + ); + expect(agentPort.updateRecord).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 82f02cc237..9e57a2dabc 100644 --- a/packages/workflow-executor/test/integration/workflow-execution.test.ts +++ b/packages/workflow-executor/test/integration/workflow-execution.test.ts @@ -39,9 +39,9 @@ const COLLECTION_SCHEMA: CollectionSchema = { collectionDisplayName: 'Customers', primaryKeyFields: ['id'], fields: [ - { fieldName: 'id', displayName: 'Id', isRelationship: false }, - { fieldName: 'email', displayName: 'Email', isRelationship: false }, - { fieldName: 'name', displayName: 'Name', isRelationship: false }, + { fieldName: 'id', displayName: 'Id', isRelationship: false, type: 'Number' }, + { fieldName: 'email', displayName: 'Email', isRelationship: false, type: 'String' }, + { fieldName: 'name', displayName: 'Name', isRelationship: false, type: 'String' }, ], actions: [], }; @@ -51,8 +51,8 @@ const COLLECTION_SCHEMA_WITH_STATUS: CollectionSchema = { collectionDisplayName: 'Customers', primaryKeyFields: ['id'], fields: [ - { fieldName: 'id', displayName: 'Id', isRelationship: false }, - { fieldName: 'status', displayName: 'Status', isRelationship: false }, + { fieldName: 'id', displayName: 'Id', isRelationship: false, type: 'Number' }, + { fieldName: 'status', displayName: 'Status', isRelationship: false, type: 'String' }, ], actions: [], }; @@ -61,7 +61,7 @@ const COLLECTION_SCHEMA_WITH_ACTIONS: CollectionSchema = { collectionName: 'customers', collectionDisplayName: 'Customers', primaryKeyFields: ['id'], - fields: [{ fieldName: 'id', displayName: 'Id', isRelationship: false }], + fields: [{ fieldName: 'id', displayName: 'Id', isRelationship: false, type: 'Number' }], actions: [ { name: 'send_email', displayName: 'Send Email', endpoint: '/forest/actions/send-email' }, ], @@ -72,7 +72,7 @@ const COLLECTION_SCHEMA_WITH_RELATION: CollectionSchema = { collectionDisplayName: 'Customers', primaryKeyFields: ['id'], fields: [ - { fieldName: 'id', displayName: 'Id', isRelationship: false }, + { fieldName: 'id', displayName: 'Id', isRelationship: false, type: 'Number' }, { fieldName: 'order', displayName: 'Order', @@ -89,8 +89,8 @@ const ORDERS_SCHEMA: CollectionSchema = { collectionDisplayName: 'Orders', primaryKeyFields: ['id'], fields: [ - { fieldName: 'id', displayName: 'Id', isRelationship: false }, - { fieldName: 'total', displayName: 'Total', isRelationship: false }, + { fieldName: 'id', displayName: 'Id', isRelationship: false, type: 'Number' }, + { fieldName: 'total', displayName: 'Total', isRelationship: false, type: 'Number' }, ], actions: [], }; From 5f4b7553dcc100b56504da8a885218b571cada4f Mon Sep 17 00:00:00 2001 From: alban bertolini Date: Mon, 1 Jun 2026 21:40:03 +0200 Subject: [PATCH 3/3] fix(workflow-executor): don't let a type-less field block the whole update step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review feedback (matthv): buildUpdateFieldTool maps over all non-relationship fields, so FieldTypeMissingError thrown by buildZodSchemaForField for a single drifted type-less field failed the entire update-record step — the global fragility this PR removes on the schema side. Exclude type-less fields from the choices offered to the AI (filter f.type != null). The AI only sees updatable fields; if none are updatable, NoWritableFieldsError. The override path still rejects an explicit type-less target via FieldTypeMissingError. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../executors/update-record-step-executor.ts | 5 ++- .../update-record-step-executor.test.ts | 44 ++++++++++++++----- 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/packages/workflow-executor/src/executors/update-record-step-executor.ts b/packages/workflow-executor/src/executors/update-record-step-executor.ts index 4c41a5a600..0b2bccbc8c 100644 --- a/packages/workflow-executor/src/executors/update-record-step-executor.ts +++ b/packages/workflow-executor/src/executors/update-record-step-executor.ts @@ -312,7 +312,10 @@ export default class UpdateRecordStepExecutor extends RecordStepExecutor !f.isRelationship); + // Exclude type-less fields: they can't be coerced/written, so offering them to the AI would + // let a single drifted field fail the whole step. The override path still rejects an explicit + // type-less target via FieldTypeMissingError. + const nonRelationFields = schema.fields.filter(f => !f.isRelationship && f.type != null); if (nonRelationFields.length === 0) { throw new NoWritableFieldsError(schema.collectionName); diff --git a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts index bab19d382f..48becbae9f 100644 --- a/packages/workflow-executor/test/executors/update-record-step-executor.test.ts +++ b/packages/workflow-executor/test/executors/update-record-step-executor.test.ts @@ -600,17 +600,22 @@ describe('UpdateRecordStepExecutor', () => { }); }); - describe('FieldTypeMissingError — writable field without a column type', () => { - it('fails the step instead of silently coercing the value to string (AI path)', async () => { - const agentPort = makeMockAgentPort(); + describe('type-less fields (orchestrator drift)', () => { + it('does not offer a type-less field to the AI, so it never blocks the whole step', async () => { + const updatedValues = { status: 'active' }; + const agentPort = makeMockAgentPort(updatedValues); const workflowPort = makeMockWorkflowPort({ customers: makeCollectionSchema({ - // Non-relationship field with no `type` (orchestrator drift). Schema accepts it (resilience), - // but update-record must not build a string schema and write the wrong type silently. - fields: [{ fieldName: 'age', displayName: 'Age', isRelationship: false }], + fields: [ + { fieldName: 'status', displayName: 'Status', isRelationship: false, type: 'String' }, + // Drifted field with no `type`: must be excluded from the AI choices, not fail the step. + { fieldName: 'age', displayName: 'Age', isRelationship: false }, + ], }), }); - const mockModel = makeMockModel({ input: { fieldName: 'Age', value: '42', reasoning: 'r' } }); + const mockModel = makeMockModel({ + input: { fieldName: 'Status', value: 'active', reasoning: 'r' }, + }); const context = makeContext({ model: mockModel.model, agentPort, @@ -621,12 +626,31 @@ describe('UpdateRecordStepExecutor', () => { const result = await executor.execute(); + expect(result.stepOutcome.status).toBe('success'); + expect(agentPort.updateRecord).toHaveBeenCalledWith( + { collection: 'customers', id: [42], values: { status: 'active' } }, + expect.objectContaining({ id: 1 }), + ); + }); + + it('throws NoWritableFieldsError when every non-relationship field lacks a type', async () => { + const workflowPort = makeMockWorkflowPort({ + customers: makeCollectionSchema({ + fields: [{ fieldName: 'age', displayName: 'Age', isRelationship: false }], + }), + }); + const context = makeContext({ + workflowPort, + stepDefinition: makeStep({ executionType: StepExecutionMode.FullyAutomated }), + }); + const executor = new UpdateRecordStepExecutor(context); + + const result = await executor.execute(); + expect(result.stepOutcome.status).toBe('error'); expect(result.stepOutcome.error).toBe( - "This field can't be updated because its type is missing from the schema. " + - 'Contact your administrator if the problem persists.', + 'This record type has no editable fields configured in Forest Admin.', ); - expect(agentPort.updateRecord).not.toHaveBeenCalled(); }); });