fix: Re-evaluate a step's template-scope let bindings in template scope

src/openjd/sessions/_runner_base.py

@@ -39,6 +39,7 @@
     "NotifyCancelMethod",
     "ScriptRunnerBase",
     "apply_let_bindings",
+    "apply_script_let_bindings",
     "resolve_action_arg_values",
     "resolve_effective_cancelation",
     "resolve_optional_int_field",
@@ -165,12 +166,14 @@ class _ExprKind(Enum):
 def _classify_expr_value(value: Any) -> _ExprKind:
     """Classify ``value`` against the EXPR type system.
 
-    This is the *only* place in this module that imports ``openjd.expr``, so
-    that every crossing into the native extension sits behind the one
+    This is one of two places in this module that import ``openjd.expr``, and
+    it is the one reached from the non-EXPR path, so it sits behind the
     ``sys.modules`` guard below. Doing the whole classification here rather
     than exposing a separate "is it a list" predicate keeps that property
     structural instead of merely documented: there is no second, unguarded
-    entry point for a future caller to reach with an arbitrary value.
+    entry point for a future caller to reach with an arbitrary value. The other
+    crossing is in :func:`_apply_template_scope_let_bindings`, guarded instead
+    by a non-zero template-scope prefix, which only an EXPR template produces.
 
     ``ExprValue`` instances are created only by the native extension, so if
     that extension has not been loaded then ``value`` cannot be one and the
@@ -494,7 +497,9 @@ def resolve_period(period: Any) -> Optional[int]:
     )
 
 
-def apply_let_bindings(*, symtab: SymbolTable, let_bindings: list[str]) -> None:
+def apply_let_bindings(
+    *, symtab: SymbolTable, let_bindings: list[str], path_format: Any = None
+) -> None:
     """Evaluate EXPR ``let`` bindings (RFC 0005) and add them to ``symtab``.
 
     ``let_bindings`` is a script's ``let`` field: an ordered list of
@@ -510,6 +515,12 @@ def apply_let_bindings(*, symtab: SymbolTable, let_bindings: list[str]) -> None:
     ``Env.File.*``/``Task.File.*`` and a file's ``data`` may reference
     let-bound values (mirroring openjd-rs's runner ordering).
 
+    ``path_format`` is the EXPR ``PathFormat`` that PATH-typed results render
+    with. ``None`` -- the default, and what every session-scope binding wants --
+    leaves the engine's default, which is the host's format. Callers
+    re-evaluating a *template*-scope binding pass ``PathFormat.POSIX``; see
+    :func:`apply_script_let_bindings`.
+
     Raises:
         ValueError (FormatStringError/ExpressionError): if a binding's
             expression cannot be evaluated, or if a binding is too long to
@@ -528,7 +539,140 @@ def apply_let_bindings(*, symtab: SymbolTable, let_bindings: list[str]) -> None:
             )
     # Single-sourced in openjd.model (parse-memoized; skips malformed
     # bindings; raises ValueError naming the failing binding).
