max visits rejects variable reference

[handcaught]

max_visits_rejects_variable_reference

Repository: criteria

Confirmed behavior

On both refs under test (origin/main at 0829f887968d42d8b5e4d53685c6f48086390caf and stable v0.5.5 at 6ac528757d2615d64b4b5f78949a12bdae1870fa), the compiler rejects a step attribute of the form max_visits = var.<name> even when the variable has a known, immutable default.

Measured behavior:

• criteria compile exit code: 1.
• Diagnostics emitted (verbatim from run-main.md and run-stable.md, Shape A):

  Error: <file>:20,16: Variables not allowed
    Variables may not be used here.
  Error: <file>:20,16: Unsuitable value type
    Unsuitable value: value must be known
  

• The same rejection occurs for max_visits = local.<name> (Shape C), string-typed variables (Shape E), fractional variables (Shape F), variables without defaults (Shape G), negative variable values (Shape H), and runtime-only references such as steps.first.done, each.value, data.store.limit, and while.index (runtime shapes).
• Workflow-level numeric policy attributes decoded as primitive struct fields (max_total_steps, max_step_retries, max_visits_warn_threshold) are rejected for var.* in the same way (Shape J). This is adjacent context for a possible separate workstream; it is not part of the required fix for this report.

Already correct — do not regress these:

• max_visits = 3 compiles cleanly and the resulting FSM JSON contains the resolved integer (run-main.md, run-stable.md, Shape I, exit 0).
• max_visits = -1 reaches the semantic validator and emits step "review_plan": max_visits must be >= 0 (Shape I-negative, exit 1).
• Omitting max_visits compiles cleanly with the default unlimited behavior.
• parallel_max = var.<name> compiles cleanly (Shape K, exit 0), confirming that a sibling numeric step attribute already uses a variable-accepting decode path.
• Literal numeric policy attributes compile cleanly and produce the expected values in the JSON output (Shape J-literal, exit 0).

Root cause identified from evidence: StepSpec.MaxVisits is declared as a primitive int struct field with an hcl:"max_visits,optional" tag, so gohcl decodes it directly and forbids variable traversals. The sibling attribute parallel_max is read from sp.Remain and evaluated through decodeIntAttr → FoldExpr, which is why it accepts var.* and local.*. The same primitive-field problem applies to PolicySpec numeric fields.

Required behavior

1. max_visits on a step block must accept a compile-time-resolvable expression, specifically:
   • a literal integer,
   • a reference to a declared variable with a known default or a value supplied via --var,
   • a reference to a declared local whose value is a known integer.
2. max_visits must continue to reject runtime-only references (steps.first.done, each.value, data.store.limit, while.index) with a clear diagnostic that says the attribute requires a compile-time value.
3. max_visits must continue to validate that the resolved value is a whole number and >= 0. Negative values must produce the existing max_visits must be >= 0 diagnostic; fractional values (e.g., a variable whose default is 3.5) must produce a type/whole-number diagnostic.
4. The duplicate generic diagnostics Variables not allowed and Unsuitable value type for max_visits must no longer appear when the expression is a valid compile-time value.
5. parallel_max = var.<name> must continue to compile cleanly and support --var overrides (regression guard).
6. A regression test must be added in the workflow/ package that compiles a workflow with max_visits = var.max_refine (and optionally local.max_refine) and asserts:
   • no diagnostics,
   • the compiled StepNode.MaxVisits equals the expected integer,
   • the FSM JSON round-trips with the same value.

Exit criteria

• criteria compile exits 0 on a workflow containing max_visits = var.max_refine where max_refine has a known default.
• criteria plan --var max_refine=7 reflects the override value in the plan output.
• criteria compile exits 1 with a clear, single diagnostic for max_visits = var.max_refine when the variable has no default and no --var override is supplied.
• criteria compile exits 1 with a type/whole-number diagnostic for a variable whose default is 3.5 used as max_visits.
• criteria compile exits 1 with max_visits must be >= 0 for max_visits = -1 (or a variable whose default is -1).
• criteria compile exits 1 with a clear diagnostic for runtime-only references such as max_visits = each.value.
• The regression test described in requirement 6 fails against the current build and passes after the fix.
• make test passes after the change.

[brokenbot]

Approved. max_visits now decodes via the same compile-time fold path as parallel_max (decodeMaxVisits → decodeIntAttr → FoldExpr), accepting literal/var./local. and rejecting runtime-only, fractional, and negative values with clear single diagnostics. All three step compile paths wired through; StepNode.MaxVisits and engine enforcement unchanged. New regression test covers var ref, local ref, missing default, fractional, negative, runtime-only, literal, and JSON round-trip. Full workflow tests pass; all CI green. Meets all workstream exit criteria.