Skip to content

[Security][Workflows] Register workflow steps synchronously using async loaders#265230

Merged
KDKHD merged 4 commits intoelastic:mainfrom
KDKHD:improvement/async-step-registration
Apr 23, 2026
Merged

[Security][Workflows] Register workflow steps synchronously using async loaders#265230
KDKHD merged 4 commits intoelastic:mainfrom
KDKHD:improvement/async-step-registration

Conversation

@KDKHD
Copy link
Copy Markdown
Member

@KDKHD KDKHD commented Apr 23, 2026

Summary

Follows up on #264788 (merged by @semd) which added async-loader support to the ServerStepRegistry and PublicStepRegistry.

Previously, registerStepDefinition was called inside a deferred getStartServices().then() callback — meaning registration happened after the setup() lifecycle completed. semd flagged this as problematic in review comments on #259159 (comment 1, comment 2).

This PR applies the fix on the security_solution side: registerStepDefinition is now called synchronously during setup(). Each step is registered with an async loader that checks the feature flag at resolution time and returns undefined to skip registration when the flag is disabled.

Changes on both server and public sides:

  • registerWorkflowSteps is now a synchronous function accepting CoreSetup instead of CoreStart
  • Each step is wrapped in an async loader that defers the feature-flag check to resolution time
  • plugin.ts / plugin.tsx call registerWorkflowSteps directly in setup() (no deferred .then())
  • New unit tests for both server and public registerWorkflowSteps

Test plan

  • node scripts/jest x-pack/solutions/security/plugins/security_solution/server/workflows/step_types/register_workflow_steps.test.ts
  • node scripts/jest x-pack/solutions/security/plugins/security_solution/public/workflows/step_types/register_workflow_steps.test.ts
  • Confirm no regression in existing workflow step tests

