diff --git a/.cargo-husky/hooks/pre-push b/.cargo-husky/hooks/pre-push index 64abafaf..60da928f 100755 --- a/.cargo-husky/hooks/pre-push +++ b/.cargo-husky/hooks/pre-push @@ -82,6 +82,25 @@ cargo clippy --workspace --no-default-features --all-targets -- -D warnings # E0599 that breaks `cargo build --release --no-default-features`. cargo clippy --workspace --no-default-features -- -D warnings +# Feature-gated code, which neither pass above COMPILES at all (#1380) — see scripts/gate.sh for the +# full reasoning and the two stated limits. Here because rot is a per-push class and this hook is the +# only check that runs on every push (#1144). Worst case (an edit in openpulse-core) this re-checks +# the workspace in a second feature configuration, ~40 s measured; a leaf-crate edit is seconds. +# +# The preflight exists so a missing distro package reads as a missing distro package: --all-features +# pulls alsa-sys, libudev-sys and libdbus-sys, whose build scripts call pkg_config and PANIC, which +# would otherwise surface as a backtrace buried in clippy output. +missing_pc="" +for pc in alsa libudev dbus-1; do + pkg-config --exists "$pc" 2>/dev/null || missing_pc="$missing_pc $pc" +done +if [ -n "$missing_pc" ]; then + echo "pre-push: FAIL — --all-features needs pkg-config:$missing_pc" + echo "pre-push: install libasound2-dev libudev-dev libdbus-1-dev (Debian/Ubuntu names)" + exit 1 +fi +cargo clippy --workspace --all-features --all-targets -- -D warnings + if [ -z "${pkgs// /}" ]; then echo "pre-push: no crate-owned changes detected; fmt + clippy only." exit 0 diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 38b6b422..b4d4ab35 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -81,6 +81,16 @@ jobs: # enforced by no machine (CLAUDE.md "Known hole"). It cost #1074: four tests red on `main`, # plus a fifth gone vacuous, unnoticed. The hook now tests touched crates on every push; this # is the full gate, and it is the thing that actually closes the hole. + # `gate.sh`'s --all-features rot guard (#1380) links against ALSA, libudev and D-Bus through + # alsa-sys / libudev-sys / libdbus-sys, whose build scripts call pkg_config and PANIC when a + # .pc file is absent. The ubuntu runner image ships none of these three -dev packages, so + # without this step the gate fails on arrival. NOT `|| true`: a missing library here means the + # rot guard cannot run, and the gate's own preflight fails loudly rather than skipping it. + - name: Install feature-gate build dependencies + run: | + sudo apt-get update + sudo apt-get install -y libasound2-dev libudev-dev libdbus-1-dev + - name: Workspace gate (fmt + clippy + tests) run: ./scripts/gate.sh @@ -178,51 +188,11 @@ jobs: - name: Build workspace on macOS run: cargo build --workspace --no-default-features - gpu-feature-gates: - name: GPU feature gates (compile + lint) - runs-on: ubuntu-latest - if: ${{ startsWith(github.head_ref, 'release/') || github.event_name == 'workflow_dispatch' }} - - steps: - - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 - - - name: Install Rust toolchain - uses: dtolnay/rust-toolchain@4cda84d5c5c54efe2404f9d843567869ab1699d4 # stable - with: - toolchain: ${{ env.REQUIRED_RUST }} - - - uses: Swatinem/rust-cache@6323deb102c322ba6fcbdcafc7e3dddab59af2b6 # v2 - with: - # One cache per job, so a lint-only job cannot evict the test job's artifacts. - shared-key: ${{ github.job }} - - - name: Toolchain preflight - run: ./scripts/check-toolchain.sh $REQUIRED_RUST - - # The per-plugin `gpu` feature is never compiled by the --no-default-features - # gates above, so its `#[cfg(feature = "gpu")]` paths rot silently (PR #424 - # found a build break + clippy + flaky-test issue only reachable here). This - # job compiles and lints the GPU paths on every change. It does NOT run the - # GPU tests — CI runners have no wgpu adapter, so those would only exercise - # the CPU fallback; compile + lint is the rot guard. - - name: GPU feature build gate - run: > - cargo build --no-default-features --features gpu - -p bpsk-plugin -p qpsk-plugin -p psk8-plugin -p qam64-plugin -p scfdma-plugin - - - name: GPU feature clippy gate - run: > - cargo clippy --no-default-features --features gpu --all-targets - -p bpsk-plugin -p qpsk-plugin -p psk8-plugin -p qam64-plugin -p scfdma-plugin - -- -D warnings - - # openpulse-gpu's adapter-requiring differential tests are behind - # `hardware-tests`, so the workspace gate never compiles them. Same rot guard - # as above: compile + lint only, never run — this runner has no adapter. - - name: GPU hardware-test rot guard (compile + lint, not run) - run: > - cargo clippy --features hardware-tests --all-targets - -p openpulse-gpu -- -D warnings + # `gpu-feature-gates` was REMOVED here (#1380). It compiled and linted the `gpu` and + # `hardware-tests` paths, which `scripts/gate.sh`'s `--all-features` pass now covers for every + # crate rather than a named five — and covers on every LOCAL gate run and in post-merge-gate, + # where this job never ran at all, being `release/**`-scoped like the rest of this file + # (#1120/#1144). Its reasoning survives in that step's comment, PR #424 precedent included. pi5-smoke-loopback: name: Pi5 smoke profile (loopback) diff --git a/.github/workflows/post-merge-gate.yml b/.github/workflows/post-merge-gate.yml index 9e6a124e..ae53c467 100644 --- a/.github/workflows/post-merge-gate.yml +++ b/.github/workflows/post-merge-gate.yml @@ -70,6 +70,16 @@ jobs: # One verdict path — `scripts/gate.sh`, never an open-coded `cargo test`. Without # `--no-fail-fast` cargo stops at the first failing binary and the count is a lower bound # (CLAUDE.md verification rule 2); the script is what guarantees the flag and the `GATE:` line. + # `gate.sh`'s --all-features rot guard (#1380) links against ALSA, libudev and D-Bus through + # alsa-sys / libudev-sys / libdbus-sys, whose build scripts call pkg_config and PANIC when a + # .pc file is absent. The ubuntu runner image ships none of these three -dev packages, so + # without this step the gate fails on arrival. NOT `|| true`: a missing library here means the + # rot guard cannot run, and the gate's own preflight fails loudly rather than skipping it. + - name: Install feature-gate build dependencies + run: | + sudo apt-get update + sudo apt-get install -y libasound2-dev libudev-dev libdbus-1-dev + - name: Workspace gate (fmt + clippy + tests) id: gate run: ./scripts/gate.sh diff --git a/CLAUDE.md b/CLAUDE.md index 98331883..67ee4eaf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -251,7 +251,7 @@ Each requirement below is done when the linked test passes. Add new links as tes | A repeater that is not running is not reported as running (#1298) — enabling with nothing to run FAILS with a reason instead of emitting `RepeaterChanged { enabled: true }`, and a thread that exited is reaped so the next command sees the truth rather than "already enabled" forever. The thread OWNS the `CrossBandRepeater`, so its exit means the repeater is gone | `cargo test -p openpulse-daemon --no-default-features --lib command_apply_tests` | | The cross-band repeater receives what the DAEMON hears (#1297, #1308) — it holds no capture stream of its own; the daemon's accumulator flushes one burst to two consumers (monitor and repeater) over a bounded lossy channel. Three lifecycle cases, because **none of them worked before 2026-09**: `[repeater] enabled = true` relays with no command (startup used to set the flag and spawn nothing), `enable_repeater` starts a config-disabled one (it used to report success and return `Ok(0)` at once), and an "already enabled" refusal is only issued when it is true. Sabotage-verified: removing the startup spawn fails cases (a) and (c) and leaves (b) green; removing the burst `try_send` fails all three | `cargo test -p openpulse-daemon --no-default-features --test repeater_relays_a_daemon_burst` | | Every binary that builds a `ModemEngine` pins `[audio] device` (#1311, #1308) — ARDOP, KISS and the TUI passed a hardcoded `None` per call, so on a multi-card host they took the OS default. The daemon's own two **repeater** engines did the same until #1308 PR 3, which is the sharpest case: a cross-band repeater is by definition a two-card station, so the default is very likely the MAIN rig. The scan now matches the pin to the constructed **binding** (`let mut rx = …` needs `rx.set_default_device`) and blanks `#[cfg(test)]` modules — a line window alone passed with `rx`'s pin deleted, because it found `tx`'s 15 lines away. Validated against a planted violation, a planted fix, a planted in-test fixture, and three independent live sabotages | `cargo test -p openpulse-modem --no-default-features --test front_ends_pin_the_audio_device` | -| The file keystore is **available as the fallback** when no usable system secret store is present, and falling back is **never silent** (REQ-CTL-04's fallback clause, #1234). The selector takes a **three-state** probe — `NotCompiledIn \| Unreachable \| Reachable` — not a boolean, because a boolean cannot distinguish "no secret service on this host" from "no keychain in this BUILD", and the second is the case that ships: the gate builds `--no-default-features`, so `probe_keychain` really does return `NotCompiledIn` and the shipped binary really does take the fallback. That is what the fourth test pins, and it is why this is a gate rather than a truth table. `select_backend` is deliberately NOT behind `cfg(feature = "keychain")` — a selector compiled only when the feature is on is never type-checked by the gate, which is #1380's shape. **Stated limit: the `KeychainStore` body itself is still never type-checked here**, so #1380 is contained, not closed, and the OS-store surface stays manual-only (its sole evidence is the `#[ignore]`d `keychain_round_trip`; REQ-CTL-03's id was retired in #1234 while the requirement stands). Sabotage-verified three ways: making `NotCompiledIn` prefer the keychain fails the fallback test AND the gate's-own-build test; returning `None` for the reason, or making the two reasons identical, each fail the reason test | `cargo test -p openpulse-keystore --no-default-features store::selection` | +| The file keystore is **available as the fallback** when no usable system secret store is present, and falling back is **never silent** (REQ-CTL-04's fallback clause, #1234). The selector takes a **three-state** probe — `NotCompiledIn \| Unreachable \| Reachable` — not a boolean, because a boolean cannot distinguish "no secret service on this host" from "no keychain in this BUILD", and the second is the case that ships: the gate builds `--no-default-features`, so `probe_keychain` really does return `NotCompiledIn` and the shipped binary really does take the fallback. That is what the fourth test pins, and it is why this is a gate rather than a truth table. `select_backend` is deliberately NOT behind `cfg(feature = "keychain")` — a selector compiled only when the feature is on is never type-checked by the gate, which is #1380's shape. **Corrected 2026-09-20 (#1380): the `KeychainStore` body IS now type-checked**, by `gate.sh`'s `--all-features` pass — on Linux, and compile+lint only, and the OS-store surface stays manual-only (its sole evidence is the `#[ignore]`d `keychain_round_trip`; REQ-CTL-03's id was retired in #1234 while the requirement stands). Sabotage-verified three ways: making `NotCompiledIn` prefer the keychain fails the fallback test AND the gate's-own-build test; returning `None` for the reason, or making the two reasons identical, each fail the reason test | `cargo test -p openpulse-keystore --no-default-features store::selection` | | The ARDOP TNC accumulates a frame **across reads**, tries `[Rs, None]` for a sticky `FECRCV`, and holds **no** capture stream while adaptive ARQ is active (#1310 PR1c). It called `receive`/`receive_with_fec` in a free-running poll loop, so on a callback backend each call saw one 5 ms poll against a seconds-long frame and the TNC could not receive on real audio — invisible to the suite because `LoopbackBackend::read` drains its whole buffer. Six keyed sites now `drop_stream()` first, with the release next to the keying rather than at the call site. `FECRCV` **stores `true` and nothing clears it** while `FECSEND` is a per-frame one-shot, so the old either/or made one `FECRCV` lose every uncoded frame for the session (the peer's `DE ` ID, any relay envelope); the burst is in hand, so the second candidate is one bounded scan. **The adaptive path is deliberately NOT converted** — the ACK listen and the adaptive IRS arm open their own stream via `stage_capture_input`, and `enable_adaptive_arq` defaults to `false`, so what is fixed is the shipped default. Its gate pins the **high-water mark of simultaneously live streams**, not an open count: a first draft counted opens and was **vacuous** — measured, it passed with the guard sabotaged — because the adaptive arm reopens per call either way. Sabotage-verified three ways, each failing its own case: IRS reverted to one-shot fails the two receive tests; `fec_rx` back to either/or fails only the uncoded-with-`FECRCV` test; making the adaptive branch tick fails only the concurrency test, at "2 capture streams open at once" | `cargo test -p openpulse-ardop --no-default-features --test receives_a_chunked_capture` | | The **OTA** coded arm scans with slices sized for the CODED frame too (#1384) — the same shape as #1310 PR1b, one caller over, and the one the daemon actually runs. Both OTA onset scans took `max_frame_samples` from `burst_onset_scan_bounds`, i.e. the plugin's RAW geometry. **Filed as a code read and then MEASURED before any fix**, which is what made it real: on BPSK250 the raw geometry is **74 624** samples while a coded frame past the one-block boundary is **131 840**, and decode at a non-zero onset went 200 B ✓ / 205 B ✓ / **210 B ✗ / 255 B ✗**. Offset 0 is exempt — the attempt before the scan decodes the whole burst — which is why it sat behind a green suite. The measurement also **corrected the boundary to payload ≤ 209 B** (`FecCodec::encode` prepends a 4-byte length prefix before blocking, so the RS input is `4 + payload + 10` against 223). Sabotage-verified: reverting both sites to raw sizing fails ONLY the two-block case, with the 200 B control still passing | `cargo test -p openpulse-modem --no-default-features --test ota_burst_sizes_for_the_fec` | | A **coded** burst is scanned with slices sized for the CODED frame (#1310 PR1b) — `decode_burst` took its per-attempt slice from the plugin's **raw** `max_frame_samples` (sized for one RS block plus envelope) and hardcoded `FecMode::None` at the decode, so it was an uncoded-only entry point and the ARDOP/KISS front ends had no coded burst path. The boundary is exact and is what the fixture is built on: `FecCodec::encode` prepends a **4-byte length prefix** (`PREFIX_LEN`) before blocking, so the RS input is `4 + payload + Frame::WIRE_OVERHEAD(10)` and one RS(255,223) block holds 223 of it — i.e. **payload ≤ 209 B**. (**Corrected 2026-09-17 from 213 B**, which omitted the prefix; #1384 MEASURED the transition between 205 B and 210 B, and 213 B is on the two-block side. The gates are unaffected — 200 B and 255 B sit on the correct sides of 209 either way.) A gate written at or below the boundary passes with the widening deleted and proves nothing — so the discriminating pair is 255 B (two blocks, must decode) against a 200 B **control** (one block, must also decode), both at a **non-zero onset**, since offset 0 is exempt by construction. Sabotage-verified twice, each failing a distinct set: raw sizing fails ONLY the two-block case while the control passes (which is what proves the failure is about sizing, not about coded bursts); re-hardcoding `FecMode::None` fails BOTH coded cases and leaves the uncoded one green | `cargo test -p openpulse-modem --no-default-features --lib burst_decode_sizes_for_the_fec` | @@ -270,7 +270,7 @@ Each requirement below is done when the linked test passes. Add new links as tes | Winlink header fields are capped, and a realistic multi-recipient message still decodes | `cargo test -p openpulse-b2f --no-default-features -- header_decode_caps header_decode_allows_a_realistic` | | B2F driver survives a hostile peer — line cap, per-operation read deadlines, framing edges | `cargo test -p openpulse-b2f-driver --no-default-features --test cmd_hardening` + `--test timeout_hardening` + `--test data_framing` | | B2F driver reports a refused or fully-rejected ISS transfer instead of silent success | `cargo test -p openpulse-b2f-driver --no-default-features --test iss_failure_paths` | -| CI gates are defined and correct (Linux core/full/gpu/pi5 + `macos-build`) — **but the `CI` workflow is `disabled_manually` by the maintainer, so they do NOT run on a PR; the gates above are run locally before every merge** | `.github/workflows/ci.yml` `on: pull_request` (definition only; check state with `gh api repos/dc0sk/OpenPulseHF/actions/workflows`) | +| CI gates are defined and correct (Linux core/full/pi5 + `macos-build`; the `gpu` job was removed in #1380, subsumed by `gate.sh`'s `--all-features` pass) — **but the `CI` workflow is `disabled_manually` by the maintainer, so they do NOT run on a PR; the gates above are run locally before every merge** | `.github/workflows/ci.yml` `on: pull_request` (definition only; check state with `gh api repos/dc0sk/OpenPulseHF/actions/workflows`) | For any new Phase 1 feature: write the test first, confirm it fails, implement until it passes. Do not mark a task done if its test does not exist. diff --git a/apps/openpulse-linksim/src/gui.rs b/apps/openpulse-linksim/src/gui.rs index d2a252e8..302f27b7 100644 --- a/apps/openpulse-linksim/src/gui.rs +++ b/apps/openpulse-linksim/src/gui.rs @@ -506,7 +506,7 @@ fn constellation_plot( .include_y(-1.8) .include_y(1.8) .show(ui, |p| { - p.points(Points::new(pts).radius(1.2).color(color)); + p.points(Points::new(pts).radius(1.2_f32).color(color)); }); }); }); diff --git a/apps/openpulse-linksim/tests/serve_integration.rs b/apps/openpulse-linksim/tests/serve_integration.rs index 780a0895..d87436cc 100644 --- a/apps/openpulse-linksim/tests/serve_integration.rs +++ b/apps/openpulse-linksim/tests/serve_integration.rs @@ -30,6 +30,10 @@ fn demo_params() -> LinkParams { turnaround_s: 0.2, max_attempts: 4, seed: 99, + // Both match `LinkParams::default()`: this fixture is about the serve/hub transport, not + // about conditioning or notching, so it takes the engine-matching defaults (#1380). + cessb_enabled: true, + notch: None, } } diff --git a/crates/openpulse-radio/src/gpio.rs b/crates/openpulse-radio/src/gpio.rs index a9593fe1..0cc3082a 100644 --- a/crates/openpulse-radio/src/gpio.rs +++ b/crates/openpulse-radio/src/gpio.rs @@ -59,14 +59,14 @@ impl GpioPtt { /// Requires the `gpio` feature; without it, returns an error. Leaves PTT released. pub fn open(spec: &str) -> Result { let (chip, offset, active_low) = parse_gpio_spec(spec)?; - #[cfg(feature = "gpio")] + #[cfg(all(target_os = "linux", feature = "gpio"))] { let line = CdevLine::request(&chip, offset)?; let mut ctrl = Self::with_line(Box::new(line), active_low); ctrl.release_ptt()?; // ensure the physical line starts in the released state Ok(ctrl) } - #[cfg(not(feature = "gpio"))] + #[cfg(not(all(target_os = "linux", feature = "gpio")))] { let _ = (chip, offset, active_low); Err(PttError::Serial( @@ -109,13 +109,13 @@ impl PttController for GpioPtt { } } -#[cfg(feature = "gpio")] +#[cfg(all(target_os = "linux", feature = "gpio"))] struct CdevLine { req: gpiocdev::Request, offset: u32, } -#[cfg(feature = "gpio")] +#[cfg(all(target_os = "linux", feature = "gpio"))] impl CdevLine { fn request(chip: &str, offset: u32) -> Result { let chip_path = if chip.starts_with('/') { @@ -133,7 +133,7 @@ impl CdevLine { } } -#[cfg(feature = "gpio")] +#[cfg(all(target_os = "linux", feature = "gpio"))] impl PttLine for CdevLine { fn set(&mut self, high: bool) -> Result<(), PttError> { let v = if high { diff --git a/docs/dev/project/traceability.md b/docs/dev/project/traceability.md index 13691859..0ffb7ebd 100644 --- a/docs/dev/project/traceability.md +++ b/docs/dev/project/traceability.md @@ -15,6 +15,78 @@ and the actually-observed results per change. --- +## 2026-09-20 — feature-gated code was compiled by nothing; #1380 + +**Change.** `#[cfg(feature = "x")]` code must still PARSE when the feature is off, so a syntax error +was caught — but nothing after parsing was: type errors, borrow errors, wrong arity, a renamed +method. Both lint passes in `gate.sh` and the hook build `--no-default-features`, so nine feature +families gating code (cpal, serial/gpio, gpu, keychain, tokio, serde, gui/serve, hardware-tests, +instruments) were compiled by no automated check at all. `ci.yml`'s `gpu-feature-gates` guarded +exactly one of them, and being `release/**`-scoped (#1120) it never ran on an ordinary PR. + +**Design decision (reviewed by Fable, `docs/dev/reviews/review-1380-feature-rot-guard.md`).** One +rule — `cargo clippy --workspace --all-features --all-targets -- -D warnings` — rather than the +issue's named `--features cpal-backend`, because a hand-maintained list of crate+feature pairs is the +same rotting mirror the guard exists to catch. Safe because every feature here is additive +(`generic-serial = ["serial"]`); a per-feature matrix was considered and rejected on measurement — +no `cfg(all(feature = A, not(feature = B)))` exists anywhere, so a 2^17 powerset would find zero +defects. + +**It found two live defects the day it was written**, neither in cpal: a `serve`-gated test left +behind when `LinkParams` gained `cessb_enabled` (`b883b5f8`) and `notch` (`26696f98`) in 2026-06 — +uncompilable, therefore **never run, for ~3 months** — and a `float-literal-f32-fallback` in the +`gui` binary that rustc says becomes a hard error. Fixed; the `serve` tests now RUN: 2 passed, 0 +failed. + +**The review's catch that would have broken `main`.** `--all-features` adds `alsa-sys`, +`libudev-sys` and `libdbus-sys`, whose build scripts call `pkg_config` and panic when a `.pc` file is +missing; the ubuntu runner ships none of the three `-dev` packages, and neither gate-running job +installed anything. My clean local run described this host only **after** a root update stamped those +`.pc` files on 2026-09-19 — `traceability.md`'s 2026-09-16 entry records `libdbus-sys` failing here +four days before. Unmodified this was red-on-arrival for `post-merge-gate.yml` (#1074's archetype). +Hence a `pkg-config` preflight in both gate and hook that **fails** with the Debian package names +rather than skipping, and an `apt-get` step (not `|| true`) in both jobs. + +**A latent defect the design exposed.** `gpiocdev` is declared under +`[target.'cfg(target_os = "linux")']` while `gpio.rs` was gated on `feature = "gpio"` alone. Cargo +enables a feature whose optional dep is target-filtered out, so `--all-features` on macOS compiles +that code with no crate. Retargeted to `all(target_os = "linux", feature = "gpio")` at all five sites +— unverified on darwin here, no darwin std on this host. + +**`gpu-feature-gates` REMOVED**, subsumed: the new pass compiles and lints `gpu` and `hardware-tests` +across every crate rather than five named plugins, on every local gate run and in post-merge-gate, +where that job never ran. Its reasoning and the PR #424 precedent survive in the new step's comment. + +**Corrections to my own framing.** I described the three passes as a 2×2 grid; the axes are not +independent, since `--all-features` turns `instruments` on regardless of `--all-targets`. And there +is a **residual off the grid**: the shipped recipe `{cpal, gpu}` with `instruments` OFF is compiled by +no pass, so an instruments-only item called from a cpal-gated production path fails only +`cargo build --release -p openpulse-cli --features cpal-backend`. Zero instances today; stated as a +limit. + +**Tests → results (actually run, at this branch).** + +- Sabotage, #1380's own probe — a planted type error in the cpal-gated `run_drive` + (`calibrate.rs:320`): pass 1 (`--no-default-features --all-targets`) **rc=0**, pass 2 + (`--no-default-features`) **rc=0**, pass 3 (`--all-features --all-targets`) **rc=101**. The two + rc=0 rows are what make the failure attributable to the new pass. +- Preflight sabotage: a nonexistent library in the list gives + `cargo clippy (all features) SKIPPED (missing pkg-config: zz-nonexistent-lib)` plus the install + line, and the gate fails. +- `cargo test -p openpulse-linksim --features serve --test serve_integration` → 2 passed, 0 failed. +- Full gate: see the `GATE:` line on PR #1421. + +**Evidence honesty.** Both defects found are in `openpulse-linksim`, which a 2026-09-10 direction +slates for replacement, and the yield on shipped cpal/gpu/serial paths was **zero**. The +justification is the class — three months undetected, PR #424 precedent — not today's haul. + +**Doc twins swept**, since this makes several statements false: `docs/features.md` and +`docs/openpulse-book.md` (both named the removed job), CLAUDE.md's acceptance row listing a `gpu` CI +gate, and CLAUDE.md's "**the `KeychainStore` body itself is still never type-checked here**, so #1380 +is contained, not closed" — now type-checked on Linux, compile+lint only. + +--- + ## 2026-09-19 — a bound test that cannot LINK the code it claims; #1405 **Change.** `req-mutation.sh` mutates every file in a requirement's scope and runs its bound tests diff --git a/docs/dev/reviews/review-1380-feature-rot-guard.md b/docs/dev/reviews/review-1380-feature-rot-guard.md new file mode 100644 index 00000000..319c9e82 --- /dev/null +++ b/docs/dev/reviews/review-1380-feature-rot-guard.md @@ -0,0 +1,96 @@ +--- +project: openpulsehf +doc: docs/dev/reviews/review-1380-feature-rot-guard.md +status: resolved +last_updated: 2026-09-20 +--- + +# Design review — the #1380 feature-rot guard + +## Prompt + +Fable was asked to **falsify** a design before implementation. The design: replace #1380's option 1 +(`cargo check --features cpal-backend`) with one complete rule in `gate.sh` — +`cargo clippy --workspace --all-features --all-targets -- -D warnings` — on the reasoning that a +hand-maintained list of crate+feature pairs is the same rotting-mirror archetype the guard exists to +prevent. Seven numbered attack points went out with the feature census, the two live defects the +command found, and the claim that the three gate passes form a 2×2 grid. It was asked specifically +whether `--all-features` hides what a per-feature matrix would catch, whether ci.yml's GPU job should +be removed as subsumed, and what `--all-features` would newly fail that a warm local build had not hit. + +## Verdict + +**Build with changes — seven of them, all applied.** The rule is right; the design was under-scoped. + +**The finding that would have broken `main`.** `--all-features` adds 22 packages, among them +`alsa-sys`, `libudev-sys` and `libdbus-sys`, whose `build.rs` each call `pkg_config` and panic when a +`.pc` file is absent. The ubuntu-24.04 runner image ships **none** of `libasound2-dev`, `libudev-dev`, +`libdbus-1-dev` (`pkg-config` itself is present — that was the filter control). Neither +`pr-hook-long-runner` nor `post-merge-gate.yml` installs anything. My clean `rc=0` described this host +**after a root system update stamped those three `.pc` files on 2026-09-19 14:16** — +`traceability.md:1030` records `libdbus-sys` failing at `pkg-config` on this very machine four days +earlier. Unmodified, the post-merge gate would have gone red on arrival and auto-opened an issue: the +#1074 red-on-arrival archetype. + +**The other six.** + +1. **A per-feature matrix buys nothing** — `git grep` finds no `cfg(all(feature = A, not(feature = B)))` + anywhere, and none of the 23 `cfg(not(feature))` arms nests inside another feature's region, so + every arm is reached by some pass. A powerset over 17 features would compile 2¹⁷ configs to find + zero defects. Noted honestly: that is a census, not a construction. +2. **My 2×2 grid claim is wrong** and would mislead the next reader. The axes are not independent: + under `--all-features`, `instruments` is on regardless of `--all-targets`, so the fourth cell is + not "the shipped feature-on configuration". They are three named configurations with a residual, + not a grid. +3. **Stated residual, off the grid entirely.** The shipped hardware recipe is `{cpal, gpu}` with + `instruments` OFF. An instruments-only accessor called from a `cpal-backend`-gated production path + passes all three passes and fails only `cargo build --release -p openpulse-cli --features + cpal-backend`. Zero instances today; recorded as a limit, not a claim of completeness. +4. **`gpio.rs` was a live latent defect**, not merely a portability nicety: `gpiocdev` is declared + under `[target.'cfg(target_os = "linux")']` while the code was gated on `feature = "gpio"` alone. + Cargo enables a feature whose optional dep is target-filtered out, so `--all-features` on macOS + compiles that code with no crate → E0433. Retargeted to `all(target_os = "linux", feature = "gpio")` + at all five sites. Not verified on darwin here (no darwin std on this host) — stated as such. +5. **Compiling is not running.** `LinkParams` gained `cessb_enabled` in `b883b5f8` and `notch` in + `26696f98` (both 2026-06); the test was last touched 2026-06-21. The `serve` feature's only tests + had been uncompilable, therefore **never run, for ~3 months**. Now run: 2 passed, 0 failed. +6. **Remove the GPU job, but only together with the apt step**, and the `--all-features` pass covers + its third step too, since `hardware-tests` is a feature and `tests/kernel_equivalence.rs` is + `#![cfg(feature = "hardware-tests")]`. + +**Evidence honesty, Fable's point and worth repeating:** both live defects are in `openpulse-linksim`, +a crate under a 2026-09-10 direction to be replaced, and the yield on shipped cpal/gpu/serial paths in +this run was **zero**. The justification is the class — three months undetected, PR #424 precedent — +not today's haul. + +**Sabotage, #1380's own probe.** A planted type error in the cpal-gated `run_drive` +(`calibrate.rs:320`): pass 1 `rc=0`, pass 2 `rc=0`, pass 3 **`rc=101`**. The preflight was separately +sabotaged with a nonexistent library name and correctly printed +`SKIPPED (missing pkg-config: zz-nonexistent-lib)` plus the package names, and failed. + +## Consumer + +`scripts/gate.sh:236` (the new `run_step`) and `.cargo-husky/hooks/pre-push:102`. In CI the consumers +are `ci.yml`'s `pr-hook-long-runner` and `post-merge-gate.yml`, both of which now install the three +`-dev` packages. The property's downstream consumer is every feature-gated production path, of which +the on-air recipe `cargo build --release -p openpulse-cli --features cpal-backend` is the one that +ships. + +## Prior art + +`grep -n "features" .github/workflows/ci.yml` → `gpu-feature-gates` (ci.yml:191-236), the existing rot +guard for one feature, with PR #424 as its recorded precedent; this change generalises it and deletes +it. `grep -n clippy scripts/gate.sh` → the two existing passes (the second added by #1418 the day +before). No other mechanism compiles feature-gated code: `git grep -n 'all-features'` over the repo +returned **zero** matches on `origin/main` (control, same command: `no-default-features` matches 105 files). + +## Twins + +The pre-push hook is the gate's twin and gets the same pass and the same preflight — it is the only +check that runs on every push (#1144), and rot is a per-push class. The two CI jobs that invoke +`gate.sh` are twins of each other and both got the apt step; `ci.yml`'s aloop job already installed +`libasound2-dev` and was left alone. The doc twins asserting the old state were swept: `docs/features.md`, +`docs/openpulse-book.md`, CLAUDE.md's acceptance row naming the `gpu` job, and CLAUDE.md's +"`KeychainStore` body is still never type-checked … #1380 is contained, not closed", which this change +makes false on Linux. #1418 is the inverse sibling — that pass narrows targets, this one widens +features — and neither closes the other. diff --git a/docs/features.md b/docs/features.md index 722c870f..88108073 100644 --- a/docs/features.md +++ b/docs/features.md @@ -2,7 +2,7 @@ project: openpulsehf doc: docs/features.md status: living -last_updated: 2026-09-14 +last_updated: 2026-09-20 --- # OpenPulseHF — Feature Reference @@ -731,7 +731,7 @@ path). Six WGSL compute kernels run on any wgpu-compatible GPU (Vulkan, Metal, D WebGPU): `bpsk_modulate`, `bpsk_demodulate`, `timing_search` (the BPSK trio), `rrc_fir` (the RRC matched filter, BPSK/QPSK/8PSK/64QAM), `soft_demod` (8PSK/64QAM soft LLRs), and `fft256` (SC-FDMA batched FFT). The GPU is API-only (`with_gpu`), -exercised by tests; a CI job (`gpu-feature-gates`) builds + lints the feature so it +exercised by tests; `scripts/gate.sh`'s `--all-features` pass builds + lints the feature so it doesn't rot. The BPSK modulate path, as one example, is three stages: 1. **Byte-to-bit expansion**: 64-thread workgroups extract LSB-first bits from input diff --git a/docs/openpulse-book.md b/docs/openpulse-book.md index 6fd55509..ca9f8b8c 100644 --- a/docs/openpulse-book.md +++ b/docs/openpulse-book.md @@ -2,7 +2,7 @@ project: openpulsehf doc: docs/openpulse-book.md status: living -last_updated: 2026-09-14 +last_updated: 2026-09-20 --- # The OpenPulseHF Book @@ -3436,7 +3436,7 @@ All read from the manifests: Consequences, spelled out: `cargo build -p openpulse-cli` includes CPAL; `cargo build -p openpulse-kiss` does not. The feature is spelled `cpal-backend` for the CLI and audio crates but `cpal` for the daemon, TNCs and testbench — `--features cpal` errors on the CLI, `--features cpal-backend` errors on the daemon. The runtime `--backend cpal` flag warns at startup when the feature is absent. (One more piece of doc drift, flagged rather than repeated: `openpulse-audio/src/lib.rs:7` claims `cpal-backend` is "enabled by default"; its own manifest says `default = []`, and the manifest is authoritative.) Platform limits from the manifest comments: `serial`/`generic-serial` are Unix-only, `gpio` is Linux-only. -GPU acceleration deserves its own row of honesty: five plugins (BPSK, QPSK, 8PSK, 64QAM, SC-FDMA) have optional wgpu paths against six WGSL kernels in `openpulse-gpu`; OFDM is not GPU-accelerated. Because the standard `--no-default-features` gates never compile the `gpu` cfg paths, they would rot silently — the CI workflow therefore has a dedicated `gpu-feature-gates` job that compiles and lints (but does not run — CI runners have no wgpu adapter) the GPU paths on every change; its comment cites the PR that found a build break reachable only there. +GPU acceleration deserves its own row of honesty: five plugins (BPSK, QPSK, 8PSK, 64QAM, SC-FDMA) have optional wgpu paths against six WGSL kernels in `openpulse-gpu`; OFDM is not GPU-accelerated. Because the standard `--no-default-features` gates never compile the `gpu` cfg paths, they would rot silently — `scripts/gate.sh` therefore carries an `--all-features` pass that compiles and lints (but does not run — CI runners have no wgpu adapter) the GPU paths on every gate run; its comment cites the PR that found a build break reachable only there. Until #1380 this was a dedicated `gpu-feature-gates` CI job, which covered five named plugins and, being `release/**`-scoped, never ran on an ordinary PR. #### 3.6.2 The canonical gate set diff --git a/scripts/gate.sh b/scripts/gate.sh index cbf0e10a..19a6b684 100755 --- a/scripts/gate.sh +++ b/scripts/gate.sh @@ -204,6 +204,44 @@ drift_check # 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 +drift_check +# FEATURE-GATED code, which neither pass above compiles at all (#1380). `#[cfg(feature = "x")]` code +# must still PARSE when the feature is off, so a syntax error is caught — but nothing after parsing +# is: type errors, borrow errors, wrong arity, a renamed method. That is exactly the code most likely +# to drift, because nobody compiles it while editing something else. Precedent is not hypothetical: +# PR #424 found a build break, a clippy finding and a flaky test reachable only through the `gpu` +# feature, and this pass found two more the day it was written — a `serve`-gated test left behind +# when `LinkParams` gained two fields (uncompilable, therefore never run, for ~3 months) and a +# `float-literal-f32-fallback` in the `gui` binary that rustc says becomes a hard error. +# +# `--all-features` rather than a list of crate+feature pairs: a hand-maintained mirror of the feature +# set is the same rotting artifact this pass exists to catch. Safe because every feature here is +# ADDITIVE (`generic-serial = ["serial"]`); there is no mutually exclusive pair to break. +# +# TWO STATED LIMITS, neither closed by this pass: +# * Not closed over TARGETS. A feature whose optional dependency is target-filtered (gpio's +# `gpiocdev` is `[target.'cfg(target_os = "linux")']`) compiles here but not elsewhere; the code +# must carry `all(target_os = ..., feature = ...)`, as gpio.rs now does. +# * Not closed over the SHIPPED recipe. `--all-features` turns `instruments` on, so an +# instruments-only item called from a `cpal-backend`-gated production path passes all three +# passes and fails only `cargo build --release -p openpulse-cli --features cpal-backend`. Zero +# instances today; it is a residual, not a claim of completeness. +# +# The preflight is NOT ceremony: --all-features pulls alsa-sys, libudev-sys and libdbus-sys, whose +# build scripts call pkg_config and PANIC when a .pc file is absent. Without this, a missing distro +# package reads as an unintelligible build-script backtrace in the middle of clippy output. +missing_pc="" +for pc in alsa libudev dbus-1; do + pkg-config --exists "$pc" 2>/dev/null || missing_pc="$missing_pc $pc" +done +if [ -n "$missing_pc" ]; then + printf ' %-38s%s\n' "cargo clippy (all features)" "SKIPPED (missing pkg-config:$missing_pc)" + echo " install: libasound2-dev libudev-dev libdbus-1-dev (Debian/Ubuntu names)" + echo "=== cargo clippy (all features): SKIPPED — missing pkg-config:$missing_pc ===" >> "$LOG" + rc_total=1 +else + run_step "cargo clippy (all features) -D warns" cargo clippy --workspace --all-features --all-targets -- -D warnings || rc_total=1 +fi TEST_CMD="none" if [ "$MODE" = "full" ]; then