feat: RFC 0007/0008 — step-level let bindings, wrap-hook step names, template extensions

pyproject.toml

@@ -29,7 +29,10 @@ classifiers = [
   "Intended Audience :: End Users/Desktop"
 ]
 dependencies = [
-  "openjd-sessions >= 0.10.7,< 0.11",
+  # 0.10.11 is the first release with `step_name` on both Session.run_task and
+  # Session.enter_environment (RFC 0007/0008). Below it, run_task() raises
+  # TypeError on every task run, since this CLI passes the keyword unconditionally.
+  "openjd-sessions >= 0.10.11,< 0.11",
   "openjd-model >= 0.9,< 0.12"
 ]
 

src/openjd/cli/_run/_local_session/_actions.py

@@ -1,6 +1,7 @@
 # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
 
 from enum import Enum
+from typing import Any, Optional
 
 from openjd.model import Step, TaskParameterSet
 from openjd.model.v2023_09 import Environment
@@ -47,7 +48,11 @@ def __init__(self, session: Session, step: Step, parameters: TaskParameterSet):
 
     def run(self):
         self._session.run_task(
-            step_script=self._step.script, task_parameter_values=self._parameters
+            step_script=self._step.script,
+            task_parameter_values=self._parameters,
+            # RFC 0008: the step name feeds the WrappedStep.Name template
+            # variable inside an active onWrapTaskRun hook.
+            step_name=self._step.name,
         )
 
     def __str__(self):
@@ -58,14 +63,46 @@ def __str__(self):
 class EnterEnvironmentAction(SessionAction):
     _environment: Environment
     _id: str
-
-    def __init__(self, session: Session, environment: Environment, env_id: str):
+    _extra_let_bindings: Optional[list[str]]
+    _step_name: Optional[str]
+
+    def __init__(
+        self,
+        session: Session,
+        environment: Environment,
+        env_id: str,
+        extra_let_bindings: Optional[list[str]] = None,
+        step_name: Optional[str] = None,
+    ):
         super(EnterEnvironmentAction, self).__init__(session)
         self._environment = environment
         self._id = env_id
+        # RFC 0007: a step's environments are entered with the step-level
+        # `let` bindings so their variables/actions can reference them.
+        self._extra_let_bindings = extra_let_bindings
+        # RFC 0007 §7.3.1 (EXPR): the owning step's name seeds Step.Name for
+        # a step environment's `let` bindings, variables, and actions. Only
+        # step-environment enters carry a step name; job/external enters
+        # leave it None.
+        self._step_name = step_name
 
     def run(self):
-        self._session.enter_environment(environment=self._environment, identifier=self._id)
+        # Both keywords are guaranteed by this package's `openjd-sessions`
+        # floor (>= 0.10.11), so neither is feature-detected. They are still
+        # only forwarded when they carry something: a step with no `let`
+        # bindings means "no extra bindings", and job/external environment
+        # enters have no owning step, so `Step.Name` must stay undefined for
+        # them rather than being seeded with None.
+        optional_kwargs: dict[str, Any] = {}
+        if self._extra_let_bindings:
+            optional_kwargs["extra_let_bindings"] = self._extra_let_bindings
+        if self._step_name is not None:
+            optional_kwargs["step_name"] = self._step_name
+        self._session.enter_environment(
+            environment=self._environment,
+            identifier=self._id,
+            **optional_kwargs,
+        )
 
     def __str__(self):
         return f"Enter Environment '{self._environment.name}'"

src/openjd/cli/_run/_local_session/_session_manager.py

@@ -102,6 +102,8 @@ def __init__(
         self._openjd_session = Session(
             session_id=self.session_id,
             job_parameter_values=job_parameter_values,
+            # RFC 0007 §7.3.1 (EXPR): seeds the Job.Name template variable.
+            job_name=str(job.name),
             path_mapping_rules=self._path_mapping_rules,
             callback=self._action_callback,
             retain_working_dir=retain_working_dir,
@@ -197,7 +199,14 @@ def _sigint_handler(self, signum: int, frame: Optional[FrameType]) -> None:
         LOG.info("Interruption signal recieved.")
         self.cancel()
 
-    def run_environment_enters(self, environments: Optional[list[Any]], type: EnvironmentType):
+    def run_environment_enters(
+        self,
+        environments: Optional[list[Any]],
+        type: EnvironmentType,
+        *,
+        extra_let_bindings: Optional[list[str]] = None,
+        step_name: Optional[str] = None,
+    ):
         """Enter one or more environments in the session."""
         if environments is None:
             return
@@ -211,10 +220,43 @@ def run_environment_enters(self, environments: Optional[list[Any]], type: Enviro
             env_id = f"{type.name} - {env.name}"
             self._action_ended.clear()
             self._current_action = EnterEnvironmentAction(
-                session=self._openjd_session, environment=env, env_id=env_id
+                session=self._openjd_session,
+                environment=env,
+                env_id=env_id,
+                # RFC 0007: a step's environments see the step-level `let`
+                # bindings.
+                extra_let_bindings=extra_let_bindings,
+                # RFC 0007 §7.3.1 (EXPR): a step's environments see Step.Name.
+                # Only step-environment enters carry a step name.
+                step_name=step_name,
             )
             self._environments_entered.append((type, env_id))
-            self._current_action.run()
+            try:
+                self._current_action.run()
+            except (RuntimeError, ValueError) as exc:
+                # Session.enter_environment raises (rather than reporting
+                # through the action-status callback) when it rejects the
+                # environment up front — e.g. the RFC 0008 "at most one wrap
+                # environment" RuntimeError, or a ValueError from the extra
+                # `let` bindings — but it can also raise *after* registering
+                # the environment (e.g. an environment `variables` expression
+                # that fails to evaluate). Only when the session did NOT
+                # register the environment may we drop it from our entered
+                # list (cleanup must not try to exit it). If the session did
+                # register it, it must stay in our list so cleanup exits it —
+                # popping it here would skip its onExit and desynchronize us
+                # from the session's LIFO exit ordering check, masking the
+                # original error with "Must exit Environment X first".
+                if env_id not in self._openjd_session.environments_entered:
+                    self._environments_entered.pop()
+                LOG.info(
+                    msg=f"Open Job Description CLI: ERROR entering environment '{env.name}': {exc}",
+                    extra={"session_id": self.session_id},
+                )
+                self.failed = True
+                self._failed_action = self._current_action
+                self._current_action = None
+                raise LocalSessionFailed(self._failed_action) from exc
             self._action_ended.wait()
             if self.failed:
                 self._failed_action = self._current_action
@@ -348,8 +390,22 @@ def run_step(
         if task_parameters is None:
             task_parameters = StepParameterSpaceIterator(space=step.parameterSpace)
 
-        # Enter all the step environments
-        self.run_environment_enters(step.stepEnvironments, EnvironmentType.STEP)
+        # Enter all the step environments. When the step defines step-level
+        # `let` bindings (RFC 0007), its environments are entered with them so
+        # their variables and actions can reference them.
+        # getattr guard: requires an openjd-model with Step.let on the
+        # instantiated Job (openjd-model PR #318+); collapse to plain
+        # `step.let` once the version pin floor guarantees it.
+        step_let_bindings = getattr(step, "let", None)
+        self.run_environment_enters(
+            step.stepEnvironments,
+            EnvironmentType.STEP,
+            extra_let_bindings=step_let_bindings or None,
+            # RFC 0007 §7.3.1 (EXPR): Step.Name is available to a step's
+            # environments (openjd-rs threads the step's resolved symbol
+            # table into enter_environment; this is the CLI counterpart).
+            step_name=step.name,
+        )
 
         try:
             # Run the tasks

src/openjd/cli/_run/_run_command.py

@@ -338,6 +338,7 @@ def _run_local_session(
     Creates a Session object and listens for log messages to synchronously end the session.
     """
 
+    error_message = "Session ended with errors; see Task logs for details"
     try:
         start_seconds = time.perf_counter()
 
@@ -367,6 +368,14 @@ def _run_local_session(
     except LocalSessionFailed:
         duration = time.perf_counter() - start_seconds
         session = None
+    except (RuntimeError, ValueError) as exc:
+        # Exceptions raised by openjd.sessions from within session actions
+        # (e.g. the RFC 0008 "at most one wrap environment" RuntimeError)
+        # rather than reported through the action-status callback. Report
+        # them as a clean error result instead of a raw traceback.
+        duration = time.perf_counter() - start_seconds
+        session = None
+        error_message = f"Session ended with errors: {exc}"
 
     preserved_message: str = ""
     if retain_working_dir and session is not None:
@@ -377,7 +386,7 @@ def _run_local_session(
     if session is None or session.failed:
         return OpenJDRunResult(
             status="error",
-            message="Session ended with errors; see Task logs for details" + preserved_message,
+            message=error_message + preserved_message,
             job_name=job.name,
             step_name=step_name,
             duration=duration,
@@ -547,9 +556,17 @@ def do_run(args: Namespace) -> OpenJDCliResult:
         return OpenJDCliResult(status="error", message=str(rte))
 
     # Create a RevisionExtensions object with the default specification version and enabled extensions
-    # We use the default v2023_09 since that's what we're currently supporting
+    # We use the default v2023_09 since that's what we're currently supporting.
+    # Extension behaviors activate only when declared by the templates being
+    # run (the job template and any external environment templates) — the
+    # CLI's --extensions list is what the CLI *accepts*, not what is enabled.
+    declared_extensions: list[str] = list(the_job.extensions or [])
+    for env_template in environments or []:
+        for ext_name in env_template.extensions or []:
+            if ext_name not in declared_extensions:
+                declared_extensions.append(ext_name)
     revision_extensions = RevisionExtensions(
-        spec_rev=the_job.revision, supported_extensions=extensions
+        spec_rev=the_job.revision, supported_extensions=declared_extensions
     )
 
     return _run_local_session(

test/openjd/cli/templates/step_name_env_job.yaml

@@ -0,0 +1,27 @@
+# RFC 0007 §7.3.1 (EXPR): Step.Name is available to a step's environments.
+# The step-level `let` binding references Step.Name, and the step's
+# environment is entered with the binding so its onEnter action can echo it.
+specificationVersion: jobtemplate-2023-09
+extensions:
+- EXPR
+name: StepNameEnvJob
+steps:
+- name: EchoStepName
+  let:
+  - bound_name = Step.Name
+  stepEnvironments:
+  - name: NameEcho
+    script:
+      actions:
+        onEnter:
+          command: python
+          args:
+          - -c
+          - print('EnvSaw={{ bound_name }}')
+  script:
+    actions:
+      onRun:
+        command: python
+        args:
+        - -c
+        - print('TaskRan')

test/openjd/cli/test_local_session.py

@@ -168,7 +168,13 @@ def test_localsession_run_success(
         assert patched_run_environment_enters.call_args_list == [
             call(session, None, EnvironmentType.EXTERNAL),
             call(session, sample_job.jobEnvironments, EnvironmentType.JOB),
-            call(session, sample_job.steps[step_index].stepEnvironments, EnvironmentType.STEP),
+            call(
+                session,
+                sample_job.steps[step_index].stepEnvironments,
+                EnvironmentType.STEP,
+                extra_let_bindings=None,
+                step_name=sample_job.steps[step_index].name,
+            ),
         ]
         # It should have run one step
         assert patched_run_step.call_args_list == [
@@ -193,6 +199,116 @@ def test_localsession_run_success(
         )
 
 
+@pytest.mark.usefixtures("sample_job_and_dirs")
+def test_localsession_step_env_enter_receives_step_name(
+    sample_job_and_dirs: tuple,
+    patched_actions,
+):
+    """
+    RFC 0007 §7.3.1 (EXPR): a step-environment enter passes the owning step's
+    name to Session.enter_environment, while job/external environment enters
+    never do.
+
+    The step-environment assertion used to be gated on feature-detecting the
+    keyword, which meant that against a sessions build without it the test took
+    the other branch and asserted the keyword was *absent* -- passing while
+    proving the opposite of its name. The `openjd-sessions >= 0.10.11` floor
+    guarantees the keyword, so the assertion is now unconditional.
+    """
+    sample_job, sample_job_parameters, template_dir, current_working_dir = sample_job_and_dirs
+    patched_enter = patched_actions[0]
+
+    with LocalSession(
+        job=sample_job, job_parameter_values=sample_job_parameters, session_id="step-name"
+    ) as session:
+        session.run_step(sample_job.steps[SampleSteps.NormalStep])
+
+    assert not session.failed
+
+    step_env_calls = [
+        c
+        for c in patched_enter.call_args_list
+        if c.kwargs["identifier"].startswith(f"{EnvironmentType.STEP.name} - ")
+    ]
+    other_env_calls = [
+        c
+        for c in patched_enter.call_args_list
+        if not c.kwargs["identifier"].startswith(f"{EnvironmentType.STEP.name} - ")
+    ]
+
+    assert step_env_calls
+    for enter_call in step_env_calls:
+        assert enter_call.kwargs["step_name"] == sample_job.steps[SampleSteps.NormalStep].name
+
+    # Job/external environment enters never carry a step name.
+    assert other_env_calls
+    for enter_call in other_env_calls:
+        assert "step_name" not in enter_call.kwargs
+
+
+@pytest.mark.usefixtures("sample_job_and_dirs", "capsys")
+def test_localsession_enter_environment_post_registration_raise(
+    sample_job_and_dirs: tuple, capsys: pytest.CaptureFixture, patched_actions
+):
+    """
+    A raise out of Session.enter_environment *after* the session registered
+    the environment (e.g. an environment `variables` expression that fails to
+    evaluate) must leave the environment in the CLI's entered list so cleanup
+    exits it. Popping it would desynchronize CLI and session state: the
+    environment's onExit would be skipped and cleanup would trip the session's
+    LIFO exit check ("Must exit Environment X first"), masking the original
+    error.
+    """
+    sample_job, sample_job_parameters, template_dir, current_working_dir = sample_job_and_dirs
+    patched_enter = patched_actions[0]
+
+    # The autouse fixture set the mock's side_effect to the real (pre-patch)
+    # Session.enter_environment; capture it before redirecting the mock.
+    real_enter = patched_enter.side_effect
+    error_text = "Failed to evaluate the environment's variables"
+
+    def register_then_raise(session, *, environment, identifier=None, **kwargs):
+        if identifier is not None and identifier.startswith(f"{EnvironmentType.STEP.name} - "):
+            # Mirror the state Session.enter_environment leaves behind when an
+            # environment `variables` expression fails to evaluate: the
+            # environment is registered in the session's entered list, but the
+            # enter raises before any action runs.
+            session._environments[identifier] = environment
+            session._environments_entered.append(identifier)
+            raise ValueError(error_text)
+        return real_enter(session, environment=environment, identifier=identifier, **kwargs)
+
+    step_env_id = f"{EnvironmentType.STEP.name} - env1"
+    job_env_id = f"{EnvironmentType.JOB.name} - rootEnv"
+
+    # Redirect the autouse fixture's Session.enter_environment mock from the
+    # real method to the registering-then-raising simulation.
+    patched_enter.side_effect = register_then_raise
+    with LocalSession(
+        job=sample_job, job_parameter_values=sample_job_parameters, session_id="post-reg"
+    ) as session:
+        with pytest.raises(LocalSessionFailed):
+            session.run_step(sample_job.steps[SampleSteps.NormalStep])
+
+        # The session registered the environment, so the CLI must keep it
+        # in its own entered list for cleanup to exit.
+        assert step_env_id in session._openjd_session.environments_entered
+        assert (EnvironmentType.STEP, step_env_id) in session._environments_entered
+
+    # Cleanup exited the registered step environment and then the job
+    # environment, in LIFO order, rather than skipping the step environment.
+    exited_ids = [
+        exit_call.kwargs["identifier"]
+        for exit_call in session._openjd_session.exit_environment.call_args_list  # type: ignore
+    ]
+    assert exited_ids == [step_env_id, job_env_id]
+
+    assert session.failed
+    output = capsys.readouterr().out
+    assert error_text in output
+    assert "Must exit Environment" not in output
+
+
 @pytest.mark.usefixtures("sample_job_and_dirs", "capsys")
 def test_localsession_run_failed(sample_job_and_dirs: tuple, capsys: pytest.CaptureFixture):
     """
@@ -221,6 +337,8 @@ def test_localsession_run_failed(sample_job_and_dirs: tuple, capsys: pytest.Capt
                 session,
                 sample_job.steps[SampleSteps.BadCommand].stepEnvironments,
                 EnvironmentType.STEP,
+                extra_let_bindings=None,
+                step_name=sample_job.steps[SampleSteps.BadCommand].name,
             ),
         ]
         session._openjd_session.exit_environment.assert_called_once()  # type: ignore