-    evaluate_let_bindings(symtab=symtab, let_bindings=let_bindings)
+    #
+    # The kwarg is forwarded only when it is set, because `path_format` does not
+    # exist on openjd-model at this package's declared floor (>= 0.11.6) and
+    # passing it there is a TypeError, not a no-op -- which would break EVERY
+    # EXPR template rather than degrading. On such a model the else branch is
+    # unreachable from any internal caller: apply_script_let_bindings reads
+    # the template-scope count through getattr, and a model without
+    # `path_format` has no `_template_scope_let_count` either, so the count is 0
+    # and nothing asks for a non-default format. Tests call it directly. On a model that does have it, `None` and "omitted"
+    # are the same call. Collapse this to an unconditional forward once the
+    # openjd-model floor carries the parameter.
+    if path_format is None:
+        evaluate_let_bindings(symtab=symtab, let_bindings=let_bindings)
+    else:
+        evaluate_let_bindings(symtab=symtab, let_bindings=let_bindings, path_format=path_format)
+
+
+def _host_path_format() -> Any:
+    """The EXPR ``PathFormat`` for this host.
+
+    The Python engine bindings expose no ``PathFormat.host()``, so it is derived
+    the same way :meth:`Session._resolved_base_entries` derives it.
+    """
+    import os
+
+    from openjd.expr import PathFormat
+
+    return PathFormat.WINDOWS if os.name == "nt" else PathFormat.POSIX
+
+
+def _apply_template_scope_let_bindings(*, symtab: SymbolTable, let_bindings: list[str]) -> None:
+    """Evaluate template-scope ``let`` bindings and seed them in host format.
+
+    Two steps, and both are load-bearing.
+
+    The bindings are evaluated with ``PathFormat.POSIX``, because that is what
+    openjd-model and openjd-rs use at job creation, so a value that leaves
+    path-space during evaluation freezes the same text it froze there. That
+    covers ``string(path(...))``, ``join(...)``, ``repr_sh(...)``, ``.parts``
+    and every comparison against a POSIX literal.
+
+    The results are then re-tagged to the host's format before they are seeded.
+    An EXPR path value carries its format, and reading one under a different
+    format is a hard error rather than a re-render::
+
+        ExpressionError: Path format mismatch for 'root':
+                         value has Posix but evaluator uses Windows
+
+    Action arguments, embedded-file ``data`` and environment-variable values all
+    resolve in the host's format, so seeding a Posix-tagged path would make
+    every one of those reads raise on Windows. The re-tag round trip leaves a
+    frozen string alone and re-renders a live path, which is exactly the split
+    the conformance suite asks for: ``expr2.2.1--string-conversion`` wants
+    ``/mnt/out`` on both platforms, while ``expr2.3.2--path-construction`` wants
+    ``\\a\\b`` on Windows.
+
+    This mirrors how a create-time table already reaches a session:
+    :meth:`Session._resolved_base_entries` deserializes it with
+    ``to_symtab(path_format=host_format)``.
+    """
+    from openjd.expr import PathFormat, SerializedSymbolTable
+
+    from openjd.model._format_strings._expr_support import symtab_to_expr_values
+
+    # A child table, so a binding can read the symbols already in scope without
+    # the POSIX evaluation writing host-format neighbours back into `symtab`.
+    scratch = SymbolTable(source=symtab)
+    apply_let_bindings(symtab=scratch, let_bindings=let_bindings, path_format=PathFormat.POSIX)
+
+    # Only the names these bindings defined. A malformed binding is skipped by
+    # the evaluator, so membership is checked rather than assumed.
+    holder = SymbolTable()
+    for binding in let_bindings:
+        name = binding.partition("=")[0].strip()
+        if name and name in scratch:
+            holder[name] = scratch[name]
+    if not holder.symbols:
+        return
+
+    engine = symtab_to_expr_values(
+        holder, types=getattr(holder, "expr_types", None), path_format=PathFormat.POSIX
+    )
+    retagged = SerializedSymbolTable.from_symtab(engine).to_symtab(path_format=_host_path_format())
+    for name in retagged.symbols:
+        symtab[name] = retagged[name]
+
+
+def apply_script_let_bindings(
+    *, symtab: SymbolTable, let_bindings: list[str], script: Any = None
+) -> None:
+    """Evaluate a script's ``let`` list into ``symtab``, honouring the
+    template-scope / session-scope boundary inside it.
+
+    An instantiated Step's script carries a *merged* ``let`` list: the
+    step-level bindings the template declared, followed by the script's own
+    (openjd-model's ``StepTemplate.resolve_syntax_sugar``). The step-level
+    prefix is template scope and was already evaluated as such at job creation.
+    Re-evaluating it here in the host's format changes its value: on Windows
+    ``startswith(path("/foo/bar"), "/foo")`` flips from ``true`` to ``false``.
+
+    So the list is split. The prefix goes through
+    :func:`_apply_template_scope_let_bindings`, which reproduces the
+    create-time value and then seeds it in host format. The remainder is the
+    script's own bindings, evaluated unchanged in the host's format, because
+    they legitimately reference host-scope symbols
+    (``Session.WorkingDirectory``, ``Task.File.*``, ``apply_path_mapping``).
+
+    Both halves land in the **same** table in the **same** order, so a
+    script-level binding can reference a step-level one.
+
+    ``script`` is the model object the ``let`` list came from. The boundary is
+    read off it as ``_template_scope_let_count`` through ``getattr`` with a
+    default of 0, so an openjd-model that predates the model-side half of this
+    fix degrades to the previous behaviour rather than raising. ``None`` --
+    what an environment script's caller passes -- means the same thing: an
+    environment script's own bindings are session scope.
+
+    Raises:
+        ValueError: as :func:`apply_let_bindings`.
+    """
+    template_scope_count = getattr(script, "_template_scope_let_count", 0)
+    if not 0 <= template_scope_count <= len(let_bindings):
+        # A model/sessions version skew reported a boundary this list cannot
+        # have. Treating everything as session scope is the previous behaviour,
+        # which is wrong on Windows but never raises; guessing a prefix could
+        # evaluate a genuinely session-scope binding in the wrong scope.
+        template_scope_count = 0
+    if template_scope_count:
+        _apply_template_scope_let_bindings(
+            symtab=symtab, let_bindings=let_bindings[:template_scope_count]
+        )
+    host_scope_bindings = let_bindings[template_scope_count:]
+    if host_scope_bindings:
+        apply_let_bindings(symtab=symtab, let_bindings=host_scope_bindings)
 
 
 class ScriptRunnerBase(ABC):