Use the async-loader pattern supported by the workflow extension registry
(added in elastic#264788) so that `registerStepDefinition` is called
synchronously during `setup()` rather than deferred via `getStartServices().then()`.

Each step is now registered with an async loader that checks the feature flag
at resolution time and returns `undefined` to skip registration when disabled.
@KDKHD
Copy link
Copy Markdown
Member Author

KDKHD commented Apr 23, 2026

/ci

@KDKHD
Copy link
Copy Markdown
Member Author

KDKHD commented Apr 23, 2026

/ci

@KDKHD KDKHD marked this pull request as ready for review April 23, 2026 09:57
@KDKHD KDKHD requested a review from a team as a code owner April 23, 2026 09:57
@KDKHD KDKHD added backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes labels Apr 23, 2026
}
workflowsExtensions.registerStepDefinition(async () => {
if (!(await isEnabled)) return undefined;
return renderAlertNarrativeStepDefinition;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Consider lazy-loading the steps themselves as well. So they do not hit the plugin's main bundle size.

Suggested change
return renderAlertNarrativeStepDefinition;
return import('./render_alert_narrative_step').then((m) => m.renderAlertNarrativeStepDefinition);

Copy link
Copy Markdown
Contributor

@semd semd left a comment

Choose a reason for hiding this comment

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

LGTM!

@KDKHD
Copy link
Copy Markdown
Member Author

KDKHD commented Apr 23, 2026

/ci

@KDKHD KDKHD enabled auto-merge (squash) April 23, 2026 11:01
@KDKHD KDKHD merged commit b8f2e20 into elastic:main Apr 23, 2026
12 checks passed
smith pushed a commit to smith/kibana that referenced this pull request Apr 23, 2026
…nc loaders (elastic#265230)

## Summary

Follows up on elastic#264788 (merged by @semd) which added
async-loader support to the `ServerStepRegistry` and
`PublicStepRegistry`.

Previously, `registerStepDefinition` was called inside a deferred
`getStartServices().then()` callback — meaning registration happened
*after* the `setup()` lifecycle completed. semd flagged this as
problematic in review comments on elastic#259159 ([comment
1](elastic#259159 (comment)),
[comment
2](elastic#259159 (comment))).

This PR applies the fix on the security_solution side:
`registerStepDefinition` is now called **synchronously** during
`setup()`. Each step is registered with an async loader that checks the
feature flag at resolution time and returns `undefined` to skip
registration when the flag is disabled.

Changes on both server and public sides:
- `registerWorkflowSteps` is now a synchronous function accepting
`CoreSetup` instead of `CoreStart`
- Each step is wrapped in an async loader that defers the feature-flag
check to resolution time
- `plugin.ts` / `plugin.tsx` call `registerWorkflowSteps` directly in
`setup()` (no deferred `.then()`)
- New unit tests for both server and public `registerWorkflowSteps`

## Test plan

- [ ] `node scripts/jest
x-pack/solutions/security/plugins/security_solution/server/workflows/step_types/register_workflow_steps.test.ts`
- [ ] `node scripts/jest
x-pack/solutions/security/plugins/security_solution/public/workflows/step_types/register_workflow_steps.test.ts`
- [ ] Confirm no regression in existing workflow step tests
rbrtj pushed a commit to walterra/kibana that referenced this pull request Apr 27, 2026
…nc loaders (elastic#265230)

## Summary

Follows up on elastic#264788 (merged by @semd) which added
async-loader support to the `ServerStepRegistry` and
`PublicStepRegistry`.

Previously, `registerStepDefinition` was called inside a deferred
`getStartServices().then()` callback — meaning registration happened
*after* the `setup()` lifecycle completed. semd flagged this as
problematic in review comments on elastic#259159 ([comment
1](elastic#259159 (comment)),
[comment
2](elastic#259159 (comment))).

This PR applies the fix on the security_solution side:
`registerStepDefinition` is now called **synchronously** during
`setup()`. Each step is registered with an async loader that checks the
feature flag at resolution time and returns `undefined` to skip
registration when the flag is disabled.

Changes on both server and public sides:
- `registerWorkflowSteps` is now a synchronous function accepting
`CoreSetup` instead of `CoreStart`
- Each step is wrapped in an async loader that defers the feature-flag
check to resolution time
- `plugin.ts` / `plugin.tsx` call `registerWorkflowSteps` directly in
`setup()` (no deferred `.then()`)
- New unit tests for both server and public `registerWorkflowSteps`

## Test plan

- [ ] `node scripts/jest
x-pack/solutions/security/plugins/security_solution/server/workflows/step_types/register_workflow_steps.test.ts`
- [ ] `node scripts/jest
x-pack/solutions/security/plugins/security_solution/public/workflows/step_types/register_workflow_steps.test.ts`
- [ ] Confirm no regression in existing workflow step tests
SoniaSanzV pushed a commit to SoniaSanzV/kibana that referenced this pull request Apr 27, 2026
…nc loaders (elastic#265230)

## Summary

Follows up on elastic#264788 (merged by @semd) which added
async-loader support to the `ServerStepRegistry` and
`PublicStepRegistry`.

Previously, `registerStepDefinition` was called inside a deferred
`getStartServices().then()` callback — meaning registration happened
*after* the `setup()` lifecycle completed. semd flagged this as
problematic in review comments on elastic#259159 ([comment
1](elastic#259159 (comment)),
[comment
2](elastic#259159 (comment))).

This PR applies the fix on the security_solution side:
`registerStepDefinition` is now called **synchronously** during
`setup()`. Each step is registered with an async loader that checks the
feature flag at resolution time and returns `undefined` to skip
registration when the flag is disabled.

Changes on both server and public sides:
- `registerWorkflowSteps` is now a synchronous function accepting
`CoreSetup` instead of `CoreStart`
- Each step is wrapped in an async loader that defers the feature-flag
check to resolution time
- `plugin.ts` / `plugin.tsx` call `registerWorkflowSteps` directly in
`setup()` (no deferred `.then()`)
- New unit tests for both server and public `registerWorkflowSteps`

## Test plan

- [ ] `node scripts/jest
x-pack/solutions/security/plugins/security_solution/server/workflows/step_types/register_workflow_steps.test.ts`
- [ ] `node scripts/jest
x-pack/solutions/security/plugins/security_solution/public/workflows/step_types/register_workflow_steps.test.ts`
- [ ] Confirm no regression in existing workflow step tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting release_note:skip Skip the PR/issue when compiling release notes v9.5.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants