Skip to content

chore(lint): clear all deny-level clippy findings across the workspace - #1885

Merged
bobleer merged 1 commit into
GCWing:mainfrom
bobleer:bob/clippy-deny-cleanup
Jul 30, 2026
Merged

chore(lint): clear all deny-level clippy findings across the workspace#1885
bobleer merged 1 commit into
GCWing:mainfrom
bobleer:bob/clippy-deny-cleanup

Conversation

@bobleer

@bobleer bobleer commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Why

[workspace.lints.clippy] sets correctness and suspicious to deny, but several crates fail to compile under cargo clippy --workspace --all-targets. Clippy short-circuits on the first compile error, so it never reached those crates — which had accumulated a backlog of hard errors that a green --all-targets run would reject.

cargo clippy --workspace --all-targets now reports 0 errors.

Compile blockers

  • relay-server/tests/library_compat.rs: the AppState initializer had fallen behind the struct (missing page_browser_auth), and let _ = routes::api::server_info(); built an unpolled future instead of pinning the symbol.
  • rmcp API drift: SseStream::from_byte_streamfrom_bytes_stream.

Deny-level fixes

  • await_holding_lock (25 sites). The subscription-auth and account-context test serialization locks now use tokio::sync::Mutex, matching the store_lock precedent already in the same file; the plain #[test] cases take them with blocking_lock, which cannot stall without an ambient runtime. The remaining sites scope their guard in a block. Note that an explicit drop(guard) before the await does not satisfy this lint — it reasons over lexical scope.
  • suspicious_open_options. The json-store lock file now states truncate(false): it carries no payload and must never be truncated while another process holds the flock on that inode.
  • items_after_test_module, never_loop, octal_escapes, unnecessary_get_then_check, drop_non_drop, plus the mechanical style/complexity/perf fixes from clippy --fix in the crates that were previously unreachable.

Two judgment calls worth a look

  • client_info.rs keeps advertising the SEP-2577-deprecated roots/sampling capabilities behind a scoped #[allow(deprecated)] with the rationale recorded inline. Dropping them is a protocol-visible behavior change pinned by mcp_remote_client_info_declares_supported_client_capabilities, not a lint cleanup.
  • stat_local_path_metadata keeps its expanded symlink check behind #[allow(clippy::needless_bool)]. Clippy only sees the #[cfg(not(windows))] { false } arm and would otherwise collapse away the Windows reparse-point detection — a silent platform regression.

Scope

Unsafe blocks in the SSH plugin source, terminal exec, and webdriver capture/evaluator paths are now documented. No behavior change anywhere else.

Left alone deliberately, since they need design judgment rather than a mechanical fix: too_many_arguments, large_enum_variant, type_complexity, and the ~195 undocumented unsafe blocks in the macOS accessibility FFI under desktop/src/computer_use/macos_*.

`correctness` and `suspicious` are deny-level in `[workspace.lints.clippy]`,
but several crates failed to compile under `--all-targets`, so clippy
short-circuited before reaching them. Fixing the compile blockers exposed a
backlog of hard errors that CI would reject.

Compile blockers:
- relay-server `library_compat` test: `AppState` initializer had fallen behind
  the struct (missing `page_browser_auth`), and `let _ = server_info();` built
  an unpolled future instead of pinning the symbol.
- rmcp API drift: `SseStream::from_byte_stream` -> `from_bytes_stream`.

Deny-level fixes:
- `await_holding_lock` (25): the subscription-auth and account-context test
  serialization locks now use `tokio::sync::Mutex` (matching the existing
  `store_lock` precedent); the remaining sites scope their guard in a block so
  it is provably released before the await. Explicit `drop()` did not satisfy
  the lint, which reasons over lexical scope.
- `suspicious_open_options`: the json-store lock file now states
  `truncate(false)` — it must never be truncated while another process holds
  the flock on that inode.
- `items_after_test_module`, `never_loop`, `octal_escapes`,
  `unnecessary_get_then_check`, `drop_non_drop`.

Also documents the previously undocumented `unsafe` blocks in the SSH plugin
source, terminal exec, and webdriver capture/evaluator paths, and records why
`client_info` keeps advertising the SEP-2577-deprecated `roots`/`sampling`
capabilities: dropping them is a protocol-visible change pinned by an existing
contract test, not a lint cleanup.

`stat_local_path_metadata` keeps its expanded symlink check behind an explicit
`#[allow]`: clippy only sees the `#[cfg(not(windows))]` arm and would collapse
away the Windows reparse-point detection.

No behavior change otherwise. Remaining warnings (`too_many_arguments`,
`large_enum_variant`, `type_complexity`, and the macOS accessibility FFI's
undocumented `unsafe` blocks) need design judgment and are left alone.
@bobleer
bobleer merged commit 5b364e7 into GCWing:main Jul 30, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant