hardening/v0.6.x: strictness waves 1+2, AGPL license, MSRV honesty, gate tightening#78
Merged
Conversation
Housekeeping — Cargo.lock version bumps that were missed by the v0.6.4 release commit (493d27c). Zero behavior change; all workspace crates already at 0.6.4 in Cargo.toml.
FileRepository::upsert_file, FileRepository::upsert_location, VolumeRepository::find_or_create, VolumeRepository::record_mount all previously took &mut self, which forced adapters to be stored as &mut references rather than Arc<dyn Trait>. The &mut was spurious — the SqliteFileRepository and SqliteVolumeRepository impls already use Mutex<Connection> for interior mutability, so each method acquires the lock via &self.conn.lock() and holds it for the full method body. WHY: this is a blocker for Arc<dyn FileRepository> sharing across Tauri commands, future axum handlers (Phase 6), and the writer- actor pattern the architecture audit recommends. No behavior change; all 139 tests across perima-core/-db/-fs/-hash/-media and perima (CLI) pass. Also applies cargo-fix unused_mut cleanups (32 fixes) across db tests and CLI bindings that held 'let mut repo = ...' — the mut is no longer required at call sites.
cargo deny check reported 'advisory-not-detected' for these two entries — the upstream unic-char-property and urlpattern deps that triggered them are no longer in the dependency graph (tauri transitive swap). Dead config removed to keep the ignore list accurate. Verified: advisories/bans/licenses/sources all ok.
Adds clippy::dbg_macro=deny, clippy::todo=deny, clippy::unimplemented=deny, clippy::undocumented_unsafe_blocks=warn to the workspace-level lint block. All four fire zero times on HEAD; this is pure forward-insurance catching AI-slop patterns (stray dbg!, todo!, unimplemented! stubs) before they land. undocumented_unsafe_blocks currently has no unsafe blocks to guard (all safe crates carry #![forbid(unsafe_code)]), so it catches the first unsafe block any future ffi crate ships without a // SAFETY: comment. Also fixes two pre-existing clippy::redundant_clone and clippy::needless_pass_by_ref_mut warnings in file_repo.rs and scan.rs that blocked the -D warnings gate before this commit. Source: docs/research/rust_strictness.md §1, §6, §8.
Enables missing_debug_implementations=warn at the workspace level and
closes the 9 resulting warnings with manual Debug impls using
f.debug_struct(…).finish_non_exhaustive() — the standard pattern for
structs holding non-Debug fields (Mutex<Connection>, fs watcher handles,
Tokio channel handles).
WHY manual impl over #[derive(Debug)]: rusqlite::Connection does not
implement Debug; neither do several of the OS-handle / channel types
the fs/media structs hold. finish_non_exhaustive() prints
'StructName { .. }', giving observers struct identity plus an indicator
that internal state is elided. Uniform pattern across all 9 sites
avoids per-struct judgment calls about which fields are safe to log.
Source: docs/research/rust_strictness.md §1 (axum adopts this pattern).
Enables clippy::unwrap_used=warn at the workspace level. All 5 existing call sites are in #[cfg(test)] modules; each module gets a scoped #[allow(clippy::unwrap_used, reason = "tests: unwrap is the assertion")] rather than per-call-site allows. WHY: .unwrap() in non-test code is an AI-slop red flag — LLMs reach for it to silence borrow errors or skip error design. In tests the panic IS the assertion, so unwrap is idiomatic. This configuration makes future non-test .unwrap() fail the clippy gate, forcing explicit per-site justification via #[allow(..., reason = ...)]. Source: docs/research/rust_strictness.md §1 and §8.
Enables clippy::print_stdout=warn and clippy::print_stderr=warn at the workspace level. 35 existing fires are all in crates/cli/ (the binary whose purpose is user-facing output). A crate-root #![allow(...)] in crates/cli/src/main.rs covers the binary and all its cmd/*.rs modules with a reason string that records the policy and names tracing as the future migration target. WHY crate-root over per-function: 35 annotations for zero added safety in a small, single-purpose binary. Non-CLI crates (core, db, fs, hash, media, desktop) must not println!/eprintln! — the lint still fires there and -D warnings will block regressions. Source: docs/research/rust_strictness.md §1, §8.
… CLI Enables the rustc unreachable_pub lint at the workspace level and converts 38 `pub` items in crates/cli/ to `pub(crate)`. Binary crates have no external consumers — any `pub` item not re-exported from main.rs should be crate-scoped. The lint is a rustc lint (not clippy), goes in [workspace.lints.rust]. Distribution of 38 fires across 13 files is listed in the wave-2 plan. All fixes are mechanical: leading `pub ` → `pub(crate) `. Nothing semantically changed — binary crates have no external API surface, so visibility tightening is a no-op observable-behavior-wise. WHY main.rs also changed: module declarations promoted from `mod` to `pub(crate) mod` so the pub(crate) items inside them satisfy unreachable_pub (items inside a private module trigger the lint even with pub(crate)). A crate-root #![allow(clippy::redundant_pub_crate)] is required because clippy::nursery's redundant_pub_crate fires on pub(crate)-inside-pub(crate)-module; the allow is scoped to the binary with a reason string recording the intentional policy. Source: docs/research/rust_strictness.md §1, §8 (research doc had it under clippy; it's actually rustc).
Commit 26f933d (wave-2 Task 2) promoted five top-level module declarations in main.rs from `mod` to `pub(crate) mod` on the belief that unreachable_pub fires on pub(crate) items inside private modules. That belief was wrong: unreachable_pub only flags `pub` items; pub(crate) items inside a private mod are already reachable from the crate root and the lint is silent. The real reason main.rs needed the crate-root allow is clippy::nursery::redundant_pub_crate firing on pub(crate) items inside private `mod` blocks (where pub(crate) is technically redundant). The allow still earns its keep; the pub(crate) mod promotion was pure churn. Reverts 5 promotions to plain `mod` and rewrites the allow's reason string to reflect the actual mechanism. Verified: clippy -D warnings is clean; 139 tests still pass. Follow-up to 26f933d per plan review.
Per project-owner direction. Replaces the previous 'MIT OR Apache-2.0' dual declaration in workspace Cargo.toml with 'AGPL-3.0-or-later' (SPDX). - Cargo.toml: license field updated; applies to every workspace crate via workspace.package inheritance. - LICENSE: canonical GNU AGPLv3 text, fetched verbatim from https://www.gnu.org/licenses/agpl-3.0.txt. No paraphrasing; bit-identical to the upstream file (661 lines). - deny.toml: AGPL-3.0-or-later added to [licenses].allow so cargo-deny accepts our own workspace crates (the allowlist applies to every crate in the graph, including workspace members). AGPL vs MIT/Apache is a meaningful policy change: copyleft with a network-use clause. Anyone providing a network service using perima (or derivative works) must offer the same source under AGPL. Deliberate choice.
The workspace previously declared rust-version = '1.85' but several
transitive dependencies (sysinfo@0.38.4, time@0.3.47, time-core@0.1.8)
require rustc 1.88+. The MSRV declaration was therefore decorative —
local builds worked only because contributors happened to run newer
stable toolchains. Anyone honoring the declared MSRV would fail to
build.
Fix:
- Cargo.toml rust-version bumped 1.85 → 1.88 to match actual dep
requirements (the honest MSRV).
- rust-toolchain.toml added pinning channel = '1.88'. Makes every
cargo invocation use exactly 1.88, so API usages that would only
compile on newer stable are caught here rather than surfacing on
a contributor's machine.
- Two clippy::similar_names errors from 1.88 (the lint's heuristic
was stricter in 1.88 than in 1.94+ where earlier development ran):
- crates/core/src/types.rs: nfc/nfd → precomposed/decomposed
- crates/db/src/search_repo.rs: cam/cap → camera/captured (plus
matching renames at the use-sites in the GroundTruthRow struct).
Verified: cargo clippy -D warnings clean; 139 tests passing; cargo
deny check passing; rustdoc -D warnings clean.
Bump rust-version + channel in lockstep on any future MSRV change.
Three coupled gate tightenings that strengthen the existing strictness posture: - apps/desktop/package.json: 'eslint src/' → 'eslint src/ --max-warnings 0'. Before this change, 'bun run lint' exited 0 on any number of warnings — 15 rules are currently at 'warn' level (including @typescript-eslint/no-explicit-any and 13 react-hooks experimental rules). Lefthook and CI called lint but didn't actually gate on it. --max-warnings 0 promotes warns to blocking errors, matching the '-D warnings' posture the Rust side already has. - justfile docs-coverage: prefix with RUSTDOCFLAGS="-D warnings". The workspace-level rustdoc lints (broken_intra_doc_links, private_intra_doc_links) only fire during cargo check/clippy. rustdoc has a separate warning path for things like invalid HTML tags, malformed code spans, bare URLs, etc. This gate catches those. - crates/cli/src/cmd/tag.rs: backtick Vec<T> in a doc comment so rustdoc doesn't parse <T> as an unclosed HTML tag. Caught today by the new RUSTDOCFLAGS=-D warnings gate — exactly the kind of silent slop it's meant to surface. No behavior change at runtime. ESLint + cargo doc both pass under the tightened gates; 139 tests still passing.
The rust-toolchain.toml pin of channel = '1.88' (commit e3113ab) was too strict: the desktop crate's transitive dep 'specta = =2.0.0-rc.24' uses 'debug_closure_helpers' and const 'TypeId::of', both stabilized after 1.88. The pre-push lefthook (which runs 'just ci' including clippy --workspace) therefore failed before the push itself could run. Fixes: - rust-toolchain.toml channel '1.88' -> 'stable'. Matches CI's dtolnay/rust-toolchain@stable so local and CI resolve identically. The declared rust-version = '1.88' in Cargo.toml stays put as an aspirational MSRV floor; true MSRV enforcement is deferred to a CI matrix job tracked in a follow-up issue. - Three clippy::collapsible_if fires surfaced by stable (1.95) that 1.88 did not flag. Collapsed each nested 'if let' using the stable if-let-chains syntax (Rust 2024 edition): * crates/fs/src/watcher.rs: map_event + bus.emit * crates/media/src/extractor.rs: stsd.as_ref() + Video descriptor * crates/cli/src/cmd/scan.rs: UpsertOutcome match + queue.enqueue Verified: clippy -D warnings clean on all six non-desktop crates; 139 tests passing; semantically identical to the original nests.
The wave-1 and wave-2 strictness commits added workspace-level clippy/rustc lints but were verified only on the non-desktop crates (gtk system deps unavailable in the sandbox). The pre-push lefthook builds the full workspace and surfaced 9 errors in crates/desktop/. Each is a mechanical follow-up to an earlier commit: - commands.rs: 2 call sites of VolumeRepository::find_or_create and ::record_mount were passing '&mut vol_repo' even after the trait methods became &self (commit 081c694). Changed to '&vol_repo'. - commands.rs: 2 functions (run_scan_live, persist_file) took '&mut FR' / '&mut R' params that are no longer used mutably after the trait-sig fix. Changed to '&FR' / '&R'. Cascaded unused-mut cleanup to 2 let-bindings. - commands.rs: 1 triple-nested if-let surfaced by clippy 1.95's stricter collapsible_if — collapsed using if-let-chains syntax. - events.rs (TauriEventEmitter), state.rs (AppState, WatcherState): 3 structs needed manual 'impl Debug' using the finish_non_exhaustive() template introduced in wave 1 (commit 1b03afe) for structs holding non-Debug fields (AppHandle, Arc< repos with Mutex<Connection>>, tokio Mutex<Option<...>>). These fixes were authored blind (sandbox can't build desktop); the pre-commit rustfmt hook applied one small reformat after the signature changes, which is included.
Root cause: `just ci` calls `cargo deny check` + `typos`, but neither binary is installed in the workflow. All 3 OS jobs failed with `error: no such command: deny` at the `cargo deny check` step. Fix: add `taiki-e/install-action@cargo-deny` + `@typos` after `@just` (before `oven-sh/setup-bun`). Prebuilt binaries, not `cargo install` — seconds per job instead of minutes. This was pre-existing on main (last green main was before the `deny` and `typos` recipes were added to `just ci`). PR #78 is the first run to surface it because prior branches didn't trigger PR-on-push.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Hardening sweep on a long-lived feature branch: two Rust strictness waves, license switch to AGPL-3.0-or-later, MSRV alignment with actual dep requirements, ESLint + rustdoc gate tightening, and the follow-up fixes they each surfaced. 14 atomic commits, each independently revertable; full local gate (`just ci`) green at HEAD.
Changes (by commit, bottom-up)
Prereq hardening
Strictness wave 1 (Rust, zero/small-scope fallout)
Strictness wave 2 (Rust, policy-sensitive)
License + MSRV + gate tightening
Follow-up fixes surfaced by the above
Deferred / tracked separately
GitHub issues #63–#77 cover the remaining work:
Test plan
Merge strategy
Create a merge commit (or rebase-and-merge). Do not squash — the 14 commits are designed to be independently revertable; each commit passes `cargo clippy -D warnings` on its own.