fix(service-automation): route every retry attempt through the node input-schema guard

.changeset/automation-input-schema-retry-parity.md

@@ -0,0 +1,23 @@
+---
+"@objectstack/service-automation": patch
+---
+
+fix(service-automation): node input-schema validation now guards EVERY attempt of a retried flow, not only the first (#9889)
+
+Before this fix, `validateNodeInputSchemas` — the guard that refuses to run a
+flow whose node `config` violates its own declared `inputSchema` — was called
+only by `execute()` (attempt 1). Under `errorHandling.strategy: 'retry'`, the
+guard's throw routed into `retryExecution`, and every retry attempt ran
+through `executeWithoutRetry` with no guard at all: the nodes attempt 1
+refused to run were executed for real, with the config the guard rejected. A
+side-effecting node (a data write, an HTTP call, an email) behind a
+mis-declared `inputSchema` was reachable simply by declaring `retry`.
+
+Now both attempt paths call the same guard, so flows that were previously
+running on retry with a mis-declared `inputSchema` will be refused on every
+attempt (`success: false`, `status: 'failed'`, with the guard's message).
+Retry accounting is unchanged: each refused attempt still consumes retry
+budget, and valid flows retry exactly as before. If a flow of yours starts
+failing with `missing required input parameter` or `expected type ... but
+got ...` after this release, it was already being refused on its first
+attempt — fix the node's `config` to match its declared `inputSchema`.

packages/services/service-automation/src/engine.ts

@@ -3184,7 +3184,10 @@ export class AutomationEngine implements IAutomationService {
  reentryHeld = true;
  }
 
- // Validate node input schemas before execution
+ // Validate node input schemas before execution. [#9889] The same
+ // call sits in `executeWithoutRetry` — every retry attempt must be
+ // refused by the same guard that refused attempt 1; see the guard's
+ // own doc for the chokepoint contract.
  this.validateNodeInputSchemas(flow, variables);
 
  // DAG traversal execution
@@ -5257,6 +5260,18 @@ export class AutomationEngine implements IAutomationService {
  /**
  * Validate node input schemas before execution.
  * Checks that node config matches declared inputSchema if present.
+ *
+ * [#9889] The ONE definition-level input-schema chokepoint, called by BOTH
+ * attempt paths — `execute()` (attempt 1) and `executeWithoutRetry` (every
+ * retry attempt) — the same chokepoint discipline `seedRunVariables`
+ * carries for the variable environment (#9704). Its verdict is a pure
+ * function of the flow definition (`_variables` is deliberately unused),
+ * so a refusal on attempt 1 must hold on every attempt: when only
+ * `execute()` called it, `errorHandling.strategy: 'retry'` ran the very
+ * nodes attempt 1 refused to run. ⛔ A repair to the validation rules
+ * belongs HERE, in the shared method — re-inlining either caller's copy
+ * re-opens the execute/executeWithoutRetry drift this file has now paid
+ * for six times (#9378, #9415, #9414, #9510, #9704, #9889).
  */
  private validateNodeInputSchemas(flow: FlowParsed, _variables: Map<string, unknown>): void {
  for (const node of flow.nodes) {
@@ -6433,6 +6448,29 @@ export class AutomationEngine implements IAutomationService {
  return { success: false, code: 'FLOW_NO_START_NODE', error: 'Flow has no start node' };
  }
 
+ // [#9889] The SAME definition-level guard attempt 1 runs under —
+ // the sixth instance of this method drifting from `execute()`
+ // (#9378, #9415, #9414, #9510, #9704 before it), and the first
+ // that skipped a GUARD rather than an exit or the environment.
+ // Without this call, a flow whose node config violates its own
+ // declared `inputSchema` under `errorHandling.strategy: 'retry'`
+ // was refused on attempt 1 (the guard throws before any node
+ // executes) and then RUN FOR REAL on attempts 2..N, because the
+ // retry handoff lives in `execute()`'s catch and every retry
+ // attempt comes back through here — a refusal that holds only
+ // until the flow is retried, i.e. `retry` as a way past
+ // authoring-time validation. The guard's verdict is a pure
+ // function of the flow definition (`_variables` is unused), so
+ // re-running it cannot refuse anything attempt 1 would have
+ // allowed; the throw lands in this method's generic failure arm
+ // below, so each refused attempt still consumes retry budget and
+ // retry accounting is unchanged. Same chokepoint discipline as
+ // `seedRunVariables` (#9704): ONE method holds the rules, both
+ // attempt paths call it — `input-schema-retry-parity.test.ts`
+ // pins the per-attempt refusal so this call cannot be dropped
+ // silently.
+ this.validateNodeInputSchemas(flow, variables);
+
  await this.executeNode(startNode, flow, variables, runContext, steps);
 
  const output: Record<string, unknown> = {};

packages/services/service-automation/src/input-schema-retry-parity.test.ts

@@ -0,0 +1,150 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+import { describe, it, expect } from 'vitest';
+import { AutomationEngine } from './engine.js';
+
+/**
+ * #9889 — node input-schema validation must hold on EVERY attempt, not only
+ * the first.
+ *
+ * `validateNodeInputSchemas` reports by throwing, and the retry handoff lives
+ * inside `execute()`'s catch. Before the repair, only `execute()` called the
+ * guard: for a flow whose node config violates its own declared `inputSchema`
+ * under `errorHandling.strategy: 'retry'`, attempt 1 threw in the guard before
+ * any node executed, the catch routed to `retryExecution`, and attempts 2..N
+ * ran through `executeWithoutRetry` — which never called the guard — so the
+ * nodes attempt 1 refused permission to run were executed for real, with the
+ * config the guard rejected. A `retry` strategy was a way past authoring-time
+ * validation.
+ *
+ * The pins here are written against the OBSERVABLE side effect (an executor
+ * spy counting real node executions), not only the thrown error: the defect's
+ * whole harm is a side-effecting node (a data write, an HTTP call, an email)
+ * running with rejected config, and an assertion on the returned error alone
+ * stays green while that node runs.
+ *
+ * On the envelope: the refusal is asserted as `success: false` +
+ * `status: 'failed'` + the guard's own message. There is no ADR-0112 `code`
+ * to assert — deliberately: #9378's classification gives `code` to the
+ * NEVER-DISPATCHED exits (`FLOW_DISABLED`, `FLOW_NO_START_NODE`) and `status:
+ * 'failed'` to the dispatched-and-failed exits, and the guard's throw rides
+ * the latter family on both attempt paths. Whether a definition-level refusal
+ * should instead be classified non-retryable (its verdict cannot change per
+ * attempt) is the open question #9889 leaves to a maintainer ruling; these
+ * pins assert the parity floor only.
+ */
+
+function createTestLogger(): any {
+ return { info: () => {}, warn: () => {}, error: () => {}, debug: () => {}, child: () => createTestLogger() };
+}
+
+/**
+ * An engine holding one flow whose single work node counts every REAL
+ * execution — the observable the negative pins are written against.
+ */
+function countingFlowEngine(opts: {
+ config: Record<string, unknown>;
+ inputSchema: Record<string, { type: string; required?: boolean }>;
+ /** What the spy executor returns; defaults to success. */
+ executeResult?: (attempt: number) => { success: boolean; error?: string };
+}) {
+ const engine = new AutomationEngine(createTestLogger());
+ const runs = { count: 0 };
+
+ engine.registerNodeExecutor({
+ type: 'script',
+ async execute() {
+ runs.count++;
+ return opts.executeResult ? opts.executeResult(runs.count) : { success: true };
+ },
+ } as any);
+
+ engine.registerFlow('guarded', {
+ name: 'guarded',
+ label: 'Guarded',
+ type: 'autolaunched',
+ errorHandling: { strategy: 'retry', maxRetries: 2, backoffMs: 0 },
+ nodes: [
+ { id: 'start', type: 'start', label: 'Start' },
+ {
+ id: 'work',
+ type: 'script' as any,
+ label: 'Work',
+ config: opts.config,
+ inputSchema: opts.inputSchema,
+ },
+ { id: 'end', type: 'end', label: 'End' },
+ ],
+ edges: [
+ { id: 'e0', source: 'start', target: 'work' },
+ { id: 'e1', source: 'work', target: 'end' },
+ ],
+ } as any);
+
+ return { engine, runs };
+}
+
+describe("#9889 — input-schema refusal holds on every attempt under strategy: 'retry'", () => {
+ it('never executes a node whose config mis-types its declared inputSchema — on ANY attempt', async () => {
+ const { engine, runs } = countingFlowEngine({
+ config: { count: 'not_a_number' },
+ inputSchema: { count: { type: 'number', required: true } },
+ });
+
+ const result = await engine.execute('guarded');
+
+ // The refusal, as the caller sees it (see header for why no `code`).
+ expect(result.success).toBe(false);
+ expect(result.status).toBe('failed');
+ expect(result.error).toContain("expected type 'number' but got 'string'");
+
+ // The point of the card: the side-effecting node ran ZERO times.
+ // Pre-repair this was 2 — refused on attempt 1, executed for real on
+ // attempts 2 and 3.
+ expect(runs.count).toBe(0);
+
+ // And the refusal happened PER ATTEMPT, not by short-circuiting the
+ // retry loop: every attempt still dispatched and consumed budget
+ // (retry accounting unchanged — the non-retryable classification is
+ // the open question, not this repair), so the run log holds one
+ // failed row per attempt (1 initial + maxRetries), each carrying the
+ // guard's own message.
+ const attemptRows = await engine.listRuns('guarded', { status: 'failed' });
+ expect(attemptRows).toHaveLength(3);
+ for (const row of attemptRows) {
+ expect(row.error).toContain("expected type 'number' but got 'string'");
+ }
+ });
+
+ it('never executes a node missing a required declared input — on ANY attempt', async () => {
+ const { engine, runs } = countingFlowEngine({
+ config: {},
+ inputSchema: { url: { type: 'string', required: true } },
+ });
+
+ const result = await engine.execute('guarded');
+
+ expect(result.success).toBe(false);
+ expect(result.status).toBe('failed');
+ expect(result.error).toContain("missing required input parameter 'url'");
+ expect(runs.count).toBe(0);
+ });
+
+ it('still retries a VALID flow normally — the guard refuses nothing attempt 1 allowed', async () => {
+ const { engine, runs } = countingFlowEngine({
+ config: { count: 42 },
+ inputSchema: { count: { type: 'number', required: true } },
+ // Attempt 1 fails downstream (a transient error, the case retry
+ // exists for); attempt 2 succeeds.
+ executeResult: attempt =>
+ attempt === 1 ? { success: false, error: 'downstream 503' } : { success: true },
+ });
+
+ const result = await engine.execute('guarded');
+
+ expect(result.success).toBe(true);
+ // Attempt 1 ran and failed, attempt 2 ran and succeeded — the fix must
+ // not turn a legitimate retry into a refusal.
+ expect(runs.count).toBe(2);
+ });
+});