fix: RFC 0005/0008 typed-arg and null-vs-empty parity with openjd-rs

Two Rust-parity fixes in the v0 session runtime:

1. WrappedAction.Args now uses RFC 0005 1.3.2 typed argument semantics.
   The enforcement path's typed arg loop (null skip, list flattening,
   display coercion) is extracted into a shared module-level helper,
   resolve_action_arg_values, in _runner_base.py; both
   _inject_wrapped_task_symbols and _inject_wrapped_env_symbols now use
   it, so a wrap hook sees exactly the argv the wrapped action would
   have run with unwrapped -- mirroring openjd-rs, whose
   seed_wrapped_action_symbols resolves through the same
   resolve_action_args as the runner. The helper also gains openjd-rs's
   plain-string-resolution fallback when typed resolution fails.

2. An empty string is no longer conflated with null.
   resolve_optional_int_field and resolve_effective_cancelation's
   deferred-mode branch now resolve typed (resolve_value) and treat
   only a typed null result as "field omitted" / "no cancelation
   declared". A genuine empty string now reaches the "must be a
   positive integer, got ''" / "must resolve to ... got ''" errors,
   matching openjd-rs's resolve_action_timeout,
   resolve_notify_period_seconds, and resolve_effective_cancelation,
   which only special-case ExprValue::Null. The
   whole_field_expression() pre-check is removed: resolve_value only
   yields a typed null for whole-field expressions, so null semantics
   remain whole-field-only by construction.

Test updates: the deferred-cancelation unit-test helper now parses its
format strings with the EXPR extension (deferred forwarding is an
RFC 0008 construct and WRAP_ACTIONS requires EXPR), since the previous
legacy parse pinned the non-Rust "" == null behavior.

Signed-off-by: David Leong <116610336+leongdl@users.noreply.github.com>

Author: leongdl

src/openjd/sessions/_runner_base.py

@@ -36,6 +36,7 @@
     "NotifyCancelMethod",
     "ScriptRunnerBase",
     "apply_let_bindings",
+    "resolve_action_arg_values",
     "resolve_effective_cancelation",
     "resolve_optional_int_field",
 )
@@ -96,6 +97,48 @@ class NotifyCancelMethod(CancelMethod):
     """Amount of time after a SIGTERM to wait to do the SIGKILL"""
 
 
