[One Workflow] Support triggering workflows for recovered and ongoing alerts#256289
[One Workflow] Support triggering workflows for recovered and ongoing alerts#256289talboren wants to merge 8 commits intoelastic:mainfrom
Conversation
Add alertStates configuration to the workflow connector, allowing users to select which alert states (firing, ongoing, recovered) should trigger workflow execution. Defaults to firing-only for full backward compat with existing rules -- no migration needed. Closes elastic/security-team#16239 Made-with: Cursor
…/alert-states-workflow
…/alert-states-workflow
- Spread original alerts into filteredAlerts to preserve `all` property - Resolve alertStates to concrete booleans to match Zod-inferred types - Update connector_types.test.ts.snap for new alertStates schema Made-with: Cursor
…/alert-states-workflow
Made-with: Cursor
There was a problem hiding this comment.
Pull request overview
Adds an alertStates configuration to the Workflows connector so users can choose which alert states (Firing/New, Ongoing, Recovered) trigger workflow execution, while keeping the default behavior unchanged for existing rules.
Changes:
- Adds
alertStatesto server/public types + server validation schemas. - Filters alert groups by selected states before building the workflow alert event, and merges selected states into
event.alerts. - Adds a “Run workflow for” checkbox group in the rule action UI + updates/extends test coverage and snapshots.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| x-pack/platform/plugins/shared/actions/server/integration_tests/snapshots/connector_types.test.ts.snap | Updates connector schema snapshot to include alertStates. |
| src/platform/plugins/shared/workflows_management/server/connectors/workflows/types.ts | Introduces server-side AlertStates type and wires it into params. |
| src/platform/plugins/shared/workflows_management/server/connectors/workflows/schema.ts | Adds Zod + config-schema validation for alertStates. |
| src/platform/plugins/shared/workflows_management/server/connectors/workflows/index.ts | Resolves defaults + filters alert groups by selected states before building the event. |
| src/platform/plugins/shared/workflows_management/server/connectors/workflows/index.test.ts | Adds adapter tests for defaulting and inclusion/exclusion by alertStates. |
| src/platform/plugins/shared/workflows_management/public/connectors/workflows/workflows_params.tsx | Adds UI checkbox group and initializes alertStates defaults. |
| src/platform/plugins/shared/workflows_management/public/connectors/workflows/workflows_params.test.tsx | Adds UI tests for rendering, defaults, toggling, and initialization. |
| src/platform/plugins/shared/workflows_management/public/connectors/workflows/types.ts | Exposes AlertStates + adds it to public action params. |
| src/platform/plugins/shared/workflows_management/common/utils/build_alert_event.ts | Merges new/ongoing/recovered alert arrays into event.alerts. |
| src/platform/plugins/shared/workflows_management/common/utils/build_alert_event.test.ts | Adds unit tests for alert merging and rule metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (actionParams.subActionParams.summaryMode === undefined) { | ||
| editAction( | ||
| 'subActionParams', | ||
| { ...actionParams.subActionParams, summaryMode: true }, | ||
| index | ||
| ); | ||
| } | ||
| if (!actionParams.subActionParams.alertStates) { | ||
| editAction( | ||
| 'subActionParams', | ||
| { | ||
| ...actionParams.subActionParams, | ||
| alertStates: { new: true, ongoing: false, recovered: false }, | ||
| }, | ||
| index | ||
| ); | ||
| } |
There was a problem hiding this comment.
The useEffect can call editAction('subActionParams', ...) twice in the same pass when both summaryMode is undefined and alertStates is missing. The second call spreads the original actionParams.subActionParams (which may still have summaryMode: undefined), potentially overwriting the first update and leaving summaryMode unset. Combine these defaults into a single computed nextSubActionParams object (setting both summaryMode and alertStates when needed) and call editAction once (or only when the computed object differs).
| if (actionParams.subActionParams.summaryMode === undefined) { | |
| editAction( | |
| 'subActionParams', | |
| { ...actionParams.subActionParams, summaryMode: true }, | |
| index | |
| ); | |
| } | |
| if (!actionParams.subActionParams.alertStates) { | |
| editAction( | |
| 'subActionParams', | |
| { | |
| ...actionParams.subActionParams, | |
| alertStates: { new: true, ongoing: false, recovered: false }, | |
| }, | |
| index | |
| ); | |
| } | |
| let nextSubActionParams = actionParams.subActionParams; | |
| let needsUpdate = false; | |
| if (nextSubActionParams.summaryMode === undefined) { | |
| nextSubActionParams = { | |
| ...nextSubActionParams, | |
| summaryMode: true, | |
| }; | |
| needsUpdate = true; | |
| } | |
| if (!nextSubActionParams.alertStates) { | |
| nextSubActionParams = { | |
| ...nextSubActionParams, | |
| alertStates: { new: true, ongoing: false, recovered: false }, | |
| }; | |
| needsUpdate = true; | |
| } | |
| if (needsUpdate) { | |
| editAction('subActionParams', nextSubActionParams, index); | |
| } |
| export interface AlertStates { | ||
| [key: string]: boolean | undefined; | ||
| new?: boolean; | ||
| ongoing?: boolean; | ||
| recovered?: boolean; | ||
| } |
There was a problem hiding this comment.
The index signature ([key: string]: ...) makes AlertStates accept arbitrary keys, which weakens type-safety (e.g., typos like { recoevered: true } compile) and diverges from the server’s fixed-key shape. Consider removing the index signature and using explicit keys only (optionally paired with a type AlertStateId = 'new' | 'ongoing' | 'recovered' for the checkbox option IDs) to keep public/server contracts aligned and prevent silent misconfiguration.
| const AlertStatesSchema = z.object({ | ||
| new: z.boolean().optional().default(true), | ||
| ongoing: z.boolean().optional().default(false), | ||
| recovered: z.boolean().optional().default(false), | ||
| }); |
There was a problem hiding this comment.
AlertStatesSchema is not strict, so unknown keys can be silently stripped/accepted by Zod parsing (depending on how parsing is performed), which can hide client typos and may differ from the stricter config-schema expectations. Consider making this schema strict (e.g., .strict()) so invalid keys are rejected consistently at validation time.
| const AlertStatesSchema = z.object({ | |
| new: z.boolean().optional().default(true), | |
| ongoing: z.boolean().optional().default(false), | |
| recovered: z.boolean().optional().default(false), | |
| }); | |
| const AlertStatesSchema = z | |
| .object({ | |
| new: z.boolean().optional().default(true), | |
| ongoing: z.boolean().optional().default(false), | |
| recovered: z.boolean().optional().default(false), | |
| }) | |
| .strict(); |
| inputs: z.any().optional(), | ||
| spaceId: z.string(), | ||
| summaryMode: z.boolean().optional().default(true), | ||
| alertStates: AlertStatesSchema.optional(), |
There was a problem hiding this comment.
AlertStatesSchema is not strict, so unknown keys can be silently stripped/accepted by Zod parsing (depending on how parsing is performed), which can hide client typos and may differ from the stricter config-schema expectations. Consider making this schema strict (e.g., .strict()) so invalid keys are rejected consistently at validation time.
| const { workflowId, summaryMode = true } = subActionParams; | ||
| const resolvedAlertStates = { | ||
| new: subActionParams.alertStates?.new !== false, | ||
| ongoing: subActionParams.alertStates?.ongoing === true, | ||
| recovered: subActionParams.alertStates?.recovered === true, | ||
| }; |
There was a problem hiding this comment.
The logic to resolve defaults for alertStates is duplicated in both the try and catch paths. Extracting this into a small helper (e.g., resolveAlertStates(alertStates?: AlertStates)) would reduce repetition and keep server-side defaulting semantics consistent if they change later.
| alertStates: params?.subActionParams?.alertStates | ||
| ? { | ||
| new: params.subActionParams.alertStates.new !== false, | ||
| ongoing: params.subActionParams.alertStates.ongoing === true, | ||
| recovered: params.subActionParams.alertStates.recovered === true, | ||
| } | ||
| : undefined, |
There was a problem hiding this comment.
The logic to resolve defaults for alertStates is duplicated in both the try and catch paths. Extracting this into a small helper (e.g., resolveAlertStates(alertStates?: AlertStates)) would reduce repetition and keep server-side defaulting semantics consistent if they change later.
⏳ Build in-progress, with failures
Failed CI StepsTest Failures
History
|
|
We've not hooked up our "add a comment about intermediate releases on connector parameter changes", so here it is manually:
In practice this means two PRs - one that introduces the schema changes, but adds no code (or minimal code) that sets a value in the new schema fields. After that has been merged and is running in serverless, the second PR setting the field can be merged. |
|
@nastasha-solomon I noticed the copy in the dialog says "Firing alerts", figured I'd ping you to see if that's the current terminology. I thought we used "active" in the past, though "firing" is probably conceptually better :-) |
Add backend-only support for configuring which alert states (new, ongoing, recovered) trigger workflow execution. This is the first part of an intermediate release strategy — schema and adapter changes land first, UI follows in a subsequent PR. - Add AlertStates interface and optional alertStates to types/schema - Add resolveAlertStates helper with backward-compatible defaults (new: true, ongoing: false, recovered: false) - Filter alert groups by resolved states in buildActionParams - Merge new/ongoing/recovered alert arrays into flat event.alerts - Schema uses optional booleans with no defaults (safe for rollback) Supersedes elastic#256289 (closed) which included both backend and UI changes. Closes elastic/security-team#16239 Made-with: Cursor
…#257363) ## Summary Adds backend-only support for configuring which alert states (`new`, `ongoing`, `recovered`) trigger workflow execution. This is the **first part of an intermediate release strategy** — schema and adapter changes land first, with the UI to follow in a subsequent PR. - Adds `AlertStates` interface and optional `alertStates` field to the workflows connector types and schema (Zod + config-schema) - Adds `resolveAlertStates()` helper with backward-compatible defaults (`new: true, ongoing: false, recovered: false`) — existing rules continue to work without migration - Filters alert groups by resolved states in `buildActionParams` before building the alert event - Updates `buildAlertEvent` to merge `new`/`ongoing`/`recovered` alert arrays into a flat `event.alerts` array (preserves Liquid template compatibility) - Schema fields are optional with no defaults, ensuring safe rollbacks during serverless rolling upgrades **Why backend-only?** Per the intermediate release pattern for serverless, schema changes must land and be deployed before the UI that writes those fields. This ensures older nodes can validate objects during rolling upgrades and rollbacks. Supersedes #256289 (closed), which included both backend and UI changes. The UI portion (alert state checkbox group in the rule action form) will follow in a separate PR. ### PR review feedback addressed from #256289: - Removed index signature from `AlertStates` interface for stricter type safety - Made `AlertStatesSchema` strict (`.strict()`) to reject unknown keys - Extracted `resolveAlertStates()` helper to avoid duplicated default logic ## References Closes elastic/security-team#16239 Made with [Cursor](https://cursor.com)
…elastic#257363) ## Summary Adds backend-only support for configuring which alert states (`new`, `ongoing`, `recovered`) trigger workflow execution. This is the **first part of an intermediate release strategy** — schema and adapter changes land first, with the UI to follow in a subsequent PR. - Adds `AlertStates` interface and optional `alertStates` field to the workflows connector types and schema (Zod + config-schema) - Adds `resolveAlertStates()` helper with backward-compatible defaults (`new: true, ongoing: false, recovered: false`) — existing rules continue to work without migration - Filters alert groups by resolved states in `buildActionParams` before building the alert event - Updates `buildAlertEvent` to merge `new`/`ongoing`/`recovered` alert arrays into a flat `event.alerts` array (preserves Liquid template compatibility) - Schema fields are optional with no defaults, ensuring safe rollbacks during serverless rolling upgrades **Why backend-only?** Per the intermediate release pattern for serverless, schema changes must land and be deployed before the UI that writes those fields. This ensures older nodes can validate objects during rolling upgrades and rollbacks. Supersedes elastic#256289 (closed), which included both backend and UI changes. The UI portion (alert state checkbox group in the rule action form) will follow in a separate PR. ### PR review feedback addressed from elastic#256289: - Removed index signature from `AlertStates` interface for stricter type safety - Made `AlertStatesSchema` strict (`.strict()`) to reject unknown keys - Extracted `resolveAlertStates()` helper to avoid duplicated default logic ## References Closes elastic/security-team#16239 Made with [Cursor](https://cursor.com)
…elastic#257363) ## Summary Adds backend-only support for configuring which alert states (`new`, `ongoing`, `recovered`) trigger workflow execution. This is the **first part of an intermediate release strategy** — schema and adapter changes land first, with the UI to follow in a subsequent PR. - Adds `AlertStates` interface and optional `alertStates` field to the workflows connector types and schema (Zod + config-schema) - Adds `resolveAlertStates()` helper with backward-compatible defaults (`new: true, ongoing: false, recovered: false`) — existing rules continue to work without migration - Filters alert groups by resolved states in `buildActionParams` before building the alert event - Updates `buildAlertEvent` to merge `new`/`ongoing`/`recovered` alert arrays into a flat `event.alerts` array (preserves Liquid template compatibility) - Schema fields are optional with no defaults, ensuring safe rollbacks during serverless rolling upgrades **Why backend-only?** Per the intermediate release pattern for serverless, schema changes must land and be deployed before the UI that writes those fields. This ensures older nodes can validate objects during rolling upgrades and rollbacks. Supersedes elastic#256289 (closed), which included both backend and UI changes. The UI portion (alert state checkbox group in the rule action form) will follow in a separate PR. ### PR review feedback addressed from elastic#256289: - Removed index signature from `AlertStates` interface for stricter type safety - Made `AlertStatesSchema` strict (`.strict()`) to reject unknown keys - Extracted `resolveAlertStates()` helper to avoid duplicated default logic ## References Closes elastic/security-team#16239 Made with [Cursor](https://cursor.com)
Add the UI portion of the alertStates feature — a "Run workflow for" checkbox group (New / Ongoing / Recovered alerts) in the rule action form, between the workflow selector and the action frequency switch. This is the second part of the intermediate release for elastic#257363: - Part 1 (merged): backend schema + adapter logic (elastic#257363) - Part 2 (this PR): UI to set alertStates on rule actions - Add AlertStates type and alertStates? to public WorkflowsActionParams - Render EuiCheckboxGroup with New/Ongoing/Recovered options - Use single editAction call in useEffect for initialization (addresses Copilot review feedback from elastic#256289) - Use "New alerts" terminology (not "Firing") per review feedback - Add 6 new tests for checkbox rendering, defaults, and toggling Closes elastic/security-team#16239 Made-with: Cursor
…59770) ## Summary Adds a "Run workflow for" checkbox group (New / Ongoing / Recovered alerts) to the workflow connector rule action form, allowing users to configure which alert states trigger workflow execution. This is the **second part of the intermediate release** for alertStates support: - **Part 1 (merged):** Backend schema + adapter logic — #257363 - **Part 2 (this PR):** UI to set `alertStates` on rule actions ### Changes - Adds `AlertStates` type and optional `alertStates` to public `WorkflowsActionParams` - Renders `EuiCheckboxGroup` with **New alerts** / **Ongoing alerts** / **Recovered alerts** options between the workflow selector and the action frequency switch - Uses a single `editAction` call in `useEffect` for initializing both `summaryMode` and `alertStates` defaults (addresses [Copilot review feedback](#256289 (comment)) from #256289) - Defaults to `{ new: true, ongoing: false, recovered: false }` — existing rules continue to work without migration - Uses "New alerts" terminology (not "Firing") per [review feedback](#256289 (comment)) from @pmuellr ### Background The original PR #256289 included both backend and UI changes but was closed to follow the [intermediate release pattern](https://docs.elastic.dev/reops/developer-guide/best-practices-rules#upgrades-and-rollbacks) for serverless. The backend changes landed first in #257363, and this PR completes the feature by enabling the UI. ## References Closes elastic/security-team#16239 Made with [Cursor](https://cursor.com) --------- Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Summary
alertStatesconfiguration ({ new, ongoing, recovered }) to the workflow connector, allowing users to select which alert states should trigger workflow execution{ new: true, ongoing: false, recovered: false }— exactly matching current behavior so existing rules require zero migrationReferences
Closes elastic/security-team#16239
Changes
server/connectors/workflows/types.tsAlertStatesinterface andalertStates?toRunWorkflowParamsserver/connectors/workflows/schema.tsalertStatesto Zod and config-schema (all optional)server/connectors/workflows/index.tsalertStatescommon/utils/build_alert_event.tsnew,ongoing,recoveredalert arrayspublic/connectors/workflows/types.tsAlertStatesandalertStates?toWorkflowsActionParamspublic/connectors/workflows/workflows_params.tsxYAML
Made with Cursor