Skip to content

jq: two escape-stashing sinks answer Demand::Stop without mark_nonretryable_escape (?// retry contract) #2924

Description

@newhoggy

Severity: Low (latent — no live repro found; surfaced by a /code-review pass on PR #2922)

Summary

mark_nonretryable_escape's own contract, stated on is_retryable_stop, is that a driver
which ends a drive because something escaped — a decode failure, a Halt from the stage it
piped into — must stash that escape and mark the stop non-retryable, because everything
between it and a ?// alternative sees only Flow::Stopped { pending: None } and would
otherwise re-run the alternative.

Two escape-stashing sinks in the path resolvers answer Demand::Stop without that call:

Both park an EvalEscape beside the drive and return Demand::Stop, which is exactly the
escape/downstream/ended idiom the contract is written for.

Repro

None found. A review pass that fuzzed ~9,000 shapes (including ?// retries, halt_error in
key/bound streams, and label/break interaction) produced no observable divergence, so this is
reported as a contract violation rather than a confirmed bug. Whether it is reachable at all
depends on whether a ?// alternative can sit above these resolvers with a Halt or decode
failure in the key/bound generator — that is the first thing to establish.

Root cause

src/jq/eval.rs — the two escape! macros expand to target_escape = Some($control); return Demand::Stop; with no mark_nonretryable_escape. stop_with_escape/stop_with_downstream/
stop_with_escape_cell are the helpers that do it correctly; neither resolver routes through
them, because their slot is an Option<EvalEscape> rather than the Option<Control> those
helpers take.

Why this is separate from #2267

#2267 is about ordering — which generator is pulled when. This is about the retry
classification of a stop, is pre-existing on one of the two sites, and needs its own oracle
work (a ?//-over-a-failing-resolver capture from jq 1.7.1) before anything is changed.
Fixing it speculatively inside an ordering PR would change ?// retry semantics with no
repro to pin it.

Suggested fix direction

Either give the two resolvers an EvalEscape-shaped stop_with_escape sibling (one
definition, so the two cannot drift — the #106 lesson these resolvers have already been
bitten by twice), or convert their slots to Option<Control> and use the existing helper.
Then pin with a ?// alternative over a halt_error in a computed key and in a slice
bound, captured live from jq 1.7.1 first.

Refs #2267, #2920, #2922, #1519.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    SonnetSuitable for a Sonnet-class model to implement

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions