fix(workflow-executor): preserve AI suggestion in pending data by Tonours · Pull Request #1586 · ForestAdmin/agent-nodejs

Tonours · 2026-05-19T19:10:18Z

Summary

Keeps pendingData immutable so the AI proposal can be diffed against the user's final choice. User overrides now land in a sibling userConfirmation: Record<string, unknown> populated by patchAndReloadPendingData from the validated PATCH body.
update-record, trigger-action and load-related-record executors read user-confirmed values (value, actionResult, name/displayName/selectedRecordId) from userConfirmation with fallback to the AI suggestion in pendingData.
userConfirmation is replaced last-write-wins (no spread-merge) to avoid stale keys bleeding across sequential PATCHes.

Test plan

yarn workspace @forestadmin/workflow-executor build
yarn workspace @forestadmin/workflow-executor test (726 tests pass)
yarn workspace @forestadmin/workflow-executor lint
Regression tests added: AI suggestion preserved with user-override of value / selectedRecordId / relation name+displayName, accept-via-PATCH without override (fallback), rejection via PATCH with userConfirmation set.

qltysh · 2026-05-19T19:15:50Z

Coverage Impact

This PR will not change total coverage.

Modified Files with Diff Coverage (5)

Rating	File	% Diff	Uncovered Line #s
	...ow-executor/src/executors/load-related-record-step-executor.ts	100.0%
	packages/workflow-executor/src/http/pending-data-validators.ts	100.0%
	packages/workflow-executor/src/executors/base-step-executor.ts	100.0%
	...workflow-executor/src/executors/update-record-step-executor.ts	100.0%
	...-executor/src/executors/trigger-record-action-step-executor.ts	100.0%
	Total	100.0%

🚦 See full report on Qlty Cloud »

🛟 Help

Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.
Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.
File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)
- Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

Scra3 · 2026-05-19T20:41:43Z

+    const isString = (v: unknown): v is string => typeof v === 'string';
+    const isRecordId = (v: unknown): v is Array<string | number> =>
+      Array.isArray(v) && v.every(e => typeof e === 'string' || typeof e === 'number');


Data are already validated by zod when we receive the http request. I think we don't need to validate two times

Scra3 · 2026-05-19T20:47:56Z

-    // Re-derive relatedCollectionName from schema using the (possibly updated) relation name.
-    // `name` is always a fieldName (set from field.fieldName in buildTarget) — search directly.
+    const name = isString(userConfirmation?.name) ? userConfirmation.name : pendingData.name;
+    const displayName = isString(userConfirmation?.displayName)


displayName seems fully derivable from the schema once we have name, so could we remove it from the HTTP payload entirely? That would avoid having to keep name / displayName in sync and reduce the risk of persisting an inconsistent state.

More generally, it feels like the invariant here should be:

changing only selectedRecordId while keeping the same name is valid,
but if name changes, then selectedRecordId should also be required,
and displayName should be recomputed from the schema rather than trusted from the client.
So maybe we should tighten this in two places:

in the Zod schema, remove displayName and require selectedRecordId whenever name is provided,
in the executor, use name as the source of truth and recompute displayName from the resolved field/schema.

Scra3

Two blocking type-safety issues need to be addressed before merge.

WithUserConfirmation.userConfirmation is typed as Record<string, unknown>, erasing the per-step shape that Zod already validated — executors end up re-implementing validation with runtime guards.
McpStepExecutionData (line 106 in step-execution-data.ts) is the only executor that calls patchAndReloadPendingData but does not extend WithUserConfirmation — the userConfirmation field will be set at runtime but is invisible to TypeScript, breaking the promise of the PR.

Fix: export one inferred type per Zod schema in pending-data-validators.ts (e.g. export type LoadRelatedRecordConfirmation = z.infer<typeof loadRelatedRecordPatchSchema>) and use those precise types in the interfaces. The runtime guards in resolveFromSelection can then be dropped.

Scra3 · 2026-05-19T21:08:07Z

+// Parsed PATCH body kept beside `pendingData` so executors can read the user's
+// final input without overwriting the AI suggestion.
+export interface WithUserConfirmation {
+  userConfirmation?: Record<string, unknown>;


issue (blocking): Record<string, unknown> erases the per-step shape that Zod already validated — executors must then add runtime type guards for fields the schema already knows. Export one inferred type per schema in pending-data-validators.ts and use them here per step instead of this catch-all.

Scra3 · 2026-05-19T21:08:07Z

    );
  }

