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

executeWithoutRetry — the method retryExecution re-runs the flow through on
every retry attempt — never called validateNodeInputSchemas, so a flow whose
node config violates its own declared inputSchema under
errorHandling.strategy: 'retry' was refused on attempt 1 and executed for
real on attempts 2..N. Both attempt paths now call the same guard (the
seedRunVariables chokepoint discipline from #9704); retry accounting is
unchanged. Pins assert the side-effecting node never runs on any attempt,
and that a valid flow still retries normally.


Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx

Co-authored-by: Claude <noreply@anthropic.com>

Author: os-warren

.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);
+    });
+});