Plan P: Nursery Promotion Sweep

On this page

Status

Step Description Status

1

Plan filing — body lands as a docs-only MR alongside epic + per-lint-step issues. nav.adoc Planned list gains a Plan P row.

Not started

2

use_self sweep — 21 emissions across impl ApiError arms in crates/craig-common/src/error.rs. Per-emission: replace ApiError::Variant { .. } with Self::Variant { .. } inside the existing impl ApiError block. Mechanical sweep; no behavior change. Flip workspace entry from allow to deny.

Done (2026-06-05) — #505 closed via this MR. Actual surface (~50 sites across 11 files in 8 crates) larger than Plan M Step 12 estimate of 21 because cascade-stop on first error masked downstream emissions. Workspace clippy::use_self allow entry removed; lint now enforced at deny via the nursery group.

3

too_long_first_doc_paragraph long-tail sweep — 45+ emissions across the workspace (the foundational-crate baseline was swept in Plan M Step 12). Per-emission: split the first doc paragraph at the first sentence boundary; add a blank /// line; everything after becomes paragraph 2. Mechanical sweep; readability win. Flip workspace entry from allow to deny.

Done (2026-06-05) — 506 closed via this MR. ~90 sites swept across 2 cascade rounds (44 first round in craig-auth + craig-cli/clients; 46 second round revealed once craig-auth compiled clean in craig-api, craig-authz, craig-test-lib). Python auto-sweep script splits first paragraph at first sentence boundary. Workspace clippy::too_long_first_doc_paragraph allow removed; lint enforced at deny via the nursery group. 2 crate-level ![allow(too_long_first_doc_paragraph)] blocks added (craig-auth + craig-api) for macro-expansion false-positives that emit with no source span and cannot be locally suppressed.

4

future_not_send !Send-audit — 33 emissions across the BFF + CLI client layers. Per-emission: classify as (a) genuinely-!Send and safe under the single-threaded runtime contract → per-fn #[allow] with rationale, (b) accidentally-!Send because of a clone-able & held across await → fix by cloning or Arc-wrapping. Investigation-heavy; estimated 1-2 weeks of cumulative work. Flip workspace entry from allow to deny only after the audit is complete.

Done (2026-06-05) — #507 closed via this MR. Audit outcome: workspace allow retained permanently with comprehensive architectural rationale per ADR-031 § 1. Real surface: 140+ emissions across 4 site classes, ALL intentionally !Send by design: (1) craig-cli/clients/** (33 sites — &Transport borrows across awaits; CLI is single-shot one-binary), (2) craig-test-lib/harness.rs (98 sites — &self methods yielding borrow futures; tests run single-threaded), (3) craig-test-lib/client.rs (4 sites), (4) craig-authz/eval_thread.rs (1 — JDM evaluator is !Send per ADR-024 architectural constraint), (5) craig-web/routes/mod.rs (3 sites — &AppState across awaits). Refactoring all 140 sites to Arc<RwLock<T>> would lose borrow-check guarantees for negative perf value. Cargo.toml allow entry carries the full audit rationale.

5

derive_partial_eq_without_eq evaluation — 32 emissions across DTOs and event payloads. Per-type: classify as (a) safe-to-add-Eq (no float-equivalent fields, no HashSet<T> callers needed) → add Eq derive next to PartialEq, (b) unsafe-to-add-Eq (carries Decimal / f64 / similarity score) → per-type #[allow] with rationale. Mixed sweep + per-site allow. Flip workspace entry from allow to deny.

Done (2026-06-05) — #508 closed via this MR. Audit: workspace grep confirmed ZERO f32/f64/Decimal/rust_decimal fields in any of the 33 emitting types (all in craig-cases-contracts + 1 in craig-store/config). Pure-sweep outcome: Eq derive added next to PartialEq at all 33 sites. Workspace clippy::derive_partial_eq_without_eq allow removed; lint now enforced at deny via the nursery group.

6

missing_const_for_fn triage — 23 emissions. Per-fn: convert to const fn IFF a caller actually needs const context (none in current codebase). Otherwise per-fn #[allow] with rationale "no const-context caller; const-promotion would be cosmetic". Likely keeps the workspace-level allow — sweep may close as "no value to enforcement". Document the decision at Step 6 close.

Done (2026-06-05) — #509 closed via this MR. Audit outcome: workspace allow retained permanently with documented rationale per ADR-031 § 2. Actual surface (38 sites across craig-common/settings + craig-db + craig-mq + craig-partner-* adapters) is larger than the Plan M Step 12 estimate of 23, but the conclusion holds: ZERO callers in the workspace use these fns in const context (no const FOO: T = some_fn(); items). Const-promotion is cosmetic — adds maintenance friction without enabling any real const-context use. Cargo.toml comment block at missing_const_for_fn allow entry records the audit decision.

7

or_fun_call long-tail sweep — 6 emissions beyond the 7 Plan M swept in tools/craig-mock-server/{doe,ies}.rs. Per-emission: same Value::as_str + unwrap_or pattern, or per-site .unwrap_or_else(|| …​) lazy form. Mechanical sweep. Flip workspace entry from allow to deny.

Done (2026-06-05) — #510 closed via this MR. 6 sites swept: craig-exchange/{adapters/standard.rs, api/transactions.rs}, craig-rules/tests/api/evil_corpus.rs, craig-web/{routes/intake/referrals.rs (×2), routes/report.rs}, xtask/cmd/api_docs.rs. All .unwrap_or(macro!(…​)) → .unwrap_or_else(|| macro!(…​)) + 1 .or(opt) → .or_else(|| opt). Workspace clippy::or_fun_call allow entry removed; lint now enforced at deny via the nursery group.

8

redundant_clone craig-auth audit — 5 emissions on Arc<…​> / Vec<u8> keypair material in crates/craig-auth/src/{introspection,jwks,keypair_env}.rs. Per-site benchmark with criterion: if clone hot-path measurable, remove the clone; if no measurable difference, per-site #[allow] with rationale "keypair clone is once-per-rotation; benchmark shows <1μs impact". Workspace entry stays allow until benchmarks land.

Done (2026-06-05) — #511 closed via this MR. Audit outcome: no benchmarks needed. All 5 craig-auth sites are legitimate redundant clones on error-return paths (audience-mismatch error variants in introspection.rs + jwks.rs; file-empty error variants in keypair_env.rs) — the cloned values were already owned locals not used after return; clippy correctly flagged them. Cascade revealed 10 more sites across craig-authz tests, craig-test-lib tests, craig-exchange/adapters, craig-intake/{main,sink/upstream_error}, craig-web/{auth,routes/report}, xtask/cmd/{coverage_matrix,identity/verify,migrate}. 15 sites total swept (all moves where ownership analysis allows; 1 needed mut arg added to verify::run; nested test-closure patterns simplified to single-clone). Workspace clippy::redundant_clone allow removed; lint enforced at deny via the nursery group.

9

Long-tail single-emission cleanup — redundant_pub_crate (1 in craig-cli/transport), needless_collect (4), suspicious_operation_groupings (1), unused_peekable (1), suboptimal_flops (1), unnecessary_lazy_evaluations (1), collection_is_never_read (1 long-tail), single_option_map (1), significant_drop_tightening (2 in craig-auth), option_if_let_else (4 in mock-server handlers). Per-site evaluation: sweep if mechanical + clear value; otherwise per-site #[allow] with rationale. Single MR for the long-tail batch. Flip applicable workspace entries from allow to deny.

Done (2026-06-05) — #512 closed via this MR. 6 long-tail lints flipped from workspace allow to deny (needless_collect, unused_peekable, suboptimal_flops, unnecessary_lazy_evaluations, collection_is_never_read, single_option_map, suspicious_operation_groupings) — these had zero real emissions or single-site sweeps. 2 retained at workspace allow with documented architectural rationale: redundant_pub_crate (CRAIG’s Plan D F-023 1,065-item pub(crate) discipline is intentional; nursery lint suggests pub which would defeat the visibility-marker intent), option_if_let_else (3 per-fn allows on mock-server handlers + 1 nursery false-positive in craig-common/pagination’s PageResponse<T> Vec<T> field that struct- and field-level allows fail to suppress). 13 sweeps shipped this MR: 11 in craig-seed/sql.rs (match Option<T> → .as_ref().map_or_else / fn-pointer), 1 in craig-common/telemetry.rs, 1 in craig-mock-server/smile.rs tuple-returning if-let, 4 test-collect→any sweeps, 1 dead Vec removal, 1 unused .peekable() drop, 1 collected-Vec → chained iterator, 1 single_option_map → .is_some().then(), 1 per-site suboptimal_flops allow.

10

Plan completion audit + archive. Per delivery-protocol.md: subagent verifies all Step 2–9 sweeps complete; nav.adoc Planned → Archive; archive.adoc new row; .claude/CLAUDE.md Phase Status final row. Workspace lint count expected to grow from "17 clippy denies + 2 rustc denies + nursery group at deny with 17 escape hatches" to roughly "17 + 2 + nursery with ≤6 escape hatches" depending on Step 4 and Step 5 outcomes.

Done (2026-06-06) — this MR closes #513. Audit subagent verified all 9 prior steps Done with MR citations; workspace nursery = deny + 12 lints flipped from allow to deny + 4 retained workspace allows with documented architectural rationale + 2 crate-level allows for macro-expansion false-positives; cargo xtask docs plan-lint clean; nextest 1603/1603 pass; clippy --workspace --all-targets -D warnings clean; #505–#512 closed; #513 + epic &38 close on this MR merge.

