clippy: fix lints that crane's cargoClippy properly enforces (#538)

The naersk → crane swap in the parent commit flips clippy from
silently passing to actually failing on `-D warnings` (naersk's
`mode = "clippy"` mangled the `--` separator so the deny never took
effect). This commit clears the surfaced lints so the workspace
builds clean under the new enforcement — every fix is mechanical and
preserves behaviour. Tests still pass (160 across the workspace).

Auto-fixes via `cargo clippy --fix`:
- `doc_markdown` (19 sites): bare identifiers in doc comments
  wrapped in backticks
- `format_in_format_args`, `explicit_into_iter_loop`,
  `redundant_closure_for_method_calls`, `useless_conversion`, and
  a few more — mechanical rewrites of the kind cargo can apply
  safely.

Hand-fixed:
- `match_same_arms` (forge_notify::is_atx_heading): two arms returning
  `true` collapsed into a single `matches!` pattern.
- `cast_sign_loss` + `format_push_string` (mcp.rs status formatter):
  guarded `i64 → u64` through `u64::try_from(…).unwrap_or(0)` (status
  timestamps are always positive in practice; clamp the skew edge to
  0) and swapped `out.push_str(&format!(…))` for `write!` into the
  buffer with an infallible-writer `let _ =`.
- `doc_lazy_continuation` in turn.rs + manager_server.rs + sh4re/lib.rs:
  doc paragraphs that the markdown parser was treating as list-item
  continuations got either a separating blank line or a `/`-for-`+`
  word swap so the parser stops seeing a list.
- `unused_async` (manager_server::handle_request_schedule_prompt):
  function has no `.await`; dropped the `async` and its `.await` call
  site.
- `needless_pass_by_value` (scheduled_prompts::submit): take
  `&NewSchedule` instead of moving the struct in; updated two prod
  callers and eight test sites to pass references.
- `type_complexity` (approvals::mark_cancelled): hoisted the
  7-tuple SELECT row shape into a `type CancelLookupRow = (…);` alias.

Allow-with-reason for intentional patterns:
- `option_option` (6 sites across dashboard / scheduled_prompts /
  manager_server): `Option<Option<T>>` carries three-state PATCH
  semantics (missing key = leave alone, `Some(None)` = clear,
  `Some(Some(v))` = set). Collapsing to `Option<T>` loses the
  "clear" state.
- `dead_code` (rebuild_queue::QueueKind::Destroy /
  QueueSource::CrashRecover; topology::parent_of / default_seed):
  wire-shape variants + API surfaces kept for the upcoming features
  (#361 follow-ups, future `Destroy` queue routing, crash-recovery
  path). Allowed at the variant / function level with the rationale
  in `reason = "…"`.
- `too_many_lines` on three specific call-sites: a 117-line
  exhaustive-variant test (dashboard_events::kind_tag_matches_…),
  the meta-flake string template renderer
  (meta::render_flake_with_lookup), and the notification poll loop
  (forge_notify::poll_once) — splitting any of them would just hide
  the contiguous shape they exist to keep visible.

`nix flake check` formatting target is still broken on main itself
(pre-existing nixfmt drift across ~28 files unrelated to this PR);
left alone here so the scope stays "crane port + lints the port
exposed" and the operator's review doesn't have to triage drive-by
nixfmt churn.
This commit is contained in:
iris 2026-05-29 01:42:06 +02:00 committed by Mara
commit 9ed58ab96d
17 changed files with 643 additions and 463 deletions

View file

@ -70,6 +70,7 @@ pub enum QueueKind {
Spawn,
/// Destroy with `--purge` (real fs work). Not yet routed here; the
/// variant exists so the wire shape doesn't need to change later.
#[allow(dead_code, reason = "wire shape — routed by a future PR")]
Destroy,
}
@ -102,10 +103,11 @@ pub enum QueueSource {
AutoUpdate,
/// Crash recovery path (future use — currently no auto-rebuild on
/// crash, but the variant exists for the imminent feature).
#[allow(dead_code, reason = "wire shape — used by a future feature")]
CrashRecover,
/// Operator approved a pending `Approval` row on the dashboard.
/// `QueueEntry.approval_id` points back at the source row so the
/// worker can fetch the kind-specific payload (commit_ref, inputs,
/// worker can fetch the kind-specific payload (`commit_ref`, inputs,
/// description) before dispatching.
Approval,
}
@ -137,7 +139,10 @@ pub enum QueueState {
impl QueueState {
pub fn is_terminal(self) -> bool {
matches!(self, QueueState::Done | QueueState::Failed | QueueState::Cancelled)
matches!(
self,
QueueState::Done | QueueState::Failed | QueueState::Cancelled
)
}
}
@ -150,7 +155,7 @@ pub struct QueueEntry {
/// so SSE upserts land in place rather than churning the list.
pub id: u64,
/// Target agent name, or the literal `"hyperhive"` for entries
/// (MetaUpdate) that affect the meta flake rather than a single
/// (`MetaUpdate`) that affect the meta flake rather than a single
/// agent.
pub agent: String,
pub kind: QueueKind,
@ -183,8 +188,8 @@ pub struct QueueEntry {
pub inputs: Vec<String>,
/// Source approval row id when this entry was created by an
/// operator-approve POST (`source == Approval`). The worker uses
/// it to re-fetch the kind-specific payload (commit_ref / inputs /
/// description / fetched_sha) and to fire `ApprovalResolved` on
/// it to re-fetch the kind-specific payload (`commit_ref` / inputs /
/// description / `fetched_sha`) and to fire `ApprovalResolved` on
/// completion. `None` for non-approval entries — preserved on
/// the wire that way too.
#[serde(default, skip_serializing_if = "Option::is_none")]
@ -312,7 +317,7 @@ impl RebuildQueue {
// docstring + #365 for why). Approval-driven entries also
// require the approval_id to match so two distinct approvals
// for the same agent never collapse into one queue slot.
for entry in inner.entries.iter_mut() {
for entry in &mut inner.entries {
if entry.state == QueueState::Queued
&& entry.kind == kind
&& entry.agent == agent
@ -320,7 +325,8 @@ impl RebuildQueue {
&& entry.approval_id == approval_id
{
if !entry.reason.contains(&reason) {
entry.reason.push_str(&format!("\nalso requested by: {reason}"));
use std::fmt::Write as _;
let _ = write!(entry.reason, "\nalso requested by: {reason}");
}
return entry.id;
}
@ -372,7 +378,10 @@ impl RebuildQueue {
/// and leaving a stale "in flight" label after a terminal
/// transition would mislead the dashboard render.
pub fn finish(&self, id: u64, state: QueueState, error: Option<String>) {
debug_assert!(state.is_terminal(), "finish() called with non-terminal {state:?}");
debug_assert!(
state.is_terminal(),
"finish() called with non-terminal {state:?}"
);
let mut inner = self.inner.lock().expect("rebuild_queue mutex poisoned");
if let Some(entry) = inner.entries.iter_mut().find(|e| e.id == id) {
entry.state = state;
@ -422,7 +431,7 @@ impl RebuildQueue {
pub fn cancel_children(&self, parent: u64) -> usize {
let mut inner = self.inner.lock().expect("rebuild_queue mutex poisoned");
let mut count = 0;
for entry in inner.entries.iter_mut() {
for entry in &mut inner.entries {
if entry.parent_id == Some(parent) && entry.state == QueueState::Queued {
entry.state = QueueState::Cancelled;
entry.finished_at = Some(now_unix());
@ -440,13 +449,13 @@ impl RebuildQueue {
/// safely interrupted). Returns true when an entry was cancelled.
pub fn cancel(&self, id: u64) -> bool {
let mut inner = self.inner.lock().expect("rebuild_queue mutex poisoned");
if let Some(entry) = inner.entries.iter_mut().find(|e| e.id == id) {
if entry.state == QueueState::Queued {
entry.state = QueueState::Cancelled;
entry.finished_at = Some(now_unix());
Self::trim_history(&mut inner);
return true;
}
if let Some(entry) = inner.entries.iter_mut().find(|e| e.id == id)
&& entry.state == QueueState::Queued
{
entry.state = QueueState::Cancelled;
entry.finished_at = Some(now_unix());
Self::trim_history(&mut inner);
return true;
}
false
}
@ -533,7 +542,7 @@ pub async fn run_worker(coord: std::sync::Arc<crate::coordinator::Coordinator>)
return;
}
}
_ = coord.rebuild_queue.notify.notified() => {
() = coord.rebuild_queue.notify.notified() => {
// New entry — back to the drain loop.
}
}
@ -556,12 +565,14 @@ async fn dispatch(
crate::actions::run_approval_apply_commit(coord, Some(entry.id), approval_id).await
}
(QueueKind::Rebuild, None) => {
let current_rev = crate::auto_update::current_flake_rev(&coord.hyperhive_flake)
.unwrap_or_default();
crate::auto_update::rebuild_agent(coord, &entry.agent, &current_rev, Some(entry.id)).await
let current_rev =
crate::auto_update::current_flake_rev(&coord.hyperhive_flake).unwrap_or_default();
crate::auto_update::rebuild_agent(coord, &entry.agent, &current_rev, Some(entry.id))
.await
}
(QueueKind::MetaUpdate, Some(approval_id)) => {
crate::actions::run_approval_update_meta_inputs(coord, Some(entry.id), approval_id).await
crate::actions::run_approval_update_meta_inputs(coord, Some(entry.id), approval_id)
.await
}
(QueueKind::MetaUpdate, None) => run_meta_update(coord, entry).await,
(QueueKind::Spawn, Some(approval_id)) => {
@ -601,7 +612,11 @@ async fn run_meta_update(
) -> anyhow::Result<()> {
let _progress = coord.meta_update_guard();
let inputs = entry.inputs.clone();
tracing::info!(?inputs, parent = entry.id, "rebuild_queue: meta-update starting");
tracing::info!(
?inputs,
parent = entry.id,
"rebuild_queue: meta-update starting"
);
coord.set_queue_step(Some(entry.id), "nix flake update");
let result = if inputs.is_empty() {
crate::meta::lock_update(&[]).await
@ -633,7 +648,7 @@ async fn run_meta_update(
/// Compute which agents a `nix flake update <inputs>` on the meta
/// flake would affect. Used by callers that pre-enqueue cascade
/// `Rebuild` entries at MetaUpdate submission time (issue #347) so the
/// `Rebuild` entries at `MetaUpdate` submission time (issue #347) so the
/// dashboard can render the dependent work alongside its parent before
/// the lock bump actually runs.
///
@ -658,7 +673,8 @@ pub async fn meta_update_cascade_agents(inputs: &[String]) -> Vec<String> {
if c == crate::lifecycle::MANAGER_NAME {
Some(crate::lifecycle::MANAGER_NAME.to_owned())
} else {
c.strip_prefix(crate::lifecycle::AGENT_PREFIX).map(str::to_owned)
c.strip_prefix(crate::lifecycle::AGENT_PREFIX)
.map(str::to_owned)
}
})
.collect()
@ -782,9 +798,11 @@ mod tests {
// Both inputs lists are preserved.
let inputs: Vec<&[String]> = snap.iter().map(|e| e.inputs.as_slice()).collect();
assert!(inputs.iter().any(|i| *i == ["nixpkgs"]));
assert!(inputs
.iter()
.any(|i| *i == ["agent-bitburner/bitburner-agent"]));
assert!(
inputs
.iter()
.any(|i| *i == ["agent-bitburner/bitburner-agent"])
);
}
#[test]