From fccf2b0a267aa59b57f1bd065065843eb87a4a5b Mon Sep 17 00:00:00 2001 From: NgoQuocViet2001 Date: Thu, 3 Sep 2026 14:51:01 +0700 Subject: [PATCH 1/2] fix(workflows): resolve negative list indices in expressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _resolve_dot_path matched only digits in the index bracket, so `task_list[-1]` never entered the indexing branch. It fell through to the dict lookup and asked for the literal key "task_list[-1]", which returns None — a template reaching for the last element of a step output rendered empty with no error, and a condition on it silently read false. Accept the negative form Python and Jinja2 both use, and bound the index from both ends so out-of-range still yields None rather than raising. --- src/specify_cli/workflows/expressions.py | 9 +++++---- tests/test_workflows.py | 19 +++++++++++++++++++ 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index 5fb036ebf2..3fc770fa09 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -139,7 +139,7 @@ def _filter_from_json(value: Any) -> Any: # against it, and the condition gate below reuses it rather than describing the # same shape a second time, so widening what indexing accepts cannot leave the # evaluator and the gate disagreeing. -_INDEXED_SEGMENT = re.compile(r"^([\w-]+)\[(\d+)\]$") +_INDEXED_SEGMENT = re.compile(r"^([\w-]+)\[(-?\d+)\]$") _PLAIN_SEGMENT = re.compile(r"^[\w-]+$") @@ -147,12 +147,13 @@ def _filter_from_json(value: Any) -> Any: def _resolve_dot_path(obj: Any, path: str) -> Any: """Resolve a dotted path like ``steps.specify.output.file`` against *obj*. - Supports dict key access and list indexing (e.g., ``task_list[0]``). + Supports dict key access and list indexing, including the negative form + Python and Jinja2 both accept (e.g., ``task_list[0]``, ``task_list[-1]``). """ parts = path.split(".") current = obj for part in parts: - # Handle list indexing: name[0] + # Handle list indexing: name[0], name[-1] idx_match = _INDEXED_SEGMENT.match(part) if idx_match: key, idx = idx_match.group(1), int(idx_match.group(2)) @@ -160,7 +161,7 @@ def _resolve_dot_path(obj: Any, path: str) -> Any: current = current.get(key) else: return None - if isinstance(current, list) and 0 <= idx < len(current): + if isinstance(current, list) and -len(current) <= idx < len(current): current = current[idx] else: return None diff --git a/tests/test_workflows.py b/tests/test_workflows.py index d5869887fb..6918e2ce98 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -1013,6 +1013,25 @@ def test_list_indexing(self): result = evaluate_expression("{{ steps.tasks.output.task_list[0].file }}", ctx) assert result == "a.md" + def test_negative_list_indexing(self): + """``list[-1]`` resolves from the end, as Python and Jinja2 both do. + + Without it the index silently fell through to a dict lookup for the + literal key ``"task_list[-1]"`` and produced ``None``, so a template + reaching for the last element rendered empty with no error. + """ + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext( + steps={"tasks": {"output": {"task_list": [{"file": "a.md"}, {"file": "b.md"}]}}} + ) + assert evaluate_expression("{{ steps.tasks.output.task_list[-1].file }}", ctx) == "b.md" + assert evaluate_expression("{{ steps.tasks.output.task_list[-2].file }}", ctx) == "a.md" + # Out of range in either direction stays None rather than raising. + assert evaluate_expression("{{ steps.tasks.output.task_list[-3] }}", ctx) is None + assert evaluate_expression("{{ steps.tasks.output.task_list[2] }}", ctx) is None + def test_context_run_id_resolves(self): """``{{ context.run_id }}`` resolves to ``StepContext.run_id``. From 20edc91639365b90cadf99f7a2b873d87555ec9f Mon Sep 17 00:00:00 2001 From: NgoQuocViet2001 Date: Tue, 29 Sep 2026 14:44:49 +0700 Subject: [PATCH 2/2] test(workflows): keep the shared-index gate test on a shape the regex still rejects --- tests/unit/test_condition_expression_block.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/unit/test_condition_expression_block.py b/tests/unit/test_condition_expression_block.py index 528798d5e2..1f7b43ed0e 100644 --- a/tests/unit/test_condition_expression_block.py +++ b/tests/unit/test_condition_expression_block.py @@ -946,16 +946,17 @@ def test_a_literal_never_reaches_the_resolver(): def test_gate_reads_the_shared_indexed_segment_definition(monkeypatch): """Widening `_INDEXED_SEGMENT` alone must reach the gate. - `steps.…​.task_list[-1]` is rejected today because `_INDEXED_SEGMENT` — the - one place `_resolve_dot_path` says what an index looks like — accepts digits - only. Widening it there and nowhere else must be enough; if the gate keeps - its own copy of the shape (as `_PATH_SEGMENT` used to), this fails. + `steps.…​.task_list[+1]` is rejected today because `_INDEXED_SEGMENT` — the + one place `_resolve_dot_path` says what an index looks like — accepts an + optional minus sign and digits only. Widening it there and nowhere else must + be enough; if the gate keeps its own copy of the shape (as `_PATH_SEGMENT` + used to), this fails. """ - path = "steps.tasks.output.task_list[-1].file" + path = "steps.tasks.output.task_list[+1].file" assert expressions._unresolvable_term(path) is not None monkeypatch.setattr( - expressions, "_INDEXED_SEGMENT", re.compile(r"^([\w-]+)\[(-?\d+)\]$") + expressions, "_INDEXED_SEGMENT", re.compile(r"^([\w-]+)\[([+-]?\d+)\]$") ) assert expressions._unresolvable_term(path) is None