ci(gate): lint the shipped configuration, not only the test one (#1418)

.cargo-husky/hooks/pre-push

@@ -74,6 +74,14 @@ scripts/check-rehomed-docs.sh --base "${upstream:-origin/main}"
 
 cargo clippy --workspace --no-default-features --all-targets -- -D warnings
 
+# The shipped (feature-off) configuration — see scripts/gate.sh for the full why (#1418). The line
+# above cannot see it: `--all-targets` pulls dev units in, and dev-dep features then unify into the
+# normal build, so the lib is linted with `instruments` ON while the release binary links it OFF.
+# It belongs here and not only in the gate because this hook is the only check that runs on EVERY
+# push (#1144), and the pass costs ~1 s warm. Measured: without it, this hook returns 0 on a planted
+# E0599 that breaks `cargo build --release --no-default-features`.
+cargo clippy --workspace --no-default-features -- -D warnings
+
 if [ -z "${pkgs// /}" ]; then
     echo "pre-push: no crate-owned changes detected; fmt + clippy only."
     exit 0

crates/openpulse-modem/src/engine.rs

@@ -14,9 +14,16 @@ use openpulse_core::conv::ConvCodec;
 use openpulse_core::dcd::DcdState;
 use openpulse_core::error::{ModemError, PluginError};
 use openpulse_core::fec::{
-    apply_window_retransmit, combine_llrs_map, combine_llrs_map_in_ranges,
-    encode_window_retransmit, hard_decide, FecCodec, FecMode, Interleaver, ShortFecCodec,
-    SoftCombiner, WindowArqFeedback, DEFAULT_INTERLEAVER_DEPTH,
+    combine_llrs_map, hard_decide, FecCodec, FecMode, Interleaver, ShortFecCodec,
+    DEFAULT_INTERLEAVER_DEPTH,
+};
+// Reached only from `#[cfg(feature = "instruments")]` methods, so the shipped library does not use
+// them (#1418). They must carry the same cfg or the feature-off build warns — which no lint pass saw
+// until #1418 added one, because `--all-targets` turns `instruments` on.
+#[cfg(feature = "instruments")]
+use openpulse_core::fec::{
+    apply_window_retransmit, combine_llrs_map_in_ranges, encode_window_retransmit, SoftCombiner,
+    WindowArqFeedback,
 };
 use openpulse_core::frame::Frame;
 use openpulse_core::hpx::{HpxEvent, HpxSession, HpxState, HpxTransition};
@@ -6927,6 +6934,10 @@ impl ModemEngine {
     }
 
     /// Baseband-I/Q counterpart of [`stage_modulate_payload`](Self::stage_modulate_payload).
+    ///
+    /// Its only caller is [`transmit_iq`](Self::transmit_iq), which is itself instruments-only, so
+    /// this carries the same cfg (#1418).
+    #[cfg(feature = "instruments")]
     fn stage_modulate_payload_iq(
         &self,
         plugin: &dyn openpulse_core::plugin::ModulationPlugin,

docs/dev/project/traceability.md

@@ -15,6 +15,58 @@ and the actually-observed results per change.
 
 ---
 
+## 2026-09-19 — the gate's `--all-targets` hid the shipped configuration; #1418
+
+**Change.** `scripts/gate.sh` and `.cargo-husky/hooks/pre-push` each 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, so the
+`openpulse-modem` lib was always linted with `instruments` **on** — while
+`cargo build --release --no-default-features` (`release.yml:60`) links it **off**. No pass anywhere
+linted the shipped configuration.
+
+**Design decision (reviewed by Fable, `docs/dev/reviews/review-1418-shipped-config-lint.md`).** Add a
+second pass rather than change the flag, and keep it `--workspace`. Dropping `--all-targets` would
+stop linting test code — the rot that put an unused binding in `session_key.rs`. Narrowing to
+`-p openpulse-modem --lib` would miss a *downstream* crate's production code calling an instruments
+item, because that crate's lib builds against the ON modem. The review corrected three things in my
+framing: the feature has **three** enabling dev-deps (`openpulse-modem/Cargo.toml:41`,
+`openpulse-daemon:92`, `openpulse-kiss:59`), not just #1277's self dev-dep, so removing that one
+would not have closed it; the trigger is *dev units in scope*, for which `--all-targets` is only a
+proxy (`--tests` alone flips it, `--bins`/`--lib` do not); and during a gate run downstream crates
+link the **ON** lib, so the claim is about the shipped binaries, not about downstream crates.
+
+**Severity, corrected.** Filed first as two cosmetic warnings; it is a build-break class. 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.toml` diff" was enforced by **release builds alone**, not by the gate or the hook.
+
+**Implementation.** `scripts/gate.sh:192` adds
+`run_step "cargo clippy (shipped cfg) -D warns" cargo clippy --workspace --no-default-features -- -D warnings`;
+`.cargo-husky/hooks/pre-push:76` adds the same command (it is the only check on every push, #1144, and
+costs ~1 s warm). `crates/openpulse-modem/src/engine.rs` gates the two items the new pass then found:
+five `openpulse_core::fec` imports moved to an `#[cfg(feature = "instruments")]` `use`, and
+`stage_modulate_payload_iq` (:6930, whose only caller `transmit_iq` is instruments-only) given the
+same cfg.
+
+**Tests → results (actually run, at `8b5896fa` + this branch).**
+
+- *Sabotage, the discriminating one* — planted `pub fn zz_probe_1418(e: &ModemEngine) -> u64 { e.notch_blocks_processed() }`
+  in `engine.rs`, then `scripts/gate.sh --quick`: the OLD step printed
+  `cargo clippy -D warnings  ok` and the NEW step printed
+  `cargo clippy (shipped cfg) -D warns  FAILED (exit 101)`. The old step passing is what makes the
+  failure attributable to the new pass rather than to the probe being loud.
+- Pre-fix, both live instances under the new pass: `cargo clippy --workspace --no-default-features`
+  → 2 warnings (`--all-targets` → 0). Post-fix: both passes rc=0.
+- Per-unit features read from cargo's `--message-format=json` `compiler-artifact` records:
+  `--all-targets` and `--tests` → `['instruments']`; `--bins`, `--lib`, no-target-flag → `[]`.
+- Full gate: see the `GATE:` line recorded on PR #1419.
+
+**Scope, stated narrowly.** This makes the shipped configuration *linted*; it was already
+*type-checked* in `ci.yml`'s macOS build, `cross check` and `release.yml` — all release-scoped. Do not
+read this entry as "the OFF configuration was never compiled".
+
+---
+
 ## 2026-09-19 — CAP-33 does not own the engine's OTA arm, and should not; #1403
 
 **Decision (maintainer, 2026-09-19): no map change.** `Refactors:` is **structural** — #1402

docs/dev/reviews/review-1418-shipped-config-lint.md

@@ -0,0 +1,86 @@
+---
+project: openpulsehf
+doc: docs/dev/reviews/review-1418-shipped-config-lint.md
+status: resolved
+last_updated: 2026-09-19
+---
+
+# Review — #1418, linting the shipped (feature-off) configuration
+
+## Prompt
+
+Fable was asked to **falsify**, not confirm, a finding and its write-up before either entered an
+issue. The framing sent was: "the gate's own `--all-targets` turns the `instruments` feature ON, so
+the library is never linted in the configuration downstream crates and shipped binaries link." It was
+given the full apparatus (five clippy invocations with their warning counts), the draft issue text,
+and six numbered attack points — the sharpest being #4, *"is 'never linted in the configuration
+downstream crates link' actually true? Downstream workspace crates build openpulse-modem as a
+dependency during the gate run — does that build see instruments on or off? If off, my headline claim
+is wrong. Test this hard."* House rules on pipelines-never-carry-verdicts, positive controls for
+grep-derived absences, and no build dirs in the worktree were included.
+
+## Verdict
+
+**Finding upheld, three corrections, and a re-classification from cosmetic to build-break.**
+
+1. **Mechanism confirmed but mis-attributed.** I blamed the #1277 self dev-dep alone. There are
+   **three** sites enabling `instruments`: `crates/openpulse-modem/Cargo.toml:41`,
+   `crates/openpulse-daemon/Cargo.toml:92`, `crates/openpulse-kiss/Cargo.toml:59`. Under `--workspace`
+   any one suffices, so removing the self dev-dep would not have closed it. Verified independently by
+   grep over all `Cargo.toml`.
+2. **`--all-targets` is a proxy, not the trigger.** The trigger is *dev units in scope*; `--tests`
+   alone flips it, `--bins`/`--lib` do not. Resolved per-unit from cargo's `--message-format=json`
+   `compiler-artifact` records — `cargo tree -e features` cannot discriminate, as it prints the dev
+   edge either way.
+3. **Point 4 was half-wrong, and the correction matters.** During the gate run there is exactly ONE
+   non-test `openpulse_modem` lib unit, features `['instruments']`, and all 14 dependents link it — so
+   in the gate, downstream crates link the **ON** lib. It is the **shipped binaries** (`release.yml:60`)
+   that link OFF. "Never linted" survives; "downstream crates link OFF" does not, and the issue text
+   was changed accordingly. Also corrected: OFF *is* type-checked (macOS build, `cross check`,
+   `release.yml`) — never **linted**. Do not let "never linted" harden into "never compiled".
+4. **Severity raised.** I had filed it as two cosmetic warnings. A planted **ungated production
+   caller** of an instruments-only accessor is invisible to both automated checks and breaks the
+   release build — re-derived here rather than taken on trust:
+
+   ```
+   cargo clippy --workspace --no-default-features --all-targets -- -D warnings  -> rc=0    (gate)
+   cargo test  -p openpulse-modem --no-default-features --no-run                -> rc=0    (hook)
+   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" is
+   enforced by **release builds alone**.
+5. **Fix shape cleared, with two constraints.** Must be an **added** pass (dropping `--all-targets`
+   stops linting test code — the `session_key.rs` rot), and must be **`--workspace`**: narrowing to
+   `-p openpulse-modem --lib` would miss a downstream crate's production code calling an instruments
+   item, since that crate's lib builds against the ON modem. Runtime 9.1 s cold / ~1 s warm.
+6. **Bottom line: file separately, not as a comment on #1380** — #1380 closes with a
+   `--features cpal-backend` PR that would leave this buried under a closed issue, and the fix is a
+   different gate step. Maintainer agreed; filed as #1418 and the #1380 comment reduced to a
+   cross-reference.
+
+Four line numbers in my draft were off by ~1; Fable's were right and were used.
+
+## Consumer
+
+`scripts/gate.sh:192` (the new `run_step`) and `.cargo-husky/hooks/pre-push:76`. Both are the direct
+production callers — this change *is* verification machinery, so its consumer is the gate itself. The
+downstream consumer of the property is `.github/workflows/release.yml:60`
+(`cargo build --release … --no-default-features`), the build whose configuration went unlinted.
+
+## Prior art
+
+`grep -n "clippy" scripts/gate.sh .cargo-husky/hooks/pre-push` → one invocation each, both
+`--all-targets`, no feature-off pass anywhere. `grep -RniE "all-targets|dev-depend" skills/` over the
+skills tree (with a known-present control phrase to prove the filter fires) → no existing rule. #1380
+is the nearest existing mechanism and is the **inverse** case (feature off, code unchecked); its
+proposed `cargo check --features cpal-backend` does not reach this.
+
+## Twins
+
+The hook is the twin of the gate and got the same pass — it is the only check that runs on every push
+(#1144), and it was measured passing the planted E0599. The other candidate twins were checked and are
+**not** affected: an awk census over every tracked `Cargo.toml`, scoped to `[dev-dependencies]`
+sections enabling features on a workspace sibling, returns exactly the three `instruments` sites and
+nothing else — so no other crate has this shape. `ci.yml`'s release-scoped jobs already compile OFF. `openpulse-cli`'s `cpal-backend` is the #1380 case, tracked
+separately.

scripts/gate.sh

@@ -189,6 +189,21 @@ drift_check
 run_step "cargo fmt --check" cargo fmt --all -- --check || rc_total=1
 drift_check
 run_step "cargo clippy -D warnings" cargo clippy --workspace --no-default-features --all-targets -- -D warnings || rc_total=1
+drift_check
+# The SHIPPED configuration, which the step above structurally cannot see (#1418). `--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` (its own Cargo.toml:41,
+# openpulse-daemon:92, openpulse-kiss:59), and under `--workspace` any one suffices. So the step
+# above always lints the lib with `instruments` ON, while `cargo build --release
+# --no-default-features` (release.yml) links it OFF. Measured: an ungated production caller of an
+# instruments-only item returned rc=0 from the step above AND from the pre-push hook, and rc=101
+# (E0599) from `cargo clippy -p openpulse-daemon --no-default-features`.
+#
+# This is an ADDED pass, not a changed flag: dropping `--all-targets` above would stop linting test
+# code, which is how an unused binding sat in session_key.rs. It must stay `--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.
+run_step "cargo clippy (shipped cfg) -D warns" cargo clippy --workspace --no-default-features -- -D warnings || rc_total=1
 
 TEST_CMD="none"
 if [ "$MODE" = "full" ]; then