jq: a wrapped module dependency sits inside the including def's scope, so that def's own name or parameters can capture names free in it

[newhoggy]

Severity: Low

Summary

Found by code review on PR #2954 (#2865), and recorded there in
docs/compliance/jq/limitations.md. #2865 makes a module's own includes work
by wrapping each exported def's body in the dependencies it reaches. The wrap
nests inside that def's own scope, so a name left free in a dependency can
be captured by the def's own name or by one of its parameters.

Not a regression: every shape below needs a transitive include, which did not
work at all before #2865 (main answers g/0 is not defined / k/0 is not defined for each). These are residual gaps in the new functionality.

Repro

All rows confirmed live against jq 1.7.1 and PR #2954's head.

1. The self-(name, arity) exclusion strands a dependency another dependency calls

$ cat inner.jq
def c: 7;
def g: c;
$ cat mid.jq
include "inner";
def c: if . == 0 then g else (. - 1 | c) end;

$ jq            -L . -nc 'include "mid"; 0 | c'   # 7
$ succinctly jq -L . -nc 'include "mid"; 0 | c'   # error: g/0 exceeded maximum recursion depth (exit 5)

mid's body calls c directly, so the dependency c is excluded to leave the
self-recursive call binding to itself — but the kept dependency g still needs
it, and its c then resolves outward to mid's own c.

2. A parameter satisfies a name a dependency left undefined, swallowing a compile error

$ cat gb.jq
def g: b;
$ cat hb.jq
include "gb";
def h(b): g;
def q: h(99);

$ jq            -L . -nc 'include "hb"; q'   # error: b/0 is not defined at gb.jq (exit 3)
$ succinctly jq -L . -nc 'include "hb"; q'   # 99 (exit 0)

The worse half of the family: a program jq rejects produces an answer.

3. A parameter captures a dependency reached only indirectly

$ cat inner3.jq
def g: 42;
def k: [g];
$ cat h3.jq
include "inner3";
def h($g): [g, k];
def q: h(7);

$ jq            -L . -nc 'include "h3"; q'   # [7,[42]]
$ succinctly jq -L . -nc 'include "h3"; q'   # [7,[7]]

h's own g is correctly the parameter; k's g should still be the
dependency, and is not.

Root cause

ModuleLoader::load_and_bind_module / visible_deps_for
(src/bin/succinctly/jq_runner.rs). Binding is wrap_defs(body, deps), which
nests the dependency block inside the Expr::FuncDef for the def itself — so
that def's name (for self-recursion) and its parameters are enclosing binders
for every dependency in the block. Real jq binds each module's block in its own
scope and only then links it, so nothing of the caller is in scope for it.

The two exclusions in visible_deps_for are the current, partial mitigation:
they keep the def's own recursion and its parameters working for names the body
uses directly. They cannot help a name reached only through another
dependency, and the cases pull in opposite directions — excluding strands the
other dependency (row 1), keeping it would shadow the def's own binding.

Suggested fix direction

Two shapes:

1. Targeted renaming. When a dependency has to be excluded, give it a fresh
   internal name instead of dropping it, and rewrite free calls to the original
   name inside the other kept dependency bodies — a scope-aware walk that
   stops at any nested def of the same (name, arity) and at parameters
   binding it. Contained, and fixes rows 1 and 3; row 2 needs (3) below as well.
2. A real module scope boundary, which is what jq: a module body sees names it should not — ~/.jq's defs, and sibling included modules' defs in declaration order #2951 is about: with a
   sealed scope the wrap would not nest inside the caller at all, and the whole
   family goes away. Subsumes (1).
3. Resolve a module's own body at load time, reporting b/0 is not defined
   against the module's own file the way jq does. That is what turns row 2 back
   into a compile error rather than something a caller can accidentally satisfy,
   and it also interacts with jq: a module's own source is parsed with no shadow-candidate seeding, so a transitively included def can't shadow a builtin inside that module #2950 (a module's source is parsed with no
   shadow-candidate seeding).

Why this is separate from #2865, #2950, #2951

#2865 delivers transitive loading and matched every ordinary shape probed
(dependencies, siblings, parameters, arity splits, self-recursion, patterns,
namespaces, diamonds, chains, $__loc__, cycles); this is the boundary of the
mechanism it uses, and needs either a scope boundary (#2951) or the narrower
rename above rather than another exclusion rule. #2950 is the shadow-candidate
half of the same module-scope story.

[newhoggy]

Triage plan (2026-09-17)

Premise check on current main (b509a8019, #2951 merged)

#2951 is merged (ADR-0023). It did not close this issue, and says so on purpose: the
dependency runs load_and_bind_module wraps into each def body are bracketed by a
barrier (RunMarker::Barrier). A barrier records which file wrote the code but does
not seal names off. ADR-0023 decision 4, resolve.rs's RunMarker::Barrier doc, and
limitations.md all name upgrading it as #2962's change.

A differential of 21 fixtures, run against /usr/bin/jq 1.7.1 and a debug build of main
(-L . -nc --arg x 5), shows the family is wider than the three rows filed:

row	fixtures	filter	jq 1.7.1	succinctly main	route
R1 (filed)	inner = def c: 7; def g: c; · mid = include "inner"; def c: if . == 0 then g else (. - 1 | c) end;	include "mid"; 0 | c	7	g/0 exceeded maximum recursion depth, exit 5	stranded exclusion
R2 (filed)	gb = def g: b; · hb = include "gb"; def h(b): g; def q: h(99);	include "hb"; q	b/0 is not defined at …/gb.jq, line 1:, exit 3	99, exit 0	no floor
R3 (filed)	inner3 = def g: 42; def k: [g]; · h3 = include "inner3"; def h($g): [g, k]; def q: h(7);	include "h3"; q	[7,[42]]	[7,[7]]	stranded exclusion
R4	B = def b1: g; · A = include "B"; def g: 1; def f: b1;	include "A"; f	g/0 is not defined at …/B.jq, exit 3	1	no floor (already pinned as the include half of test_import_alias_does_not_reach_a_dependency_body_2951)
R5	inner5 = def k: $g; · h5 = include "inner5"; def h($g): k; def q: h(7);	include "h5"; q	$g is not defined at …/inner5.jq, line 1:, exit 3	7	no variable floor
R6	a6 = def x: 1; · b6 = def y: x; · m6 = include "a6"; include "b6"; def z: y;	include "m6"; z	x/0 is not defined at …/b6.jq, exit 3	1	no floor (sibling dependency group)
R8	inner8 = def c: 7; def c(f): f; def g: [c, c(8)]; · mid8 = R1's mid with inner8	include "mid8"; 0 | c	[7,8]	recursion depth, exit 5	stranded exclusion, arity split
R13	mid13 = import "inner" as i; def c: if . == 0 then i::g else (. - 1 | c) end;	include "mid13"; 0 | c	7	i::g/0 exceeded maximum recursion depth, exit 5	a dependency run carries no import alias, so i::g's bare c gets no #2989 retry and falls outward to mid13's c
R18	ib = def g: b; · hib = import "ib" as i; def h(b): i::g; def q: h(99);	include "hib"; q	b/0 is not defined at …/ib.jq, exit 3	99	no floor (import form of R2)
R20	order = def g: c; def c: 7; · morder = include "order"; def c: 1; def q: g;	include "morder"; q	c/0 is not defined at …/order.jq, exit 3	1	no floor (a later sibling cannot bind; the includer's does)

Already matching (keep as must-not-change rows): R2u include "hb"; 1 → 1 (an
unreferenced dependency is an error in neither tool), R9, R11 (nested def c inside a
dependency body), R12 (a dependency's own param named c), R15 (--arg global $x is
visible in a dependency body → "5"), R16/R21/R22 below, R17 (def h: def g: 6; [g, k] →
[6,5]).

A jq quirk the fix must preserve (R16/R21/R22). Inside a module, an included
dependency binds before the module's own siblings. l1 = def c: 1; def g: c;,
l2f = include "l1"; def c: 2; def g2: c; → include "l2f"; g2 is 1 in jq, while the
same text as a main filter is 2. Precedence inside a module def body is params > self >
dependencies > module siblings. The current "wrap dependencies innermost inside each body"
model already gets this right (succinctly matches on all three), so the fix keeps the wrap
and changes only what the wrap can see and what it hides.

Root cause (two independent mechanisms)

1. No floor on dependency runs. A barrier does not seal names off, so a name left free in a
   dependency body resolves outward into the including def's params and self, the including
   module's siblings, and earlier dependency groups (R2, R4, R6, R18, R20). The same is true
   of variables: VarScope is a plain Vec<String> scanned by in_var_scope, which
   knows nothing about run markers, so even a floored run would still leak a $param (R5).
2. Exclusion strands a dependency. visible_deps_for drops any dependency that clashes
   with the def's own (name, arity) or a 0-arity param (when the body calls it directly), so
   the body's own binding wins. The dependency then vanishes for other dependencies that
   need it (R1, R3, R8). Adding a floor alone turns those rows from a wrong answer into a
   compile error (c/0 / g/0 is not defined), which is still not jq's answer. That is why
   jq: a module body sees names it should not — ~/.jq's defs, and sibling included modules' defs in declaration order #2951 held off.

R13 needs a third, smaller thing: an import dependency group needs its alias on its own run
marker, as ADR-0023 already does for top-level imports.

Plan

1. Floor dependency runs (loader + resolver). In load_and_bind_module, bracket each
   dependency group with ModuleRun::begin_marker(id, alias) instead of barrier_marker(id).
   DepRuns becomes Vec<(u32, Option<String>, FuncDefList)>, and module_dep_defs records
   the alias for each import group (None for include). Fixes R2, R4, R6, R13, R18, R20.
   • Then delete RunMarker::Barrier, barrier_marker, wrap_barrier_run and the
     Barrier arm of scan_scope, if nothing else uses them. Before deleting, confirm with
     test_import_alias_does_not_reach_a_dependency_body_2951 that a flooring Begin
     (alias None) still stops an enclosing import's alias. It should: scan_scope returns at
     the first open Begin and records it as run.
   • The evaluator stays untouched for the reason ADR-0023 decision 5 gives: the floor only
     narrows the scope to a suffix of the evaluator's own innermost-first stack.
2. Floor variables the same way (resolve.rs). In check's Expr::FuncDef arm, when
   name parses as a run marker, also push it onto var_scope for the then walk and
   truncate afterwards. Make in_var_scope go through scan_scope, just as in_scope does,
   so a variable scan stops at the same floor. --arg/$ENV/$__loc__ must stay visible
   (R15): confirm how globals are resolved (the VarScope miss path) before relying on it.
   Give UnboundVar an origin: Option<u32> like UnresolvedCall, and have the jq runner
   use it, so R5 reports at …/inner5.jq, line 1: plus the source echo instead of
   <top-level>. build_call_graph tracks no variables, so only check changes, but say so
   in a comment so the lock-step note in the file header stays true. Fixes R5.
3. Rename instead of exclude (visible_deps_for). Replace the two exclusions with a
   rename:
   • A dependency whose (name, arity) equals the def's own, or whose name matches a param
     at arity 0, is kept under a fresh internal name: NUL-prefixed, so it can never be
     lexed, and unique per entry (e.g. \0dep:<run>:<index>:<name>). A body call to that key
     then binds to self or the param as today.
   • Drop the "only when called directly" condition. Renaming is correct whether or not the
     body uses the name, which also removes the "a body that both uses such a name directly
     and reaches a dependency needing the other binding cannot be expressed" caveat in the
     doc comment.
   • Rewrite free calls of that exact (name, arity) inside the same group's dependency
     bodies at or after the renamed entry's position: the renamed dependency's own body (its
     self-recursion, R9-shaped), and later siblings. Never earlier ones, which cannot see it
     in jq (R20). With duplicate definitions, the innermost earlier-or-equal entry wins.
   • The rewrite walk has to respect scope. It stops at any nested Expr::FuncDef of the
     same (name, arity) (its body and its then, R11). It stops in the body of a def with
     a param of that name when arity is 0 (R12). And it does not descend into the def
     bodies of a nested flooring run (the dependency's own dependencies, already wrapped
     inside it), because those cannot see this name and a rewrite there would turn a correct
     c/0 is not defined into an error naming a mangled symbol. Code after the run's end
     marker is still walked.
   • Arity for a call carrying jq: a user def cannot shadow a builtin — the builtin silently wins, producing a wrong value with no error #2036's builtin_fallback must come from the fallback
     (resolve.rs's builtin_fallback_arity), not args.len(), which is 0 by construction
     there (see called_func_names's doc).
   • Closure: split wanted into names the body wants (which never pull a renamed entry)
     and names dependencies want (which may). Compute the body's direct calls from the
     original body, not from wrapped. Once runs are floored, an inner group's calls cannot
     reach an outer group, so the current called_func_names(&wrapped) only over-keeps. That
     is harmless but unnecessary, and slightly positive for perf(jq): transitively included modules compound in memory when a module's defs each call more than one def below #2955.
   • Any rebuilt FuncDef gets a fresh FuncDefBound::default() (jq: def declared inside a loop body re-substitutes on every iteration (no BoundBody-style cache for FuncDef) #2094).
   • Fixes R1, R3, R8.
4. Display. A renamed name must never surface. Two sites can print a def name:
   bind_def_call's …/N exceeded maximum recursion depth (both evaluators: check
   eval_generic.rs), and UnresolvedCall's Display. Add one ModuleRun-owned helper
   that maps a renamed name back to the name as written, and route both through it. Test with
   a renamed dependency that recurses without bound: the message should read c/0, which is
   what jq's shape would say.
5. Must-not-change sweep. Run the full module suite (cargo test --features cli --test jq_cli_tests module, plus the resolve.rs unit tests), and re-run the 21-row matrix above
   against the new build. R2u, R9, R11, R12, R15, R16, R17, R21 and R22 must stay identical.
   Also test whether the floor changes anything for ~/.jq or top-level runs; it should not,
   since only dependency runs change marker kind.

Tests

• test_dependency_capture_by_the_including_defs_scope_2962: flip to jq's answers (7;
  exit 3 with the full b/0 is not defined at …/gb.jq, line 1: + echo + jq: 1 compile error; [7,[42]]), as its doc comment already asks.
• test_import_alias_does_not_reach_a_dependency_body_2951: flip the include half to
  g/0 is not defined at B.jq, as its assertion message already asks.
• New CLI rows R5, R6, R8, R13, R18, R20, plus must-not-change rows R16/R21 (the jq quirk),
  R11, R12 and R15. Compare full stderr and exit code against captured jq output, not
  contains.
• resolve.rs unit tests: replace a_barrier_claims_authorship_without_flooring with a
  variable-floor test (a $param below a Begin is invisible, a global is not), and a test
  that a closed run's variables stay visible to the code after its end marker (the original
  body still sees $g).
• Loader unit tests for the rename walk: self-recursion inside the renamed dependency, a
  later sibling, an earlier sibling left alone, a nested shadowing def, a param shadow, a
  nested sealed run left alone, a builtin_fallback call at the right arity, and duplicate
  definitions.
• Per memory feedback_validate_differential_harness_against_known_bad_build: before
  trusting "0 diffs", confirm the matrix script reports the known diffs against main. It
  does today: 10 DIFF rows, plus R19 below.

Docs

• ADR-0023: add an amendment section rather than a new ADR, because 0023 already names
  this as the follow-up. Decision 4 (barrier) becomes "dependency runs floor like any
  module". Record the variable floor and the rename mechanism, and why renaming here is
  sound, unlike the rejected "name mangling" alternative: this rename is invisible to the
  body by construction and touches only calls the resolver would bind to that entry. Update
  the Consequences bullet about jq: a wrapped module dependency sits inside the including def's scope, so that def's own name or parameters can capture names free in it #2962.
• docs/compliance/jq/limitations.md: mark the jq: a wrapped module dependency sits inside the including def's scope, so that def's own name or parameters can capture names free in it #2962 section closed, keeping the matrix
  and the jq-quirk note about dependencies binding before siblings.
• The doc comments on visible_deps_for, load_and_bind_module and
  RunMarker/ModuleRun, and the module_dep_defs comment that still says ~/.jq "leak[s]
  in today through the top-level chain" (closed by jq: a module body sees names it should not — ~/.jq's defs, and sibling included modules' defs in declaration order #2951, now stale).

Risks

• Rename walk vs. nested runs. The easiest bug to write is renaming inside a dependency's
  own nested dependency bodies, or missing a shadowing binder. Both are covered by the loader
  unit tests above. Run the fuzz/matrix after the rename is done, not only against the
  hand-picked rows (memory feedback_hand_picked_oracle_matrix_is_biased).
• Mangled names leaking into any other diagnostic ($__loc__, builtins, error paths
  in eval_generic.rs). Grep every def.name / FuncDef { name print site.
• Two evaluators (eval.rs / eval_generic.rs): the rename is load-time AST, so both
  see it. Only the display helper has to be applied in both.
• Performance: the floor adds nothing per lookup (scan_scope already returns at the
  first open Begin). The rename is a load-time walk over the dependency bodies already being
  cloned. No A/B needed unless the module-chain test (test_transitive_include_chain_does_not_blow_up_2865)
  slows measurably.

Dependencies / out of scope

• No blocker. jq: a module's own source is parsed with no shadow-candidate seeding, so a transitively included def can't shadow a builtin inside that module #2950 (shadow-candidate seeding for module sources) is independent. Keep its
  rows untouched.
• Separate divergence found while probing, not part of this issue (to file):
  def h($__loc__): 1; h(1) is a syntax error in jq 1.7.1 (unexpected $__loc__, expecting IDENT or BINDING, exit 3) but succinctly accepts it and answers 1. This happens in the
  main filter too, so it is a parser issue, not a module-scope one.

Class: Opus (unchanged)

• Not Sonnet: this is judgment, not mechanics. It is three coordinated changes across the
  loader and the resolver (the function floor, a new variable floor in check, and a
  scope-aware rename walk that has to respect nested sealed runs, shadowing defs and params,
  and builtin_fallback arity). The semantics come from a 21-row oracle matrix that
  includes a non-obvious jq precedence quirk. Mangled names must be kept out of diagnostics in
  both evaluators.
• Not Fable: there is no new representation and no new architecture. ADR-0023 already
  chose the mechanism (markers plus the scan_scope floor) and named "upgrade the barrier"
  as this issue's change. The rename is the targeted fix the issue itself proposed, now pinned
  down. The ADR work is an amendment to an accepted ADR, not a new design.

[newhoggy]

Filed the out-of-scope $__loc__ binding divergence from the triage plan as #3029. It also covers a related $ENV shadowing gap found while scoping it.

[newhoggy]

🔧 Claimed by agent Miles — branch issue-2962-dependency-runs-floor-scope