Skip to content

fix(workflow): replace automatedExecution with executionType#1590

Open
EnkiP wants to merge 6 commits into
feat/prd-214-server-step-mapperfrom
fix/execution-type
Open

fix(workflow): replace automatedExecution with executionType#1590
EnkiP wants to merge 6 commits into
feat/prd-214-server-step-mapperfrom
fix/execution-type

Conversation

@EnkiP
Copy link
Copy Markdown
Member

@EnkiP EnkiP commented May 21, 2026

Definition of Done

fixes PRD-378

need frontend PR: https://github.com/ForestAdmin/forestadmin/pull/9708
and backend PR: https://github.com/ForestAdmin/forestadmin-server/pull/8254
branch name is always fix/execution-type

General

  • Write an explicit title for the Pull Request, following Conventional Commits specification
  • Test manually the implemented changes
  • Validate the code quality (indentation, syntax, style, simplicity, readability)

Security

  • Consider the security impact of the changes made

Note

[!NOTE]

Replace automaticExecution with executionType enum across workflow step executors

  • Removes the automaticExecution boolean from all step schemas in step-definition.ts and replaces it with an optional executionType enum (ServerStepExecutionTypeEnum: FullyAutomated, AutomatedWithConfirmation, Manual).
  • Updates all executors (TriggerRecordActionStepExecutor, UpdateRecordStepExecutor, LoadRelatedRecordStepExecutor, McpStepExecutor) to branch on executionType === FullyAutomated instead of automaticExecution.
  • Adds manual execution handling to ConditionStepExecutor: when executionType=Manual and no incoming pending data exists, the step returns awaiting-input without calling the AI.
  • Changes the default polling interval in build-workflow-executor.ts from 5000 ms to 30000 ms.
  • Risk: automaticExecution is no longer accepted in step definitions; any callers still passing this field will have it ignored or fail schema validation.

Changes since #1590 opened

  • Replaced ServerStepExecutionTypeEnum with StepExecutionMode enum throughout workflow-executor package [77f5301]
  • Added warn method to Logger interface and all logger implementations and test mocks [77f5301]
  • Implemented unsupported execution type warning feature in step executors [77f5301]

Macroscope summarized 851e5ee.

@qltysh
Copy link
Copy Markdown

qltysh Bot commented May 21, 2026

2 new issues

Tool Category Rule Count
qlty Structure Function with many returns (count = 4): toStepOutcome 2

@linear
Copy link
Copy Markdown

linear Bot commented May 21, 2026

PRD-378

@qltysh
Copy link
Copy Markdown

qltysh Bot commented May 21, 2026

Qlty


Coverage Impact

⬇️ Merging this pull request will decrease total coverage on feat/prd-214-server-step-mapper by 0.01%.

Modified Files with Diff Coverage (15)

RatingFile% DiffUncovered Line #s
Coverage rating: A Coverage rating: C
packages/workflow-executor/src/adapters/console-logger.ts0.0%9
Coverage rating: A Coverage rating: A
...ow-executor/src/executors/load-related-record-step-executor.ts100.0%
Coverage rating: A Coverage rating: A
packages/workflow-executor/src/build-workflow-executor.ts100.0%
Coverage rating: A Coverage rating: A
packages/workflow-executor/src/adapters/pretty-logger.ts0.0%15
Coverage rating: A Coverage rating: A
...s/workflow-executor/src/executors/read-record-step-executor.ts100.0%
Coverage rating: A Coverage rating: A
...ges/workflow-executor/src/executors/condition-step-executor.ts100.0%
Coverage rating: A Coverage rating: A
packages/workflow-executor/src/executors/base-step-executor.ts100.0%
Coverage rating: A Coverage rating: A
...workflow-executor/src/executors/update-record-step-executor.ts100.0%
Coverage rating: A Coverage rating: A
...-executor/src/executors/trigger-record-action-step-executor.ts100.0%
Coverage rating: A Coverage rating: A
...workflow-executor/src/adapters/run-to-available-step-mapper.ts100.0%
Coverage rating: A Coverage rating: A
packages/workflow-executor/src/types/validated/step-definition.ts100.0%
Coverage rating: A Coverage rating: A
packages/workflow-executor/src/executors/mcp-step-executor.ts100.0%
Coverage rating: A Coverage rating: A
packages/workflow-executor/src/adapters/step-definition-mapper.ts100.0%
Coverage rating: B Coverage rating: B
...ages/workflow-executor/src/executors/guidance-step-executor.ts100.0%
New Coverage rating: A
packages/workflow-executor/src/adapters/server-types.ts100.0%
Total96.7%
🤖 Increase coverage with AI coding...
In the `fix/execution-type` branch, add test coverage for this new code:

- `packages/workflow-executor/src/adapters/console-logger.ts` -- Line 9
- `packages/workflow-executor/src/adapters/pretty-logger.ts` -- Line 15

🚦 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.

Comment thread packages/workflow-executor/src/executors/base-step-executor.ts
Copy link
Copy Markdown
Member

@Scra3 Scra3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good direction on the enum migration. A few architectural and documentation points before merge.


import { z } from 'zod';

import { ServerStepExecutionTypeEnum } from '../../adapters/server-types';
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): ServerStepExecutionTypeEnum from the adapter layer leaks into the domain schema — declare a StepExecutionMode enum here in types/validated/ instead and have step-definition-mapper.ts be the single translation point. Executors should import from types/validated/, never from adapters/.

const baseRecordFields = {
...baseFields,
automaticExecution: z.boolean().optional(),
executionType: z.enum(ServerStepExecutionTypeEnum).optional(),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: z.enum() is designed for string-tuple arrays; z.nativeEnum(StepExecutionMode) is the idiomatic form for TypeScript enum objects — use it once the domain enum is declared above.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the commentary: Use z.enum(EnumObject), not z.nativeEnum — the latter is deprecated in zod 4.

}

// Branch C -- Awaiting confirmation
// Branch C -- Awaiting confirmation (also covers Manual fallback)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): also covers Manual fallback hides an intentional design choice — document explicitly that manual and automatedWithConfirmation share Branch C because the AI suggestion is always computed and the distinction is UI-only (how the front presents the confirmation to the user).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, add a warning logs if we receive "manual". Add it on each step

}

// Branch C -- pre-fetch candidates, await user confirmation
// Branch C -- pre-fetch candidates, await user confirmation (also covers Manual fallback)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): same as update-record — document why manual and automatedWithConfirmation intentionally share Branch C.

}

// Branch C -- Awaiting confirmation (frontend executes the action, including forms)
// Branch C -- Awaiting confirmation (frontend executes the action, including forms).
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): same — document the intentional equivalence of manual and automatedWithConfirmation in Branch C.

}

// Branch C -- Awaiting confirmation
// Branch C -- Awaiting confirmation (also covers Manual fallback)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

issue (non-blocking): same — document the intentional equivalence of manual and automatedWithConfirmation in Branch C.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants