[pull] main from keephq:main

keep/step/step.py

@@ -163,14 +163,13 @@ def _run_single(self, dont_render=False):
         # Initialize all conditions
         conditions = []
 
-        aliases = self.config.get("alias", {})
-
-        # if aliases are defined, set them in the context
-        for alias_key, alias_val in aliases.items():
-            aliases[alias_key] = self.io_handler.render(alias_val)
-
+        # set the step's vars in the context so they're available for condition/if evaluation
+        # note: aliases are intentionally NOT rendered/set here - they are only computed
+        # once we know the step is actually going to run (see below), since rendering them
+        # can fail/throw if the step (and its "if") is meant to be skipped (e.g. because they
+        # reference results of another step that was itself skipped)
         self.context_manager.set_step_vars(
-            self.step_id, _vars=self.vars, _aliases=aliases
+            self.step_id, _vars=self.vars, _aliases={}
         )
 
         for condition in self.conditions:
@@ -301,6 +300,18 @@ def _run_single(self, dont_render=False):
                 },
             )
 
+        # Now that we know the step is actually going to run, render and set its aliases.
+        # This must happen after the "if" check above (and not before, and not
+        # unconditionally) since aliases are only meaningful/safe to compute when the
+        # step runs - e.g. they may reference results of another step that was skipped,
+        # in which case rendering them here (before the "if" gate) could throw.
+        aliases = self.config.get("alias", {})
+        for alias_key, alias_val in aliases.items():
+            aliases[alias_key] = self.io_handler.render(alias_val)
+        self.context_manager.set_step_vars(
+            self.step_id, _vars=self.vars, _aliases=aliases
+        )
+
         # Third, check throttling
         # Now check if throttling is enabled
         self.logger.info(

tests/test_steps.py

@@ -88,6 +88,37 @@ def test_run_single_and_trigger_keep_function(sample_step, mocked_context_manage
         assert mock_len.call_count == 1
 
 
+def test_run_single_if_false_skips_alias_evaluation(sample_step, mocked_context_manager):
+    # Regression test: when a step's "if" condition evaluates to false, its "alias"
+    # block must NOT be evaluated at all - just like the provider call is skipped.
+    #
+    # Here the alias expression references another step's result
+    # ("steps.get-incident.results.body") that is missing from the context (e.g.
+    # because that other step was itself skipped), which renders as an empty string
+    # and turns "keep.dictget({{ ... }}, ...)" into the syntactically invalid
+    # "keep.dictget(, ...)". If the alias were evaluated despite "if" being false,
+    # this would raise a SyntaxError instead of the step being cleanly skipped.
+    sample_step.config["if"] = "'{{ steps.get-incident.results.body }}' != ''"
+    sample_step.config["alias"] = {
+        "incident_name": (
+            "keep.dictget({{ steps.get-incident.results.body }}, "
+            "'user_generated_name', 'Untitled incident')"
+        )
+    }
+
+    sample_step.io_handler.context_manager = mocked_context_manager
+    sample_step.provider.query = Mock(return_value="result")
+    sample_step.provider.notify = Mock(return_value="result")
+
+    # Should not raise (the alias must never be rendered), and the step should be
+    # reported as not having run.
+    result = sample_step._run_single()
+
+    assert result is None
+    sample_step.provider.query.assert_not_called()
+    sample_step.provider.notify.assert_not_called()
+
+
 def test_run_single_exception(sample_step):
     # Simulate an exception
     sample_step.provider.query = Mock(side_effect=Exception("Test exception"))