From 6e24b85bc343892abbad37d2eee9a5a162a32109 Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Mon, 29 Jun 2026 19:19:16 +0500 Subject: [PATCH 1/5] fix: interpolate multi-expression templates instead of returning None (#3208) `evaluate_expression` returned None for templates containing two or more `{{ }}` blocks with no surrounding literal text, e.g. `"{{ context.run_id }} {{ inputs.issue }}"`. The single-expression fast path used `_EXPR_PATTERN.fullmatch()`, but `fullmatch` defeats the pattern's non-greedy `(.+?)` body: for two adjacent expressions it still matches, capturing everything between the first `{{` and the last `}}` (`"context.run_id }} {{ inputs.issue"`) as the body. That garbage failed dot-path resolution and returned None directly, bypassing the `sub()` interpolation path that would have resolved each expression. Downstream this surfaced as the literal string "None" reaching commands. Guard the fast path on `stripped.count("{{") == 1` so only genuine single-expression templates take the typed return; multi-expression templates fall through to `sub()` and interpolate correctly. Add regression tests for two expressions separated by a space and for adjacent expressions with no separator. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/workflows/expressions.py | 19 +++++++++++++++---- tests/test_workflows.py | 21 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index b7ed17e801..d6f3ea21bc 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -383,10 +383,21 @@ def evaluate_expression(template: str, context: Any) -> Any: namespace = _build_namespace(context) - # Single expression: return typed value - match = _EXPR_PATTERN.fullmatch(template.strip()) - if match: - return _evaluate_simple_expression(match.group(1).strip(), namespace) + # Single expression: return typed value (preserving type). + # + # Guard the fast path on there being exactly one ``{{`` block. The pattern + # uses a non-greedy body ``(.+?)``, but ``fullmatch`` defeats non-greediness: + # for ``"{{ a }} {{ b }}"`` it still matches, expanding the body to capture + # everything between the first ``{{`` and the last ``}}`` (``"a }} {{ b"``). + # That garbage body fails resolution and returns ``None``, bypassing the + # ``sub()`` interpolation path that would handle each expression correctly. + # Only take the typed fast path for genuine single-expression templates + # (issue #3208). + stripped = template.strip() + if stripped.count("{{") == 1: + match = _EXPR_PATTERN.fullmatch(stripped) + if match: + return _evaluate_simple_expression(match.group(1).strip(), namespace) # Multi-expression: string interpolation def _replacer(m: re.Match[str]) -> str: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 988730d783..5854014a38 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -226,6 +226,27 @@ def test_string_interpolation(self): result = evaluate_expression("Feature: {{ inputs.name }} done", ctx) assert result == "Feature: login done" + def test_multi_expression_no_surrounding_text(self): + """Two adjacent expressions with no literal text must interpolate each, + not collapse to None via the fullmatch fast path (#3208).""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"issue": "23"}, run_id="47c5eb4b") + result = evaluate_expression( + "{{ context.run_id }} {{ inputs.issue }}", ctx + ) + assert result == "47c5eb4b 23" + + def test_multi_expression_adjacent_no_separator(self): + """Back-to-back expressions with no separator still interpolate (#3208).""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"a": "foo", "b": "bar"}) + result = evaluate_expression("{{ inputs.a }}{{ inputs.b }}", ctx) + assert result == "foobar" + def test_comparison_equals(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext From 2d726480415011454e6a702272b7ddb2f77e930f Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Tue, 30 Jun 2026 00:14:27 +0500 Subject: [PATCH 2/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/test_workflows.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 5854014a38..587b5e9af5 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -227,8 +227,8 @@ def test_string_interpolation(self): assert result == "Feature: login done" def test_multi_expression_no_surrounding_text(self): - """Two adjacent expressions with no literal text must interpolate each, - not collapse to None via the fullmatch fast path (#3208).""" +"""Two expressions with no surrounding literal text must interpolate each, +not collapse to None via the fullmatch fast path (#3208).""" from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext From a66fa43c744cf919670be8a8962a314e5c63c084 Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Tue, 30 Jun 2026 17:27:43 +0500 Subject: [PATCH 3/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/test_workflows.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 587b5e9af5..415fd7bf2d 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -227,8 +227,8 @@ def test_string_interpolation(self): assert result == "Feature: login done" def test_multi_expression_no_surrounding_text(self): -"""Two expressions with no surrounding literal text must interpolate each, -not collapse to None via the fullmatch fast path (#3208).""" + """Two expressions with no surrounding literal text must interpolate each, + not collapse to None via the fullmatch fast path (#3208).""" from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext From 53c899a8c8a1219ce13ba299244a9d5d1c20ddde Mon Sep 17 00:00:00 2001 From: Noor-ul-ain001 Date: Tue, 30 Jun 2026 20:24:29 +0500 Subject: [PATCH 4/5] fix(expressions): use match-span guard so single expressions with literal {{ keep their type The previous `stripped.count("{{") == 1` guard misclassified a genuine single expression whose string argument contains a literal `{{` (e.g. `{{ inputs.text | contains('{{') }}`) as multi-expression, routing it through `sub()` interpolation and coercing the typed (bool/int/list) return value to a string -- breaking the type-preservation the docstring promises (Copilot review on #3228). Anchor a single match at the start and require it to consume the whole stripped string instead. The non-greedy body stops at the first `}}`, so a two-block template fails the span check (falls through to interpolation, fixing #3208) while a lone expression -- including one with a `{{` inside a string literal -- matches to the end and keeps its typed value. Add a regression test for the literal-brace single-expression case. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/workflows/expressions.py | 29 ++++++++++++++---------- tests/test_workflows.py | 11 +++++++++ 2 files changed, 28 insertions(+), 12 deletions(-) diff --git a/src/specify_cli/workflows/expressions.py b/src/specify_cli/workflows/expressions.py index d6f3ea21bc..b3e1de8016 100644 --- a/src/specify_cli/workflows/expressions.py +++ b/src/specify_cli/workflows/expressions.py @@ -385,19 +385,24 @@ def evaluate_expression(template: str, context: Any) -> Any: # Single expression: return typed value (preserving type). # - # Guard the fast path on there being exactly one ``{{`` block. The pattern - # uses a non-greedy body ``(.+?)``, but ``fullmatch`` defeats non-greediness: - # for ``"{{ a }} {{ b }}"`` it still matches, expanding the body to capture - # everything between the first ``{{`` and the last ``}}`` (``"a }} {{ b"``). - # That garbage body fails resolution and returns ``None``, bypassing the - # ``sub()`` interpolation path that would handle each expression correctly. - # Only take the typed fast path for genuine single-expression templates - # (issue #3208). + # The fast path must fire only when the whole template is one ``{{ ... }}`` + # block. ``fullmatch`` cannot decide this: the pattern's non-greedy body + # ``(.+?)`` is defeated by ``fullmatch``, so ``"{{ a }} {{ b }}"`` still + # matches with the body expanded to ``"a }} {{ b"`` -- garbage that fails + # resolution and returns ``None``, bypassing the ``sub()`` interpolation + # path that handles each expression correctly (issue #3208). + # + # Anchor a single match at the start instead and require it to consume the + # entire stripped string. The non-greedy body then stops at the first + # ``}}``: a genuine two-block template leaves a trailing ``}}`` and fails the + # span check, while a lone expression -- even one with a literal ``{{`` in a + # string argument such as ``{{ inputs.text | contains('{{') }}`` -- matches + # to the end and keeps its typed return value. (Counting ``{{`` would + # misclassify that expression as multi-block and coerce it to ``str``.) stripped = template.strip() - if stripped.count("{{") == 1: - match = _EXPR_PATTERN.fullmatch(stripped) - if match: - return _evaluate_simple_expression(match.group(1).strip(), namespace) + match = _EXPR_PATTERN.match(stripped) + if match and match.end() == len(stripped): + return _evaluate_simple_expression(match.group(1).strip(), namespace) # Multi-expression: string interpolation def _replacer(m: re.Match[str]) -> str: diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 415fd7bf2d..03cd456533 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -247,6 +247,17 @@ def test_multi_expression_adjacent_no_separator(self): result = evaluate_expression("{{ inputs.a }}{{ inputs.b }}", ctx) assert result == "foobar" + def test_single_expression_with_literal_braces_preserves_type(self): + """A lone expression whose string argument contains a literal ``{{`` + must still take the typed fast path and return a bool, not a string + (the fix for #3208 must not coerce it to ``"True"``).""" + from specify_cli.workflows.expressions import evaluate_expression + from specify_cli.workflows.base import StepContext + + ctx = StepContext(inputs={"text": "uses {{ jinja }} syntax"}) + result = evaluate_expression("{{ inputs.text | contains('{{') }}", ctx) + assert result is True + def test_comparison_equals(self): from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext From e3c69cae66bb6ce57b2d5bf8ed4235c6bff6b58b Mon Sep 17 00:00:00 2001 From: Noor ul ain Date: Tue, 30 Jun 2026 21:49:39 +0500 Subject: [PATCH 5/5] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- tests/test_workflows.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/tests/test_workflows.py b/tests/test_workflows.py index 03cd456533..8e6393b963 100644 --- a/tests/test_workflows.py +++ b/tests/test_workflows.py @@ -248,15 +248,17 @@ def test_multi_expression_adjacent_no_separator(self): assert result == "foobar" def test_single_expression_with_literal_braces_preserves_type(self): - """A lone expression whose string argument contains a literal ``{{`` + """A lone expression whose string argument contains a literal ``{{`` or ``}}`` must still take the typed fast path and return a bool, not a string - (the fix for #3208 must not coerce it to ``"True"``).""" + (the fix for #3208 must not coerce it to ``\"True\"``).""" from specify_cli.workflows.expressions import evaluate_expression from specify_cli.workflows.base import StepContext ctx = StepContext(inputs={"text": "uses {{ jinja }} syntax"}) - result = evaluate_expression("{{ inputs.text | contains('{{') }}", ctx) - assert result is True + assert evaluate_expression("{{ inputs.text | contains('{{') }}", ctx) is True + + ctx = StepContext(inputs={"text": "uses }} syntax"}) + assert evaluate_expression("{{ inputs.text | contains('}}') }}", ctx) is True def test_comparison_equals(self): from specify_cli.workflows.expressions import evaluate_expression