fix(workflows): reject a condition that has no {{ }} block

[ntdatt812]

Defect

evaluate_condition resolves its argument through evaluate_expression, which only substitutes {{ ... }} blocks. A string with no such block comes back unchanged, and — unless it reads true/false — is then coerced by bool(). So a condition authored without the braces is never evaluated at all.

With inputs.count == 5:

evaluate_condition("{{ inputs.count > 100 }}", ctx)   # False  — correct
evaluate_condition("inputs.count > 100", ctx)         # True   — never evaluated
evaluate_condition("inputs.name == 'zzz'", ctx)       # True
evaluate_condition("inputs.count < 3", ctx)           # True

Every brace-less condition is true, whatever it says. An if step always takes then; a while/do-while step never terminates on its condition and runs to max_iterations — ten agent invocations for a loop the author expected to stop after one.

Nothing reports it. The workflow validates, runs, and takes the wrong branch silently.

Why this is worth a validation error

The three step validators already reject a list/dict/number condition, and the comment there states the reason exactly:

a list/dict/number condition silently resolves to its truthiness (e.g. condition: [1, 2] is always True) with no error, branching wrongly on an authoring mistake. Reject those at validation […]

A brace-less string is the same failure mode, and a likelier mistake: GitHub Actions accepts a bare expression in if: (if: github.event_name == 'push'), so an author arriving from Actions writes the brace-less form by habit — and unlike [1, 2], condition: inputs.count > 100 looks completely correct on the page.

Fix

condition_is_never_evaluated() in expressions.py, wired into the if, while and do-while validators. The error names the problem and hands back the corrected form:

If step 's1': 'condition' 'inputs.count > 100' has no '{{ }}' block, so it is
never evaluated and is always true. Wrap the expression: "{{ inputs.count > 100 }}".

Validation only — no runtime behaviour changes. Still valid, and covered by tests: "{{ ... }}" in any position, "true"/"false" in any case, real bools, empty and whitespace strings, and every non-string type (already handled by the branch above).

Tests

New tests/unit/test_condition_expression_block.py, 40 cases:

• the runtime behaviour is pinned first — same expression with and without braces, asserting False vs True — so the defect stays documented even if the validator changes
• each of the three step validators rejects the brace-less form and echoes the corrected expression
• no false positives, parametrised across all three step types
• the helper itself across strings, bools, containers and numbers

tests/unit + tests/test_workflows.py

before:  22 failed, 1083 passed, 9 skipped
after:   22 failed, 1123 passed, 9 skipped

The same 22 pre-existing failures in both runs — all symlink_to on Windows without the privilege (OSError: [WinError 1314]), unrelated to this change. Everything added is the +40 new tests.

[Copilot]

Pull request overview

Adds validation to reject brace-less workflow conditions that would otherwise always evaluate as true.

Changes:

• Adds a shared condition-validation helper.
• Integrates validation into if, while, and do-while steps.
• Adds runtime and validator regression tests.

Show a summary per file

File	Description
src/specify_cli/workflows/expressions.py	Adds condition detection helper.
src/specify_cli/workflows/steps/if_then/__init__.py	Validates if conditions.
src/specify_cli/workflows/steps/while_loop/__init__.py	Validates while conditions.
src/specify_cli/workflows/steps/do_while/__init__.py	Validates do-while conditions.
tests/unit/test_condition_expression_block.py	Adds regression coverage.

Review details

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

• Files reviewed: 5/5 changed files
• Comments generated: 4
• Review effort level: Balanced

[ntdatt812]

Both review points were correct — thanks. Fixed in c0c291c.

1. Unterminated / reversed delimiters slipped through

Confirmed against the interpolator rather than assumed. _interpolate_expressions() takes its raw_close == -1 branch and appends the tail verbatim, so nothing is substituted:

