[Alerting V2] [ResponseOps] Recovery delay is configurable and persisted even when it has no effect on execution

x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/rule_data_schema.test.ts

@@ -9,6 +9,7 @@ import { RUNBOOK_ARTIFACT_TYPE, RUNBOOK_CONTENT_LIMIT } from '@kbn/alerting-v2-c
 import {
   createRuleDataBaseSchema,
   createRuleDataSchema,
+  isRecoveryDelayAllowed,
   updateRuleDataSchema,
   IMMUTABLE_RULE_FIELDS,
   getBreachEsqlQuery,
@@ -55,6 +56,7 @@ describe('createRuleDataSchema', () => {
         metadata: { name: 'test rule', owner: 'team-a', tags: ['label-1', 'label-2'] },
         time_field: 'event.created',
         schedule: { every: '5m', lookback: '10m' },
+        recovery_strategy: 'no_breach',
         grouping: { fields: ['host.name'] },
         state_transition: {
           pending_operator: 'AND',
@@ -72,6 +74,7 @@ describe('createRuleDataSchema', () => {
           metadata: { name: 'test rule', owner: 'team-a', tags: ['label-1', 'label-2'] },
           time_field: 'event.created',
           schedule: { every: '5m', lookback: '10m' },
+          recovery_strategy: 'no_breach',
           grouping: { fields: ['host.name'] },
           state_transition: {
             pending_operator: 'AND',
@@ -702,6 +705,7 @@ describe('createRuleDataSchema', () => {
     it('accepts state_transition with only recovering fields', () => {
       const result = createRuleDataSchema.parse({
         ...validCreateData,
+        recovery_strategy: 'no_breach',
         state_transition: {
           recovering_operator: 'OR',
           recovering_count: 5,
@@ -863,6 +867,83 @@ describe('createRuleDataSchema', () => {
     });
   });
 
+  describe('recovery delay allowed', () => {
+    it('rejects a recovering_count when recovery_strategy is unset', () => {
+      const result = createRuleDataSchema.safeParse({
+        ...validCreateData,
+        state_transition: { pending_count: 0, recovering_count: 2 },
+      });
+
+      expect(result.success).toBe(false);
+    });
+
+    it('rejects a recovering_count when recovery_strategy is "none"', () => {
+      const result = createRuleDataSchema.safeParse({
+        ...validCreateData,
+        recovery_strategy: 'none',
+        state_transition: { recovering_count: 2 },
+      });
+
+      expect(result.success).toBe(false);
+    });
+
+    it('rejects a recovering_timeframe when recovery is disabled', () => {
+      const result = createRuleDataSchema.safeParse({
+        ...validCreateData,
+        recovery_strategy: 'none',
+        state_transition: { recovering_timeframe: '5m' },
+      });
+
+      expect(result.success).toBe(false);
+    });
+
+    it('rejects an inert recovery delay even when no_data_strategy is "recover"', () => {
+      const result = createRuleDataSchema.safeParse({
+        ...validCreateData,
+        recovery_strategy: 'none',
+        no_data_strategy: 'recover',
+        query: {
+          format: 'standalone',
+          breach: { query: 'FROM logs-* | LIMIT 1' },
+          no_data: { query: 'FROM logs-* | STATS c = COUNT(*)' },
+        },
+        state_transition: { recovering_count: 2 },
+      });
+
+      expect(result.success).toBe(false);
+    });
+
+    it('accepts recovering_count of 0 when recovery is disabled', () => {
+      const result = createRuleDataSchema.safeParse({
+        ...validCreateData,
+        recovery_strategy: 'none',
+        state_transition: { pending_count: 0, recovering_count: 0 },
+      });
+
+      expect(result.success).toBe(true);
+    });
+
+    it('accepts pending-only state_transition when recovery is disabled', () => {
+      const result = createRuleDataSchema.safeParse({
+        ...validCreateData,
+        recovery_strategy: 'none',
+        state_transition: { pending_count: 3 },
+      });
+
+      expect(result.success).toBe(true);
+    });
+
+    it('accepts a recovering delay when recovery_strategy is "no_breach"', () => {
+      const result = createRuleDataSchema.safeParse({
+        ...validCreateData,
+        recovery_strategy: 'no_breach',
+        state_transition: { recovering_count: 2, recovering_timeframe: '5m' },
+      });
+
+      expect(result.success).toBe(true);
+    });
+  });
+
   describe('artifacts envelope', () => {
     const parseWithArtifact = (artifact: Record<string, unknown>) =>
       createRuleDataSchema.safeParse({ ...validCreateData, artifacts: [artifact] });
@@ -1677,3 +1758,52 @@ describe('tagsResponseSchema', () => {
     expect(() => tagsResponseSchema.parse({})).toThrow();
   });
 });
+
+describe('isRecoveryDelayAllowed', () => {
+  it('returns true when recovery is enabled, regardless of recovering delay', () => {
+    expect(
+      isRecoveryDelayAllowed({
+        recovery_strategy: 'no_breach',
+        state_transition: { recovering_count: 3, recovering_timeframe: '5m' },
+      })
+    ).toBe(true);
+    expect(
+      isRecoveryDelayAllowed({
+        recovery_strategy: 'query',
+        state_transition: { recovering_count: 3 },
+      })
+    ).toBe(true);
+  });
+
+  it('returns true when recovery is disabled but no recovering delay is set', () => {
+    expect(isRecoveryDelayAllowed({ recovery_strategy: 'none' })).toBe(true);
+    expect(isRecoveryDelayAllowed({ recovery_strategy: null })).toBe(true);
+    expect(isRecoveryDelayAllowed({})).toBe(true);
+    expect(isRecoveryDelayAllowed({ recovery_strategy: 'none', state_transition: {} })).toBe(true);
+  });
+
+  it('treats recovering_count 0 as no delay even when recovery is disabled', () => {
+    expect(
+      isRecoveryDelayAllowed({
+        recovery_strategy: 'none',
+        state_transition: { recovering_count: 0 },
+      })
+    ).toBe(true);
+  });
+
+  it('returns false for a positive recovering delay when recovery is disabled', () => {
+    expect(
+      isRecoveryDelayAllowed({
+        recovery_strategy: 'none',
+        state_transition: { recovering_count: 1 },
+      })
+    ).toBe(false);
+    expect(
+      isRecoveryDelayAllowed({
+        recovery_strategy: null,
+        state_transition: { recovering_timeframe: '5m' },
+      })
+    ).toBe(false);
+    expect(isRecoveryDelayAllowed({ state_transition: { recovering_count: 2 } })).toBe(false);
+  });
+});

x-pack/platform/packages/shared/response-ops/alerting-v2-schemas/src/rule_data_schema.ts

@@ -541,6 +541,36 @@ export const isNoDataQueryProvidedForStrategy = (data: {
 export const isNoDataStrategyNotEmit = (data: {
   no_data_strategy?: NoDataStrategy | null;
 }): boolean => data.no_data_strategy !== noDataStrategy.emit;
+
+/**
+ * A recovery delay is inert when recovery is disabled (`recovery_strategy` is
+ * `none` or unset), so we reject it. Only a positive delay counts
+ * (`recovering_count > 0` or any `recovering_timeframe`); `recovering_count: 0`
+ * means "recover immediately" and is always allowed.
+ */
+export const isRecoveryDelayAllowed = (data: {
+  recovery_strategy?: RecoveryStrategy | null;
+  state_transition?: {
+    recovering_count?: number | null;
+    recovering_timeframe?: string | null;
+  } | null;
+}): boolean => {
+  const recoveryEnabled =
+    data.recovery_strategy != null && data.recovery_strategy !== recoveryStrategy.none;
+  if (recoveryEnabled) {
+    return true;
+  }
+
+  const stateTransition = data.state_transition;
+  if (stateTransition == null) {
+    return true;
+  }
+
+  const hasRecoveryDelay =
+    (stateTransition.recovering_count != null && stateTransition.recovering_count > 0) ||
+    stateTransition.recovering_timeframe != null;
+  return !hasRecoveryDelay;
+};
 const rejectEmitNoDataStrategy = {
   message: 'no_data_strategy "emit" is not currently supported.',
   path: ['no_data_strategy'],
@@ -577,6 +607,11 @@ export const createRuleDataSchema = createRuleDataBaseSchema
     path: ['query', 'no_data'],
   })
   .refine(isNoDataStrategyNotEmit, rejectEmitNoDataStrategy)
+  .refine(isRecoveryDelayAllowed, {
+    message:
+      'state_transition.recovering_count and recovering_timeframe have no effect when recovery is disabled (recovery_strategy is "none" or unset).',
+    path: ['state_transition', 'recovering_count'],
+  })
   .meta({ id: 'alerting_new_rule' });
 
 export type CreateRuleData = z.infer<typeof createRuleDataSchema>;

x-pack/platform/plugins/shared/alerting_v2/server/agent_builder/tools/manage_rule/operations.test.ts

@@ -706,6 +706,38 @@ describe('executeRuleOperations', () => {
       );
     });
 
+    it('throws when a recovering delay is set while recovery is disabled', async () => {
+      const ops: RuleOperation[] = [
+        { operation: 'set_kind', kind: 'alert' },
+        {
+          operation: 'set_query',
+          query: { format: 'standalone', breach: { query: 'FROM metrics-* | STATS COUNT(*)' } },
+        },
+        { operation: 'set_state_transition', pending_count: 0, recovering_count: 2 },
+      ];
+
+      await expect(executeRuleOperations({}, ops)).rejects.toThrow(
+        'state_transition.recovering_count and recovering_timeframe have no effect when recovery is disabled'
+      );
+      await expect(executeRuleOperations({}, ops)).rejects.toBeInstanceOf(
+        RuleOperationValidationError
+      );
+    });
+
+    it('allows recovering_count 0 (immediate) while recovery is disabled', async () => {
+      const ops: RuleOperation[] = [
+        { operation: 'set_kind', kind: 'alert' },
+        {
+          operation: 'set_query',
+          query: { format: 'standalone', breach: { query: 'FROM metrics-* | STATS COUNT(*)' } },
+        },
+        { operation: 'set_state_transition', pending_count: 0, recovering_count: 0 },
+      ];
+
+      const result = await executeRuleOperations({}, ops);
+      expect(result.data.state_transition).toEqual({ pending_count: 0, recovering_count: 0 });
+    });
+
     it('throws when signal rule uses composed query format', async () => {
       const ops: RuleOperation[] = [
         { operation: 'set_kind', kind: 'signal' },

x-pack/platform/plugins/shared/alerting_v2/server/agent_builder/tools/manage_rule/operations.ts

@@ -32,6 +32,7 @@ import {
   isStateTransitionAllowed,
   isSignalUsingStandaloneFormat,
   isSignalQueryBreachOnly,
+  isRecoveryDelayAllowed,
   isRecoveryQueryConsistentWithStrategy,
   isRecoveryQueryProvidedForStrategy,
   isNoDataQueryConsistentWithStrategy,
@@ -570,6 +571,12 @@ export const executeRuleOperations = async (
     );
   }
 
+  if (!isRecoveryDelayAllowed(next)) {
+    throw new RuleOperationValidationError(
+      'state_transition.recovering_count and recovering_timeframe have no effect when recovery is disabled (recovery_strategy is "none" or unset).'
+    );
+  }
+
   return {
     data: next,
     ...(lastQueryColumns ? { queryColumns: lastQueryColumns } : {}),

x-pack/platform/plugins/shared/alerting_v2/server/lib/errors/README.md

@@ -66,6 +66,7 @@ backwards compatible. Renaming or removing a code is a breaking change.
 | `INVALID_STATE_TRANSITION`          | 400    | `state_transition` is incompatible with the rule's `kind`                                                                           | `{ rule_id, kind, transition }`            |
 | `INVALID_SIGNAL_RULE`               | 400    | An update would leave a signal rule in an invalid shape                                                                             | `{ rule_id, rule_kind }`                   |
 | `INVALID_RULE_QUERY_CONFIG`         | 400    | An update would desynchronize a `recovery_strategy`/`no_data_strategy` from its `query.recovery`/`query.no_data` block              | `{ rule_id }`                              |
+| `INVALID_STATE_TRANSITION_CONFIG`   | 400    | A recovery delay (`state_transition.recovering_count`/`recovering_timeframe`) is set while recovery is disabled, where it is inert  | `{ rule_id }`                              |
 | `BULK_QUERY_MATCH_LIMIT_EXCEEDED`   | 400    | By-query bulk operation with `force: true` matched more resources than the per-request cap; rejected before any resource is mutated | `{ match_count, limit }`                   |
 | `IMMUTABLE_FIELDS_CHANGED`          | 400    | PUT (upsert) request changes a field flagged as immutable                                                                           | `{ fields }`                               |
 | `INVALID_FILTER_FIELD`              | 400    | The `filter` references a field that is not in the allow-list                                                                       | `{ field, allowed_fields }`                |

x-pack/platform/plugins/shared/alerting_v2/server/lib/errors/error_codes.ts

@@ -35,6 +35,13 @@ export const ALERTING_ERROR_CODES = {
   INVALID_ARTIFACT_DATA: 'INVALID_ARTIFACT_DATA',
   /** `state_transition` cannot be applied to the rule's `kind`. */
   INVALID_STATE_TRANSITION: 'INVALID_STATE_TRANSITION',
+  /**
+   * A rule's merged shape carries a recovery delay
+   * (`state_transition.recovering_count` / `recovering_timeframe`) while
+   * recovery is disabled (`recovery_strategy` is `none` or unset), where it
+   * would never take effect.
+   */
+  INVALID_STATE_TRANSITION_CONFIG: 'INVALID_STATE_TRANSITION_CONFIG',
   /** A signal rule's merged shape violates signal constraints. */
   INVALID_SIGNAL_RULE: 'INVALID_SIGNAL_RULE',
   /**

x-pack/platform/plugins/shared/alerting_v2/server/lib/rules_client/rules_client.test.ts

@@ -458,6 +458,36 @@ describe('RulesClient', () => {
       ).resolves.not.toThrow();
     });
 
+    it('throws 400 when disabling recovery leaves a stored recovering delay inert', async () => {
+      const client = createClient();
+
+      const existingAttributes: RuleSavedObjectAttributes = {
+        ...baseSoAttrs,
+        kind: 'alert',
+        recovery_strategy: 'no_breach',
+        state_transition: { recovering_count: 3 },
+      };
+
+      rulesSavedObjectService.get.mockResolvedValueOnce({
+        id: 'rule-id-inert-recovery-delay',
+        attributes: existingAttributes,
+        version: 'WzEsMV0=',
+      });
+
+      await expect(
+        client.updateRule({
+          id: 'rule-id-inert-recovery-delay',
+          data: { recovery_strategy: 'none' },
+        })
+      ).rejects.toMatchObject({
+        output: { statusCode: 400 },
+        message:
+          'state_transition.recovering_count and recovering_timeframe have no effect when recovery is disabled (recovery_strategy is "none" or unset).',
+      });
+
+      expect(rulesSavedObjectService.update).not.toHaveBeenCalled();
+    });
+
     it('throws 400 when updating a signal rule query to composed format', async () => {
       const client = createClient();
 

x-pack/platform/plugins/shared/alerting_v2/server/lib/rules_client/utils.test.ts

@@ -870,6 +870,58 @@ describe('utils', () => {
 
       expect(() => validateMergedRuleAttributes('rule-1', attrs)).not.toThrow();
     });
+
+    it('throws INVALID_STATE_TRANSITION_CONFIG (400) when a recovering delay is set while recovery is disabled', () => {
+      const attrs = createRuleSoAttributes({
+        kind: 'alert',
+        recovery_strategy: 'none',
+        state_transition: { recovering_count: 3 },
+      });
+
+      expect(() => validateMergedRuleAttributes('rule-1', attrs)).toThrow(
+        expect.objectContaining({
+          isBoom: true,
+          output: expect.objectContaining({ statusCode: 400 }),
+          message:
+            'state_transition.recovering_count and recovering_timeframe have no effect when recovery is disabled (recovery_strategy is "none" or unset).',
+          data: { code: 'INVALID_STATE_TRANSITION_CONFIG', details: { rule_id: 'rule-1' } },
+        })
+      );
+    });
+
+    it('throws INVALID_STATE_TRANSITION_CONFIG when a recovering_timeframe is set while recovery is unset', () => {
+      const attrs = createRuleSoAttributes({
+        kind: 'alert',
+        recovery_strategy: undefined,
+        state_transition: { recovering_timeframe: '5m' },
+      });
+
+      expect(() => validateMergedRuleAttributes('rule-1', attrs)).toThrow(
+        expect.objectContaining({
+          data: { code: 'INVALID_STATE_TRANSITION_CONFIG', details: { rule_id: 'rule-1' } },
+        })
+      );
+    });
+
+    it('does not throw for a recovering delay when recovery is enabled', () => {
+      const attrs = createRuleSoAttributes({
+        kind: 'alert',
+        recovery_strategy: 'no_breach',
+        state_transition: { recovering_count: 3, recovering_timeframe: '5m' },
+      });
+
+      expect(() => validateMergedRuleAttributes('rule-1', attrs)).not.toThrow();
+    });
+
+    it('does not throw for recovering_count 0 (immediate) when recovery is disabled', () => {
+      const attrs = createRuleSoAttributes({
+        kind: 'alert',
+        recovery_strategy: 'none',
+        state_transition: { pending_count: 0, recovering_count: 0 },
+      });
+
+      expect(() => validateMergedRuleAttributes('rule-1', attrs)).not.toThrow();
+    });
   });
 
   describe('pickImmutable', () => {