diff --git a/packages/workflow-executor/CLAUDE.md b/packages/workflow-executor/CLAUDE.md index c6f8b581e6..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<>`. 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/` 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..0b2bccbc8c 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 { @@ -301,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); @@ -316,7 +330,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 e488f599cf..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,21 +38,15 @@ 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(); +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 +58,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 +78,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, ''])( 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..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,6 +600,60 @@ describe('UpdateRecordStepExecutor', () => { }); }); + 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({ + 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: 'Status', value: 'active', 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('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 record type has no editable fields configured in Forest Admin.', + ); + }); + }); + 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 +1900,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: [], };