Enforce strict Rust lints with an audit checker

Merged
JS
3d ago
+3029-1422

Clippy already ran at -D warnings, but nothing stopped a suppression without a reason, a glob import, a .clone() on an Arc, or a lossy cast from coming back, and nothing stopped the lint table itself from being loosened. This tightens the table in Cargo.toml and adds a checker for what a lint cannot ask.

The lint table now also denies allow attributes and any suppression without a reason, .clone() on Arc/Rc, redundant clones, + on strings, more than three bool parameters, needless Option/Result wrapping, a panic inside a Result function, glob imports, sign-losing casts, pub items nothing outside the crate can name, ignored must-use results, stale lint expectations, locks held across .await, discarded futures and mem::forget. Every finding in src/ and tests/ is fixed rather than waived; the 37 remaining #[expect] sites each carry a reason naming the constraint.

scripts/check-rust-style.py is a new gate that holds the rest: the required lints cannot be loosened, even by a lint group set to allow at a higher priority; no #[allow] or bare #[ignore]; no TODO, FIXME or ponytail: markers; no CJK in Rust or web/ (one fixture in tests/agent/framework.rs now uses Greek); and no Rust file may pass its recorded line count, with --ratchet lowering the recorded counts and --baseline origin/main refusing a raised one. --workspace checks every Cargo member inherits the policy. Its tests are tests/test_check_rust_style.py.

The lints deliberately left off are listed in the new codetrial-rust skill with the reason for each: missing_debug_implementations would ask for the Debug derive that keeps credentials out of logs, and needless_pass_by_value and unused_async would churn handler signatures for no clarity. The skill also records what the build enforces against what it leaves to review.

The numeric edits keep their behavior: negative whole numbers still serialize as 0, and the try_from conversions replace as casts on values already range-checked. The gate now needs Python 3.11 in every lane except browser, because the checker reads Cargo.toml with tomllib.

Smaller changes ride along, each in its own commit. Provider discovery, room routing and the pool summary move from src/config.rs into src/config/providers.rs with the code unchanged. A separate commit then stops the provider Debug output and the startup pool summary from printing a configured LiveKit URL whole, since it can carry credentials in its userinfo, path, query or fragment; they print the host and the URL authority, with tests for each position. Three flake fixes stand alone: a shellcheck-flagged cleanup trap, an undo chord macOS does not treat as undo, and a board test that paused the clock around a real socket.

Validation: cargo fmt --check, cargo clippy --locked --workspace --all-targets, cargo doc, cargo test --locked and ./scripts/test.sh pass locally, apart from two unrelated flakes seen once each under load (review-memory-tests, and a browser test that passes on rerun). Not run: a live interview, and the Linux CI runner.

Summary by cubic

Enforces the Rust lint policy through the workspace lint table in Cargo.toml and a new scripts/check-rust-style.py gate, fixing every finding in src/ and tests/ instead of waiving it.

Lint and audit checks

  • The workspace lint table denies un-reasoned suppressions, .clone() on Arc/Rc, glob imports, sign-losing casts, more than three bool parameters, panics in Result functions, locks held across .await, discarded futures, and unreachable pub items; the remaining #[expect] sites each name the constraint.
  • The checker refuses a loosened required lint even via a lint group, rejects bare #[allow]/#[ignore], TODO/FIXME markers, and CJK text in Rust or web/, and keeps a file under its recorded line ceiling; --ratchet lowers ceilings and --baseline origin/main refuses a raised one.
  • The new codetrial-rust skill records the enforced rules, the deliberately-off lints, and how to suppress one; the gate needs Python 3.11 in every lane except browser because the checker reads Cargo.toml with tomllib.

Also in this PR

  • Provider discovery, room routing, and the pool summary move from src/config.rs into src/config/providers.rs unchanged, with their tests into tests/config/recording.rs.
  • Provider Debug output and the startup pool summary print the host and URL authority, never a configured LiveKit URL's userinfo, path, query, or fragment; tests pin credentials in each position.
  • Numeric behavior is unchanged: negative whole numbers still serialize as 0, and try_from replaces as casts on already range-checked values.
  • Three flake fixes: a shellcheck-flagged cleanup trap, an undo chord macOS does not recognize, and a board test that paused the clock around a real socket.
  • Mutation testing tightened two tests: the provider-order test pins which warning each miss gets, and the recording sweep test only lets the sweep requeue an orphaned delivery.