ci(gate): lint the shipped configuration, not only the test one (#1418) - #1419
Merged
Merged
Conversation
Both lint sites ran exactly one clippy pass, `--workspace --no-default-features
--all-targets -- -D warnings`. `--all-targets` puts dev units in scope, and resolver 2
then unifies dev-dependency features into the normal build. Three dev-deps enable
`openpulse-modem`'s `instruments` feature — its own Cargo.toml:41, openpulse-daemon:92,
openpulse-kiss:59 — and under `--workspace` any one suffices. So the lib was always
linted with `instruments` ON, while `cargo build --release --no-default-features`
(release.yml:60) links it OFF. No pass linted the shipped configuration.
Not cosmetic. An ungated production caller of an instruments-only accessor is invisible
to BOTH automated checks and breaks the release build:
gate cargo clippy --workspace --no-default-features --all-targets -D warnings -> rc=0
hook cargo test -p openpulse-modem --no-default-features --no-run -> rc=0
ship cargo clippy -p openpulse-daemon --no-default-features -D warnings -> rc=101 E0599
Corollary: #1277's "a production call cannot compile without a visible Cargo.toml diff"
was enforced by release builds alone.
An ADDED pass, not a changed flag — dropping `--all-targets` would stop linting test
code, the rot that put an unused binding in session_key.rs. It stays `--workspace`: a
DOWNSTREAM crate's production code calling an instruments item also escapes, because
that crate's lib builds against the ON modem. ~1 s warm, 9.1 s from a cold modem lib.
The hook gets it too because it is the only check that runs on every push (#1144).
engine.rs carries the two items the new pass then found, both already unreachable in
shipped builds: five `openpulse_core::fec` imports move to an instruments-gated `use`,
and `stage_modulate_payload_iq` takes the same cfg as its only caller, `transmit_iq`.
No product behaviour changes.
Sabotage-verified, and the control is what makes it attributable: with the probe
planted, `gate.sh --quick` prints `cargo clippy -D warnings ok` for the OLD step and
`cargo clippy (shipped cfg) -D warns FAILED (exit 101)` for the NEW one.
Fable review corrected three things in my framing before this was built: the three
enabling dev-deps (I had blamed #1277's self dev-dep alone, whose removal would not have
closed it), `--all-targets` being a proxy for "dev units in scope" (`--tests` alone
flips it), and that during a gate run downstream crates link the ON lib — so the claim
is about shipped binaries, not downstream crates.
Verification-objective: the workspace lint must cover the feature configuration the
release build links, not only the one that includes dev units
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1418.
Both lint sites ran exactly one clippy pass,
--workspace --no-default-features --all-targets -- -D warnings.--all-targetsputs dev units in scope, and resolver 2 then unifies dev-dependencyfeatures into the normal build. Three dev-deps enable
openpulse-modem'sinstrumentsfeature —crates/openpulse-modem/Cargo.toml:41,openpulse-daemon:92,openpulse-kiss:59— and under--workspaceany one suffices. So the lib was always linted withinstrumentsON, whilecargo build --release --no-default-features(release.yml:60) links it OFF. Nothing linted theshipped configuration.
Not cosmetic — what the hole admits
An ungated production caller of an instruments-only accessor is invisible to both automated
checks and breaks the release build:
Corollary: #1277's "a production call cannot compile without a visible
Cargo.tomldiff" wasenforced by release builds alone — not by the gate, not by the hook.
The change
An added pass, not a changed flag. Dropping
--all-targetswould stop linting test code — the rotthat put an unused binding in
session_key.rs. It stays--workspace, because a downstream crate'sproduction code calling an instruments item escapes too: that crate's lib builds against the ON modem.
~1 s warm, 9.1 s from a cold modem lib. The hook gets it as well, since it is the only check that runs
on every push (#1144).
engine.rscarries the two items the new pass then found, both already unreachable in shipped builds:five
openpulse_core::fecimports move to an instruments-gateduse, andstage_modulate_payload_iqtakes the same cfg as its only caller,
transmit_iq. No product behaviour changes.Sabotage-verified, in both directions
The control is what makes the failure attributable — with the probe planted, the OLD step still passes:
And on a clean tree the new step reports
ok, so it is not a step that always fails.Test results
scripts/gate.shon the branch tip, clean tree:All 12 steps
ok, including the new one. held-out (runtime, #1274): notch_rescues_interferer, ota_channel_adaptation — run scripts/slow-tests.shNot covered by this run, unchanged by this PR: the two
#[ignore]d acceptance suites above.Honesty note
The first gate run on this branch failed, on my own review artifact's frontmatter (
doc:not equalto the path,
project:missing,status: livingwithout a manifest entry). Fixed and amended; theverdict above is the re-run. Worth stating plainly in a PR that adds a gate step.
Scope
This makes the shipped configuration linted. It was already type-checked in
ci.yml's macOS build,cross checkandrelease.yml— all release-scoped. Do not read this as "the OFF configuration wasnever compiled".
#1380 stays open. That is the inverse case — a feature that is OFF in the gate, whose code
is never type-checked — and it still needs its own
--features cpal-backendstep. The comment on #1380records the distinction.
Verification-objective: the workspace lint must cover the feature configuration the release
build links, not only the one that includes dev units
Review: docs/dev/reviews/review-1418-shipped-config-lint.md
🤖 Generated with Claude Code
https://claude.ai/code/session_0188ATCj6DZ9aRVQ2vSirua6