+def resolve_action_arg_values(args: Optional[Sequence], symtab: SymbolTable) -> list[str]:
+    """Resolve an action's ``args`` field into its flat list of argument
+    strings (excluding the command).
+
+    RFC 0005 §1.3.2 argument semantics, mirroring openjd-rs's
+    resolve_action_args: a whole-field expression argument resolves typed —
+    a null result skips the argument, a list result flattens inline (one
+    argument per element, rendered with the engine's display coercion), and
+    a scalar becomes a single argument. Multi-segment format strings and
+    legacy (non-EXPR) expressions resolve to their string form.
+
+    Shared by the enforcement path (:meth:`ScriptRunnerBase._run_action`)
+    and the RFC 0008 ``WrappedAction.Args`` injection, so a wrap hook sees
+    exactly the arguments the wrapped action would have run with unwrapped.
+
+    Raises:
+        FormatStringError: If an argument's expression cannot be resolved.
+    """
+    resolved: list[str] = []
+    if args is not None:
+        for arg in args:
+            try:
+                value = arg.resolve_value(symtab=symtab)
+            except FormatStringError:
+                # Mirror openjd-rs's resolve_action_args: when typed
+                # resolution fails (e.g. a legacy-parsed expression meeting
+                # a typed symbol value), fall back to plain string
+                # resolution — which raises FormatStringError itself if the
+                # argument is genuinely unresolvable.
+                resolved.append(arg.resolve(symtab=symtab))
+                continue
+            if isinstance(value, str):
+                resolved.append(value)
+            elif getattr(value, "is_null", False):
+                continue
+            elif str(getattr(value, "type", "")).startswith("list["):
+                resolved.extend(str(element) for element in value)
+            else:
+                resolved.append(str(value))
+    return resolved
+
+
 def resolve_optional_int_field(
     value: Any,
     symtab: SymbolTable,
@@ -111,13 +154,16 @@ def resolve_optional_int_field(
     - ``None`` (field omitted) stays ``None``.
     - A literal ``int`` passes through unchecked: literal values were
       bounds-checked by the static validator at parse time.
-    - A FormatString (FEATURE_BUNDLE_1) is resolved against ``symtab``. A
-      whole-field expression that resolves to null renders as the empty
-      string and is treated as if the field were not provided (``None`` —
-      the caller applies any positional schema default). Otherwise the
-      resolved value must be an integer within the given bounds; the bounds
-      apply here because format-string values could not be checked at parse
-      time.
+    - A FormatString (FEATURE_BUNDLE_1) is resolved against ``symtab``
+      using typed resolution. A whole-field expression that resolves to a
+      typed null is treated as if the field were not provided (``None`` —
+      the caller applies any positional schema default). Any other result
+      — including a genuine empty string — must be an integer within the
+      given bounds; the bounds apply here because format-string values
+      could not be checked at parse time. This matches the openjd-rs
+      runtime (resolve_action_timeout / resolve_notify_period_seconds),
+      which only treats an ExprValue::Null result as "field omitted" and
+      errors on an empty string.
 
     Raises:
         ValueError: If the resolved value is not an integer, or violates
@@ -128,9 +174,15 @@ def resolve_optional_int_field(
         return None
     if isinstance(value, int):
         return value
-    resolved = value.resolve(symtab=symtab)
-    if resolved == "":
+    # Typed resolution: a whole-field EXPR expression yields the engine's
+    # typed value, so a null result is distinguishable from a genuine empty
+    # string. Multi-segment and legacy (non-EXPR) format strings fall back
+    # to plain string resolution — correct, since typed nulls only exist
+    # under EXPR whole-field semantics (Template Schemas 5.3).
+    resolved_value = value.resolve_value(symtab=symtab)
+    if getattr(resolved_value, "is_null", False):
         return None
+    resolved = str(resolved_value)
     if ge is not None and le is not None:
         constraint = f"between {ge} and {le}"
     elif ge == 1 and le is None:
@@ -195,19 +247,23 @@ def resolve_period(period: Any) -> Optional[int]:
     if cancelation is None:
         return (None, None)
     if isinstance(cancelation, CancelationMethodDeferred_2023_09):
-        # Null semantics apply only to a whole-field expression
-        # ("{{ ... }}" with no surrounding text, target type string? —
-        # Template Schemas 5.3). A normal format string that happens to
-        # resolve to the empty string is NOT null; it falls through to the
-        # "must resolve to..." error below.
-        is_whole_field = cancelation.mode.whole_field_expression() is not None
-        mode = cancelation.mode.resolve(symtab=symtab)
-        if mode == "" and is_whole_field:
+        # Typed resolution. Null semantics apply only to a whole-field
+        # expression ("{{ ... }}" with no surrounding text, target type
+        # string? — Template Schemas 5.3), and resolve_value only yields a
+        # typed null for a whole-field EXPR expression; every other format
+        # string resolves to its plain string form. A format string that
+        # happens to resolve to the empty string is NOT null; it falls
+        # through to the "must resolve to..." error below (matching the
+        # openjd-rs runtime, which errors on any non-null, non-mode-name
+        # result).
+        mode_value = cancelation.mode.resolve_value(symtab=symtab)
+        if getattr(mode_value, "is_null", False):
             # Null mode drops the ENTIRE cancelation object: mode is the
             # object's required discriminator, so an "omitted" mode cannot
             # leave a partial object behind. The action behaves exactly as
             # if no <Cancelation> were declared.
             return (None, None)
+        mode = str(mode_value)
         if mode == CancelationMode_2023_09.TERMINATE.value:
             # Post-resolution the object must validate against the resolved
             # variant's shape: TERMINATE admits no notify period.
@@ -644,24 +700,10 @@ def _run_action(
         assert isinstance(action, Action_2023_09)
         try:
             command = [action.command.resolve(symtab=symtab)]
-            if action.args is not None:
-                # RFC 0005 §1.3.2 argument semantics, mirroring openjd-rs's
-                # resolve_action_args: a whole-field expression argument
-                # resolves typed — a null result skips the argument, a list
-                # result flattens inline (one argument per element, rendered
-                # with the engine's display coercion), and a scalar becomes a
-                # single argument. Multi-segment format strings and legacy
-                # (non-EXPR) expressions resolve to their string form.
-                for arg in action.args:
-                    value = arg.resolve_value(symtab=symtab)
-                    if isinstance(value, str):
-                        command.append(value)
-                    elif getattr(value, "is_null", False):
-                        continue
-                    elif str(getattr(value, "type", "")).startswith("list["):
-                        command.extend(str(element) for element in value)
-                    else:
-                        command.append(str(value))
+            # RFC 0005 §1.3.2 typed argument semantics (null skip, list
+            # flattening) — see resolve_action_arg_values, shared with the
+            # RFC 0008 WrappedAction.Args injection.
+            command.extend(resolve_action_arg_values(action.args, symtab))
         except FormatStringError as exc:
             # Extremely unlikely since a JobTemplate needs to have passed
             # validation before we could be running it, but just to be safe.
@@ -670,11 +712,10 @@ def _run_action(
             time_limit: Optional[timedelta] = default_timeout
             # A FormatString timeout (FEATURE_BUNDLE_1) is resolved right
             # before the action runs. A whole-field expression that
-            # resolves to null renders as the empty string — e.g.
-            # forwarding `timeout: "{{WrappedAction.Timeout}}"`
-            # (RFC 0008) when the wrapped action specified no
-            # timeout — and is treated as if the field were not
-            # provided, so the positional default applies.
+            # resolves to a typed null — e.g. forwarding
+            # `timeout: "{{WrappedAction.Timeout}}"` (RFC 0008) when the
+            # wrapped action specified no timeout — is treated as if the
+            # field were not provided, so the positional default applies.
             try:
                 seconds = resolve_optional_int_field(
                     action.timeout, symtab, ge=1, description="timeout"

src/openjd/sessions/_session.py

@@ -45,6 +45,7 @@
 from ._runner_base import (
     ScriptRunnerBase,
     apply_let_bindings,
+    resolve_action_arg_values,
     resolve_effective_cancelation,
     resolve_optional_int_field,
 )
@@ -1771,9 +1772,13 @@ def _inject_wrapped_env_symbols(
         is removed from tracking, so the wrapped environment's own
         variables are included."""
         command = inner_action.command.resolve(symtab=inner_symtab)
-        args = (
-            [a.resolve(symtab=inner_symtab) for a in inner_action.args] if inner_action.args else []
-        )
+        # RFC 0005 §1.3.2 typed argument semantics (null skip, list
+        # flattening), shared with the enforcement path
+        # (ScriptRunnerBase._run_action) so the hook sees exactly the
+        # arguments the wrapped action would have run with unwrapped —
+        # mirroring openjd-rs's seed_wrapped_action_symbols, which resolves
+        # via the same resolve_action_args as the runner.
+        args = resolve_action_arg_values(inner_action.args, inner_symtab)
         symtab["WrappedAction.Command"] = command
         symtab["WrappedAction.Args"] = args
         symtab["WrappedAction.Environment"] = (
@@ -1807,9 +1812,10 @@ def _inject_wrapped_task_symbols(
         on_run = step_script.actions.onRun
 
         symtab["WrappedAction.Command"] = on_run.command.resolve(symtab=inner_symtab)
-        symtab["WrappedAction.Args"] = (
-            [arg.resolve(symtab=inner_symtab) for arg in on_run.args] if on_run.args else []
-        )
+        # RFC 0005 §1.3.2 typed argument semantics (null skip, list
+        # flattening), shared with the enforcement path — see
+        # _inject_wrapped_env_symbols.
+        symtab["WrappedAction.Args"] = resolve_action_arg_values(on_run.args, inner_symtab)
         symtab["WrappedAction.Environment"] = self._collect_session_env_list()
         symtab["WrappedAction.Timeout"] = self._resolve_action_timeout(on_run, inner_symtab)
         self._inject_wrapped_cancelation_symbols(

test/openjd/sessions_v0/test_session_let_bindings.py

@@ -228,6 +228,16 @@ def test_whole_field_null_resolves_to_none(self) -> None:
         symtab["X"] = None
         assert resolve_optional_int_field(field, symtab, ge=1, description="timeout") is None
 
+    def test_whole_field_empty_string_rejected(self) -> None:
+        # A genuine empty STRING is not null (openjd-rs parity: only an
+        # ExprValue::Null result means "field omitted"; an empty string
+        # falls through to the integer parse and errors).
+        field = _format_string_field("{{ X }}")
+        symtab = SymbolTable()
+        symtab["X"] = ""
+        with pytest.raises(ValueError, match="timeout must be a positive integer, got ''"):
+            resolve_optional_int_field(field, symtab, ge=1, description="timeout")
+
     def test_resolved_value_below_ge_rejected(self) -> None:
         field = _format_string_field("{{ X }}")
         symtab = SymbolTable()

test/openjd/sessions_v0/test_wrap_cancelation.py

@@ -269,7 +269,12 @@ def _deferred(self, mode: str, period: str | None = None):
         from openjd.model._format_strings import FormatString
         from openjd.model.v2023_09 import CancelationMethodDeferred, ModelParsingContext
 
-        ctx = ModelParsingContext()
+        # A deferred mode is an RFC 0008 forwarding construct, and
+        # WRAP_ACTIONS requires the EXPR extension — so the format strings
+        # here parse as EXPR expressions, giving the typed null semantics
+        # the runtime relies on (a whole-field null drops the cancelation
+        # object; an empty STRING is an error, matching openjd-rs).
+        ctx = ModelParsingContext(supported_extensions=["FEATURE_BUNDLE_1", "EXPR"])
         return CancelationMethodDeferred(
             mode=FormatString(mode, context=ctx),
             notifyPeriodInSeconds=(
@@ -313,6 +318,17 @@ def test_mode_resolving_notify_then_terminate_with_period(self) -> None:
         )
         assert result == ("NOTIFY_THEN_TERMINATE", 45)
 
+    def test_whole_field_mode_resolving_empty_string_raises(self) -> None:
+        # A genuine empty STRING is not null, even for a whole-field
+        # expression (openjd-rs parity: only an ExprValue::Null result
+        # drops the cancelation object; an empty string is an invalid
+        # mode). E.g. a STRING parameter whose value is "".
+        from openjd.sessions._runner_base import resolve_effective_cancelation
+
+        cancelation = self._deferred("{{X}}")
+        with pytest.raises(ValueError, match="must resolve to .* got ''"):
+            resolve_effective_cancelation(cancelation, self._symtab(X=""))
+
     def test_mode_resolving_garbage_raises(self) -> None:
         from openjd.sessions._runner_base import resolve_effective_cancelation
 

test/openjd/sessions_v0/test_wrap_task_run.py

@@ -105,6 +105,45 @@ def test_injects_empty_args_when_step_has_no_args(self) -> None:
         finally:
             session.cleanup()
 
+    def test_injects_typed_args_null_skip_and_list_flatten(self) -> None:
+        # RFC 0005 §1.3.2 typed argument semantics (openjd-rs parity: the
+        # wrapped path in seed_wrapped_action_symbols resolves through the
+        # same resolve_action_args as the runner): a whole-field list
+        # expression flattens inline (one argument per element), a
+        # whole-field null is skipped, and the hook sees exactly the argv
+        # the wrapped action would have run with unwrapped.
+        from openjd.model.v2023_09 import ModelParsingContext
+        from openjd.sessions._runner_base import resolve_action_arg_values
+
+        context = ModelParsingContext(supported_extensions=["EXPR"])
+        script = StepScript_2023_09.model_validate(
+            {
+                "actions": {
+                    "onRun": {
+                        "command": "echo",
+                        "args": ["front", '{{ ["a", "b c"] }}', "{{ null }}", "back"],
+                    }
+                }
+            },
+            context=context,
+        )
+        session = Session(session_id=uuid.uuid4().hex, job_parameter_values={})
+        try:
+            symtab = SymbolTable()
+            session._inject_wrapped_task_symbols(symtab, script, "MyStep", inner_symtab=symtab)
+            assert symtab["WrappedAction.Args"] == ["front", "a", "b c", "back"]
+            # The unwrapped enforcement path (_run_action) resolves the same
+            # action's args via the same shared helper — wrapped and
+            # unwrapped runs of this action use identical argv.
+            assert resolve_action_arg_values(script.actions.onRun.args, symtab) == [
+                "front",
+                "a",
+                "b c",
+                "back",
+            ]
+        finally:
+            session.cleanup()
+
     def test_injects_wrapped_environment_as_key_value_list(self) -> None:
         # RFC 0008 (openjd-rs #277): WrappedAction.Environment carries every
         # session-defined variable — openjd_env definitions (applied via