auto-fix batch 2026-04-28 (claude/friendly-maxwell-Oggvw)

.claude/skills/resolving-issues/SKILL.md

@@ -42,6 +42,7 @@ All sub-fixes land on one master branch per session. Master PR is opened **only
 - **Sub-PR base ≠ main means GH Actions workflows scoped to `pull_request: branches: [main]` won't fire.** Implementer treats local `just check` green (fmt + clippy + test + wasm) as the merge gate; do not park sub-PR open waiting for CI that won't run. Master PR (base=main, opened end-of-run) runs full CI before human merge — the load-bearing gate.
 - Implementer watches CI on sub-PR ONLY when CI actually runs (rare; usually requires PR base = main). CI green → merge sub-PR into master branch. No CI run → local `just check` green is the gate, then merge.
 - CI red after one fix attempt → file follow-up issue (caveman body, link blocker), close sub-PR. Move on. Do NOT leave it as a draft for someone to resume — the next scheduled run picks up the follow-up issue automatically.
+- **`just` may be absent in some sandboxes.** Fall back to raw `cargo` equivalents — `cargo fmt --check`, `cargo clippy --workspace --all-targets -- -D warnings`, `cargo test --workspace`, `cargo check --target wasm32-unknown-unknown -p <crate>` for dual-target lib crates. Same gate, different binary. Report which path was used in the sub-PR body.
 
 ## Core Loop
 
@@ -130,6 +131,7 @@ Never defer skill edits to a follow-up — they ship with the run that surfaced
 
 - Pre-worktree: `git stash` or `git restore` main dir; `.claude/worktrees/` in `.gitignore`.
 - Worktree per issue, branched off master branch. Tear down after sub-PR merges or parks as draft.
+- **Worktree dir may be pre-populated** with residue from a prior session that didn't tear down cleanly. Inspect first: if it contains the same logical work the implementer would do, incorporate it (run gates, finish the workflow). Don't blindly `git worktree remove --force` — that destroys legitimate in-progress work. Reset only if the residue is from an unrelated branch.
 
 ## Quality
 
@@ -139,3 +141,4 @@ Never defer skill edits to a follow-up — they ship with the run that surfaced
 - Master PR opened only at end of run, **non-draft**, with whatever sub-PRs landed. Master PR runs full CI when opened — the actual quality net for the run.
 - Anything blocked → follow-up issue, sub-PR closed, next scheduled run picks it up. Never open the master PR as draft. Only skip opening the PR entirely if literally zero sub-PRs landed.
 - No in-session continuation. The issue queue is the durable handoff.
+- **Browser tests may be compile-only when wasm-pack / firefox / geckodriver are absent in the sandbox.** `cargo check --target wasm32-unknown-unknown -p willow-web --tests` is the fallback gate — it confirms the test compiles. The full headless run executes on real CI when the master PR opens. Implementer must flag the gap explicitly in the sub-PR body so the human knows what was actually run vs. compile-checked.

crates/actor/src/fsm.rs

@@ -215,7 +215,10 @@ mod tests {
     struct StrictDoor;
 
     #[derive(Debug, Clone, PartialEq)]
-    #[allow(dead_code)]
+    #[allow(
+        dead_code,
+        reason = "Open variant exists for completeness; tests start in Closed and assert StrictDoor rejects further transitions."
+    )]
     enum DoorState {
         Open,
         Closed,

crates/client/src/lib.rs

@@ -68,6 +68,14 @@ mod tests_profile_view;
 #[path = "tests/ephemeral.rs"]
 mod tests_ephemeral;
 
+#[cfg(test)]
+#[path = "tests/voice.rs"]
+mod tests_voice;
+
+#[cfg(test)]
+#[path = "tests/governance.rs"]
+mod tests_governance;
+
 /// How long a typing indicator remains visible after the last typing event, in milliseconds.
 pub const TYPING_INDICATOR_TTL_MS: u64 = 5_000;
 

crates/client/src/tests/governance.rs

@@ -0,0 +1,263 @@
+//! Happy-path tests for the governance mutation API.
+//!
+//! Covers the five governance mutators in [`crate::mutations::ClientMutations`]
+//! that previously only had Playwright coverage (or none at all):
+//!
+//! * `propose_grant_admin`     — `EventKind::Propose { ProposedAction::GrantAdmin }`
+//! * `propose_revoke_admin`    — `EventKind::Propose { ProposedAction::RevokeAdmin }`
+//! * `propose_kick_member`     — `EventKind::Propose { ProposedAction::KickMember }`
+//! * `propose_set_threshold`   — `EventKind::Propose { ProposedAction::SetVoteThreshold }`
+//! * `delete_role`             — `EventKind::DeleteRole`
+//!
+//! These run against the in-memory `test_client` harness. The genesis
+//! author (`test_client()`'s identity) is the server owner, which by
+//! the authority model is automatically an admin and the root of
+//! trust — so `Propose { … }` and `DeleteRole` events are accepted
+//! without further permission setup.
+//!
+//! Scope: each test only asserts that the mutator emits the expected
+//! `EventKind` (variant + payload) into the local DAG. We deliberately
+//! do not assert *downstream* state-machine effects (vote tally,
+//! actual admin promotion, role removal) — those live in the
+//! tier-1 state-machine tests in `crates/state/src/materialize.rs`.
+//! Mirroring the convention established by `voice.rs` (PR #464), the
+//! focus here is "the mutator emitted the right event".
+//!
+//! Why poke the DAG instead of intercepting the wire broadcast?
+//! `test_client()` does not subscribe to any topic, so
+//! `broadcast_event` drops the bytes with a warning. Reading the DAG
+//! captures the same signed event the broadcast would carry — the
+//! mutator's `apply_event` and `broadcast_event` calls are fed the
+//! identical `Event` value. See `mutations.rs` for the call sequence.
+
+use willow_identity::Identity;
+use willow_state::{Event, EventKind, ProposedAction, VoteThreshold};
+
+use crate::test_client;
+use crate::ClientHandle;
+
+/// Snapshot every event currently in `client`'s managed DAG, in
+/// topological order. The owner-authored mutation under test lands at
+/// the tail of this vector once the mutator's `apply_event` returns.
+async fn dag_events<N: willow_network::Network>(client: &ClientHandle<N>) -> Vec<Event> {
+    willow_actor::state::select(&client.dag_addr, |ds| {
+        ds.managed
+            .dag()
+            .topological_sort()
+            .into_iter()
+            .cloned()
+            .collect()
+    })
+    .await
+}
+
+/// Find the first event in the DAG matching `predicate`. Tests use
+/// this to assert *exactly one* event of the expected variant landed,
+/// without depending on the ordering of unrelated genesis / channel
+/// events that `test_client()` seeds.
+async fn find_event<N, F>(client: &ClientHandle<N>, predicate: F) -> Option<Event>
+where
+    N: willow_network::Network,
+    F: Fn(&EventKind) -> bool,
+{
+    dag_events(client)
+        .await
+        .into_iter()
+        .find(|e| predicate(&e.kind))
+}
+
+// ───── propose_grant_admin ──────────────────────────────────────────────
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn propose_grant_admin_emits_propose_grant_admin_event() {
+    let (client, _broker) = test_client();
+    let target = Identity::generate().endpoint_id();
+
+    client
+        .mutations()
+        .propose_grant_admin(target)
+        .await
+        .expect("owner can propose grant_admin");
+
+    let event = find_event(&client, |kind| {
+        matches!(
+            kind,
+            EventKind::Propose {
+                action: ProposedAction::GrantAdmin { .. }
+            }
+        )
+    })
+    .await
+    .expect("propose_grant_admin must emit a Propose { GrantAdmin } event into the DAG");
+
+    match event.kind {
+        EventKind::Propose {
+            action: ProposedAction::GrantAdmin { peer_id },
+        } => {
+            assert_eq!(
+                peer_id, target,
+                "GrantAdmin proposal must target the requested peer_id"
+            );
+        }
+        other => panic!("expected Propose {{ GrantAdmin }}, got {other:?}"),
+    }
+    assert_eq!(
+        event.author,
+        client.identity.endpoint_id(),
+        "propose event must be authored by the local (owner) identity"
+    );
+}
+
+// ───── propose_revoke_admin ─────────────────────────────────────────────
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn propose_revoke_admin_emits_propose_revoke_admin_event() {
+    let (client, _broker) = test_client();
+    let target = Identity::generate().endpoint_id();
+
+    client
+        .mutations()
+        .propose_revoke_admin(target)
+        .await
+        .expect("owner can propose revoke_admin");
+
+    // `propose_revoke_admin` *also* fires a follow-up
+    // `RevokePermission(SendMessages)` (best-effort untrust). That side
+    // effect is documented on the mutator and is fine — we only assert
+    // that the Propose event is among the resulting DAG entries.
+    let event = find_event(&client, |kind| {
+        matches!(
+            kind,
+            EventKind::Propose {
+                action: ProposedAction::RevokeAdmin { .. }
+            }
+        )
+    })
+    .await
+    .expect("propose_revoke_admin must emit a Propose { RevokeAdmin } event into the DAG");
+
+    match event.kind {
+        EventKind::Propose {
+            action: ProposedAction::RevokeAdmin { peer_id },
+        } => {
+            assert_eq!(
+                peer_id, target,
+                "RevokeAdmin proposal must target the requested peer_id"
+            );
+        }
+        other => panic!("expected Propose {{ RevokeAdmin }}, got {other:?}"),
+    }
+}
+
+// ───── propose_kick_member ──────────────────────────────────────────────
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn propose_kick_member_emits_propose_kick_member_event() {
+    let (client, _broker) = test_client();
+    let target = Identity::generate().endpoint_id();
+
+    client
+        .mutations()
+        .propose_kick_member(target)
+        .await
+        .expect("owner can propose kick_member");
+
+    let event = find_event(&client, |kind| {
+        matches!(
+            kind,
+            EventKind::Propose {
+                action: ProposedAction::KickMember { .. }
+            }
+        )
+    })
+    .await
+    .expect("propose_kick_member must emit a Propose { KickMember } event into the DAG");
+
+    match event.kind {
+        EventKind::Propose {
+            action: ProposedAction::KickMember { peer_id },
+        } => {
+            assert_eq!(
+                peer_id, target,
+                "KickMember proposal must target the requested peer_id"
+            );
+        }
+        other => panic!("expected Propose {{ KickMember }}, got {other:?}"),
+    }
+}
+
+// ───── propose_set_threshold ────────────────────────────────────────────
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn propose_set_threshold_emits_propose_set_threshold_event() {
+    let (client, _broker) = test_client();
+
+    client
+        .mutations()
+        .propose_set_threshold(VoteThreshold::Unanimous)
+        .await
+        .expect("owner can propose set_threshold");
+
+    let event = find_event(&client, |kind| {
+        matches!(
+            kind,
+            EventKind::Propose {
+                action: ProposedAction::SetVoteThreshold { .. }
+            }
+        )
+    })
+    .await
+    .expect("propose_set_threshold must emit a Propose { SetVoteThreshold } event into the DAG");
+
+    match event.kind {
+        EventKind::Propose {
+            action: ProposedAction::SetVoteThreshold { threshold },
+        } => {
+            assert_eq!(
+                threshold,
+                VoteThreshold::Unanimous,
+                "SetVoteThreshold proposal must carry the requested threshold variant"
+            );
+        }
+        other => panic!("expected Propose {{ SetVoteThreshold }}, got {other:?}"),
+    }
+}
+
+// ───── delete_role ──────────────────────────────────────────────────────
+
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn delete_role_emits_delete_role_event() {
+    let (client, _broker) = test_client();
+
+    // `delete_role` is a direct admin mutation. We don't have to seed
+    // a matching `CreateRole` first — the state machine accepts the
+    // event regardless of whether the role exists, and this tier of
+    // test is asserting the mutator emits the right event, not the
+    // downstream materialisation. Use a fixed role id so we can
+    // pinpoint the event in the DAG.
+    let role_id = "role-to-delete";
+    client
+        .mutations()
+        .delete_role(role_id)
+        .await
+        .expect("owner can delete_role");
+
+    let event = find_event(&client, |kind| matches!(kind, EventKind::DeleteRole { .. }))
+        .await
+        .expect("delete_role must emit a DeleteRole event into the DAG");
+
+    match event.kind {
+        EventKind::DeleteRole { role_id: emitted } => {
+            assert_eq!(
+                emitted, role_id,
+                "DeleteRole event must carry the requested role_id"
+            );
+        }
+        other => panic!("expected DeleteRole, got {other:?}"),
+    }
+    assert_eq!(
+        event.author,
+        client.identity.endpoint_id(),
+        "delete_role event must be authored by the local (owner) identity"
+    );
+}