Epic: TBD (file alongside Step 1)
Issues: TBD (one per step, #N0X–#N0Y)
Branch prefix: refactor/plan-p-
*Milestone: TBD

Context

Plan M Step 12 (#503, !581) promoted clippy::nursery = deny workspace-wide and triaged 130+ emissions across 16 unique nursery lints. Per the plan’s <10-same-MR-threshold rule (codified as ADR-031 § 1), small-surface + clear-value lints were swept in the Step 12 MR (24 sites across 6 lints). 17 lints with either >10 emissions or case-by-case judgment were allow-listed at workspace priority 1 with documented rationale.

ADR-031 § 4 (Successor plan handoff) requires that deferred sweeps go to a named successor plan with explicit sweep scope — NOT a "potential improvements" section or a vague follow-up issue without owner. This plan IS that successor: each deferred lint becomes one bounded sweep MR (Steps 2–9), and the plan completes when each workspace allow entry has been re-evaluated and either flipped to deny after sweep, or replaced with per-site #[allow] blocks documenting why each surviving emission is correct.

Threat Model

Plan P’s threat model is the same as Plan M’s: workspace lint configs that allow-list a lint silently can hide real new emissions added by clippy updates, by codebase growth, or by lint behavior changes. The Plan M Step 12 allow-lists are time-bound (each allow entry’s comment cites Plan P as the home for the sweep); if Plan P never lands, the allow-lists become permanent and the regression-gate value of nursery = deny is partially defeated.

Cross-cutting Invariants

  1. Each Step 2–9 MR flips its corresponding workspace allow entry to deny (or removes it entirely if defeating the lint workspace-wide). Leaving an entry at allow after its sweep step closes means the sweep didn’t actually take.

  2. Per-site #[allow(clippy::X, reason = "…​")] blocks survive only if the reason describes an idiomatic correctness invariant (e.g. "JDM eval thread is !Send by design"), NOT just "this lint is annoying."

  3. ADR-031 patterns apply to Plan P MRs too. New [workspace.lints.clippy] allow entries (if any) carry the same comment-block requirement: surface measurement + rationale + successor-plan pointer.

  4. Per-step sweeps do not introduce new lint emissions in unrelated lints. Pre-push battery covers this; an MR that touches craig-cases/error.rs to flip 21 use_self arms must not introduce, say, new wildcard_enum_match_arm violations.

Step DAG

1 (plan filing)
  ↓
  ── per-lint sweeps run in any order; each independent ──
  ↓
  2 (use_self) → 3 (too_long_first_doc_paragraph) → 7 (or_fun_call) → 9 (long-tail batch)
  ↓
  ── investigation-heavy steps run in parallel with mechanical sweeps ──
  ↓
  4 (future_not_send !Send-audit) || 5 (derive_partial_eq_without_eq triage) || 6 (missing_const_for_fn) || 8 (redundant_clone benchmarks)
  ↓
  ── all sweeps converge ──
  ↓
  10 (audit + archive)

Risks

  1. Step 4 future_not_send audit may surface real concurrency bugs. !Send futures held across tokio::spawn boundaries silently deadlock or single-task-serialize work that callers expected to parallelize. If the audit finds these, the fix may be larger than expected (refactor to Arc<RwLock<T>>). Mitigation: each finding files its own issue + fixes in a separate MR; Plan P Step 4 closes with audit findings + linked fix MRs.

  2. Step 8 redundant_clone benchmarks may inconclusive. Keypair clone is rare (once-per-rotation). If criterion shows <1μs impact, the entry stays at workspace allow. Documented as "benchmark confirms negligible" and the step closes — Plan P Step 10 audit accepts the outcome.

  3. Step 6 missing_const_for_fn may close as "no value". If no caller in the codebase needs const context, the lint adds maintenance friction without benefit. Documented at Step 6 close.

Successor plans

After Plan P closes, future work that builds on the workspace lint discipline:

  • Plan Q — clippy::restriction probe. The restriction group fires on patterns that are sometimes correct, sometimes wrong (e.g. clippy::indexing_slicing, clippy::shadow_reuse, clippy::float_arithmetic). Probe with the same <10-same-MR-threshold rule from ADR-031. Most lints will allow-list; a few will sweep.

  • Plan R — cargo-mutants integration. Once Plans M + P land, the lint config is tight enough that mutation testing won’t surface trivially-broken code (no panic!() slipping past tests, no .unwrap() masking errors). Run cargo mutants on the workspace and triage the surviving mutants.

References

  • Plan M archived body: plans/archive/quality-lint-followthrough.adoc

  • ADR-031 — Nursery Lint Triage and Promotion Pattern: adrs/adr-031-nursery-lint-triage.adoc

  • Cargo.toml [workspace.lints.clippy] — current 17 escape-hatch entries from Plan M Step 12

  • delivery-protocol.md § Plan completion audit — audit protocol Plan P Step 10 follows

Edit this page · latest