From c4687337c096a65d488514207dadef7f5f600448 Mon Sep 17 00:00:00 2001 From: Robert M1 <50460704+githubrobbi@users.noreply.github.com> Date: Tue, 19 May 2026 13:06:09 -0700 Subject: [PATCH] fix(features): gate 6 items on streamable-http/async + add lint-ci-no-default regression guard (Phase 8e, refs #295) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Post-Phase-8 audit found that `cargo check --workspace --no-default-features` (playbook §996, the second of the four required Phase 8 validations) was never run during Phase 8. Doing so reveals 6 items reachable only when feature X is on but not themselves `#[cfg(feature = "X")]`-gated. Under strict CI (`-D warnings`) all 6 would have hard-failed. Source fixes (3 surgical cfg/cfg_attr annotations, zero behavior change in default + --all-features configs): * `uffs-mcp::McpStats::{avg_tool_latency_us,to_json}` → `#[cfg(feature = "streamable-http")]` (both only called by `http.rs`'s `/status` endpoint). * `uffs-client::daemon_ctl::keepalive_send_blocking` → `#[cfg(feature = "async")]` (only called by `connect_keepalive`, itself `async`-gated in `lib.rs`). * 3 `#[expect(clippy::cognitive_complexity, …)]` in `uffs-mcp::lib.rs` + `handler/mod.rs` → `#[cfg_attr(feature = "streamable-http", expect(…))]` (the lint score only crosses threshold when streamable-http is on; the conditional expect preserves the strict check when it actually triggers without leaving an unfulfilled expect when it doesn't). Regression guard: * New `lint-ci-no-default` gate added to `scripts/ci/gates.toml`: `cargo clippy --workspace --all-targets --no-default-features --no-deps -- -D warnings`. Mirrors `lint-ci` in flag stack + strictness; differs only in `--no-default-features` vs `--all-features`. Together the pair enforces the additivity invariant. * `just lint-ci-no-default` recipe added to `just/test.just`. * `pr-fast.yml::clippy-no-default` job added (mirrors `clippy` job). * Registered in `required.needs:` + aggregator table. * Pre-push hook regenerated via `just gen-hooks` (auto from manifest). * All 4 drift detectors green: gates-drift, hooks-drift, workflow-drift, fast-drift. Documentation: * `dependency_policy.md` §2: 5-tool table → 6-tool table; new paragraph explaining the additivity invariant the pair establishes. * `dependency_policy.md` §10: 8e decisions-log row. * `trait_policy.md:193`: Phase 7f TBD → #291 (Phase 8 follow-up #1). Adherence to the 5-rule contract: * **Rule 1** (no suppression hacks): zero `#[allow]` added. All 6 fixes are minimum-correct `#[cfg]` annotations that express the actual reachability; the 3 `expect`s become conditional rather than absolute. * **Rule 2** (surgical, correct fixes): 3 source files touched with 9 attribute lines added; no logic change. * **Rule 3** (preserve behavior + contracts): default + --all-features binary surface unchanged. All API contracts preserved. * **Rule 4** (improve tests, don't dodge them): new CI gate gives the whole workspace structural feature-additivity verification it lacked. * **Rule 5** (document & commit well): this commit message + 2 policy-doc updates + 4-line decisions-log entries explain the why, the what, and the guard against regression. --- .github/workflows/pr-fast.yml | 34 ++++++++++++++++ crates/uffs-client/src/daemon_ctl.rs | 4 ++ crates/uffs-mcp/src/handler/mod.rs | 18 ++++++--- crates/uffs-mcp/src/lib.rs | 9 +++-- crates/uffs-mcp/src/stats.rs | 8 ++++ .../code-quality/dependency_policy.md | 15 ++++++- .../architecture/code-quality/trait_policy.md | 2 +- just/test.just | 9 +++++ scripts/ci/gates.toml | 40 +++++++++++++++++++ scripts/hooks/_lint_pre_push.sh | 1 + 10 files changed, 128 insertions(+), 12 deletions(-) diff --git a/.github/workflows/pr-fast.yml b/.github/workflows/pr-fast.yml index ad5cca6bb..36175aa67 100644 --- a/.github/workflows/pr-fast.yml +++ b/.github/workflows/pr-fast.yml @@ -403,6 +403,38 @@ jobs: save-if: 'false' - run: cargo clippy --workspace --all-targets --all-features --locked --no-deps -- -D warnings + # ───────────────────────────────────────────────────────────────────── + # clippy-no-default — Phase 8e regression guard (issue #295). Same + # strict clippy flag stack as `clippy` above but with + # `--no-default-features` instead of `--all-features`, catching items + # reachable only when a feature is enabled but not themselves + # `#[cfg(feature = "…")]`-gated. Cache key is distinct from `clippy` + # because the feature configuration produces a different artifact set. + # ───────────────────────────────────────────────────────────────────── + clippy-no-default: + name: Clippy (--no-default-features) + runs-on: ubuntu-22.04 + needs: [classify, sanity] + if: needs.classify.outputs.code == 'true' + timeout-minutes: 30 + steps: + - uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2 + with: + ref: ${{ github.event.pull_request.head.sha || github.sha }} + + - name: Free up disk space + run: | + sudo rm -rf /usr/local/lib/android /usr/share/dotnet /opt/ghc + sudo rm -rf /usr/local/share/boost /usr/local/graalvm + df -h / + + - run: rustup show + - uses: Swatinem/rust-cache@c19371144df3bb44fab255c43d04cbc2ab54d1c4 # v2.9.1 + with: + shared-key: pr-fast-clippy-no-default + save-if: 'false' + - run: cargo clippy --workspace --all-targets --no-default-features --locked --no-deps -- -D warnings + # ───────────────────────────────────────────────────────────────────── # docs — rustdoc with `-Dwarnings` PLUS doctest execution. Catches # the exact class of bug Phase 1 / Phase 2 locally catch too — keeping @@ -661,6 +693,7 @@ jobs: - fmt - sanity - clippy + - clippy-no-default - docs - test-build - tests @@ -687,6 +720,7 @@ jobs: [fmt]='${{ needs.fmt.result }}' [sanity]='${{ needs.sanity.result }}' [clippy]='${{ needs.clippy.result }}' + [clippy-no-default]='${{ needs.clippy-no-default.result }}' [docs]='${{ needs.docs.result }}' [test-build]='${{ needs.test-build.result }}' [tests]='${{ needs.tests.result }}' diff --git a/crates/uffs-client/src/daemon_ctl.rs b/crates/uffs-client/src/daemon_ctl.rs index 4ad8173ee..307606ab3 100644 --- a/crates/uffs-client/src/daemon_ctl.rs +++ b/crates/uffs-client/src/daemon_ctl.rs @@ -183,6 +183,10 @@ pub(crate) fn verify_daemon_after_connect_strict_at( /// On Unix, opens the `AF_UNIX` socket at `sock_path`. /// On Windows, opens the named pipe (no `ws2_32` cost) — `sock_path` is /// unused but kept for API stability. +/// +/// Only consumed by the async keepalive task, hence the `async` feature +/// gate (matches `connect_keepalive` in `lib.rs`). +#[cfg(feature = "async")] pub(crate) fn keepalive_send_blocking(sock_path: &std::path::Path) { #[cfg(unix)] { diff --git a/crates/uffs-mcp/src/handler/mod.rs b/crates/uffs-mcp/src/handler/mod.rs index c9a8ad8c3..51ac87d57 100644 --- a/crates/uffs-mcp/src/handler/mod.rs +++ b/crates/uffs-mcp/src/handler/mod.rs @@ -321,9 +321,12 @@ impl ServerHandler for UffsMcpServer { .with_instructions(AGENT_INSTRUCTIONS.to_owned()) } - #[expect( - clippy::cognitive_complexity, - reason = "async match + await + iteration + logging contributes to Clippy score" + #[cfg_attr( + feature = "streamable-http", + expect( + clippy::cognitive_complexity, + reason = "async match + await + iteration + logging contributes to Clippy score" + ) )] async fn on_roots_list_changed(&self, context: rmcp::service::NotificationContext) { // Ask the client for the current list of roots. @@ -361,9 +364,12 @@ impl ServerHandler for UffsMcpServer { }) } - #[expect( - clippy::cognitive_complexity, - reason = "tool dispatch with timing, error handling, stats, and reconnect" + #[cfg_attr( + feature = "streamable-http", + expect( + clippy::cognitive_complexity, + reason = "tool dispatch with timing, error handling, stats, and reconnect" + ) )] async fn call_tool( &self, diff --git a/crates/uffs-mcp/src/lib.rs b/crates/uffs-mcp/src/lib.rs index 6dd8f2380..913c2e3ed 100644 --- a/crates/uffs-mcp/src/lib.rs +++ b/crates/uffs-mcp/src/lib.rs @@ -227,9 +227,12 @@ impl Default for McpConfig { /// /// Returns an error if the daemon connection fails or the MCP transport /// encounters an I/O error. -#[expect( - clippy::cognitive_complexity, - reason = "MCP server startup with daemon connect, idle timer, and transport orchestration" +#[cfg_attr( + feature = "streamable-http", + expect( + clippy::cognitive_complexity, + reason = "MCP server startup with daemon connect, idle timer, and transport orchestration" + ) )] pub async fn run_mcp_server_with_config(config: &McpConfig) -> anyhow::Result<()> { info!( diff --git a/crates/uffs-mcp/src/stats.rs b/crates/uffs-mcp/src/stats.rs index c6712bad0..9eadba20d 100644 --- a/crates/uffs-mcp/src/stats.rs +++ b/crates/uffs-mcp/src/stats.rs @@ -86,6 +86,10 @@ impl McpStats { } /// Average tool call latency in microseconds (0 if no calls). + /// + /// Only consumed by the HTTP `/status` endpoint, hence the + /// `streamable-http` feature gate. + #[cfg(feature = "streamable-http")] #[must_use] pub(crate) fn avg_tool_latency_us(&self) -> u64 { let total = self.total_tool_calls.load(Ordering::Relaxed); @@ -96,6 +100,10 @@ impl McpStats { } /// Serialize stats to a JSON value for the `/status` endpoint. + /// + /// Only consumed by the HTTP `/status` endpoint, hence the + /// `streamable-http` feature gate. + #[cfg(feature = "streamable-http")] #[must_use] pub(crate) fn to_json(&self) -> serde_json::Value { serde_json::json!({ diff --git a/docs/architecture/code-quality/dependency_policy.md b/docs/architecture/code-quality/dependency_policy.md index d86f78cb4..6f2fbee35 100644 --- a/docs/architecture/code-quality/dependency_policy.md +++ b/docs/architecture/code-quality/dependency_policy.md @@ -52,7 +52,7 @@ test-substitution boundary documented in ## 2 The lint posture Phase 8 introduces **no new clippy lints**. Feature and dependency -hygiene is enforced through five complementary tools wired into the +hygiene is enforced through six complementary tools wired into the pre-push gate and CI: | Tool | What it catches | Where | @@ -62,6 +62,16 @@ pre-push gate and CI: | `cargo vet` | Unaudited crate-versions in the dep graph | `supply-chain/{config,audits,imports.lock}.toml` + `pr-fast.yml::security` | | `cargo tree --workspace -d` | Cross-version duplicate inventory | Surfaced by `scripts/dev/feature_dep_audit.sh --with-cargo`; no hard gate (workspace runs `multiple-versions = "warn"`) | | `cargo doc --no-deps --all-features` | Broken intra-doc links inside `# Features` sections | Pre-push gate (`rustdoc`) + `pr-fast.yml::docs` | +| `cargo clippy --workspace --all-targets --no-default-features --no-deps -- -D warnings` | **Feature-additivity regressions** — `pub`/`pub(crate)` items reachable only when feature X is on but not themselves `#[cfg(feature = "X")]`-gated (manifests as `dead_code` warnings + unfulfilled `expect` warnings when feature X is off) | Pre-push gate (`lint-ci-no-default` — Phase 8e) + `pr-fast.yml::clippy-no-default` | + +The sixth tool — `lint-ci-no-default` — is the Phase 8e regression +guard (issue #295). It mirrors the existing `lint-ci` gate +(`--all-features -D warnings`) but swaps in `--no-default-features`, +so the pair establishes the **additivity invariant**: every item +reachable only with feature X must compile cleanly both with feature X +off AND with all features on. Without this gate, the `--all-features` +build masks the dead-code warning the `--no-default-features` build +emits, and additivity regresses silently. `deny.toml` is configured at: @@ -331,4 +341,5 @@ Append-only. Each entry: date, sub-phase, decision, PR. |---|---|---|---| | 2026-05-19 | 8a | Add `scripts/dev/feature_dep_audit.sh` baseline tool | #292 | | 2026-05-19 | 8b | Document the 3 feature contracts per playbook §988 in rustdoc + Cargo.toml | #293 | -| 2026-05-19 | 8c | Add this `dependency_policy.md` + `CONTRIBUTING.md` cross-link | this PR | +| 2026-05-19 | 8c | Add this `dependency_policy.md` + `CONTRIBUTING.md` cross-link | #294 | +| 2026-05-19 | 8e | Add `lint-ci-no-default` regression guard + fix 6 feature-additivity gaps (`McpStats::{avg_tool_latency_us,to_json}`, `keepalive_send_blocking`, 3 `cognitive_complexity` expects) | this PR | diff --git a/docs/architecture/code-quality/trait_policy.md b/docs/architecture/code-quality/trait_policy.md index 3cfcccc11..464e94565 100644 --- a/docs/architecture/code-quality/trait_policy.md +++ b/docs/architecture/code-quality/trait_policy.md @@ -190,5 +190,5 @@ Append-only. Each entry: date, sub-phase, decision, PR. | 2026-05-19 | 7c | Audit 127 generic-fn sites — all G1/G2/G5; zero refactor PRs | findings-only | | 2026-05-19 | 7d | Audit 40 true dispatch sites — all D1/D2; zero refactor PRs | findings-only | | 2026-05-19 | 7e | Keep all 3 surviving `pub trait`s OPEN — `FileReader` (J3), `FormatRow` (J3), `RuntimeDir` (structurally sealed by private fields of `RuntimeFile`) | findings-only | -| 2026-05-19 | 7f | Add 4 clippy lints: `too_many_arguments`, `trait_duplication_in_bounds`, `wrong_self_convention` (deny) + `multiple_bound_locations` (warn) | TBD | +| 2026-05-19 | 7f | Add 4 clippy lints: `too_many_arguments`, `trait_duplication_in_bounds`, `wrong_self_convention` (deny) + `multiple_bound_locations` (warn) | #291 | | 2026-05-19 | 7g | Add this `trait_policy.md` + `CONTRIBUTING.md` cross-link | this PR | diff --git a/just/test.just b/just/test.just index de9fe6595..ccce80cab 100644 --- a/just/test.just +++ b/just/test.just @@ -69,6 +69,15 @@ lint-ci: @printf "\033[0;34m🚨 CI lint gating (FAST-FAIL)...\033[0m\n" cargo clippy --workspace --all-targets --all-features --no-deps -- -D warnings +# Feature-additivity regression guard (Phase 8e — issue #295). Runs the +# same strict clippy flag stack as `lint-ci` but with `--no-default-features` +# instead of `--all-features`, catching items that are reachable only when +# a feature is enabled but not themselves `#[cfg(feature = "…")]`-gated. +# MUST mirror .github/workflows/pr-fast.yml `clippy-no-default` job exactly. +lint-ci-no-default: + @printf "\033[0;34m🚨 CI lint gating (--no-default-features, FAST-FAIL)...\033[0m\n" + cargo clippy --workspace --all-targets --no-default-features --no-deps -- -D warnings + # Cross-platform CI lint gate via Docker (Linux target). # Catches Linux-only clippy drift that the local host (macOS) cannot reproduce # — e.g. platform-dependent type aliases like `libc::time_t` where nightly diff --git a/scripts/ci/gates.toml b/scripts/ci/gates.toml index 6914ba4bf..634af829a 100644 --- a/scripts/ci/gates.toml +++ b/scripts/ci/gates.toml @@ -340,6 +340,46 @@ The exact clippy flag stack `pr-fast.yml`'s `clippy` job runs. Kept in lockstep manually until Phase 2 codegen lands. """ +[[gate]] +id = "lint-ci-no-default" +label = "CI-mirror clippy --no-default-features (-D warnings)" +command = ["just", "lint-ci-no-default"] +tiers = ["pre-push", "pr-fast"] +gate_when = "rust_changed" +hard = true +tool = "cargo" +expected_runtime_secs = 30 +bucket = "seq" +order = 25 +consumer_names = { "pr-fast" = "clippy-no-default" } +notes = """ +Phase 8e regression guard for issue #295 — feature additivity. + +`cargo clippy --workspace --no-default-features --all-targets -- -D warnings` +catches the class of bug that surfaced post-Phase-8 closeout: items +reachable only when feature X is on, but not themselves +`#[cfg(feature = "X")]`-gated. The existing `lint-ci` gate runs +`--all-features` which masks the dead-code + unfulfilled-`expect` +warnings these gaps emit. + +Specifically prevents regression of the 6 sites fixed by issue #295: +- `uffs-mcp::McpStats::{avg_tool_latency_us,to_json}` (now + `cfg(feature = "streamable-http")`-gated). +- `uffs-client::daemon_ctl::keepalive_send_blocking` (now + `cfg(feature = "async")`-gated). +- 3 `cfg_attr(feature = "streamable-http", expect(cognitive_complexity, …))` + in `uffs-mcp::lib.rs` + `handler/mod.rs`. + +Mirrors `lint-ci` in flag stack and `-D warnings` strictness; differs +only in `--no-default-features` instead of `--all-features`. Together +the two gates establish the additivity property: every `pub`/`pub(crate)` +item reachable only with feature X must compile cleanly both with +feature X off AND with all features on. + +Documented in +`docs/architecture/code-quality/dependency_policy.md` §2. +""" + [[gate]] id = "rustdoc" label = "rustdoc -D warnings" diff --git a/scripts/hooks/_lint_pre_push.sh b/scripts/hooks/_lint_pre_push.sh index abb0a4b5a..acb49450d 100755 --- a/scripts/hooks/_lint_pre_push.sh +++ b/scripts/hooks/_lint_pre_push.sh @@ -235,6 +235,7 @@ command -v reuse >/dev/null 2>&1 && spawn_bg "reuse" reuse lint --quiet if (( CODE_CHANGED )); then run_seq "cargo-check" cargo check --workspace --all-targets --all-features --locked run_seq "lint-ci" just lint-ci + run_seq "lint-ci-no-default" just lint-ci-no-default run_seq "lint-prod" just lint-prod run_seq "lint-tests" just lint-tests run_seq "rustdoc" env RUSTDOCFLAGS=-Dwarnings cargo doc --workspace --all-features --no-deps --locked