[Alerting V2] [ResponseOps] Recovery delay is configurable and persisted even when it has no effect on execution by doakalexi · Pull Request #288802 · elastic/kibana

doakalexi · 2026-09-02T19:08:15Z

Summary

Recovery delay (state_transition.recovering_count / state_transition.recovering_timeframe) only ever does real work when a recovery strategy is enabled. This PR updates the schema validation on create/update to not allow creating a rule with state_transition.recovering_count > 0 / state_transition.recovering_timeframe when the recovery_strategy is null or undefined.

Note: the validation does allow recovering_count: 0 it is not a delay and the episode will recover immediately. I'm happy to change this if that is confusing.

Checklist

Unit or functional tests were updated or added to match the most common scenarios

To verify

Start Kibana and enable alerting V2
Go to dev tools and run the following:

Verifyrecovery_strategy: "none" + recovering_count > 0 fails

POST kbn:/api/alerting/v2/rules
{
  "kind": "alert",
  "metadata": { "name": "verify-inert-a1" },
  "schedule": { "every": "5m" },
  "query": { "format": "standalone", "breach": { "query": "FROM logs-* | LIMIT 1" } },
  "recovery_strategy": "none",
  "state_transition": { "pending_count": 0, "recovering_count": 3 }
}

Verify recovery_strategy unset + recovering_timeframe fails

POST kbn:/api/alerting/v2/rules
{
  "kind": "alert",
  "metadata": { "name": "verify-inert-a2" },
  "schedule": { "every": "5m" },
  "query": { "format": "standalone", "breach": { "query": "FROM logs-* | LIMIT 1" } },
  "state_transition": { "recovering_timeframe": "5m" }
}

Verify it still fails with no_data_strategy: "recover"

POST kbn:/api/alerting/v2/rules
{
  "kind": "alert",
  "metadata": { "name": "verify-inert-a3" },
  "schedule": { "every": "5m" },
  "query": {
    "format": "standalone",
    "breach": { "query": "FROM logs-* | LIMIT 1" },
    "no_data": { "query": "FROM logs-* | STATS c = COUNT(*)" }
  },
  "recovery_strategy": "none",
  "no_data_strategy": "recover",
  "state_transition": { "recovering_count": 2 }
}

Verify recovering_count: 0 is allowed even when recovery is disabled

POST kbn:/api/alerting/v2/rules
{
  "kind": "alert",
  "metadata": { "name": "verify-ok-b1" },
  "schedule": { "every": "5m" },
  "query": { "format": "standalone", "breach": { "query": "FROM logs-* | LIMIT 1" } },
  "recovery_strategy": "none",
  "state_transition": { "pending_count": 0, "recovering_count": 0 }
}

Verify pending-only state_transition is allowed while recovery is disabled

POST kbn:/api/alerting/v2/rules
{
  "kind": "alert",
  "metadata": { "name": "verify-ok-b2" },
  "schedule": { "every": "5m" },
  "query": { "format": "standalone", "breach": { "query": "FROM logs-* | LIMIT 1" } },
  "recovery_strategy": "none",
  "state_transition": { "pending_count": 3 }
}

Verify a recovery delay is allowed with recovery enabled

Keep the id from the response for the next section

POST kbn:/api/alerting/v2/rules
{
  "kind": "alert",
  "metadata": { "name": "verify-ok-b3" },
  "schedule": { "every": "5m" },
  "query": { "format": "standalone", "breach": { "query": "FROM logs-* | LIMIT 1" } },
  "recovery_strategy": "no_breach",
  "state_transition": { "pending_count": 0, "recovering_count": 3, "recovering_timeframe": "5m" }
}

Update the rule above to disable recovery without clearing the delay, expect 400

PATCH kbn:/api/alerting/v2/rules/<B3_RULE_ID>
{
  "recovery_strategy": "none"
}

Update the rule to disable recovery with also clearing the delay, expect 200

PATCH kbn:/api/alerting/v2/rules/<B3_RULE_ID>
{
  "recovery_strategy": "none",
  "state_transition": { "pending_count": 0 }
}

…elay

infra-vault-gh-plugin-prod · 2026-09-03T16:31:54Z

Pinging @elastic/response-ops (Team:ResponseOps)

…elay

baileycash-elastic

looks good, one rec on the naming for the isRecoveryDelayAllowed naming. I realized we'll have UI work to do so I'll raise a separate ticket and race you! :D

baileycash-elastic · 2026-09-04T20:59:28Z

+      expect(result.success).toBe(false);
+    });
+
+    it('accepts recovering_count of 0 when recovery is disabled', () => {


iiiinteresting

I can change this if needed, it is kind of weird

I was trying to minimize the disruption from adding the new validation

might create some drift since I plan to hide the ui field entirely, we'll see. I want to avoid over-engineering the mapper if possible

Okay yeah that makes sense, I can remove it

Removed in this commit, 00f662b

baileycash-elastic · 2026-09-04T21:02:07Z

+  });
+
+  it('returns true when recovery is disabled but no recovering delay is set', () => {
+    expect(isRecoveryDelayAllowed({ recovery_strategy: 'none' })).toBe(true);


isRecoveryDelayAllowed feels slightly misleading in this case

Updated in this commit, 00f662b

So sorry, backend changes look good... a few naming conventions I thought were a little weird.

The UI changes are still needed, best to do here to avoid any problems. Thanks for making these changes!

…thub.com:doakalexi/kibana into alerting-v2/update-validation-for-recovery-delay

doakalexi · 2026-09-08T17:32:06Z

@baileycash-elastic I added UI changes in this commit, 9b736cc

…elay

…thub.com:doakalexi/kibana into alerting-v2/update-validation-for-recovery-delay

…ate-validation-for-recovery-delay

github-actions

One compatibility concern on the update path: the new isRecoveryTransitionConsistentWithStrategy invariant runs on every update against merged attributes, which can make pre-existing rules that carry an inert recovering delay un-updatable. Details inline. The rest of the change (schema refinement, form/compose mappers gating on isRecoveryEnabled, agent-builder op check, error code, and the accompanying tests) looks consistent and well-covered.

Generated by Claude Reviewer for #288802 · claude · opus · 368.7 AIC · ⌖ 22.6 AIC · ⊞ 5.5K

github-actions · 2026-09-08T22:17:43Z

      details: { rule_id: ruleId },
    },
+    {
+      valid: isRecoveryTransitionConsistentWithStrategy(attrs),


This invariant runs on every updateRule call (validateMergedRuleAttributes(id, nextAttrs) is unconditional), and nextAttrs preserves the stored state_transition/recovery_strategy when the update body doesn't touch them. That makes it a backward-compatibility concern for rules already in storage.

Before this PR, createRuleDataSchema had no recovering-vs-strategy refinement, and the old form mapper emitted recovering_count: 0 unconditionally for immediate recovery mode (see the assertions removed in rule_request_mappers.test.ts). So a rule with recovery_strategy: 'none'/unset + state_transition.recovering_count: 0 (or a positive value) was createable via both the API and the UI and can exist on disk today.

After this change, any such pre-existing rule becomes un-updatable: even an unrelated PATCH (e.g. renaming) merges to the same inert recovering delay and now throws INVALID_STATE_TRANSITION_CONFIG (400), forcing the user to clear the recovering delay in the same request before any other edit can go through.

If alerting_v2 already has persisted rules in this shape, consider normalizing the inert recovering fields out of nextAttrs on update (drop them when recovery is disabled) rather than rejecting, or adding a saved-object migration to strip them — so existing rules stay editable. If there's no such data yet (pre-GA, no migration needed), it'd be worth confirming that explicitly.

This is okay because we are experimental.

…elay

baileycash-elastic

lgtm, proposed a few areas to 🧹 clean

tysm for implementing this!

baileycash-elastic · 2026-09-11T17:39:32Z


-      <EuiSpacer size="m" />
-      <RecoveryDelayField />
+      {/*


nit: self-explanatory, comment not needed

removed in this commit, e95a043

baileycash-elastic · 2026-09-11T17:41:12Z

-    if (stateTransition?.recoveringTimeframe != null)
-      out.recovering_timeframe = stateTransition.recoveringTimeframe;
-    if (stateTransition?.recoveringCount != null)
+  // Recovering thresholds are only meaningful when recovery is enabled; emitting them


nit: self explanatory

removed in this commit, e95a043

baileycash-elastic · 2026-09-11T17:42:10Z

+/**
+ * Recovery is enabled only for condition-based strategies (`no_breach` / `query`).
+ * `none`/unset means condition-based recovery never fires, so the recovering delay
+ * thresholds (`recovering_count` / `recovering_timeframe`) are inert and must not be
+ * emitted — the write API rejects them (see `isRecoveryTransitionConsistentWithStrategy`).
+ * `no_data_strategy: 'recover'` does NOT re-enable them: the director bypasses recovering
+ * gating for no-data recovery (see `count_timeframe_strategy`).
+ */


probably not needed

removed in this commit, e95a043

…rror_codes.ts Co-authored-by: Bailey Cash <bailey.cash@elastic.co>

…_v2/common/builders.ts Co-authored-by: Bailey Cash <bailey.cash@elastic.co>

…elay

kibanamachine · 2026-09-11T19:52:18Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 11eae65
Build duration: 70 mins

Failed CI Steps

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
`shared-packages`	4.6MB	4.6MB	+496.0B

Unknown metric groups

total optimizer output size

id	before	after	diff
`all`	64.0MB	64.0MB	+496.0B

warm start memory

id	before	after	diff
`post forced gc heap baseline`	-	835791304	+835791304
`post forced gc heap delta`	-	-1943612	-1943612
`post forced gc heap delta standard deviation`	-	2801204	+2801204
`post forced gc heap target`	-	833847692	+833847692
`tail heap delta`	-	-31759699	-31759699
total			+1638736889

History

💛 Build #500487 was flaky 350a627
💚 Build #498620 succeeded f26d7c5
💔 Build #498610 failed a3eeb71
💔 Build #498526 failed d5f8df3
💔 Build #498490 failed 57e01b6
💚 Build #496440 succeeded a5aa772

Updating validation for recovery delay

cb02bad

doakalexi changed the title ~~Updating validation for recovery delay~~ [Alerting V2] [ResponseOps] Recovery delay is configurable and persisted even when it has no effect on execution Sep 2, 2026

Merge branch 'main' into alerting-v2/update-validation-for-recovery-d…

bce43cc

…elay

doakalexi added release_note:skip Skip the PR/issue when compiling release notes Team:ResponseOps Platform ResponseOps team (formerly the Cases and Alerting teams) t// backport:version Backport to applied version labels v9.6.0 labels Sep 3, 2026

doakalexi marked this pull request as ready for review September 3, 2026 16:31

doakalexi requested a review from a team as a code owner September 3, 2026 16:31

Merge branch 'main' into alerting-v2/update-validation-for-recovery-d…

a5aa772

…elay

kibanamachine added the reviewer:scout Agentic PR Scout test review label Sep 3, 2026

baileycash-elastic previously approved these changes Sep 4, 2026

View reviewed changes

baileycash-elastic mentioned this pull request Sep 4, 2026

[Alerting V2] [UI] Rule form can still configure an inert recovery delay after server-side rejection #289390

Closed

5 tasks

baileycash-elastic self-requested a review September 4, 2026 21:30

doakalexi added 3 commits September 8, 2026 10:28

Addressing PR comments

00f662b

Adding UI changes

9b736cc

Merge branch 'alerting-v2/update-validation-for-recovery-delay' of gi…

06a7cd9

…thub.com:doakalexi/kibana into alerting-v2/update-validation-for-recovery-delay

doakalexi and others added 7 commits September 8, 2026 10:32

Merge branch 'main' into alerting-v2/update-validation-for-recovery-d…

57e01b6

…elay

Merge branch 'main' into alerting-v2/update-validation-for-recovery-d…

d5f8df3

…elay

Merge branch 'main' into alerting-v2/update-validation-for-recovery-d…

1eff571

…elay

Fixing check after merge with main

d34739c

Merge branch 'alerting-v2/update-validation-for-recovery-delay' of gi…

a3eeb71

…thub.com:doakalexi/kibana into alerting-v2/update-validation-for-recovery-delay

Merge branch 'main' of github.com:elastic/kibana into alerting-v2/upd…

7a88ad9

…ate-validation-for-recovery-delay

Fixing test failures

f26d7c5

github-actions Bot reviewed Sep 8, 2026

View reviewed changes

Merge branch 'main' into alerting-v2/update-validation-for-recovery-d…

350a627

…elay

baileycash-elastic approved these changes Sep 11, 2026

View reviewed changes

doakalexi and others added 4 commits September 11, 2026 11:40

Removing comments

e95a043

Update x-pack/platform/plugins/shared/alerting_v2/server/lib/errors/e…

c6ea536

…rror_codes.ts Co-authored-by: Bailey Cash <bailey.cash@elastic.co>

Update x-pack/platform/plugins/shared/alerting_v2/test/scout_alerting…

684bc24

…_v2/common/builders.ts Co-authored-by: Bailey Cash <bailey.cash@elastic.co>

Merge branch 'main' into alerting-v2/update-validation-for-recovery-d…

11eae65

…elay

doakalexi enabled auto-merge (squash) September 11, 2026 18:46

doakalexi merged commit 5395241 into elastic:main Sep 11, 2026
41 checks passed

kibanamachine added backport:skip This PR does not require backporting and removed backport:version Backport to applied version labels labels Sep 11, 2026

Uh oh!

Conversation

doakalexi commented Sep 2, 2026 • edited by baileycash-elastic Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

To verify

Uh oh!

infra-vault-gh-plugin-prod Bot commented Sep 3, 2026

Uh oh!

baileycash-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

doakalexi commented Sep 8, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot Sep 8, 2026

Choose a reason for hiding this comment

Uh oh!

doakalexi Sep 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baileycash-elastic left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kibanamachine commented Sep 11, 2026

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Page load bundle

total optimizer output size

warm start memory

History

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

doakalexi commented Sep 2, 2026 •

edited by baileycash-elastic

Loading

doakalexi Sep 9, 2026 •

edited

Loading