Skip to content

ci(guard): a sibling crate can arm a test target gated on a runner capability - #3246

Open
noahgift wants to merge 2 commits into
mainfrom
PMAT-1098-dev-dep-feature-leak
Open

noahgift wants to merge 2 commits into
mainfrom
PMAT-1098-dev-dep-feature-leak

Conversation

@noahgift

Copy link
Copy Markdown
Contributor

crates/aprender-test-lib/tests/falsify_chromium_driver_is_real.rs opens with
#![cfg(feature = "browser")], and its own header states the containment:

These require Chrome/Chromium on PATH. They are NOT on ci.yml's beat list, because
the clean-room image is not known to ship a browser; they belong on a Chrome-equipped
runner the way the GPU falsifiers belong on a CUDA one. Running them without a
browser FAILS — deliberately. There is no skip.

A cargo feature could not hold that. crates/aprender-orchestrate's [dev-dependencies]
line 10:

jugar-probar = { path = "../aprender-test-lib", package = "aprender-test-lib", features = ["browser"] }

The BSE-17 quick tier runs one nextest invocation with --lib --tests over the whole
selection, and --tests builds dev-dependencies. A PR touching both crates unified
browser on, compiled aprender-test-lib's own gated target, and ran three browser
tests on a browser-less clean-room runner — 30,281 passed, 3 failed, on the operator's
priority PR (#3205, run 34803859272).

Feature unification is a property of the build graph. "Default features" is a property
of a package. ci_test_tier.sh's header asserts the second over the first.

Measured, not suspected

33 such edges today, over 22 (crate, feature) gates. The enabler reaches the feature
two different ways, and covering only one misses half the surface:

spelling example
features = ["browser"] on the dependency aprender-orchestrate [dev]
a feature table entry probar/browser aprender-test-cli [normal]

optional = true does not exempt an edge either — it means "off until a feature turns it
on", which is exactly how aprender-cgp reaches aprender-gpu/cuda.

A ratchet, not a hard fail

Most of the 33 are probably harmless: a crate the quick tier never selects, or a gate whose
capability every runner has. Failing on all 33 would be an assertion the tree has not
earned. So the baseline is the measured number, it only ever shrinks, and a new edge is
what fails — named:

NEW since the baseline:
  aprender-distribute        [dev   ] -> aprender-test-lib/gpu    via features=[] on the dependency

That is a real run: adding "gpu" to aprender-distribute's dev-dep, then reverting it.

The case table

Six rows over committed fixture manifests, no cargo. R3 is the discrimination row
identical manifests, and the only difference is that the feature gates no test target, so
there is nothing to arm and nothing to report. R4 holds that a crate arming its own
target is not a leak. R5 holds that an optional dependency still counts. E1 makes an
unreadable input ENV rather than an empty (and therefore passing) answer.

Two placement decisions

  • The jq program is its own file (scripts/lib/feature_leak_edges.jq). Inline, bashrs
    lints the guard as shell and reads jq's $r[0] / $deps[] as unbraced array expansions:
    nine SC1087 errors on a program that is not shell. Silencing a false positive by
    rewriting the code is worse than the finding; removing it by putting the jq where jq
    belongs is just correct. bashrs: 0 errors.
  • Wired in guard-cargo, not guard-tree. ci.yml runs guard_tree.sh --no-cargo, so a
    cargo-reading guard placed there would never be dispatched — the class this repo calls "a
    facility with a self-test and no caller". cargo metadata --no-deps means no build.

Vacuity floor: fewer than 5 feature-gated test targets found means the scan is broken,
not that the tree is clean — and a broken scan reports zero edges, which reads exactly like
a pass.

Verified: --self-test 6/6; check_guards_are_wired.sh PASS (baseline did not grow);
check_baseline_ratchets.sh classifies the new baseline as set; the ratchet fires and
names a synthetic new edge.

Refs #3242

no-close: #3242 stays OPEN. This is the guard it asks for, but the ticket also owns the
un-run browser falsifiers (no runner in the fleet has Chromium) and the 33 existing edges,
none of which this PR examines.

🤖 Generated with Claude Code

…pability

crates/aprender-test-lib/tests/falsify_chromium_driver_is_real.rs opens with
`#![cfg(feature = "browser")]` and its own header states the containment: "NOT on
ci.yml's beat list, because the clean-room image is not known to ship a browser;
they belong on a Chrome-equipped runner the way the GPU falsifiers belong on a
CUDA one. Running them without a browser FAILS -- deliberately. There is no
skip."

A cargo feature could not hold that. crates/aprender-orchestrate's
[dev-dependencies] line 10:

    jugar-probar = { path = "../aprender-test-lib",
                     package = "aprender-test-lib", features = ["browser"] }

The BSE-17 quick tier runs ONE nextest invocation with `--lib --tests` over the
whole selection, and `--tests` builds dev-dependencies. A PR touching both crates
unified `browser` on, compiled aprender-test-lib's OWN gated target, and ran
three browser tests on a browser-less clean-room runner: 30,281 passed, 3 failed,
on the operator's priority PR (#3205, run 34803859272).

Feature unification is a property of the BUILD GRAPH. "Default features" is a
property of a PACKAGE. ci_test_tier.sh's header asserts the second over the
first, and this is the gap.

MEASURED, NOT SUSPECTED: 33 such edges today over 22 (crate, feature) gates. The
enabler reaches the feature two different ways and covering only one misses half
the surface --

  features = ["browser"] on the dependency        aprender-orchestrate  [dev]
  a feature table entry `probar/browser`          aprender-test-cli     [normal]

-- and `optional = true` does not exempt an edge: it means "off until a feature
turns it on", which is how aprender-cgp reaches aprender-gpu/cuda.

A RATCHET, NOT A HARD FAIL. Most of the 33 are probably harmless: a crate the
quick tier never selects, or a gate whose capability every runner has. Failing on
all 33 would assert something the tree has not earned. So the baseline is the
measured number, it only ever shrinks, and a NEW edge is what fails -- with the
new edge named:

    NEW since the baseline:
      aprender-distribute  [dev] -> aprender-test-lib/gpu  via features=[] ...

(that line is a real run, from adding `"gpu"` to aprender-distribute's dev-dep and
reverting it.)

Six rows over committed fixture manifests, no cargo. R3 is the discrimination
row: identical manifests, and the only difference is that the feature gates no
test target -- nothing to arm, nothing to report. R4 holds that a crate arming
its OWN target is not a leak.

The jq program lives in scripts/lib/feature_leak_edges.jq rather than inline
because bashrs lints the guard as shell and reads jq's `$r[0]` / `$deps[]` as
unbraced array expansions: nine SC1087 errors on a program that is not shell.
Silencing a false positive by rewriting the code would be worse than the finding;
removing it by putting the jq where jq belongs is just correct. bashrs: 0 errors.

Wired in guard-cargo, not guard-tree: ci.yml runs `guard_tree.sh --no-cargo`, so
a cargo-reading guard placed there would never be dispatched -- the class this
repo calls a facility with a self-test and no caller. `--no-deps` means no build.

Vacuity floor: fewer than 5 feature-gated test targets found means the SCAN is
broken, not that the tree is clean -- and a broken scan reports zero edges, which
reads exactly like a pass.

Refs #3242

Pmat-Ticket: PMAT-1098

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@noahgift
noahgift enabled auto-merge September 14, 2026 05:10
@github-actions

github-actions Bot commented Sep 14, 2026

Copy link
Copy Markdown

§13.11 rung 1 — quorum shadow verdict

S13-SHADOW pr=3246 head=eaf9043e6b62f8fc18bdc1ca3db2a51b62877222 verdict=REFUSE class=Q1 arm_rc=1

Shadow mode: this records a verdict and merges nothing. A refusal
to arm is not a block (§13 adds zero rows to §7) — the pull request is
exactly as green as it was.

noahgift added a commit that referenced this pull request Sep 14, 2026
…ped the other

workspace-test failed again on this PR, run 34806202374, same class and a
different file:

    aprender-test-lib::falsify_playbook_drives_a_browser
      a_playbook_click_changes_the_real_page
      evaluate_is_decided_by_the_page
      screenshots_are_written_as_real_files
    could not launch a browser-backed executor

30,481 passed, 1 failed. Same mechanism as the previous commit:
aprender-orchestrate's dev-dependency carries features = ["browser"], the BSE-17
quick tier builds the selected crates' --lib --tests in ONE graph, and this
crate's own browser-gated target compiled and ran on a browser-less clean-room
runner.

The survey that found the first file printed `2 #![cfg(feature = "browser")]`.
I read the count and acted on one file. That is the same incomplete-fix shape
this branch's book.yml work is about -- "the fix stopped at the step that hurt"
-- committed one commit after writing that sentence.

Both targets now gate on `browser-falsify`. Verified in both directions:

    --features browser          --test falsify_playbook_drives_a_browser  ->  0 tests
    --features browser-falsify  --test falsify_playbook_drives_a_browser  ->  6 tests

and `grep -rln '^#!\[cfg(feature = "browser")\]' crates/*/tests/` is now 0, which
is the check I should have run the first time rather than reading a count.

#3246's ratchet is the mechanical version: `aprender-orchestrate [dev] ->
aprender-test-lib/browser` is in its baseline precisely because `browser` still
gated a target, so this change makes that baseline SHRINK. A count I read is not
a check; a baseline that moves is.

Pmat-Ticket: PMAT-1098

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@noahgift
noahgift added this pull request to the merge queue Sep 14, 2026
noahgift added a commit that referenced this pull request Sep 14, 2026
…as none

workspace-test, run 34809941798: 39,613 passed, 2 failed --

    aprender-verify-ml::book verification::oracles::test_io_oracle_example
    aprender-verify-ml::book verification::oracles::test_python_executor_example
      assertion failed: verdict.is_ok()

Third instance tonight of one class: a test that asserts a RUNNER CAPABILITY,
gated by nothing, compiled and run because the BSE-17 quick tier reached its
crate. The browser falsifiers were the first two.

MEASURED, not inferred, on both sides of the seam:

    docker run --rm localhost:5000/sovereign-ci:stable command -v python3
      -> NO_PYTHON3   (and NO_PYTHON; rustc IS present)

and the SAME built test binary, run twice:

    with python3 on PATH       3 passed, 1 ignored
    with an empty PATH         3 FAILED, 1 ignored

So it is the interpreter, not the code, and not the platform.

CI reported 2 failures, not 3: `test_verification_with_input_example` did not run
in that selection. It fails identically without an interpreter -- the run above is
how I know -- so the gate covers the module, not the two rows that happened to be
reported. Fixing only what the log named is what left the second browser falsifier
armed two commits ago.

`python-oracle`, on `mod oracles;`, and nothing enables it. A capability needs a
name no sibling reaches by accident; `browser-falsify` in aprender-test-lib is the
same shape, and #3246's ratchet is the mechanical version of not letting one leak.

NO SKIP. Where `python-oracle` is on, the rows still fail hard without python3 --
they are examples from the book and their whole point is that the executor really
executes. Verified both directions:

    (default)                  --test book verification::   -> 0 tests, module compiled out
    --features python-oracle   --test book ...::oracles     -> 3 passed, 1 ignored

Also dark, and not fixed here: `grep -c aprender-verify-ml .github/workflows/ci.yml`
is 0, so this target has never run in CI. It surfaced only because this PR's
selection reached the crate. And `test_timeout_handling_example` carries a bare
`#[ignore]` ("Timeout handling currently hangs - needs executor fix") which the
repo bans; it predates this change and needs its own ticket.

The close-out is python3 in the clean-room image, then arm the feature. Measure
first, assert second.

Pmat-Ticket: PMAT-1098

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@noahgift noahgift added this to the 0.68.0 milestone Sep 14, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Sep 14, 2026
@noahgift
noahgift added this pull request to the merge queue Sep 14, 2026
@noahgift
noahgift removed this pull request from the merge queue due to a manual request Sep 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A dev-dependency's feature arms a sibling's runner-gated test: the BSE-17 quick tier ran three browser falsifiers on a browser-less runner

1 participant