fix(jq): admit a null/bool navigated-bind marker at an equal-valued sibling

docs/compliance/jq/limitations.md

@@ -863,7 +863,11 @@ is the revert that established what the other one costs.
    equal-valued sibling `path(.a as $y \| .c \| $y)` still refuses — exactly the #1466 class
    the frame witness exists to keep closed, now for navigated bindings too. yq mode is
    unchanged: `substitute_bound_var`'s widening is jq-mode only
-   ([#2643](https://github.com/rust-works/succinctly/issues/2643)).
+   ([#2643](https://github.com/rust-works/succinctly/issues/2643)). The sibling refusal is
+   itself the general (non-`null`/`bool`) case: jq's own `jv_identical` admits
+   `null`/`true`/`false` by value regardless of node, so the same shape on `{"a":true,"c":true}`
+   answers `["c"]` on both
+   ([#3136](https://github.com/rust-works/succinctly/issues/3136)).
 
    Fourteen rows stay refuse-only, each pinned in `test_path_bind_origin_matrix_refuse_only_2042`
    (`src/jq/eval.rs`) and `scripts/jq-bind-origin-oracle-sweep.sh`'s own `REFUSE_ONLY` list:
@@ -894,7 +898,10 @@ is the revert that established what the other one costs.
    `Snapshot` marker not proven to name that call's own document node to `Untracked` first, so
    `Frame::certifies`'s unconditional `Snapshot => true` never gets a chance to admit the
    rebuilt copy — refuse-only by construction, since the only transition is `Snapshot →
-   Untracked` and `Untracked` never certifies. A documented residual remains: a handful of
+   Untracked` and `Untracked` never certifies by node identity (a `null`/`bool` rebuilt copy
+   still certifies by jq's own value-identity rule, #3136 — sound, since jq's `jv_identical`
+   has no pointer identity for those three values at all, rebuilt or not). A documented
+   residual remains: a handful of
    constructions jq's own reference-counted `jv` passes an embedded node through *without
    copying it* (`{k:.} \| .k`, `. + {}`, `reduce empty as $i (.; .)`, and `path(.[0] | $x)`
    navigation *inside* a `path()` call over `[.]`) now refuse rather than silently accept a
@@ -1036,11 +1043,14 @@ is the revert that established what the other one costs.
    is no cursor at that funnel to promote against), a navigated bind on an *owned-rooted*
    document (`input | .a as $y | .a | path($y)`, `jq -n '{a:{b:1}} | .a as $y | .a | ($y.b) = 9'`,
    a `tojson|fromjson`-rebuilt root: the marker's node is an `OwnedIdentity` position, and
-   `marker_is_root` reads only a document node against a live cursor — no `OwnedRoot` twin),
-   and a `null`/`bool` marker at an
-   equal-valued sibling (`.a as $y | .c | $y |= 5` on `{"a":true,"c":true}`: jq's
-   `jv_identical` admits those by value regardless of node, the resolver's `TrackedVar` arm
-   consults the origin first — pre-existing). The library entry point `succinctly::jq::eval`
+   `marker_is_root` reads only a document node against a live cursor — no `OwnedRoot` twin).
+   A `null`/`bool` marker at an equal-valued sibling (`.a as $y | .c | $y |= 5` on
+   `{"a":true,"c":true}`) used to be refuse-only the same way — jq's `jv_identical` admits
+   those by value regardless of node, but the resolver's `TrackedVar` arm consulted the
+   origin first — closed by
+   [#3136](https://github.com/rust-works/succinctly/issues/3136), which ORs in the same
+   null/bool value-identity carve-out `register_identical` already makes, gated the same way
+   by the existing node-identity check. The library entry point `succinctly::jq::eval`
    takes the eager evaluator for a program `needs_path_context` does not route (a `path(f)`
    with an argument, a write), so through it these rows keep refusing as before; the CLI's
    route is the generic evaluator, where they answer.

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

@@ -497,7 +497,6 @@ navigated-bind-positional-assign:#3037 residual -- same as navigated-bind-positi
 navigated-bind-embed:#2889 -- the owned-embed residual ({k:.a} | .k), unchanged by #3037
 navigated-bind-reduce-update:#3037 residual -- the UPDATE of reduce re-enters the eager evaluator with an owned accumulator; its eval_as carries no node for a navigated bind (#2072 gave the generic evaluator that, not this one), and there is no cursor at the funnel to promote against
 navigated-bind-catch-handler:#3037 residual -- same as navigated-bind-reduce-update, through a catch handler
-navigated-bind-bool-sibling:pre-existing -- the jv_identical rule of jq admits a null/bool by value regardless of node, but the TrackedVar arm of the resolver consults the origin first; ($y | .) = 5 already answers since the . stage re-establishes by value
 navigated-bind-owned-root:#3037 residual -- a navigated bind on an owned-rooted document; the marker node is an OwnedIdentity position and marker_is_root reads only a document node against a live cursor, no OwnedRoot twin
 navigated-bind-input-root:#3037 residual -- same as navigated-bind-owned-root, on the input-queue route
 identity-if-arms-differ:#2978 -- identity_bind_position is static: an if whose arms sit at different positions ($p at [], . at ["a"]) proves neither, so the bind stays a bare Snapshot and getpath has no position to compose from; jq evaluates the condition

src/jq/eval.rs

@@ -29212,10 +29212,14 @@ fn marker_needs_demotion(marker: &Tracked, root: &RootWitness) -> bool {
 
 /// Rewrite every [`Expr::TrackedVar`] marker in `expr` whose
 /// [`marker_needs_demotion`] against `root` into an [`Origin::Untracked`]
-/// copy (#2642) -- a demotion, never a promotion: `Frame::certifies` already
-/// refuses `Untracked` unconditionally, so this is the only change needed to
-/// stop a rebuilt copy from being admitted, without touching `Frame`,
-/// `Origin`, or `resolve_node`'s certification rule itself.
+/// copy (#2642) -- a demotion, never a promotion: `Frame::certifies` still
+/// refuses `Untracked` by node identity unconditionally, so this is the
+/// only change needed to stop a rebuilt copy from being admitted *that
+/// way*. [`marker_identical`]'s null/bool value-identity carve-out (#3136)
+/// applies uniformly to every `Origin` including a demoted `Untracked`
+/// one -- correctly, since jq's own `jv_identical` has no pointer identity
+/// for `null`/booleans at all, rebuilt or not -- so this demotion still
+/// costs only node-identity-based acceptance, never the null/bool one.
 ///
 /// The walk itself is [`rewrite_markers`], shared with the one promotion
 /// this evaluator performs ([`reroot_markers`], #3037): `Cow::Borrowed`
@@ -31628,9 +31632,19 @@ fn resolve_node_eager<'a, S: EvalSemantics>(
         // `Origin::Untracked` (#2072: a navigated binding wrapped only for
         // the cursor routes' sake, made outside any resolver invocation)
         // never certifies, so this refuses exactly as it did before #2072
-        // gave every `as` binding a `TrackedVar` wrapper.
+        // gave every `as` binding a `TrackedVar` wrapper -- except the one
+        // case jq's own `jv_identical` admits regardless of node identity
+        // (#3136): `null`/`true`/`false` are identical by value alone (the
+        // same carve-out `null_bool_identical` makes for `register_identical`
+        // and the pattern-walk's own first-step check), so an
+        // `Origin::Untracked` marker at an equal-valued sibling still
+        // certifies when its value is one of those three, where the
+        // node-identity rule above never can.
         Expr::TrackedVar(marker) => {
-            if trackable && marker.value == *value && frame.certifies(&marker.origin) {
+            if trackable
+                && marker.value == *value
+                && (frame.certifies(&marker.origin) || null_bool_identical(&marker.value, value))
+            {
                 Ok(vec![PathBranch::new(
                     PathPrefix::root(),
                     Cow::Borrowed(value),
@@ -33549,6 +33563,18 @@ fn null_bool_identical(a: &OwnedValue, b: &OwnedValue) -> bool {
     matches!(a, OwnedValue::Null | OwnedValue::Bool(_)) && a == b
 }
 
+/// Whether `marker` certifies against `target` -- node identity
+/// (`Frame::certifies`) or, failing that, jq's `jv_identical` null/bool
+/// value-identity carve-out (#3136). Shared by `resolve_node_eager`'s and
+/// `resolves_to_register`'s otherwise-identical `Expr::TrackedVar` arms so
+/// a future refinement of this rule can't apply to one and silently miss
+/// the other, the same duplication risk [`null_bool_identical`]'s own doc
+/// comment names.
+fn marker_identical(marker: &Tracked, target: &OwnedValue, frame: &Frame) -> bool {
+    marker.value == *target
+        && (frame.certifies(&marker.origin) || null_bool_identical(&marker.value, target))
+}
+
 /// Whether `expr`, given it runs to completion without raising or
 /// yielding zero outputs, is *provably* `reg` -- `resolve_as_pattern`'s
 /// first-step identity test (jq's `path_intact`) needs exactly this, which
@@ -33573,9 +33599,9 @@ fn null_bool_identical(a: &OwnedValue, b: &OwnedValue) -> bool {
 /// `is_identity_passthrough(then_branch) && is_identity_passthrough
 /// (else_branch) => trackable` arm answered `["a","x"]` and `del` wrote
 /// through the mismatch. This function instead re-runs the bare-
-/// `TrackedVar` arm's own check (`marker.value == *reg &&
-/// frame.certifies(&marker.origin)`) at every level it recurses to, so a
-/// stale marker refuses wherever it's reached, not just at the top.
+/// `TrackedVar` arm's own check ([`marker_identical`]) at every level it
+/// recurses to, so a stale marker refuses wherever it's reached, not just
+/// at the top.
 ///
 /// Mirrors `is_identity_passthrough`'s grammar otherwise, with one
 /// difference: `Alternative` is gated on the *runtime* register being
@@ -33587,7 +33613,10 @@ fn null_bool_identical(a: &OwnedValue, b: &OwnedValue) -> bool {
 fn resolves_to_register(expr: &Expr, trackable: bool, reg: &OwnedValue, frame: &Frame) -> bool {
     match unwrap_paren(expr) {
         Expr::Identity => trackable,
-        Expr::TrackedVar(marker) => marker.value == *reg && frame.certifies(&marker.origin),
+        // #3136: [`marker_identical`], shared with `resolve_node_eager`'s
+        // own `TrackedVar` arm, so a future refinement of this rule can't
+        // apply to one and silently miss the other.
+        Expr::TrackedVar(marker) => marker_identical(marker, reg, frame),
         Expr::If {
             then_branch,
             else_branch,

src/jq/expr.rs

@@ -161,10 +161,13 @@ pub struct Tracked {
 /// - [`Origin::Untracked`] -- the #2072 witness for a binding whose source
 ///   navigated, made *outside* any resolver invocation (an ordinary `as`
 ///   binding, not one syntactically inside `path()`'s argument): there is
-///   no invocation to certify against, so this marker always refuses,
-///   exactly as an un-wrapped literal did before #2072 gave every `as`
-///   binding a `TrackedVar` wrapper so its `Tracked::node` could carry
-///   cursor identity. `path(.a as $y | .c | $y)` still refuses, matching jq.
+///   no invocation to certify against, so this marker refuses by node
+///   identity, exactly as an un-wrapped literal did before #2072 gave every
+///   `as` binding a `TrackedVar` wrapper so its `Tracked::node` could carry
+///   cursor identity. `path(.a as $y | .c | $y)` on `{"a":{"b":1},"c":{"b":1}}`
+///   still refuses, matching jq -- but jq's own `jv_identical` admits
+///   `null`/`true`/`false` by value regardless of node (#3136), so the same
+///   filter on `{"a":true,"c":true}` answers `["c"]` on both.
 #[derive(Debug, Clone, PartialEq)]
 pub enum Origin {
     /// Frozen from the ambient input itself; certified by value equality
@@ -190,7 +193,8 @@ pub enum Origin {
         path: super::eval::BindPath,
     },
     /// Frozen from a navigated position outside any resolver invocation
-    /// (#2072); never certified.
+    /// (#2072); never certified by node identity, only by the null/bool
+    /// value-identity carve-out (#3136) every other `Origin` also gets.
     Untracked,
 }
 

tests/jq_cli_tests.rs

@@ -60792,10 +60792,6 @@ fn test_navigated_bind_traps_still_refuse_3037() -> Result<()> {
 ///   `tojson|fromjson`-rebuilt root): the marker's node is an
 ///   `OwnedIdentity` position, and `marker_is_root` reads only a document
 ///   node against a live cursor -- no `OwnedRoot` twin (review finding).
-/// - A `null`/`bool` marker at an equal-valued sibling: jq's `jv_identical`
-///   admits those by value regardless of node, but the resolver's
-///   `TrackedVar` arm consults the origin first (pre-existing; `($y | .)
-///   = 5` already answers, since the `.` stage re-establishes by value).
 #[test]
 // jq filter literals like `{b:1}`/`{k:.a}` are not formatting strings;
 // clippy cannot tell the two apart from the brace shape alone (as `*_2642`).
@@ -60813,7 +60809,6 @@ fn test_navigated_bind_residuals_refuse_cleanly_3037() -> Result<()> {
             r#"{"a":{"b":1}}"#,
             r"try error(.) catch (.a as $y | .a | path($y))",
         ),
-        (r#"{"a":true,"c":true}"#, r".a as $y | .c | $y |= 5"),
         (
             r#"{"a":{"b":1}}"#,
             r"(tojson|fromjson) | .a as $y | .a | path($y)",
@@ -60833,6 +60828,68 @@ fn test_navigated_bind_residuals_refuse_cleanly_3037() -> Result<()> {
     Ok(())
 }
 
+/// #3136: jq's own `jv_identical` treats `null`/`true`/`false` as identical
+/// by value regardless of node (the same carve-out `null_bool_identical`
+/// already made for `register_identical` and the pattern-walk's own
+/// first-step check) -- but `resolve_node_eager`'s `Expr::TrackedVar` arm
+/// consulted node identity first, so a `null`/`bool` marker bound from a
+/// navigated position refused at an equal-valued *sibling* where jq
+/// answers. All three values, and both write forms (`|=`, `del`), not just
+/// the `path()` read `test_navigated_bind_residuals_refuse_cleanly_3037`
+/// used to pin as refuse-only. The last two rows are
+/// `resolve_as_pattern`'s own sibling gap (review): a bare-var pattern
+/// source (`$y as $z | ...`) reaches `resolves_to_register`'s identical
+/// `TrackedVar` arm, which had the same gap independently.
+#[test]
+fn test_navigated_bind_null_bool_sibling_answers_3136() -> Result<()> {
+    for (input, filter, want) in [
+        (r#"{"a":true,"c":true}"#, r".a as $y | .c | path($y)", "[]"),
+        (r#"{"a":true,"c":true}"#, r".a as $y | .c | $y |= 5", "5"),
+        (r#"{"a":null,"c":null}"#, r".a as $y | .c | path($y)", "[]"),
+        (
+            r#"{"a":false,"c":false}"#,
+            r"del(.a as $y | .c | $y)",
+            r#"{"a":false}"#,
+        ),
+        (
+            r#"{"a":true,"c":true}"#,
+            r".a as $y | .c | $y as $z | path($z)",
+            "[]",
+        ),
+        (
+            r#"{"a":false,"c":false}"#,
+            r"del(.a as $y | .c | $y as $z | $z)",
+            r#"{"a":false}"#,
+        ),
+    ] {
+        let (stdout, stderr, code) = run_jq_full(&["-c", filter], Some(input))?;
+        assert_eq!(code, 0, "#3136: `{filter}`: stderr={stderr:?}");
+        assert_eq!(stdout.trim_end(), want, "#3136: `{filter}`");
+    }
+    // Controls: a non-null/bool equal-valued sibling still refuses -- jq's
+    // own jv_identical requires actual pointer identity for those, which
+    // this fix does not (and must not) widen. One direct bind, one through
+    // a pattern source, exercising both fixed call sites' own gates.
+    for (filter, input) in [
+        (r".a as $y | .c | path($y)", r#"{"a":{"b":1},"c":{"b":1}}"#),
+        (
+            r".a as $y | .c | $y as $z | path($z)",
+            r#"{"a":{"b":1},"c":{"b":1}}"#,
+        ),
+    ] {
+        let (stdout, stderr, code) = run_jq_full(&["-c", filter], Some(input))?;
+        assert_eq!(
+            code, 5,
+            "#3136 control: non-null/bool sibling must still refuse, `{filter}`: got stdout={stdout:?} stderr={stderr:?}"
+        );
+        assert!(
+            stderr.contains("Invalid path expression"),
+            "#3136 control: `{filter}` -- stderr: {stderr:?}"
+        );
+    }
+    Ok(())
+}
+
 /// yq mode is untouched by #3037: real yq v4.53.3 treats `($y.b) = 9`
 /// through a variable as a no-op and prints the document unchanged
 /// (`b: 1`), at the marker's own node and at a sibling alike, where