condition	evaluate_condition	old helper	new helper
inputs.count > 100	True	flags	flags
{{ inputs.count > 100	True	misses	flags
}} inputs.count > 100 {{	True	misses	flags
{{ inputs.count > 100 }}	False	—	—

The helper now requires a complete block: it finds the first {{ and checks a }} follows it. {{ a }} {{ b }} and {{ inputs.text | default('}}') }} stay unflagged.

2. The correction was not valid YAML

Reproduced exactly as described — condition: "{{ inputs.name == "zzz" }}" raises yaml.parser.ParserError. A remediation the author cannot paste is not a remediation.

Added format_condition_correction() in expressions.py, used by all three validators as suggested. It picks quoting from the content — double by default, single when the expression holds a double quote, double with backslash escapes when it holds both — and drops a stray delimiter instead of nesting a second one, so {{ inputs.count > 100 corrects to "{{ inputs.count > 100 }}" rather than "{{ {{ ... }} }}".

condition	emitted correction
inputs.name == "zzz"	'{{ inputs.name == "zzz" }}'
inputs.name == 'zzz'	"{{ inputs.name == 'zzz' }}"
inputs.a == "x" and inputs.b == 'y'	"{{ inputs.a == \"x\" and inputs.b == 'y' }}"
inputs.path == 'C:\tmp'	"{{ inputs.path == 'C:\tmp' }}"

Tests — the file goes from 40 to 70 cases. The new ones cover the incomplete forms, and run every correction through yaml.safe_load asserting it loads back to exactly {{ <expr> }} and that the corrected form is not itself re-flagged — the invariant that would have caught both of these.

uvx ruff@0.15.0 check src tests clean; pytest tests/unit 250 passed (the 2 symlink failures are a local Windows privilege limitation, green on CI).

[Copilot]

Review details

• Files reviewed: 5/5 changed files
• Comments generated: 2
• Review effort level: Balanced

[mnriem]

Please address Copilot feedback

[ntdatt812]

@mnriem — both follow-up findings addressed in dc3c7de. Reproduced each one first rather than taking them on trust; both were real.

1. The }} check was not quote-aware

Confirmed exactly as described:

condition	evaluate_condition	validator, before	after
{{ inputs.x == '}}'	True	misses it	flags
{{ inputs.x == '}}' }}	False	—	not flagged
{{ inputs.text | default('}}') }}	—	not flagged	not flagged

_interpolate_expressions finds no quote-aware close, falls to its raw-close branch, evaluates a truncated body and leaves residual text (False') — non-empty, so bool() makes it true.

Rather than restate the quote rules a third time in this module, I lifted the scan out of _interpolate_expressions into _find_block_close() and had the validator call it. The check and the substitution it is predicting now share one implementation and cannot drift — which is the same failure mode this PR is about. _interpolate_expressions keeps its exact previous behaviour; the extraction is behaviour-preserving.

2. Hand-rolled quoting missed control characters

Also reproduced — a condition carrying a newline did not round-trip:

condition	before	after
inputs.x == 1\nand inputs.name == 'abc'	newline folded, round-trip fails	exact
he said "hi"\nthen left	ParserError	exact
inputs.a == 'x\ry'	round-trip fails	exact

Switched to json.dumps(..., ensure_ascii=False) as suggested — json was already imported, every JSON string is a valid YAML double-quoted scalar, and it removes the escape enumeration entirely. ensure_ascii=False keeps a non-ASCII operand readable instead of expanding it into \uXXXX; there is a test for that.

Tests 70 → 83. The quoted-delimiter condition joins the incomplete-block set; the round-trip set gains multiline, newline-with-quote, tab, carriage return and a non-ASCII operand. All four new cases fail on the previous commit (4 failed, 79 passed) and pass here (83 passed) — they are not vacuous.

Verification on this head: uvx ruff@0.15.0 check src tests — All checks passed. pytest tests/unit — 263 passed. pytest tests/workflows — 179 passed. The handful of local failures in both directories are the Windows symlink-privilege cases, identical on unpatched main; the CI matrix is the authority there and was green on the previous head.