fix(jq): carry the fold register into its UPDATE/EXTRACT nested pipes (#3145)

docs/compliance/jq/limitations.md

@@ -1262,12 +1262,16 @@ is the revert that established what the other one costs.
    now reaching nested pipes `... \| $x \| .b \| .c` would have navigated through it too.
    Such a bind is `Origin::Untracked` now (`identity_bind_position`); a marker source keeps
    its own origin, and a `null`/`bool` `.` loses nothing since `jv_identical` admits those by
-   value. The same register does **not** yet reach a fold's UPDATE/EXTRACT body: that route
-   (`FoldRegister::resolve`) passes its register to `resolve_seq` explicitly under a frame that
-   carries none, so `del(foreach .a as $v (.; try ($v \| .b); .))` on `{"a":{"b":1}}` still
-   echoes the document where jq writes `{"a":{}}`, as does `del(.a as $y \| .a \| 5 \| foreach
-   range(1) as $i (0; .; try ($y \| .b)))` — pre-existing, filed as
-   [#3145](https://github.com/rust-works/succinctly/issues/3145). Two refuse-only residuals,
+   value. [#3145](https://github.com/rust-works/succinctly/issues/3145) extends the same
+   carrying to a fold's UPDATE/EXTRACT body, whose own route (`FoldRegister::resolve`) passed
+   its register to `resolve_seq` explicitly under a frame that carried none: `del(foreach .a as
+   $v (.; try ($v \| .b); .))` on `{"a":{"b":1}}` echoed the document and now writes
+   `{"a":{}}`, as jq does. It does not reach a fold whose INIT is untracked *after a literal
+   stage* — that fold re-seeds its register from the ambient value, so the marker is not
+   recognised at all (the pre-existing `literal-then-fold-untracked-init` /
+   `carried-register-passthrough` class), and with a generator source the enclosing `try` then
+   swallows that refusal into a no-op write: `del(.a as $y \| .a \| 5 \| foreach range(1) as $i
+   (0; .; try ($y \| .b)))` is `{"a":{}}` in jq and echoes here. Two refuse-only residuals,
    both in the sweep: `path(.a \| try error(.) catch .)` —
    `error(.)` raises the register node itself and jq answers `["a"]`, but a payload equal to
    the register by value cannot be told from a rebuilt copy (`error({"a":1,"b":2})` refuses in

scripts/jq-bind-origin-fuzz.py

@@ -415,6 +415,11 @@ def main():
     ap.add_argument("-n", type=int, default=500)
     ap.add_argument("--seed", type=int, default=1)
     ap.add_argument("--show", type=int, default=8)
+    ap.add_argument("--fold-p", type=float, default=None,
+                    help="probability a program is a fold (default FOLD_P). A fold-only fix's "
+                         "changed code is reachable only from a fold, so a stock run draws too "
+                         "few to see its rare shapes -- #3145's review found a fabricated write "
+                         "at --fold-p 1.0 that 18,000 stock programs had missed")
     ap.add_argument("--self-test", action="store_true")
     a = ap.parse_args()
     pin = open("tests/data/jq-golden/JQ_VERSION").read().strip()
@@ -430,6 +435,11 @@ def main():
             print(f"{name} ({len(pool)}): " + " ; ".join(pool))
         return 0
     rng = random.Random(a.seed)
+    # `--fold-p 1.0` drives every program through `fold_program`; the other
+    # routes keep their own shares of what is left.
+    fold_p = FOLD_P if a.fold_p is None else a.fold_p
+    route_p = ROUTE_P * (1.0 - fold_p) / max(1.0 - FOLD_P, 1e-9)
+    value_bind_p = VALUE_BIND_P * (1.0 - fold_p) / max(1.0 - FOLD_P, 1e-9)
     kinds = ["agree", "fabricate", "mismatch", "refuse-only", "refuse-early", "both-reject",
              "fabricate-baseline", "mismatch-baseline", "timeout"]
     counts = {k: 0 for k in kinds}
@@ -438,13 +448,13 @@ def main():
         dv = doc(rng)
         d = json.dumps(dv)
         r = rng.random()
-        if r < ROUTE_P:
+        if r < route_p:
             # #3036: the document twice, so `input` reads a second copy.
             f, d = route_program(rng, dv), d + "\n" + d
-        elif r < ROUTE_P + VALUE_BIND_P:
+        elif r < route_p + value_bind_p:
             # #3037: same ROUTES, so `input` needs its second copy too.
             f, d = value_bind_program(rng, dv), d + "\n" + d
-        elif r < ROUTE_P + VALUE_BIND_P + FOLD_P:
+        elif r < route_p + value_bind_p + fold_p:
             f = fold_program(rng)
         else:
             f = program(rng)
@@ -458,7 +468,7 @@ def main():
         counts[c] += 1
         if c != "agree" and len(examples[c]) < a.show:
             examples[c].append((d, f, j, s))
-    print(f"seed={a.seed} n={a.n} " + " ".join(f"{k}={v}" for k, v in counts.items() if v or k in kinds[:4]))
+    print(f"seed={a.seed} n={a.n} fold_p={fold_p:g} " + " ".join(f"{k}={v}" for k, v in counts.items() if v or k in kinds[:4]))
     for c in kinds[1:]:
         for d, f, j, s in examples[c]:
             print(f"[{c}] {f}\n    on {d}\n    jq[{j[0]}]: {j[1].strip()[:120]}\n    sc[{s[0]}]: {s[1].strip()[:120]}")

scripts/jq-bind-origin-oracle-sweep.sh

@@ -454,6 +454,12 @@ computed-identity-bind-marker-source	{"a":{"b":{"c":1}}}	path(. as $x | 5 | $x a
 computed-identity-bind-mixed-if	{"a":{"b":{"c":1}}}	path(. as $x | 5 | (if true then $x else . end) as $y | $y)
 catch-break-payload-not-register	{"a":null}	(.a | label $out | try (break $out) catch .b) = 1
 catch-break-restores-register	{"a":null}	path(.a as $y | .a | label $out | try (break $out) catch $y)
+fold-body-nested-try	{"a":{"b":1}}	del(foreach .a as $v (.; try ($v | .b); .))
+fold-body-nested-if	{"a":{"b":1}}	path(foreach .a as $v (.; if true then ($v | .b) else . end; .))
+fold-extract-nested-try	{"a":{"b":1}}	path(.a as $y | .a | foreach range(1) as $i (0; .; try ($y | .b)))
+fold-body-nested-try-sibling-control	{"a":{"b":1},"c":{"b":1}}	del(.a as $y | foreach .c as $v (.; try ($y | .b); .))
+fold-body-fanout-declines	{"a":{"b":1}}	(foreach .a as {a:$v} ?// {c:$v} (0; ($v[0]?, $v))) = 9
+fold-body-fanout-declines-del	{"a":{"b":1}}	del(foreach .a as $v (.; (($v | .b?), 1); try ($v | .b?)))
 CASES_EOF
 
 # Known refuse-only rows (jq answers, succinctly refuses), each with the
@@ -500,6 +506,8 @@ untracked-opaque-stage-lost-register:#3120 review -- an opaque stage (reduce, a
 untracked-later-step-refusal-no-retry:#3120 review -- refusal_is_exact is decided per source, and a marker that is the register is value-equal to it, so a later-step refusal after a certified first step is treated as a guess and does not retry; jq retries onto $w. The trackable twin retries and agrees
 catch-payload-own-node-refuse-only:#3133 -- error(.) raises the register node itself and jq answers ["a"]; the payload equals the register by value but is not null/bool and carries no marker, so it cannot be told from a rebuilt copy (catch-rebuilt-payload-refuses) and the handler stays untracked
 computed-identity-bind-mixed-if:#3133 -- an if source with one arm a computed `.` and the other a marker binds Untracked on an untracked stage (the condition is not evaluated); jq evaluates it and binds the marker
+fold-body-fanout-declines:#3145 review -- the register reaches a fold body only when that body cannot fan out: a nested pipe sees it while a sibling branch of the same multi-output body does not, so an UPDATE that refused wholesale could half-succeed and drive a ?// retry jq never performs, writing a key jq never names. Refusing the whole body is the safe side of that asymmetry
+fold-body-fanout-declines-del:#3145 review -- the del twin: jq writes {"a":{}}, the half-success wrote nothing at exit 0, and declining refuses loudly instead
 REFUSE_EOF
 
 if [[ "${1:-}" == "--list-cases" ]]; then

src/jq/eval.rs

@@ -33456,6 +33456,28 @@ fn getpath_preserves_register<S: EvalSemantics>(
             .is_some_and(|(invocation, path)| !stage_frame.names(invocation, path))
 }
 
+/// Whether `expr` can produce more than one output -- a syntactic
+/// over-approximation, used by [`FoldRegister::resolve`] to decline
+/// carrying the register into a body whose branches would then see it
+/// unevenly (#3145 review). `Comma` and the iterating/recursing shapes
+/// count; so does anything this predicate cannot see inside, since the
+/// answer must be "maybe" for those.
+fn fans_out(expr: &Expr) -> bool {
+    any_subexpr(expr, &mut |e| {
+        matches!(
+            e,
+            Expr::Comma(_)
+                | Expr::Iterate
+                | Expr::RecursiveDescent
+                | Expr::AsPattern { .. }
+                | Expr::Reduce { .. }
+                | Expr::Foreach { .. }
+                | Expr::FuncCall { .. }
+                | Expr::NamespacedCall { .. }
+        )
+    })
+}
+
 /// jq's own `jv_identical(v, jq->value_at_path)`, modeled for a value type
 /// that has no pointer to compare — the single definition shared by every
 /// site that has to answer "is this branch still sitting *on* the path
@@ -33785,12 +33807,44 @@ impl FoldRegister {
         // accumulator itself currently sits there (the seed below carries
         // the register for exactly that case). A lost register has no
         // position to offer.
+        //
+        // #3145: the frame carries the register's *value* too, for the same
+        // reason `resolve_seq_stage` puts it on a pipe stage's frame
+        // (#3133): a pipe nested in UPDATE/EXTRACT under `try`/`if`/`,` is
+        // reached through `resolve_node_sink`, which has no `PathBranch` to
+        // take the register from, so `try ($v | .b)` could not re-establish
+        // `$v` and `try` swallowed its refusal -- `del(foreach .a as $v (.;
+        // try ($v | .b); .))` echoed the document where jq writes
+        // `{"a":{}}`. Only for a body that cannot fan out (`fans_out`),
+        // only while the register is **live** (`self.trackable`)
+        // and only in jq mode, exactly as the pipe-stage site gates it: an
+        // untracked `FoldRegister`'s `value` is not a register at all
+        // (`enter`'s own untracked arm seeds it from the ambient), and
+        // handing that to a nested pipe let markers re-establish where jq
+        // refuses -- three fabricated writes in 18,000 fuzzed programs,
+        // none of which the sweep or the unit rows reached.
+        //
+        // The `fans_out` half is the review's finding, and the same shape
+        // one level up: a nested pipe sees the register while a *sibling*
+        // branch of the same multi-output body does not (it goes through
+        // `resolve_node`, whose non-pipe arms this frame does not reach),
+        // so an UPDATE that used to refuse wholesale could half-succeed --
+        // `(foreach .a as {a:$v} ?// {c:$v} (0; ($v[0]?, $v))) = 9` wrote
+        // `.a.c`, a key jq never names, because only the second output
+        // refused and drove a `?//` retry jq does not perform. A body with
+        // one output path cannot split that way. Widening the other
+        // direction (re-establishing in `resolve_node`'s own arms) is
+        // #2046's documented scope limit, not this fix's.
         let update_frame = if self.trackable {
             self.frame.clone()
         } else {
             self.frame.unknown()
         };
         let resolved = if let Expr::Pipe(exprs) = unwrap_paren(expr) {
+            // The register goes in as the explicit argument here, which
+            // `resolve_seq_sink` prefers over the frame's and every later
+            // stage overwrites from `carried_register` -- so the frame does
+            // not carry it too (that copy would never be read).
             resolve_seq::<S>(
                 exprs,
                 &input,
@@ -33801,6 +33855,13 @@ impl FoldRegister {
                 self.trackable.then_some(&self.value),
             )
         } else {
+            let update_frame = update_frame.with_register(
+                if S::TAG == EvalTag::Jq && self.trackable && !tr && !fans_out(expr) {
+                    Some(&self.value)
+                } else {
+                    None
+                },
+            );
             resolve_node::<S>(expr, &input, tr, snapshot, &update_frame, keep)
         };
         let owned = match resolved {
@@ -99970,6 +100031,78 @@ mod tests {
         }
     }
 
+    /// #3145: a pipe nested under `try`/`if`/`,` inside a fold's
+    /// UPDATE/EXTRACT reaches `resolve_node_sink` with no `PathBranch` to
+    /// take the register from, so `try ($v | .b)` could not re-establish
+    /// `$v`, raised the resolver's own refusal, and `try` swallowed it --
+    /// `del(foreach .a as $v (.; try ($v | .b); .))` echoed the document
+    /// where jq writes `{"a":{}}` (#3133 fixed the pipe-stage half; the
+    /// fold's own `FoldRegister::resolve` frame carried no register).
+    /// Every row confirmed live against jq 1.7.1.
+    #[test]
+    fn test_fold_body_nested_pipe_keeps_the_register_3145() {
+        for (doc, filter, want) in [
+            (
+                &br#"{"a":{"b":1}}"#[..],
+                r"path(foreach .a as $v (.; try ($v | .b); .))",
+                r#"["a","b"]"#,
+            ),
+            (
+                &br#"{"a":{"b":1}}"#[..],
+                r"del(foreach .a as $v (.; try ($v | .b); .))",
+                r#"{"a":{}}"#,
+            ),
+            (
+                &br#"{"a":{"b":1}}"#[..],
+                r"path(foreach .a as $v (.; if true then ($v | .b) else . end; .))",
+                r#"["a","b"]"#,
+            ),
+            // EXTRACT, not just UPDATE.
+            (
+                &br#"{"a":{"b":1}}"#[..],
+                r"path(.a as $y | .a | foreach range(1) as $i (0; .; try ($y | .b)))",
+                r#"["a","b"]"#,
+            ),
+            // The sibling-copy control: `$v` is the register here, `$y` is
+            // not, and jq writes nothing through the latter either.
+            (
+                &br#"{"a":{"b":1},"c":{"b":1}}"#[..],
+                r"del(foreach .c as $v (.; try ($v | .b); .))",
+                r#"{"a":{"b":1},"c":{}}"#,
+            ),
+            (
+                &br#"{"a":{"b":1},"c":{"b":1}}"#[..],
+                r"del(.a as $y | foreach .c as $v (.; try ($y | .b); .))",
+                r#"{"a":{"b":1},"c":{"b":1}}"#,
+            ),
+        ] {
+            assert_eq!(outputs(doc, filter), [want], "{filter}");
+        }
+        // A *fan-out* body declines the register entirely (review): a
+        // nested pipe would see it while a sibling branch of the same body
+        // does not, and that half-success fabricated a write. jq emits
+        // `["a","b"]` here and then raises; succinctly refuses outright,
+        // with no prefix -- the safe side (`fold-body-fanout-declines` in
+        // the sweep).
+        query!(br#"{"a":{"b":1}}"#, r"path(foreach .a as $v (.; ($v | .b), 1; .))",
+            QueryResult::Error(e) | QueryResult::Partial(_, Control::Error(e)) => {
+                assert!(is_resolver_refusal(&e), "{}", e.message);
+            }
+        );
+        // Unchanged: a fold whose INIT is untracked *after a literal stage*
+        // re-seeds its register from the ambient, so the marker is not
+        // recognised at all -- pre-existing (`literal-then-fold-untracked-
+        // init`, `carried-register-passthrough`), and with a generator
+        // source the `try` then swallows that refusal into a no-op write.
+        assert_eq!(
+            outputs(
+                br#"{"a":{"b":1}}"#,
+                r"del(.a as $y | .a | 5 | foreach range(1) as $i (0; .; try ($y | .b)))"
+            ),
+            [r#"{"a":{"b":1}}"#]
+        );
+    }
+
     /// #2676: a fold's own destructuring pattern is tracked exactly the way
     /// a plain `. as PATTERN` bind is (#2649's walk, `PathPatternMode`), whenever the
     /// fold's SOURCE resolves through the path register. Every row here is