diff --git a/pyproject.toml b/pyproject.toml index 1d0ded5..bb88b5e 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -29,11 +29,18 @@ classifiers = [ "Intended Audience :: End Users/Desktop" ] dependencies = [ - # 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" + # 0.12.0 is the first release carrying `resolved_symtab` on all three of + # Session.enter_environment, Session.exit_environment and Session.run_task. + # It is also the release that removed `extra_let_bindings` from + # enter_environment: 0.10.11 through 0.11.0 have that keyword and no + # `resolved_symtab`, so there is no version satisfying both channels and the + # floor has to move rather than being feature-detected. Below 0.12.0 every + # call site here raises TypeError on the unexpected `resolved_symtab`. + "openjd-sessions >= 0.12.0,< 0.13", + # 0.11.4 is the first release exporting `create_job_with_symbol_tables` and + # `JobWithSymbolTables`. 0.11.3 and below have `create_job` only, so the + # step-scope resolved symbol tables cannot be obtained at all. + "openjd-model >= 0.11.4,< 0.12" ] [project.urls] diff --git a/src/openjd/cli/_common/__init__.py b/src/openjd/cli/_common/__init__.py index be487bd..93cdf3c 100644 --- a/src/openjd/cli/_common/__init__.py +++ b/src/openjd/cli/_common/__init__.py @@ -4,7 +4,7 @@ from dataclasses import asdict, dataclass from enum import Enum from pathlib import Path -from typing import Callable, Literal +from typing import TYPE_CHECKING, Callable, Literal import json import yaml import os @@ -23,6 +23,10 @@ ) from openjd.model import DecodeValidationError, Job, JobParameterValues, EnvironmentTemplate +if TYPE_CHECKING: + # Annotations only; see the note in _job_from_template.py. + from openjd.expr import SerializedSymbolTable + __all__ = [ "add_extensions_argument", "get_doc_type", @@ -118,7 +122,10 @@ def generate_job( environments: list[EnvironmentTemplate] = [], *, supported_extensions: list[str], -) -> tuple[Job, JobParameterValues]: +) -> tuple[Job, JobParameterValues, dict[str, "SerializedSymbolTable"]]: + """Returns the job, its parameter values, and the per-step resolved symbol + tables keyed by step name. See :func:`job_from_template` for why the tables + must be carried alongside the job rather than discarded.""" try: # Raises: RuntimeError, DecodeValidationError template = read_job_template(args.path, supported_extensions=supported_extensions) diff --git a/src/openjd/cli/_common/_job_from_template.py b/src/openjd/cli/_common/_job_from_template.py index 11727d4..7c13dbb 100644 --- a/src/openjd/cli/_common/_job_from_template.py +++ b/src/openjd/cli/_common/_job_from_template.py @@ -4,7 +4,7 @@ import json from pathlib import Path import re -from typing import Optional, Union +from typing import TYPE_CHECKING, Optional, Union import yaml from ._validation_utils import get_doc_type @@ -15,10 +15,17 @@ Job, JobParameterValues, JobTemplate, - create_job, + create_job_with_symbol_tables, preprocess_job_parameters, ) +if TYPE_CHECKING: + # Only for annotations: `openjd.expr` is a facade over the native + # extension, and importing the CLI must not load it. The tables the model + # hands back are already instances of this type, so nothing here + # constructs one. + from openjd.expr import SerializedSymbolTable + def get_params_from_file(parameter_string: str) -> Union[dict, list]: """ @@ -111,10 +118,18 @@ def job_from_template( parameter_args: list[str] | None, job_template_dir: Path, current_working_dir: Path, -) -> tuple[Job, JobParameterValues]: +) -> tuple[Job, JobParameterValues, dict[str, "SerializedSymbolTable"]]: """ Given a decoded Job Template and a user-input parameter dictionary, - generates a Job object and the parameter values for running the job. + generates a Job object, the parameter values for running the job, and the + per-step resolved symbol tables, keyed by step name. + + A step's template-scope `let` bindings (RFC 0005 §3.6) are evaluated once + here, at job creation, and their resolved values reach a session only + through those tables -- neither the model nor the session re-derives them + from the source expressions. Sessions for a step must therefore be given + `step_symbol_tables[step.name]`, or the step's `let` produces no bindings + at all. Raises: RuntimeError if parameters are an unsupported type or don't correspond to the template """ @@ -132,11 +147,14 @@ def job_from_template( raise RuntimeError(str(ve)) try: - job = create_job( + # `create_job_with_symbol_tables` returns the same Job that + # `create_job` does; it additionally returns the symbol tables that + # instantiation built instead of discarding them. + created = create_job_with_symbol_tables( job_template=template, job_parameter_values=parameter_values, environment_templates=environments, ) - return (job, parameter_values) + return (created.job, parameter_values, created.step_symbol_tables) except DecodeValidationError as dve: raise RuntimeError(f"Could not generate Job from template and parameters: {str(dve)}") diff --git a/src/openjd/cli/_run/_local_session/_actions.py b/src/openjd/cli/_run/_local_session/_actions.py index 9e46538..483038c 100644 --- a/src/openjd/cli/_run/_local_session/_actions.py +++ b/src/openjd/cli/_run/_local_session/_actions.py @@ -1,12 +1,18 @@ # Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. from enum import Enum -from typing import Any, Optional +from typing import TYPE_CHECKING, Optional from openjd.model import Step, TaskParameterSet from openjd.model.v2023_09 import Environment from openjd.sessions import Session +if TYPE_CHECKING: + # Annotations only: `openjd.expr` is a facade over the native extension and + # importing the CLI must not load it. These tables are produced by + # `create_job_with_symbol_tables` and forwarded unchanged. + from openjd.expr import SerializedSymbolTable + class EnvironmentType(str, Enum): """ @@ -40,11 +46,24 @@ def run(self): class RunTaskAction(SessionAction): _step: Step _parameters: TaskParameterSet + _resolved_symtab: Optional["SerializedSymbolTable"] - def __init__(self, session: Session, step: Step, parameters: TaskParameterSet): + def __init__( + self, + session: Session, + step: Step, + parameters: TaskParameterSet, + resolved_symtab: Optional["SerializedSymbolTable"] = None, + ): super(RunTaskAction, self).__init__(session) self._step = step self._parameters = parameters + # The step's create-time resolved symbol table + # (`create_job_with_symbol_tables().step_symbol_tables[step.name]`). + # It is the only channel for the step's template-scope `let` values: + # the model does not merge them into `script.let`, and the session does + # not re-evaluate the source expressions. + self._resolved_symtab = resolved_symtab def run(self): self._session.run_task( @@ -53,6 +72,7 @@ def run(self): # RFC 0008: the step name feeds the WrappedStep.Name template # variable inside an active onWrapTaskRun hook. step_name=self._step.name, + resolved_symtab=self._resolved_symtab, ) def __str__(self): @@ -63,7 +83,7 @@ def __str__(self): class EnterEnvironmentAction(SessionAction): _environment: Environment _id: str - _extra_let_bindings: Optional[list[str]] + _resolved_symtab: Optional["SerializedSymbolTable"] _step_name: Optional[str] def __init__( @@ -71,15 +91,18 @@ def __init__( session: Session, environment: Environment, env_id: str, - extra_let_bindings: Optional[list[str]] = None, + resolved_symtab: Optional["SerializedSymbolTable"] = 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 0005 §3.6: a step's environments are entered with the step's + # create-time resolved symbol table, which carries the step's + # template-scope `let` values so their variables/actions can reference + # them. Only step-environment enters have one; job and external + # environment enters leave it None. + self._resolved_symtab = resolved_symtab # 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 @@ -87,20 +110,26 @@ def __init__( self._step_name = step_name def run(self): - # 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 + # `step_name` is omitted rather than passed as None when this enter has + # no owning step. Passing None would be equivalent today, since + # enter_environment skips a None `step_name`, but omitting it keeps the + # call site's intent legible at the boundary and it is what + # test_localsession_step_env_enter_receives_step_name asserts — that + # assertion is currently the only check that job and external enters + # do not seed Step.Name, because a template referencing Step.Name + # outside a step is rejected by static validation before any session + # is built, so no loadable template can observe it. + # + # `resolved_symtab` is passed unconditionally: None is its documented + # "no table" value and there is no analogous assertion keyed on its + # absence. + optional_kwargs: dict[str, str] = {} if self._step_name is not None: optional_kwargs["step_name"] = self._step_name self._session.enter_environment( environment=self._environment, identifier=self._id, + resolved_symtab=self._resolved_symtab, **optional_kwargs, ) @@ -111,15 +140,34 @@ def __str__(self): class ExitEnvironmentAction(SessionAction): _id: str _keep_session_running: bool + _resolved_symtab: Optional["SerializedSymbolTable"] - def __init__(self, session: Session, id: str, keep_session_running: bool): + def __init__( + self, + session: Session, + id: str, + keep_session_running: bool, + resolved_symtab: Optional["SerializedSymbolTable"] = None, + ): super(ExitEnvironmentAction, self).__init__(session) self._id = id self._keep_session_running = keep_session_running + # The same table the environment was entered with, so its onExit + # resolves in the same scope as its onEnter (what + # Session.exit_environment documents for this argument). + # + # Unlike the worker agent, the CLI holds the table as an object rather + # than a JSON string served by the service, so there is no parse step + # here that could fail and no wrapper degrading a parse failure to None + # to keep teardown unconditional. An environment entered without a + # table simply exits without one. + self._resolved_symtab = resolved_symtab def run(self): self._session.exit_environment( - identifier=self._id, keep_session_running=self._keep_session_running + identifier=self._id, + keep_session_running=self._keep_session_running, + resolved_symtab=self._resolved_symtab, ) def __str__(self): diff --git a/src/openjd/cli/_run/_local_session/_session_manager.py b/src/openjd/cli/_run/_local_session/_session_manager.py index 3aeb1d0..e4b5273 100644 --- a/src/openjd/cli/_run/_local_session/_session_manager.py +++ b/src/openjd/cli/_run/_local_session/_session_manager.py @@ -3,7 +3,7 @@ from queue import Queue from threading import Event import time -from typing import Any, Iterable, Optional, Type +from typing import TYPE_CHECKING, Any, Iterable, Optional, Type from types import FrameType, TracebackType from signal import signal, SIGINT, SIGTERM, SIG_DFL from itertools import islice @@ -39,6 +39,10 @@ PathMappingRule, ) +if TYPE_CHECKING: + # Annotations only; see the note in _actions.py. + from openjd.expr import SerializedSymbolTable + class LocalSessionFailed(RuntimeError): """ @@ -118,6 +122,8 @@ class LocalSession: _path_mapping_rules: Optional[list[PathMappingRule]] _environments: Optional[list[Any]] _environments_entered: list[tuple[EnvironmentType, str]] + _step_symbol_tables: dict[str, "SerializedSymbolTable"] + _entered_env_symtabs: dict[str, "SerializedSymbolTable"] _log_handler: LocalSessionLogHandler _cleanup_called: bool @@ -127,6 +133,7 @@ def __init__( job: Job, job_parameter_values: JobParameterValues, session_id: str, + step_symbol_tables: Optional[dict[str, "SerializedSymbolTable"]] = None, timestamp_format: LoggingTimestampFormat = LoggingTimestampFormat.RELATIVE, path_mapping_rules: Optional[list[PathMappingRule]] = None, environments: Optional[list[Any]] = None, @@ -142,6 +149,21 @@ def __init__( self._timestamp_format = timestamp_format self._path_mapping_rules = path_mapping_rules self._environments = environments + # The create-time resolved symbol tables from + # `create_job_with_symbol_tables`, keyed by step name. A step's + # template-scope `let` (RFC 0005 §3.6) is evaluated once at job + # creation, and this is the only channel its resolved values have into + # a session: the model does not fold them into `script.let` and the + # session does not re-evaluate the source expressions. A caller that + # omits them gets a session in which every step-level `let` is simply + # undefined. + self._step_symbol_tables = step_symbol_tables or {} + # The table each entered environment was entered with, keyed by + # environment identifier, so its exit can be given the same one -- + # `Session.exit_environment` documents that as the way an onExit + # resolves in the same scope as its onEnter. Environments entered + # without a table are simply absent. + self._entered_env_symtabs = {} # Create an OpenJD Session self._openjd_session = Session( @@ -249,7 +271,7 @@ def run_environment_enters( environments: Optional[list[Any]], type: EnvironmentType, *, - extra_let_bindings: Optional[list[str]] = None, + resolved_symtab: Optional["SerializedSymbolTable"] = None, step_name: Optional[str] = None, ): """Enter one or more environments in the session.""" @@ -268,27 +290,39 @@ def run_environment_enters( 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 0005 §3.6: a step's environments see the step's resolved + # template-scope `let` values through its symbol table. + resolved_symtab=resolved_symtab, # 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)) + # Recorded before the enter runs, and deliberately not undone on + # failure: whether the enter succeeds or not, the exit that follows + # must be given the table the enter was attempted with. + if resolved_symtab is not None: + self._entered_env_symtabs[env_id] = resolved_symtab 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 — + # environment up front, before registering it — the RFC 0008 + # "at most one Environment defining wrap hooks" RuntimeError is + # the trigger that reaches us in practice. Every failure it + # detects *after* registering goes through + # _fail_action_before_start() and returns normally instead, so + # as of openjd-sessions 0.12.0 nothing raises post-registration + # and the else-branch below is defensive. It is kept because the + # cost of being wrong is asymmetric: if a future release does + # raise after registering, dropping the environment from our + # list 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". So: 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". @@ -325,7 +359,12 @@ def run_environment_exits(self, type: EnvironmentType, *, keep_session_running: prev_action_failed = self.failed self._action_ended.clear() self._current_action = ExitEnvironmentAction( - session=self._openjd_session, id=env_id, keep_session_running=keep_session_running + session=self._openjd_session, + id=env_id, + keep_session_running=keep_session_running, + # The same table the enter used, so onExit resolves in the same + # scope as onEnter. `pop` because an environment is exited once. + resolved_symtab=self._entered_env_symtabs.pop(env_id, None), ) self._current_action.run() self._action_ended.wait() @@ -345,7 +384,11 @@ def run_task(self, step: Step, parameter_set: TaskParameterSet) -> None: self._action_ended.clear() self._current_action = RunTaskAction( - session=self._openjd_session, step=step, parameters=parameter_set + session=self._openjd_session, + step=step, + parameters=parameter_set, + # RFC 0005 §3.6: the step's resolved template-scope `let` values. + resolved_symtab=self._step_symbol_tables.get(step.name), ) self._current_action.run() self._action_ended.wait() @@ -436,17 +479,16 @@ def run_step( if task_parameters is None: task_parameters = StepParameterSpaceIterator(space=step.parameterSpace) - # 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) + # Enter all the step environments with the step's create-time resolved + # symbol table, so a step-level `let` (RFC 0005 §3.6) is visible to + # their variables and actions. The source expressions are not + # re-evaluated anywhere downstream, so a missing table here means the + # step's `let` names are undefined rather than merely stale. + resolved_symtab = self._step_symbol_tables.get(step.name) self.run_environment_enters( step.stepEnvironments, EnvironmentType.STEP, - extra_let_bindings=step_let_bindings or None, + resolved_symtab=resolved_symtab, # 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). diff --git a/src/openjd/cli/_run/_run_command.py b/src/openjd/cli/_run/_run_command.py index 6f1b837..8011b40 100644 --- a/src/openjd/cli/_run/_run_command.py +++ b/src/openjd/cli/_run/_run_command.py @@ -4,7 +4,7 @@ from dataclasses import dataclass from pathlib import Path import json -from typing import Iterable, Optional +from typing import TYPE_CHECKING, Iterable, Optional import re import logging import time @@ -40,6 +40,10 @@ ) from openjd.sessions import PathMappingRule, LOG +if TYPE_CHECKING: + # Annotations only; see the note in _local_session/_actions.py. + from openjd.expr import SerializedSymbolTable + @dataclass class OpenJDRunResult(OpenJDCliResult): @@ -321,6 +325,7 @@ def _run_local_session( *, job: Job, job_parameter_values: JobParameterValues, + step_symbol_tables: Optional[dict[str, "SerializedSymbolTable"]] = None, step_list: list[Step], selected_step: Optional[Step], timestamp_format: LoggingTimestampFormat, @@ -346,6 +351,9 @@ def _run_local_session( with LocalSession( job=job, job_parameter_values=job_parameter_values, + # RFC 0005 §3.6: each step's create-time resolved `let` values. The + # only channel they have into the session. + step_symbol_tables=step_symbol_tables, timestamp_format=timestamp_format, session_id="CLI-session", path_mapping_rules=path_mapping_rules, @@ -464,7 +472,7 @@ def do_run(args: Namespace) -> OpenJDCliResult: try: # Raises: RuntimeError - the_job, job_parameter_values = generate_job( + the_job, job_parameter_values, step_symbol_tables = generate_job( args, environments, supported_extensions=extensions ) @@ -572,6 +580,7 @@ def do_run(args: Namespace) -> OpenJDCliResult: return _run_local_session( job=the_job, job_parameter_values=job_parameter_values, + step_symbol_tables=step_symbol_tables, step_list=step_list, selected_step=selected_step, task_parameter_values=task_parameter_values, diff --git a/src/openjd/cli/_summary/_summary_command.py b/src/openjd/cli/_summary/_summary_command.py index ebcb52a..86226b1 100644 --- a/src/openjd/cli/_summary/_summary_command.py +++ b/src/openjd/cli/_summary/_summary_command.py @@ -35,7 +35,7 @@ def do_summary(args: Namespace) -> OpenJDCliResult: try: # Raises: RuntimeError - sample_job, _ = generate_job(args, supported_extensions=extensions) + sample_job, _, _ = generate_job(args, supported_extensions=extensions) except RuntimeError as rte: return OpenJDCliResult(status="error", message=str(rte)) diff --git a/test/openjd/cli/conftest.py b/test/openjd/cli/conftest.py index 111ca35..98ab7ec 100644 --- a/test/openjd/cli/conftest.py +++ b/test/openjd/cli/conftest.py @@ -28,14 +28,20 @@ def sample_job_and_dirs(request): os.makedirs(current_working_dir) template = decode_job_template(template=MOCK_TEMPLATE) + # `job_from_template` also returns the per-step resolved symbol tables; + # this fixture's consumers construct LocalSessions without them, so it + # keeps yielding the (job, parameters, dirs...) shape. Tests that need + # the tables use the `step_let_job` fixture in test_step_symbol_tables.py. + job, parameters, _ = job_from_template( + template=template, + environments=[], + parameter_args=request.param, + job_template_dir=template_dir, + current_working_dir=current_working_dir, + ) yield ( - *job_from_template( - template=template, - environments=[], - parameter_args=request.param, - job_template_dir=template_dir, - current_working_dir=current_working_dir, - ), + job, + parameters, template_dir, current_working_dir, ) diff --git a/test/openjd/cli/templates/env_declares_redacted_env.yaml b/test/openjd/cli/templates/env_declares_redacted_env.yaml new file mode 100644 index 0000000..e2775f6 --- /dev/null +++ b/test/openjd/cli/templates/env_declares_redacted_env.yaml @@ -0,0 +1,10 @@ +specificationVersion: environment-2023-09 +extensions: + - REDACTED_ENV_VARS +environment: + name: EnvDeclaresRedactedEnv + script: + actions: + onEnter: + command: python + args: ["-c", "print('openjd_redacted_env: SECRET=s3cret')"] diff --git a/test/openjd/cli/templates/env_wrap_echoes_step_name.yaml b/test/openjd/cli/templates/env_wrap_echoes_step_name.yaml new file mode 100644 index 0000000..24e1fd4 --- /dev/null +++ b/test/openjd/cli/templates/env_wrap_echoes_step_name.yaml @@ -0,0 +1,17 @@ +specificationVersion: environment-2023-09 +extensions: + - EXPR + - WRAP_ACTIONS +environment: + name: WrapEchoesStepName + script: + actions: + onWrapEnvEnter: + command: python + args: ["-c", "print('WrapEnvEnter')"] + onWrapTaskRun: + command: python + args: ["-c", "print('WrappedStepName={{ WrappedStep.Name }}')"] + onWrapEnvExit: + command: python + args: ["-c", "print('WrapEnvExit')"] diff --git a/test/openjd/cli/templates/env_wrap_second.yaml b/test/openjd/cli/templates/env_wrap_second.yaml new file mode 100644 index 0000000..be20744 --- /dev/null +++ b/test/openjd/cli/templates/env_wrap_second.yaml @@ -0,0 +1,17 @@ +specificationVersion: environment-2023-09 +extensions: + - EXPR + - WRAP_ACTIONS +environment: + name: WrapSecond + script: + actions: + onWrapEnvEnter: + command: python + args: ["-c", "print('WrapSecondEnvEnter')"] + onWrapTaskRun: + command: python + args: ["-c", "print('WrapSecondTaskRun')"] + onWrapEnvExit: + command: python + args: ["-c", "print('WrapSecondEnvExit')"] diff --git a/test/openjd/cli/templates/feature_bundle_1_timeout.yaml b/test/openjd/cli/templates/feature_bundle_1_timeout.yaml index 3e21a31..6a7d461 100644 --- a/test/openjd/cli/templates/feature_bundle_1_timeout.yaml +++ b/test/openjd/cli/templates/feature_bundle_1_timeout.yaml @@ -5,7 +5,11 @@ extensions: parameterDefinitions: - name: Timeout type: INT - default: 5 + # Generous relative to the ~1s the action needs: the point of the test is + # that the format string resolves into `timeout`, not that the timeout + # fires. A tight value made the action race its own budget under a loaded + # parallel test run and exit non-zero. + default: 20 steps: - name: TimeoutStep script: diff --git a/test/openjd/cli/templates/job_env_refs_step_name.yaml b/test/openjd/cli/templates/job_env_refs_step_name.yaml new file mode 100644 index 0000000..90a438f --- /dev/null +++ b/test/openjd/cli/templates/job_env_refs_step_name.yaml @@ -0,0 +1,19 @@ +specificationVersion: "jobtemplate-2023-09" +extensions: + - EXPR +name: JobEnvRefsStepName + +jobEnvironments: + - name: JobEnvRefsStep + script: + actions: + onEnter: + command: python + args: ["-c", "print('JobEnvSawStepName={{ Step.Name }}')"] +steps: + - name: TheStep + script: + actions: + onRun: + command: python + args: ["-c", "print('TaskRan')"] diff --git a/test/openjd/cli/templates/job_name_expr_job.yaml b/test/openjd/cli/templates/job_name_expr_job.yaml new file mode 100644 index 0000000..31336a9 --- /dev/null +++ b/test/openjd/cli/templates/job_name_expr_job.yaml @@ -0,0 +1,21 @@ +specificationVersion: "jobtemplate-2023-09" +extensions: + - EXPR +name: JobNameExprJob + +steps: + - name: EchoJobName + let: + - bound_job_name = Job.Name + stepEnvironments: + - name: JobNameEcho + script: + actions: + onEnter: + command: python + args: ["-c", "print('EnvSawJobName={{ bound_job_name }}')"] + script: + actions: + onRun: + command: python + args: ["-c", "print('TaskRan')"] diff --git a/test/openjd/cli/templates/job_sleep_exit_normal.yaml b/test/openjd/cli/templates/job_sleep_exit_normal.yaml index f0f5e2f..9a96a19 100644 --- a/test/openjd/cli/templates/job_sleep_exit_normal.yaml +++ b/test/openjd/cli/templates/job_sleep_exit_normal.yaml @@ -12,9 +12,21 @@ "args": [ "-c", # Obfuscate "EXIT_NORMAL" so it doesn't appear in the log when Windows prints the command that's run to the log. - "import time,sys; print('SLEEP'); sys.stdout.flush(); time.sleep(5); print(chr(69)+'XIT_NORMAL')", + "import time,sys; print('SLEEP'); sys.stdout.flush(); time.sleep(60); print(chr(69)+'XIT_NORMAL')", ], - "timeout": 2, + # The margins are deliberately wide, in both + # directions, because this test asserts on a + # race: SLEEP must be printed *before* the + # timeout fires, and EXIT_NORMAL must not be. + # A 2s timeout left under 2s for interpreter + # startup, which a loaded parallel test run + # can exceed -- the action was then killed + # before printing SLEEP and the test failed + # intermittently. 5s of startup headroom, and + # a sleep an order of magnitude longer than + # the timeout, make both directions robust + # without changing what is verified. + "timeout": 5, } } }, diff --git a/test/openjd/cli/templates/no_extensions_shows_secret.yaml b/test/openjd/cli/templates/no_extensions_shows_secret.yaml new file mode 100644 index 0000000..a8b81b7 --- /dev/null +++ b/test/openjd/cli/templates/no_extensions_shows_secret.yaml @@ -0,0 +1,11 @@ +specificationVersion: "jobtemplate-2023-09" +name: No Extensions Shows Secret +description: Declares no extensions; prints whether SECRET reached the task + +steps: + - name: ShowSecret + script: + actions: + onRun: + command: python + args: ["-c", "import os; print('SECRET_IS=' + str(os.environ.get('SECRET')))"] diff --git a/test/openjd/cli/templates/redacted_env_undeclared.yaml b/test/openjd/cli/templates/redacted_env_undeclared.yaml new file mode 100644 index 0000000..0943511 --- /dev/null +++ b/test/openjd/cli/templates/redacted_env_undeclared.yaml @@ -0,0 +1,18 @@ +specificationVersion: "jobtemplate-2023-09" +name: Redacted Env Undeclared +description: Emits openjd_redacted_env without declaring the REDACTED_ENV_VARS extension + +jobEnvironments: + - name: EmitRedactedEnv + script: + actions: + onEnter: + command: python + args: ["-c", "print('openjd_redacted_env: SECRET=s3cret')"] +steps: + - name: ShowSecret + script: + actions: + onRun: + command: python + args: ["-c", "import os; print('SECRET_IS=' + str(os.environ.get('SECRET')))"] diff --git a/test/openjd/cli/templates/step_let_symtab_job.yaml b/test/openjd/cli/templates/step_let_symtab_job.yaml new file mode 100644 index 0000000..074205b --- /dev/null +++ b/test/openjd/cli/templates/step_let_symtab_job.yaml @@ -0,0 +1,39 @@ +specificationVersion: "jobtemplate-2023-09" +extensions: + - EXPR +name: StepLetSymtabJob +parameterDefinitions: + - name: Region + type: STRING + default: us-west-2 +jobEnvironments: + # Present so the "job environments get no step table" assertion has a call to + # inspect. Without it that assertion is vacuous. + - name: JobEnv + script: + actions: + onEnter: + command: python + args: ["-c", "print('JobEnvEntered')"] +steps: + - name: EchoStepLet + let: + # Template-scope `let`: resolved once at job creation. Its resolved value + # reaches a session only through the step's symbol table, so every + # assertion below fails outright if the CLI forwards nothing. + - bucket = "assets-" + Param.Region + stepEnvironments: + - name: StepLetEnv + script: + actions: + onEnter: + command: python + args: ["-c", "print('EnterSawStepLet={{ bucket }}')"] + onExit: + command: python + args: ["-c", "print('ExitSawStepLet={{ bucket }}')"] + script: + actions: + onRun: + command: python + args: ["-c", "print('TaskSawStepLet={{ bucket }}')"] diff --git a/test/openjd/cli/templates/wrapped_step_name_job.yaml b/test/openjd/cli/templates/wrapped_step_name_job.yaml new file mode 100644 index 0000000..ccc7cb9 --- /dev/null +++ b/test/openjd/cli/templates/wrapped_step_name_job.yaml @@ -0,0 +1,11 @@ +specificationVersion: "jobtemplate-2023-09" +name: WrappedStepNameJob +description: A single step whose onRun is replaced by an active onWrapTaskRun hook + +steps: + - name: WrappedTaskStep + script: + actions: + onRun: + command: python + args: ["-c", "print('TaskRanUnwrapped')"] diff --git a/test/openjd/cli/test_common.py b/test/openjd/cli/test_common.py index dad8dab..af501c5 100644 --- a/test/openjd/cli/test_common.py +++ b/test/openjd/cli/test_common.py @@ -370,7 +370,7 @@ def test_job_from_template_success( template_dir, current_working_dir = template_dir_and_cwd template = decode_job_template(template=template_dict) - result, _ = job_from_template(template, [], mock_params, template_dir, current_working_dir) + result, _, _ = job_from_template(template, [], mock_params, template_dir, current_working_dir) assert result.name == expected_job_name assert [step.model_dump(exclude_none=True) for step in result.steps] == [ step.model_dump(exclude_none=True) for step in template.steps diff --git a/test/openjd/cli/test_feature_bundle_1.py b/test/openjd/cli/test_feature_bundle_1.py index 09079a6..94f29ec 100644 --- a/test/openjd/cli/test_feature_bundle_1.py +++ b/test/openjd/cli/test_feature_bundle_1.py @@ -45,7 +45,7 @@ def test_format_string_timeout(self, capsys) -> None: """Test that format string timeout is resolved.""" template = TEMPLATES_DIR / "feature_bundle_1_timeout.yaml" outerr = run_openjd_cli_main(capsys, args=["run", str(template)], expected_exit_code=0) - assert "Running with timeout 5s" in outerr.out + assert "Running with timeout 20s" in outerr.out def test_format_string_amount_minmax(self, capsys) -> None: """Test that format string min/max in AmountRequirement is resolved.""" diff --git a/test/openjd/cli/test_local_session.py b/test/openjd/cli/test_local_session.py index 712f37f..d226d63 100644 --- a/test/openjd/cli/test_local_session.py +++ b/test/openjd/cli/test_local_session.py @@ -172,7 +172,7 @@ def test_localsession_run_success( session, sample_job.steps[step_index].stepEnvironments, EnvironmentType.STEP, - extra_let_bindings=None, + resolved_symtab=None, step_name=sample_job.steps[step_index].name, ), ] @@ -337,7 +337,7 @@ 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, + resolved_symtab=None, step_name=sample_job.steps[SampleSteps.BadCommand].name, ), ] diff --git a/test/openjd/cli/test_redacted_env.py b/test/openjd/cli/test_redacted_env.py index 2affa5c..e42847b 100644 --- a/test/openjd/cli/test_redacted_env.py +++ b/test/openjd/cli/test_redacted_env.py @@ -43,3 +43,61 @@ def test_run_job_with_redacted_env(capsys): assert ( unexpected_message not in outerr.out ), f"Found unexpected line in output:\n{format_capsys_outerr(outerr)}" + + +def test_run_job_redacted_env_not_enabled_by_accept_list(capsys): + """ + Extension behaviors activate only when a template *declares* the extension. + The CLI's --extensions list is what the CLI accepts, not what it enables. + + This job template declares no extensions, so REDACTED_ENV_VARS must stay + off even though --extensions defaults to every supported extension: the + variable does not reach the task, and the session warns that the extension + is not enabled. Building the session's RevisionExtensions from the + accept-list instead of the declared list flips both observations. + """ + outerr = run_openjd_cli_main( + capsys, + args=[ + "run", + str(TEMPLATE_DIR / "redacted_env_undeclared.yaml"), + ], + expected_exit_code=0, + ) + + assert ( + "SECRET_IS=None" in outerr.out + ), f"openjd_redacted_env set SECRET despite the extension not being declared:\n{format_capsys_outerr(outerr)}" + assert ( + "REDACTED_ENV_VARS extension is not enabled" in outerr.out + ), f"Expected the not-enabled warning:\n{format_capsys_outerr(outerr)}" + + +def test_run_job_redacted_env_declared_by_environment_template(capsys): + """ + The enabled extensions are the union of what the job template and every + external environment template declare. Here only the environment template + declares REDACTED_ENV_VARS, so redaction is active for the whole session: + SECRET reaches the task and its value is masked in the logs. Dropping the + environment templates from the union leaves the extension off. + """ + outerr = run_openjd_cli_main( + capsys, + args=[ + "run", + str(TEMPLATE_DIR / "no_extensions_shows_secret.yaml"), + "--environment", + str(TEMPLATE_DIR / "env_declares_redacted_env.yaml"), + ], + expected_exit_code=0, + ) + + assert ( + "SECRET_IS=" + "*" * 8 in outerr.out + ), f"SECRET was not set-and-redacted for the task:\n{format_capsys_outerr(outerr)}" + assert ( + "REDACTED_ENV_VARS extension is not enabled" not in outerr.out + ), f"The extension declared by the environment template was not enabled:\n{format_capsys_outerr(outerr)}" + assert ( + "s3cret" not in outerr.out + ), f"The redacted value leaked into the output:\n{format_capsys_outerr(outerr)}" diff --git a/test/openjd/cli/test_run_command.py b/test/openjd/cli/test_run_command.py index 5550c15..433c130 100644 --- a/test/openjd/cli/test_run_command.py +++ b/test/openjd/cli/test_run_command.py @@ -676,14 +676,21 @@ def test_run_local_session_enter_environment_raises(capsys: pytest.CaptureFixtur def test_do_run_step_name_in_step_environment(capsys: pytest.CaptureFixture) -> None: """ RFC 0007 §7.3.1 (EXPR) parity with openjd-rs: a step-level `let` binding - may reference Step.Name, and the step's environments are entered with the - binding so their actions can echo it. - - This is the end-to-end proof of the feature. It used to be `skipif`-gated on - feature-detecting the `step_name` keyword, which meant it did not run at all - against a sessions build that lacked it -- so the only test that actually - exercised Step.Name in a step environment was silently skipped. The - `openjd-sessions >= 0.10.11` floor guarantees the keyword, so it always runs. + may reference Step.Name, and the resolved value reaches the step's + environments. + + Same shape as test_do_run_job_name_in_step_let_binding: this pins the + create-time forward path -- openjd-model resolves `bound_name = Step.Name` + at job creation, the value travels in `step_symbol_tables`, the CLI hands + that table to the step-environment enter, and the onEnter action echoes it. + + Scope: it does NOT pin the CLI's `step_name` keyword on that enter. The + binding is step-*level*, so it is already a literal in the resolved table; + dropping `step_name=step_name` leaves this test passing (measured). That + keyword is pinned by test_localsession_step_env_enter_receives_step_name. + It also used to be `skipif`-gated on feature-detecting the keyword, so it + did not run at all against a sessions build that lacked it; the declared + openjd-sessions floor guarantees the keyword, so it always runs. """ template_dir = Path(__file__).parent / "templates" args = [ @@ -699,6 +706,120 @@ def test_do_run_step_name_in_step_environment(capsys: pytest.CaptureFixture) -> assert "TaskRan" in outerr.out +def test_do_run_job_name_in_step_let_binding(capsys: pytest.CaptureFixture) -> None: + """ + RFC 0007 §7.3.1 (EXPR): a step-level `let` binding may reference Job.Name, + and the resolved value reaches the step's environments. + + What this pins is the create-time forward path, end to end: openjd-model + resolves `bound_job_name = Job.Name` at job creation, the value travels in + `step_symbol_tables`, the CLI hands that table to the step-environment + enter, and the onEnter action echoes it. The assertion is on the value, so + any break in that chain fails here. + + Scope: because the binding is step-*level*, it is resolved before a Session + exists and is already a literal in the resolved table. Deleting + `job_name=str(job.name)` from the Session construction therefore leaves this + test passing (measured), so it does NOT pin the Session's Job.Name seeding. + """ + template_dir = Path(__file__).parent / "templates" + args = [ + "run", + str(template_dir / "job_name_expr_job.yaml"), + "--step", + "EchoJobName", + ] + outerr = run_openjd_cli_main(capsys, args=args, expected_exit_code=0) + assert ( + "EnvSawJobName=JobNameExprJob" in outerr.out + ), f"Job.Name did not resolve to the job's name:\n{format_capsys_outerr(outerr)}" + assert "TaskRan" in outerr.out + + +def test_do_run_wrapped_step_name_is_the_running_step(capsys: pytest.CaptureFixture) -> None: + """ + RFC 0008: the step name the CLI passes to Session.run_task feeds + WrappedStep.Name inside an active onWrapTaskRun hook. The hook echoes it, + so the assertion pins the resolved *value* against the step's real name -- + a constant or otherwise wrong step name fails here. + """ + template_dir = Path(__file__).parent / "templates" + args = [ + "run", + str(template_dir / "wrapped_step_name_job.yaml"), + "--environment", + str(template_dir / "env_wrap_echoes_step_name.yaml"), + ] + outerr = run_openjd_cli_main(capsys, args=args, expected_exit_code=0) + assert ( + "WrappedStepName=WrappedTaskStep" in outerr.out + ), f"WrappedStep.Name did not resolve to the running step's name:\n{format_capsys_outerr(outerr)}" + # The hook runs *instead of* the wrapped onRun. + assert "TaskRanUnwrapped" not in outerr.out + + +def test_do_run_step_name_undefined_outside_a_step(capsys: pytest.CaptureFixture) -> None: + """ + Step.Name exists only within a step's own scope: a job environment that + references it is rejected outright, with the model naming Job.Name as the + symbol that *is* in scope there. + + Scope of what this pins: the rejection is openjd-model *static* validation + at template-read time, so it fires before a Session is ever constructed. + That makes it a genuine end-to-end guarantee that Step.Name cannot be used + outside a step -- but it does NOT pin the CLI's own decision to leave + `step_name` off job/external environment enters, because the template never + reaches that code. The same rejection applies to every environment-template + location (onEnter, onExit, and RFC 0008 wrap hooks), so no loadable template + can observe whether the CLI seeded Step.Name there. That decision is pinned + only by test_localsession_step_env_enter_receives_step_name. + """ + template_dir = Path(__file__).parent / "templates" + args = [ + "run", + str(template_dir / "job_env_refs_step_name.yaml"), + ] + outerr = run_openjd_cli_main(capsys, args=args, expected_exit_code=1) + assert ( + "Variable Step.Name does not exist at this location" in outerr.out + ), f"Expected Step.Name to be out of scope for a job environment:\n{format_capsys_outerr(outerr)}" + assert "JobEnvSawStepName=" not in outerr.out + + +def test_do_run_enter_failure_before_registration_surfaces_real_error( + capsys: pytest.CaptureFixture, +) -> None: + """ + Two external environment templates both defining wrap hooks make + Session.enter_environment reject the second one up front, per RFC 0008, + *before* the session registers it. The CLI must drop that environment from + its own entered list so cleanup does not try to exit it -- otherwise the + session's "Cannot exit unknown Environment" RuntimeError escapes cleanup + and replaces the real error in the result. + """ + template_dir = Path(__file__).parent / "templates" + args = [ + "run", + str(template_dir / "wrapped_step_name_job.yaml"), + "--environment", + str(template_dir / "env_wrap_echoes_step_name.yaml"), + "--environment", + str(template_dir / "env_wrap_second.yaml"), + ] + outerr = run_openjd_cli_main(capsys, args=args, expected_exit_code=1) + + assert ( + "at most one Environment defining wrap hooks" in outerr.out + ), f"The RFC 0008 rejection is not the surfaced error:\n{format_capsys_outerr(outerr)}" + assert ( + "Cannot exit unknown Environment" not in outerr.out + ), f"Cleanup tried to exit the unregistered environment:\n{format_capsys_outerr(outerr)}" + # The unregistered environment is never exited, so the result keeps the + # default message rather than one built from a cleanup-time RuntimeError. + assert "Session ended with errors; see Task logs for details" in outerr.out + assert "WrapSecondEnvExit" not in outerr.out + + class TestProcessTaskParams: """Testing that we properly handle the values of the --task-param/-tp command-line argument""" diff --git a/test/openjd/cli/test_step_symbol_tables.py b/test/openjd/cli/test_step_symbol_tables.py new file mode 100644 index 0000000..2924bdd --- /dev/null +++ b/test/openjd/cli/test_step_symbol_tables.py @@ -0,0 +1,215 @@ +# Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. +"""The create-time step symbol table's path from job creation into a session. + +A step's template-scope ``let`` (RFC 0005 §3.6) is evaluated once, at job +creation. openjd-model does not merge the resolved bindings into ``script.let`` +and openjd-sessions does not re-evaluate the source expressions, so the only +channel those values have into a session is the ``resolved_symtab`` argument on +``Session.enter_environment``, ``Session.run_task`` and +``Session.exit_environment``. A CLI that forwards nothing produces a step whose +``let`` names are simply undefined -- not stale, absent. + +These tests pin the forwarding at each of the three call sites, and the +end-to-end result of all three together. +""" + +from pathlib import Path +from typing import Any +from unittest.mock import patch + +import pytest +import yaml + +from . import format_capsys_outerr, run_openjd_cli_main +from openjd.cli._common import SUPPORTED_EXTENSIONS +from openjd.cli._common._job_from_template import job_from_template +from openjd.cli._run._local_session._session_manager import EnvironmentType, LocalSession +from openjd.model import ( + RevisionExtensions, + StepParameterSpaceIterator, + decode_job_template, +) +from openjd.sessions import Session + +TEMPLATE_PATH = Path(__file__).parent / "templates" / "step_let_symtab_job.yaml" +STEP_NAME = "EchoStepLet" +# The resolved value of the template's `bucket = "assets-" + Param.Region` +# binding, under the Region parameter's default. +EXPECTED_BUCKET = "assets-us-west-2" + + +@pytest.fixture +def step_let_job(tmp_path: Path) -> Any: + """The step-``let`` job, together with its per-step resolved symbol tables. + + Deliberately a real ``job_from_template`` call rather than a stub table: the + point under test is that the values the model resolves at job creation reach + the session, and a hand-built table would not exercise that. + """ + template = decode_job_template( + template=yaml.safe_load(TEMPLATE_PATH.read_text()), + supported_extensions=SUPPORTED_EXTENSIONS, + ) + job, parameters, step_symbol_tables = job_from_template( + template=template, + environments=[], + parameter_args=[], + job_template_dir=tmp_path, + current_working_dir=tmp_path, + ) + return job, parameters, step_symbol_tables + + +def _local_session(job, parameters, step_symbol_tables, session_id: str) -> LocalSession: + """A LocalSession built the way ``do_run`` builds one for this job. + + ``revision_extensions`` must carry the extensions the job declares, or the + session parses the step's EXPR constructs against the default surface. + """ + return LocalSession( + job=job, + job_parameter_values=parameters, + session_id=session_id, + step_symbol_tables=step_symbol_tables, + revision_extensions=RevisionExtensions( + spec_rev=job.revision, supported_extensions=list(job.extensions or []) + ), + ) + + +def test_the_model_resolves_the_step_let_into_the_step_table(step_let_job) -> None: + """Precondition for the three forwarding tests: the table is worth sending. + + Not a forwarding test. It pins that ``create_job_with_symbol_tables`` + returns a table for the step, keyed by step name, carrying the step's + resolved ``let`` -- so a later forwarding test that finds nothing has a + forwarding bug rather than an empty producer. + """ + _, _, step_symbol_tables = step_let_job + + assert STEP_NAME in step_symbol_tables + symbols = step_symbol_tables[STEP_NAME].to_symtab().symbols + assert "bucket" in symbols + + +def test_run_task_forwards_the_steps_resolved_symtab(step_let_job) -> None: + """``Session.run_task`` receives the table for the step being run. + + Spied rather than stubbed, and driven through ``run_step``: the real + ``Session.run_task`` is what eventually fires the action-status callback + that ``LocalSession.run_task`` blocks on, so a no-op stub deadlocks. + """ + job, parameters, step_symbol_tables = step_let_job + step = next(s for s in job.steps if s.name == STEP_NAME) + parameter_set = next(iter(StepParameterSpaceIterator(space=step.parameterSpace))) + + with patch.object( + Session, "run_task", autospec=True, side_effect=Session.run_task + ) as patched_run_task: + with _local_session(job, parameters, step_symbol_tables, "run-task-symtab") as session: + session.run_step(step, task_parameters=[parameter_set]) + + assert not session.failed + patched_run_task.assert_called_once() + forwarded = patched_run_task.call_args.kwargs["resolved_symtab"] + # Identity, not merely truthiness: the step's own table, not another step's + # and not a rebuilt one. + assert forwarded is step_symbol_tables[STEP_NAME] + + +def test_step_env_enter_forwards_the_steps_resolved_symtab(step_let_job) -> None: + """A step environment is entered with its owning step's table. + + The job/external assertion at the end is a negative control: it cannot fail + against a revert of the forwarding (a revert makes *every* enter get no + table, which is what it asserts for these). It guards the other direction -- + forwarding one step's table to job or external environments, which are + outside step scope. + """ + job, parameters, step_symbol_tables = step_let_job + step = next(s for s in job.steps if s.name == STEP_NAME) + + with patch.object( + Session, "enter_environment", autospec=True, side_effect=Session.enter_environment + ) as patched_enter: + with _local_session(job, parameters, step_symbol_tables, "enter-symtab") as session: + session.run_step(step) + + assert not session.failed + + step_prefix = f"{EnvironmentType.STEP.name} - " + step_env_calls = [ + c for c in patched_enter.call_args_list if c.kwargs["identifier"].startswith(step_prefix) + ] + assert step_env_calls + for enter_call in step_env_calls: + assert enter_call.kwargs["resolved_symtab"] is step_symbol_tables[STEP_NAME] + + for enter_call in patched_enter.call_args_list: + if not enter_call.kwargs["identifier"].startswith(step_prefix): + assert enter_call.kwargs.get("resolved_symtab") is None + + +def test_step_env_exit_forwards_the_table_the_enter_used(step_let_job) -> None: + """A step environment's exit receives the same table its enter did. + + ``Session.exit_environment`` documents that as what makes an ``onExit`` + resolve in the same scope as its ``onEnter``. Identity against the enter's + argument rather than against the mapping, so a second lookup that happened + to return an equal-but-distinct table would still be visible. + """ + job, parameters, step_symbol_tables = step_let_job + step = next(s for s in job.steps if s.name == STEP_NAME) + + with ( + patch.object( + Session, "enter_environment", autospec=True, side_effect=Session.enter_environment + ) as patched_enter, + patch.object( + Session, "exit_environment", autospec=True, side_effect=Session.exit_environment + ) as patched_exit, + ): + with _local_session(job, parameters, step_symbol_tables, "exit-symtab") as session: + session.run_step(step) + + assert not session.failed + + step_prefix = f"{EnvironmentType.STEP.name} - " + entered = { + c.kwargs["identifier"]: c.kwargs["resolved_symtab"] + for c in patched_enter.call_args_list + if c.kwargs["identifier"].startswith(step_prefix) + } + assert entered + + exited = { + c.kwargs["identifier"]: c.kwargs.get("resolved_symtab") + for c in patched_exit.call_args_list + if c.kwargs["identifier"].startswith(step_prefix) + } + assert set(exited) == set(entered) + for identifier, table in entered.items(): + assert exited[identifier] is table + + +def test_do_run_step_let_reaches_all_three_action_kinds( + capsys: pytest.CaptureFixture, +) -> None: + """End to end: a step-level ``let`` produces its bindings in the session. + + One run covers all three entry points -- the step environment's ``onEnter`` + and ``onExit`` and the task's ``onRun`` each echo the bound value. The + assertion is on the resolved *value*, so a template-scope binding that + arrived unevaluated, or as a different step's value, fails here rather than + passing on mere presence. + """ + outerr = run_openjd_cli_main( + capsys, + args=["run", str(TEMPLATE_PATH), "--step", STEP_NAME], + expected_exit_code=0, + ) + + for label in ("EnterSawStepLet", "TaskSawStepLet", "ExitSawStepLet"): + assert ( + f"{label}={EXPECTED_BUCKET}" in outerr.out + ), f"{label} did not see the step's resolved `let`:\n{format_capsys_outerr(outerr)}"