@@ -1020,10 +1164,17 @@ def _materialize_files(
         symtab: SymbolTable,
         let_bindings: Optional[list[str]] = None,
         preallocated_records: Optional[list[_FileRecord]] = None,
+        script: Any = None,
     ) -> None:
         """Helper for derived classes that wraps all of the logic around
         materializing embedded files to disk.
 
+        ``script`` is the model object ``let_bindings`` came from, forwarded to
+        :func:`apply_script_let_bindings` so a step script's merged list is
+        split at its template-scope boundary. Omitting it evaluates every
+        binding in the host's path format, which is what an environment script
+        wants.
+
         When ``let_bindings`` is given, they are evaluated between file-path
         allocation and content writing (RFC 0005, mirroring the openjd-rs
         runners): a file's *path* never depends on ``let`` values (filenames
@@ -1061,20 +1212,25 @@ def _materialize_files(
             else:
                 records = file_writer.allocate_file_paths(files, symtab)
             if let_bindings:
-                apply_let_bindings(symtab=symtab, let_bindings=let_bindings)
+                apply_script_let_bindings(symtab=symtab, let_bindings=let_bindings, script=script)
             file_writer.write_file_contents(records, symtab)
         except (RuntimeError, ValueError) as exc:
             # Had a problem writing at least one file to disk, or evaluating
             # a `let` binding (FormatStringError/ExpressionError subclass
             # ValueError). Surface the error.
             self._fail_action(str(exc))
 
-    def _apply_let_bindings_or_fail(self, symtab: SymbolTable, let_bindings: list[str]) -> bool:
+    def _apply_let_bindings_or_fail(
+        self, symtab: SymbolTable, let_bindings: list[str], script: Any = None
+    ) -> bool:
         """Evaluate the script's EXPR ``let`` bindings into ``symtab``. On an
         evaluation error the action is failed through the normal failure path
-        (openjd_fail log, FAILED state, callback). Returns True on success."""
+        (openjd_fail log, FAILED state, callback). Returns True on success.
+
+        ``script`` is the model object the bindings came from; see
+        :func:`apply_script_let_bindings` for what it is read for."""
         try:
-            apply_let_bindings(symtab=symtab, let_bindings=let_bindings)
+            apply_script_let_bindings(symtab=symtab, let_bindings=let_bindings, script=script)
         except ValueError as exc:
             self._fail_action(str(exc))
             return False

src/openjd/sessions/_runner_step_script.py

@@ -102,6 +102,10 @@ def run(self) -> None:
         # the script's EXPR `let` bindings evaluate (so bindings can reference
         # Task.File.*), and contents are written after (so `data` can
         # reference let-bound values) — mirroring the openjd-rs runner.
+        #
+        # `script=self._script` is what tells the evaluation where this merged
+        # `let` list stops being template scope and starts being session scope;
+        # see apply_script_let_bindings.
         if self._script.embeddedFiles is not None:
             symtab = SymbolTable(source=self._symtab)
             self._materialize_files(
@@ -110,12 +114,13 @@ def run(self) -> None:
                 self._session_files_directory,
                 symtab,
                 let_bindings=let_bindings,
+                script=self._script,
             )
             if self.state == ScriptRunnerState.FAILED:
                 return
         elif let_bindings:
             symtab = SymbolTable(source=self._symtab)
-            if not self._apply_let_bindings_or_fail(symtab, let_bindings):
+            if not self._apply_let_bindings_or_fail(symtab, let_bindings, self._script):
                 return
         else:
             symtab = self._symtab

src/openjd/sessions/_session.py

@@ -44,7 +44,7 @@
 from ._path_mapping import PathMappingRule
 from ._runner_base import (
     ScriptRunnerBase,
-    apply_let_bindings,
+    apply_script_let_bindings,
     resolve_action_arg_values,
     resolve_effective_cancelation,
     resolve_optional_int_field,
@@ -1998,6 +1998,7 @@ def _build_wrapped_inner_scope(
         let_bindings: Optional[list[str]],
         embedded_files: Optional[Any],
         base: SymbolTable,
+        script: Any = None,
     ) -> SymbolTable:
         """Build the scope a wrapped action would have resolved against had
         it run unwrapped: a copy of ``base`` (the session-scope table) plus
@@ -2012,6 +2013,17 @@ def _build_wrapped_inner_scope(
         symmetrically, the inner entity's lets never apply to the hook's own
         resolution scope. Mirrors openjd-rs's ``build_wrapped_inner_scope``.
 
+        ``script`` is the inner entity's script -- the model object
+        ``let_bindings`` came from -- forwarded so that a wrapped *step* script's
+        merged ``let`` list is split at its template-scope boundary exactly as
+        the step runner splits it (see
+        :func:`~._runner_base.apply_script_let_bindings`). Without it a wrapped
+        action would resolve against template-scope values re-rendered in the
+        host's path format, i.e. against a scope that differs from the one it
+        would have had unwrapped -- which is the whole property this method
+        exists to reproduce. An inner *environment* script has no such prefix and
+        is unaffected.
+
         Raises:
             ValueError (FormatStringError/ExpressionError): a binding or file
                 reference did not resolve.
@@ -2027,10 +2039,10 @@ def _build_wrapped_inner_scope(
             )
             records = file_writer.allocate_file_paths(embedded_files, symtab)
             if let_bindings:
-                apply_let_bindings(symtab=symtab, let_bindings=let_bindings)
+                apply_script_let_bindings(symtab=symtab, let_bindings=let_bindings, script=script)
             file_writer.write_file_contents(records, symtab)
         elif let_bindings:
-            apply_let_bindings(symtab=symtab, let_bindings=let_bindings)
+            apply_script_let_bindings(symtab=symtab, let_bindings=let_bindings, script=script)
         return symtab
 
     def _try_inject_wrapped_symbols(
@@ -2061,6 +2073,7 @@ def _try_inject_wrapped_symbols(
                 inner_script.let if inner_script is not None else None,
                 inner_script.embeddedFiles if inner_script is not None else None,
                 symtab,
+                inner_script,
             )
             inject(inner_symtab)
         except (FormatStringError, ValueError, RuntimeError) as e: