Skip to content

arazzo-executor: recover criterion failures and retain partial reports - #284

Merged
SVilgelm merged 1 commit into
mainfrom
fix/arazzo-criterion-recovery
Sep 8, 2026
Merged

arazzo-executor: recover criterion failures and retain partial reports#284
SVilgelm merged 1 commit into
mainfrom
fix/arazzo-criterion-recovery

Conversation

@SVilgelm

@SVilgelm SVilgelm commented Sep 7, 2026

Copy link
Copy Markdown
Member

Follow up #282 with Stage 2: criterion recovery and report-preserving execution APIs.

Runtime criterion evaluation errors now produce failed conditions with their original typed diagnostics in CriterionOutcome.error. Success criteria remain ordered, while action criteria stop at the first failure and allow later eligible actions to match. StepRecord.action_criteria records the actions actually considered, and text reports include their diagnostics. Missing data stays distinct from explicit null; null regex/JSONPath contexts fail normally, while a JSONPath node containing null selected from a non-null document still counts as a match.

Unsupported XPath/AsyncAPI capabilities remain terminal, as do operation, parameter, output, action-reference, client, and limit failures. Preparation-time syntax checks and the Stage 1 dependency-ordering policies remain unchanged. The same runtime recovery policy applies after v1.0 upconversion.

Add Run::partial_report(), ExecutionFailure, and the execute_with_report, execute_async_with_report, and execute_v1_0_with_report functions. Interrupted reports use Outcome::Incomplete, never success; preparation failures have no report. Retain earlier attempts and actual responses/completed workflow calls before fallible output evaluation or action dispatch. Do not fabricate completed records for unsent requests or requests that never received a response. Terminal engine errors stop further progress; Awaiting and NotWaiting remain correctable driver errors, and completed reports remain inspectable.

Existing execution function signatures are unchanged. Callers that previously expected runtime criterion errors in Err must now inspect the report's outcome and diagnostic fields; successful recovery can leave a successful run containing earlier failed attempts. The new report fields and non-exhaustive enum variants are additive API changes. No dependencies are added or upgraded.

crates/roas-arazzo-executor/tests/recovery_test.rs includes a step with "condition": "$response.body.ready" and an onFailure retry, driven by {} then {"ready":true} responses. It also covers runtime-generated invalid patterns ("condition": "{$inputs.pattern}" with pattern: "["), action fallthrough, nested workflow recovery, null versus missing data, and retained history after terminal errors. Three Stage 1 grammar cases now assert the same typed diagnostics in failed criterion outcomes instead of expecting a terminal error.

The behavior follows the Arazzo 1.1 evaluation-error rules. Full preparation, external source resolution, and later stages are not included.

Assisted-by: Codex
Signed-off-by: Sergey Vilgelm <sergey@vilgelm.com>
Copilot AI lite review requested due to automatic review settings September 7, 2026 23:54

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@SVilgelm

SVilgelm commented Sep 8, 2026

Copy link
Copy Markdown
Member Author

Reviewed at 4628f1b (based on f533aa6; main has since moved to e35761a). cargo fmt --all --check, cargo clippy --workspace --all-features --all-targets -- -D warnings, cargo test -p roas-arazzo-executor --all-features --doc and cargo nextest run --workspace --all-features (3282 tests) are green. Crate coverage is 98.41% lines / 96.35% regions, with run.rs at 99.26%, lib.rs 98.70% and report.rs 95.98%.

No confirmed defects. The API surface is handled carefully — every report type is already #[non_exhaustive], so the new CriterionOutcome.error, StepRecord.action_criteria and Outcome::Incomplete are additive; execute / execute_async / execute_v1_0 keep their signatures by delegating; and is_success() moving from != Failed to matches!(Succeeded | Ended) is identical for every pre-existing variant.

I re-derived the README's claims rather than reading them, and they hold: XPath and $message stay terminal with an Incomplete partial report that retains the attempt and its diagnostic; a terminal output error keeps the real response record (passed=false, Performed::Request { status: 200 }); repeated advance returns the same completed report; supply after a terminal error gives Stopped while partial_report() keeps the attempt; a null regex or JSONPath context fails without a missing-value diagnostic; and $.a selecting a null node out of {"a": null} passes while $.b fails.

Possible Risks

1. Static document defects are now recoverable and can end in a successful run — Medium

run.rs:1335-1362 treats only the Unsupported variants as terminal, so everything else becomes a false condition — including two things that cannot become valid at runtime.

An undeclared step id, with a recovery action that reaches a passing step:

{ "stepId": "a", "operationId": "check",
  "successCriteria": [{ "condition": "$steps.typo.outputs.value" }],
  "onFailure": [{ "name": "skip", "type": "goto", "stepId": "b" }] }
outcome=Succeeded is_success=true
  step a passed=false errs=["`$steps.typo.outputs.value` names step `typo`, which this workflow has not got"]
  step b passed=true errs=[]

And a typed criterion missing its required context (criterion.rs:91) — {"condition": "ok", "type": "regex"} with no context — evaluates false carrying "a regexcriterion needs acontext", recoverable the same way.

Runtime data errors belong in the recovery path: a missing body field or an interpolated pattern that will not compile are exactly what onFailure is for, and the PR is right about those. These two are different — both are decidable before the first request, and #282 deliberately made the first of them loud (short_circuiting_does_not_hide_undeclared_steps_or_workflows asserted a terminal error; it now asserts a report field).

The consumer impact is concrete: roas-cli arazzo run exits on report.is_success() (crates/roas-cli/src/arazzo.rs:507), so a workflow with a typo'd step id in a criterion exits 0, with the diagnostic only on stderr.

Fix: hoist both checks into ordered_steps, which already walks every criterion through criterion::references and has the workflow's declared step ids and the description's workflow ids in hand. Runtime recovery then covers only what actually depends on runtime values, and Stage 1's guarantee survives Stage 2.

2. ExecutionError::Stopped covers two conditions and its message fits only one — Low

run.rs:412 and run.rs:491 return it after a terminal error, which is exactly what the message says (report.rs:306, "the run already stopped after an engine error"). run.rs:441 returns the same error when the frame stack empties with no report — a path that previously returned Progress::Done(self.finish()). I could not construct a document that reaches it (the root leave() either sets self.report or enters the next queued workflow), so it reads as defensive; but if it ever is reached the caller is told an engine error stopped the run when none did, and nothing covers the path. Worth either a debug_assert! plus a note that it is unreachable, or a test if it is not.

Nice-to-Have Improvements

3. Two unreachable arms in the terminal-error filter — Low

run.rs:1351-1355 matches CriterionError::Select(SelectError::Unsupported(_)) and Select(SelectError::Expression(ExpressionError::Unsupported(_))). criterion::passes produces CriterionError::Select only through selects()select::apply, which returns SelectError::Malformed and nothing else; SelectError::Unsupported comes from kind_of, reached only by select::select on a ValueOrSelector::Selector, which no criterion path uses. Harmless, but they imply a route that does not exist and account for part of the uncovered regions in that function.

4. roas-cli arazzo run does not use the new API — Low

crates/roas-cli/src/arazzo.rs:487 still calls execute, so a terminal engine error there discards the partial report this PR adds. Switching to execute_with_report and printing failure.report next to failure.error would give the command its history for free — it already prints the report at :501, and that output now carries criterion diagnostics. Out of this PR's scope, but the CLI is in the same workspace.

— Reviewed by Claude Opus 5

@SVilgelm
SVilgelm merged commit 8e6b84a into main Sep 8, 2026
126 checks passed
@SVilgelm
SVilgelm deleted the fix/arazzo-criterion-recovery branch September 8, 2026 01:58
SVilgelm added a commit that referenced this pull request Sep 8, 2026
Add IO-free `prepare` and `required_sources` APIs and an immutable,
reusable
`PreparedWorkflow`. Preparation aggregates deterministic diagnostics
with field
paths, workflow/step context and byte offsets before a checked run can
send a
request. It composes structural validation with static
expression/reference,
capability, effective-parameter, operation and dependency checks across
the
selected workflow's potential calls and recovery branches.

Prepared runs reuse condition/runtime-expression syntax, interpolation
templates,
constant regex/JSONPath programs, endpoints and ordering while keeping
inputs,
attempts and outputs independent. A named condition profile and optional
portability warnings make implementation policy explicit. Retained token
offsets
keep diagnostics accurate with quoted expression lookalikes and cached
ASTs.

The CLI now prepares before execution and prints available partial
history on
terminal runtime errors unless quiet. Source discovery avoids loading
unrelated
documents for qualified operations while preserving bare operation-ID
uniqueness
checks. Existing validation-ignore options remain honored.

This intentionally makes the CLI stricter: a criterion such as
`{"condition":"true || $steps.typo.outputs.value"}`, a typed criterion
without
its required context, or a constant malformed regex fails before
requests,
including on a potential recovery branch. Runtime-dependent criterion
failures
retain the recovery/reporting behavior from #284. Existing `execute`,
`execute_async`, `execute_v1_0` and `Run::start` signatures and lazy
validation
remain unchanged; strict preparation is opt-in for library callers.

The unreleased 0.2 API also adds `CriterionError::Syntax::offset` and
removes the
location prefix from that variant's `message`. Downstream code
constructing or
destructuring all fields must adapt; consumers should read the typed
byte offset
instead of parsing display text. The README migration notes cover this
breaking
change. Preparation uses the runtime's typed missing-reference errors,
displays
syntax offsets once, and sorts bracketed diagnostic indices numerically.
Cached
condition validation remains per use site, including shared reusable
actions.

XPath, AsyncAPI, external workflow calls, non-RFC9535 JSONPath and
nested workflow
calls that require their own dependency scheduling are explicit
checked-path
capability errors. Input schemas remain available for caller validation;
fetching,
cross-document resolution and JSON Schema execution stay in later
stages.

---------

Signed-off-by: Sergey Vilgelm <sergey@vilgelm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants