[Shaun1]

Summary

This PR brings simple Criterion Object evaluation closer to the Arazzo 1.0.1 specification. It makes four changes, one per commit:

1. Compound conditions. &&, || and parentheses now work in simple conditions.
2. Null operands give null operands defined comparison semantics
3. Empty responses. When a request or response value is missing (for example, a body on an HTTP 204), the engine now uses null instead of failing the whole step. A reference to an unknown step, workflow, input or component is still an error.

4. Numeric literals. 0 and 1 are now read as numbers, not booleans.

Motivation

1. Logical operators and grouping

The spec's Operators table lists && (and), || (or) and () (logical grouping) next to the comparison operators. Before this change, evaluateSimpleConditionString split a condition on the first comparison operator it found, so only a single left <op> right comparison worked. A condition like this one, which is common in auth and MFA flows, could not be expressed:

successCriteria:
  - condition: >-
      $statusCode == 204 ||
      ($statusCode == 200 &&
        ($response.body#/challengeName == 'NEW_PASSWORD_REQUIRED' ||
         $response.body#/challengeName == 'SOFTWARE_TOKEN_MFA'))

Simple conditions are now tokenized and parsed with a small recursive-descent parser. || binds more loosely than &&, and both bind more loosely than comparisons. A runtime expression ($...) is read as one operand up to the next whitespace, operator or parenthesis, so a character like > inside a JSON pointer path is not treated as an operator.

2. Null operand handling

Fixes an upstream bug: an explicit JSON null passed ordering comparisons such as > 0. Missing values now follow the same null rules.

Null has no ordering: >, <, >= and <= now return false when either side is
null, == is true only when both sides are null, and != is its inverse. A
null used as a bare operand is falsy, so the rest of the condition still
decides the outcome. A present but non-boolean bare operand is still an
error.

https://spec.openapis.org/arazzo/v1.0.1.html#literals

3. Missing response values on empty responses

Runtime expressions such as $response.body#/id or $response.header.Location can legitimately have no value. A 204 has no body, and an optional header may be missing. Before this change, any such miss aborted the step, even inside an || branch the author meant as a fallback.

Missing request and response values (headers, query and path parameters, JSON-pointer segments, and absent bodies or headers) now resolve to null:

• In criteria: a missing operand becomes null. If a whole &&/|| branch fails only because a value was missing, that branch counts as false, so the rest of the condition still decides the result.
• In step and workflow outputs: the output is set to null instead of failing the step.

This is deliberately narrow. Unknown steps, workflows, inputs, outputs, components and source descriptions are authoring mistakes, so they still raise errors. The existing atomicity guarantees are unchanged (TestEngine_StepOutputsAreNotPartiallyPopulatedOnEvaluationFailure, TestEngine_WorkflowOutputsAreNotPartiallyPopulatedOnEvaluationFailure): any non-optional failure still publishes no partial outputs, and workflow outputs are still committed only after every output has been evaluated.

4. 0 and 1 are numbers

(Fixed upstream)

Out of scope

These spec operators and literals are still not supported in simple conditions and could be follow-ups:

• ! (logical not)
• [] (array index) and . (property dereference) as operators
• null as a literal

Tests

• TestEvaluateCriterion_SimpleCondition_BooleanOperators: nested &&/||/() conditions, including the 204-or-challenge example above.
• TestEvaluateCriterion_NumericLiteralZeroIsNotBoolean: totalCount > 0 passes when the count is positive and fails when it is zero.
• TestEvaluateSimpleOperand_ZeroAndOneAreIntegers: operand parsing for 0 and 1.
• Each commit passes go vet ./arazzo/ and go test ./arazzo/... on its own; go test ./... passes on the final commit.

🤖 Generated with Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (f6e97f4) to head (6e7c78c).

Additional details and impacted files

@@            Coverage Diff             @@
##              main      #694    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files          315       315            
  Lines        39762     39939   +177     
==========================================
+ Hits         39762     39939   +177     

Flag	Coverage Δ
unittests	100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Shaun1]

Not sure if you are accepting external PRs or what the procedure is, but I'm using your lib in an arazzo tool I'm building and want to see if I can get these fixes for issues I ran into merged upstream. This should bring it closer in line with the full arazzo spec.

I see there is already a PR for handling 1/0 properly so I can rebase and modify if that gets pushed.

Thanks for the work you do on this library, love it.

[daveshanley]

Thanks for this Shaun, and thanks for the kind words. Compound criteria have been a gap for a while, so this is very welcome. The parser is clean, the precedence is right, and the null rules are spot on.

A few things need sorting before it can go in. There are two tokenizer regressions (operators in JSON pointers, and UTF-8), a performance hit, and the output change, which I'd like to split out. Details inline.

Also, the description still talks about the 0/1 change and its tests. That fix (#690) already landed on main, so worth a tidy up.

[daveshanley]

This tokenizes and parses the condition on every single evaluation. I benchmarked it against main (alternating runs, n=12):

$statusCode == 200               72ns -> 181ns  (+151%)   2 -> 3 allocs
$response.body#/status == 'ok'  114ns -> 252ns  (+121%)   5 -> 6 allocs
true                              6ns ->  55ns  (+816%)   0 -> 1 allocs

About 76% of that is the tokenizer, mostly slice growth. We can't be dropping performance in libopenapi, so this needs to get back to parity.

The engine already keeps criterionCaches for regex and JSONPath. Compile the condition once into a small tree, cache it there, and evaluation becomes a cheap walk. Short-circuiting && and || falls out of that for free.

[daveshanley]

rune(input[i]) looks at one byte at a time, and unicode.IsSpace counts 0xA0 and 0x85 as whitespace. Those are UTF-8 continuation bytes, so characters like à get cut in half:

$response.body#/voilà == 1   -> invalid UTF-8      (true on main)
$response.body#/s == voilà   -> false, no error    (true on main)

The second one is the nasty one. A silent wrong answer.

Either decode with utf8.DecodeRuneInString, or only treat ASCII whitespace as a separator. Same fix applies to the operand loops further down.

[daveshanley]

This ends a runtime expression at the first operator, paren or space, so any of those inside a JSON pointer breaks the whole condition. Both of these work on main and fail here:

$response.body#/limits/>=minimum != true   -> unexpected token "!="
$response.body#/price(usd) == 5            -> unexpected token "("

The first one is lifted straight from the SplitSimpleCondition tests, and RFC 6901 allows all of these characters in a pointer. Main ends a $ operand at whitespace. I'd keep that rule, and only peel off a trailing ) while a group is open.

Also, this branch and the else below it are identical, so one of them can go.

[daveshanley]

Matching on error message text is going to bite us. Someone rewords an error in expression and the runtime quietly changes behaviour here.

It also misses a few cases that are just as "missing" as the ones listed:

{"items": []}   $response.body#/items/0/id   -> array index 0 out of bounds
{"data": null}  $response.body#/data/id      -> cannot traverse into scalar node
no headers sent $request.header.X            -> no request headers available

The last one is inconsistent. If the step sent any other header, the same expression comes back as null. The engine leaves that map nil when a step sends no headers, so this happens for real.

I'd add a sentinel error in expression (like ErrLocalSymbolNotFound), wrap it with %w everywhere a value isn't there, and check it here with errors.Is.

And because || always evaluates both sides, $statusCode == 200 || $response.body#/items/0/id == 'x' errors on an empty list, even though the left side is true. I'd short-circuit.

[daveshanley]

I'd pull this out of the PR. A missing output now becomes nil and the workflow carries on.

I ran a two-step workflow. create gets a 204 and has output id: $response.body#/id. get uses $steps.create.outputs.id as a path param.

• main: create fails with no response body available. Clear and early.
• this PR: get is sent with userId: nil (which fmt.Sprint renders as "<nil>") and the workflow reports success.

The null rules make sense inside criteria, and the spec backs them there. Outputs are a different story. The spec doesn't ask for this, and failing fast is the safer default. If your tool needs it, put it behind an EngineConfig option.

Same goes for populateWorkflowOutputs below.