fix: soundness recovery for wrapped libs, ppx-runtime, virtual-impl deps

[robinbb]

Layer 5 of 9 of #14492.

Restores correctness for four cases the bare BFS filter (the prior layer) mishandles: virtual-impl deps (fall back to glob via has_virtual_impl), wrapped local libs reached through the wrapper name (glob the wrapped lib's Lib.closure), ppx_runtime_libraries introduced by pps (glob their Lib.closure), and stanza-(flags -open Foo) references that ocamldep on a dep-lib module cannot see (BFS-expand the opened module names into the consumer's reference set).

Promotes cross-lib-instrumentation-barrier.t/run.t and mixed-per-module-preprocess-precision.t to reflect the new dep shape. The cram tests broken on layer 4 pass again here without test file changes.

Lands the changelog (doc/changes/added/14492.md).

Part of #14492. Related to #4572.

[RyanJamesStewart]

While going through must_glob_libs against the cases #14516 narrows away, I
think I have a fourth genuinely-needed class that the three recovery branches
(has_virtual_impl, wrapped_libs_referenced, pps_runtime_libs) do not
cover.
It is not a regression in #14517 as it stands; the default build is still
exit 0. It is a soundness incompleteness in the recovery set plus a latent
stale-rebuild hazard, and it becomes a hard default-build regression at the
next layer (#14518). Reproduction first, then the mechanism and the bounding.

Three directories, all (wrapped false) on the intermediate, with the token
that names the third library absent from every source file on both ends:

$ cat > dune-project <<'EOF'
(lang dune 3.23)
EOF
$ mkdir prelude middle consumer
$ cat > prelude/dune <<'EOF'
(library (name prelude) (wrapped false))
EOF
$ cat > prelude/prelude.ml <<'EOF'
type color = Red | Green | Blue
EOF
$ cat > middle/dune <<'EOF'
(library
 (name middle) (libraries prelude) (wrapped false)
 (flags (:standard -open Prelude)))
EOF
$ cat > middle/middle.mli <<'EOF'
val pick : unit -> color
EOF
$ cat > middle/middle.ml <<'EOF'
let pick () = Green
EOF
$ cat > consumer/dune <<'EOF'
(executable (name consumer) (libraries middle prelude))
EOF
$ cat > consumer/consumer.ml <<'EOF'
let () = match Middle.pick () with
  | Green -> print_endline "g"
  | Red | Blue -> print_endline "nb"
EOF
$ dune build --sandbox=copy consumer/consumer.exe

middle.mli surfaces color, which is Prelude.color, but middle is built
with (flags (:standard -open Prelude)) so the token Prelude appears in no
source file under middle/, and consumer never names Prelude either. The
consumer's compile genuinely needs prelude.cmi to resolve the color
constructors, but ocamldep -modules reports Prelude for neither side, so
the per-module BFS in #14516 never reaches prelude, and none of #14517's
three recovery branches re-add it (prelude is not wrapped, not a ppx-runtime
lib, not a virtual-impl).

The --sandbox=copy build is the discriminator here, since a copy sandbox
contains exactly the rule's declared deps. That is the same soundness oracle
the in-stack fixtures use (cross-lib-preprocess-barrier.t,
wrapped-from-vlib-soundness.t). Built from the stack heads:

dune state	what this commit is, in plain terms	dune build --sandbox=copy consumer/consumer.exe
d57d159	stock, before the per-module dep filter	exit 0
4c95b95	#14516 head: the per-module filter on, recovery not yet added	exit 1, Error: Unbound constructor "Green"
a94a5c6	#14517 head: filter plus the must_glob_libs recovery	exit 1, Error: Unbound constructor "Green"

#14517 fails identically to #14516, so the recovery layer does not close this
class.

Mechanism, from dune rules on the consumer compile action:

• stock d57d159 deps include glob(prelude/.prelude.objs/byte/*.cmi), so
  prelude's cmis are tracked.
• fix: soundness recovery for wrapped libs, ppx-runtime, virtual-impl deps #14517 deps are {ocamlc/ocamlopt, Consumer.cmi, consumer.ml, File middle/.middle.objs/byte/middle.cmi}. prelude.cmi is absent from the
  tracked dep set. The action still carries -I prelude/.prelude.objs/byte
  (the cctx-wide includes feat: per-module library dependency filter via ocamldep BFS #14516 leaves unnarrowed), which is why the
  non-sandbox build still succeeds: the compiler finds prelude.cmi on disk
  via -I even though it is not a declared dependency. So on fix: soundness recovery for wrapped libs, ppx-runtime, virtual-impl deps #14517 today
  this is a silent under-invalidation (a latent stale-rebuild hazard), and a
  hard error only under the sandbox oracle.

The source-level reason the BFS cannot see it: Ocaml_flags .extract_open_module_names parses -open from the consumer's own flags only;
it does not consult the dep library middle's stanza flags, and ocamldep on
middle's source emits nothing for Prelude because it is -open'd away.

One rebuttal worth pre-empting: it is not a missing user declaration. With
consumer/dune set to (executable (name consumer) (libraries middle prelude)) (the dep declared explicitly), #14517 still drops prelude.cmi
from the consumer compile rule and --sandbox=copy still fails Unbound constructor "Green". The library closure is correct; the per-module cmi-dep
derivation is what is incomplete. prelude.cmi is the only supplier of the
color constructors (middle.cmi only references Prelude.color), so this
is a dropped-only-supplier case, not a precision win over a redundant edge.

On scope, to keep this honestly bounded:

• Not a hard regression on fix: soundness recovery for wrapped libs, ppx-runtime, virtual-impl deps #14517 today: default dune build consumer/consumer.exe is exit 0 (the wide -I masks the missing tracked
  dep).
• It is a hard failure under --sandbox=copy / --sandbox=hard on fix: soundness recovery for wrapped libs, ppx-runtime, virtual-impl deps #14517
  today, which is the same symptom class as the cram tests feat: per-module library dependency filter via ocamldep BFS #14516 breaks and
  fix: soundness recovery for wrapped libs, ppx-runtime, virtual-impl deps #14517 restores.
• It becomes a hard default-build regression at feat: per-module [-I]/[-H] include flags via [kept_libs] #14518: building that layer
  (8634a3050) and running the same fixture, the default (non-sandbox) build
  is exit 1 Unbound constructor "Green" and the consumer action's -I no
  longer includes prelude/..., since filtered_include_flags restricts
  -I/-H to kept_libs and prelude is not in kept_libs. So the gap is
  load-bearing for the next layer in the stack, not sandbox-only academic.

One structural note that might be useful for the fixture set rather than the
fix: the existing -open tests (wrapped-reexport-via-open-flag.t,
auto-wrapped-child-reexport.t) put -open on the wrapper/consumer side and
re-export through a module alias via a wrapped lib, both of which the recovery
does handle (exit 0 here, including under --sandbox=copy). Their oracle is
dune build @check plus rebuild-counting, and under that non-sandbox oracle
this fixture is invisible on #14517 (the @check alias build is exit 0).
--sandbox=copy (the stack's own convention in the soundness fixtures) is
what surfaces it.
The unwrapped-intermediate, type-via-.mli-inference, token-absent
conjunction is the shape that slips past both the BFS and the current -open
fixtures.

I have a discriminating cram fixture in the per-module-lib-deps/ style using
the --sandbox=copy convention (fails pre-recovery and still fails on #14517
head; would pass once the class is recovered). Happy to attach it if it is
useful, or to defer entirely on where the recovery belongs.

[robinbb]

Thanks for the careful reproduction — the gap is real and now fixed.

The diagnosis matches what I see: ocamldep on middle's source emits no token for Prelude because the -open removes the need to name it, so the BFS cannot reach prelude, and prelude is not wrapped / not a ppx-runtime lib / not a virtual-impl, so none of the three existing must_glob_libs recoveries catch it. Under --sandbox=copy the consumer's compile fails at L5 head; at L6 it becomes a hard default-build regression once filtered_include_flags removes the wide -I that was masking the missing tracked dep.

The fix sits inside the BFS rather than alongside the existing must_glob_libs set: the per-iteration step now extends the frontier with the modules named by each visited library's stanza -open flags, in addition to the entry's impl + intf ocamldep raw refs. The reachability rule becomes a three-way disjunction (consumer references it, or some reached module's ocamldep names it, or some reached module's owning lib stanza-opens it), which closes the gap through the same fixpoint that handles ocamldep-visible references. Chosen over the alternative — extending the BFS's initial referenced set with stanza opens of all cctx libs — because the BFS-internal version is both more precise (only reached libs contribute) and reads more clearly in code.

Regression test landed at test/blackbox-tests/test-cases/per-module-lib-deps/cross-lib-open-flag-barrier.t — same shape as your fixture, asserts dune build --sandbox=copy consumer/consumer.exe succeeds. Fails on L4 and L5 head before this patch (Unbound constructor Green), passes after. The fourth recovery class is also documented in doc/dev/per-module-narrowing.md (the algorithm reference added in L9).

Commit: 34a19142a3 fix: BFS-expand through dep libs' stanza -open modules on L5; the L6..L9 chain has been rebased and #14492 cherry-picked to match.

[robinbb]

@RyanJamesStewart Please verify whether the cram test that you volunteered to attach is covered by the newly added test cases. Are you an LLM that is running wild, fixing "highly active" PRs? What criteria do you use to find PRs to which to contribute? Does @stuboo monitor your work?

[RyanJamesStewart]

On the test: the fixture I offered is the one you have added as cross-lib-open-flag-barrier.t. Same prelude/middle/consumer shape, same (wrapped false) and stanza -open, same --sandbox=copy oracle, so it is covered and a separate attachment would be redundant. The one case it does not pin down is the #14518 layer, where the same construction becomes a hard default-build regression once filtered_include_flags drops the wide -I. If a guard there is useful I am happy to add one.

On the rest: I am not an LLM running wild. I post under my full name and stand behind what I post under my own judgment. My criteria for participating in a PR are deliberately narrow: I sweep open issues and recently merged PRs, look for a load-bearing case the existing patches do not cover, reproduce it myself under the same oracle the in-tree tests use, and only post when the finding is clearly outside what is already addressed. I set aside far more than I send. I have no idea who stuboo is. This is my first contribution here; it will not be the last.

[stuboo]

I have no connection to this account or this repository.

[robinbb]

I sweep open issues and recently merged PRs, look for a load-bearing case the existing patches do not cover, reproduce it myself under the same oracle the in-tree tests use, and only post when the finding is clearly outside what is already addressed. I set aside far more than I send.

That is a lot of work, @RyanJamesStewart! What inspires you to go to such lengths?

(In any case, I appreciate your having found a weakness in this PR.)

[robinbb]

I have no idea who stuboo is.

@stuboo Is the user who committed the following to your (@RyanJamesStewart's) time tracker repo: RyanJamesStewart/time-tracker@77cfb81

[robinbb]

I have no connection to this account or this repository.

@stuboo Perhaps a bot has falsely claimed to have made this commit by you: RyanJamesStewart/time-tracker@77cfb81

[Copilot]

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated no new comments.

[Alizter]

@robinbb Do we have a test for -open Foo.Bar, I'm interested in seeing how the new detection will handle such cases.

[Alizter]

Other than my question above, this LGTM

[robinbb]

@robinbb Do we have a test for -open Foo.Bar, I'm interested in seeing how the new detection will handle such cases.

No, and in fact we did not handle such cases properly. I have created a test for exactly this, and will assign the PR to you. The fix is in the "L4" PR (this is the "L5" PR).

[robinbb]

Since we now have a test for -open Foo.Bar, I'm going to interpret the status as safe to merge.