+  // Keeps `pendingData` immutable; mirrors `userConfirmed` only because


nit: "immutable" is misleading — userConfirmed is written back into pendingData four lines below. The intent is that the original AI suggestion is preserved (not overwritten by a full merge), which is different from immutability.

Scra3 · 2026-05-19T21:08:07Z

    }

-    const { name, displayName, selectedRecordId } = pendingData;
+    const isString = (v: unknown): v is string => typeof v === 'string';


issue (non-blocking): these guards exist only because userConfirmation is typed as Record<string, unknown> — Zod already validated that name is a string and selectedRecordId is Array<string | number>. Fix the WithUserConfirmation typing and both guards can be dropped in favour of direct property access.

Scra3 · 2026-05-19T21:08:07Z

-    // Re-derive relatedCollectionName from schema using the (possibly updated) relation name.
-    // `name` is always a fieldName (set from field.fieldName in buildTarget) — search directly.
+    const name = isString(userConfirmation?.name) ? userConfirmation.name : pendingData.name;
+    const displayName = isString(userConfirmation?.displayName)


suggestion: displayName can be re-derived from the schema once name is resolved (field.displayName), so there is no need to accept it from the PATCH body — trusting client-provided display names risks surfacing stale or inconsistent data.

Scra3 · 2026-05-20T13:59:14Z

https://linear.app/forestadmin/issue/PRD-378/executor-shouldnt-execute-non-automated-tasks

Scra3 · 2026-05-20T14:00:01Z

linked to: PRD-378

qltysh · 2026-05-25T15:33:28Z

All good ✅

… and re-derive displayName from schema - Export named Zod schemas and inferred types from pending-data-validators (UpdateRecordConfirmation, TriggerActionConfirmation, McpConfirmation, LoadRelatedRecordConfirmation) - Make WithUserConfirmation generic so each interface carries the exact confirmation shape instead of Record<string, unknown> - Add WithUserConfirmation<McpConfirmation> to McpStepExecutionData (was missing despite mcp executor writing userConfirmation) - Remove isString/isRecordId runtime guards in resolveFromSelection — now unnecessary with precise typing - Re-derive displayName from FieldSchema instead of accepting it from the PATCH body; remove displayName from loadRelatedRecordPatchSchema and contract - Fix inaccurate comment "Keeps pendingData immutable" in patchAndReloadPendingData Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

… in load-related-record patch Sending a different relation name without a new record ID would silently reuse the AI-suggested record ID from the original collection, producing a wrong or non-existent record in the new relation. Zod refine now enforces that selectedRecordId is required when name is overridden. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ata and validators test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…tion for load-related-record Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…tors Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…kup, warn on timeout discard, fix logStartup default, tighten key type - Replace non-null assertion on patchBodySchemas lookup with an explicit StepStateError guard - Downgrade post-timeout discard log from info to warn (correct severity) - Fix logStartup reporting undefined for pollingIntervalMs when env var is unset - Narrow patchBodySchemas key type from string to PatchableStepType union - Remove unused GuidanceConfirmation export Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…step type in patchAndReloadPendingData Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…onfirmableStepExecutionData union - Export ConfirmableStepExecutionData (explicit union of the 4 step types that use the confirmation flow) from step-execution-data.ts - Drop local WithPendingData intersection type — it re-declared constraints already present in the concrete types - Drop PatchBody cast — cast directly to TExec['userConfirmation'] after safeParse Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ution-data Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…date-record confirmation flow Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…pollingIntervalMs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…serConfirmation pendingData now holds only the AI suggestion. handleConfirmationFlow reads userConfirmed from userConfirmation instead of pendingData. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ion terminology Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ation split - Section 3 rewritten: pendingData = AI suggestion (never overwritten), userConfirmation = validated POST body stored separately - trigger-action and mcp split into separate blocks; actionResult documented - Remove stale userConfirmed from RunStore pendingData shapes - Fix "PATCH" → "POST" in two comments Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…ecord pendingData Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Tonours force-pushed the fix/workflow-executor-preserve-ai-suggestion-in-pending-data branch from c797768 to 7966b7f Compare May 19, 2026 19:22

macroscopeapp Bot reviewed May 19, 2026

View reviewed changes

Comment thread packages/workflow-executor/src/executors/load-related-record-step-executor.ts

Tonours force-pushed the fix/workflow-executor-preserve-ai-suggestion-in-pending-data branch from 7966b7f to d58877a Compare May 19, 2026 19:30

macroscopeapp Bot reviewed May 19, 2026

View reviewed changes

Comment thread packages/workflow-executor/src/executors/base-step-executor.ts

Scra3 reviewed May 19, 2026

View reviewed changes

Scra3 requested changes May 19, 2026

View reviewed changes

macroscopeapp Bot reviewed May 19, 2026

View reviewed changes

Comment thread packages/workflow-executor/src/executors/load-related-record-step-executor.ts

macroscopeapp Bot reviewed May 19, 2026

View reviewed changes

Comment thread packages/workflow-executor/src/http/pending-data-validators.ts Outdated

Scra3 force-pushed the fix/workflow-executor-preserve-ai-suggestion-in-pending-data branch from 96808c0 to 13499c5 Compare May 25, 2026 15:32

Scra3 changed the base branch from feat/prd-214-server-step-mapper to fix/execution-type May 25, 2026 15:33

Base automatically changed from fix/execution-type to feat/prd-214-server-step-mapper May 25, 2026 20:06

Scra3 force-pushed the fix/workflow-executor-preserve-ai-suggestion-in-pending-data branch from 64c7f81 to 6862dfc Compare May 25, 2026 20:16

macroscopeapp Bot reviewed May 25, 2026

View reviewed changes

Comment thread packages/workflow-executor/src/cli-core.ts Outdated

Tonours and others added 12 commits May 26, 2026 09:57

fix(workflow-executor): preserve AI suggestion in pending data

77bdd7b

style(workflow-executor): fix prettier formatting in step-execution-d…

d0d3705

…ata and validators test Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

fix(workflow-executor): reject empty string name and fix refine condi…

9d52092

…tion for load-related-record Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

style(workflow-executor): remove stale comment in pending-data-valida…

5ef2890

…tors Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

test(workflow-executor): cover StepStateError guard for unregistered …

fd1b153

…step type in patchAndReloadPendingData Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

style(workflow-executor): remove spurious Unions comment in step-exec…

eb5e37f

…ution-data Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(workflow-executor): simplify UpdateTarget construction in up…

91d90b8

…date-record confirmation flow Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

style(workflow-executor): fix prettier formatting

d3f3a5e

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Scra3 force-pushed the fix/workflow-executor-preserve-ai-suggestion-in-pending-data branch from 6862dfc to d3f3a5e Compare May 26, 2026 08:01

alban bertolini and others added 6 commits May 26, 2026 10:14

refactor(workflow-executor): remove redundant fallback in logStartup …

c54c3fa

…pollingIntervalMs Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(workflow-executor): move userConfirmed from pendingData to u…

150122a

…serConfirmation pendingData now holds only the AI suggestion. handleConfirmationFlow reads userConfirmed from userConfirmation instead of pendingData. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(workflow-executor): update comments to reflect userConfirmat…

6dbdf45

…ion terminology Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

docs(workflow-executor): remove stale displayName comment from contract

08ee945

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor(workflow-executor): extract FieldWithValue type for update-r…

240675f

…ecord pendingData Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Scra3 approved these changes May 26, 2026

View reviewed changes

Tonours merged commit f254266 into feat/prd-214-server-step-mapper May 26, 2026
49 of 57 checks passed

Tonours deleted the fix/workflow-executor-preserve-ai-suggestion-in-pending-data branch May 26, 2026 13:05

Conversation

Tonours commented May 19, 2026

Summary

Test plan

Uh oh!

qltysh Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Scra3 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Scra3 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Scra3 left a comment

Choose a reason for hiding this comment

Uh oh!

Scra3 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Scra3 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Scra3 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Scra3 May 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Scra3 commented May 20, 2026

Uh oh!

Scra3 commented May 20, 2026

Uh oh!

qltysh Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

qltysh Bot commented May 19, 2026 •

edited

Loading

qltysh Bot commented May 25, 2026 •

edited

Loading