mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(desktop): resolve clippy warnings introduced in Phase 3 seam commits
- items_after_test_module: move SCOPE_GENERATION_TEST_LOCK before mod tests in scope.rs - await_holding_lock: add #[allow] + SAFETY comments on all tests holding std::sync::Mutex across .await (single-threaded tokio runtime, no deadlock risk) - needless_borrow: drop redundant & on &handle calls to async import cores - too_many_arguments: suppress on spawn_agent_child_at (8-arg function required by approved Area-5 signature; bounded to two call sites) - redundant_closure: replace wrapper closures with function-item references for stop_managed_agent_process and write_agent_runtime_receipt - unused_imports: remove unused load_managed_agents_at from two test imports - cloned_ref_to_slice_refs: replace &[x.clone()] with std::slice::from_ref(&x) in three test save_managed_agents_at calls - doc_lazy_continuation: add two extra spaces to continuation lines in the team_snapshot.rs docstring list item 3 - dead_code: suppress on make_captured_scope helper prepared for future tests Co-authored-by: Will Pfleger <pfleger.will@gmail.com> Signed-off-by: Will Pfleger <pfleger.will@gmail.com>
This commit is contained in:
co-authored by
Will Pfleger
parent
fcfd41dbff
commit
1a1571b165
@@ -352,7 +352,7 @@ async fn restart_local_agent_on_config_change(
|
||||
})
|
||||
},
|
||||
// Production stop function.
|
||||
|app_ref, record_mut, runtimes| stop_managed_agent_process(app_ref, record_mut, runtimes),
|
||||
stop_managed_agent_process,
|
||||
// Production spawn function.
|
||||
|app_ref, rec, relay, owner, personas, global, teams| {
|
||||
crate::managed_agents::spawn_agent_child_at(
|
||||
@@ -360,7 +360,7 @@ async fn restart_local_agent_on_config_change(
|
||||
)
|
||||
},
|
||||
// Production receipt function.
|
||||
|app_ref, receipt| crate::managed_agents::write_agent_runtime_receipt(app_ref, receipt),
|
||||
crate::managed_agents::write_agent_runtime_receipt,
|
||||
)
|
||||
.await
|
||||
}
|
||||
@@ -1390,8 +1390,8 @@ mod tests {
|
||||
#[test]
|
||||
fn test_record_mesh_change_after_preflight_aborts_before_stop() {
|
||||
use crate::managed_agents::{
|
||||
storage::{load_managed_agents_at, save_managed_agents_at},
|
||||
BackendKind, ManagedAgentPairRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey,
|
||||
storage::save_managed_agents_at, BackendKind, ManagedAgentPairRuntime,
|
||||
ManagedAgentRecord, ManagedAgentRuntimeKey,
|
||||
};
|
||||
use tauri::Manager;
|
||||
|
||||
@@ -1466,7 +1466,7 @@ mod tests {
|
||||
runtime: None,
|
||||
name_pool: vec![],
|
||||
};
|
||||
save_managed_agents_at(tmp.path(), &[record.clone()]).unwrap();
|
||||
save_managed_agents_at(tmp.path(), std::slice::from_ref(&record)).unwrap();
|
||||
|
||||
let app = make_mock_app();
|
||||
let app_handle = app.handle().clone();
|
||||
@@ -1585,8 +1585,8 @@ mod tests {
|
||||
#[tokio::test]
|
||||
async fn test_full_tail_stop_spawn_receipt_register_save() {
|
||||
use crate::managed_agents::{
|
||||
storage::{load_managed_agents_at, save_managed_agents_at},
|
||||
BackendKind, ManagedAgentPairRuntime, ManagedAgentRecord, ManagedAgentRuntimeKey,
|
||||
storage::save_managed_agents_at, BackendKind, ManagedAgentPairRuntime,
|
||||
ManagedAgentRecord, ManagedAgentRuntimeKey,
|
||||
};
|
||||
use std::sync::{Arc, Mutex};
|
||||
use tauri::Manager;
|
||||
@@ -1664,7 +1664,7 @@ mod tests {
|
||||
};
|
||||
|
||||
// Write initial store.
|
||||
save_managed_agents_at(tmp.path(), &[record.clone()]).unwrap();
|
||||
save_managed_agents_at(tmp.path(), std::slice::from_ref(&record)).unwrap();
|
||||
std::fs::write(tmp.path().join("personas.json"), b"[]").unwrap();
|
||||
std::fs::write(tmp.path().join("global-agent-config.json"), b"{}").unwrap();
|
||||
|
||||
|
||||
@@ -866,6 +866,9 @@ async fn test_active_client_stop_then_transition_preflight_succeeds() {
|
||||
/// on the same lock. When the test body drops the guard the install task
|
||||
/// acquires, validates, and finds the captured scope stale.
|
||||
#[tokio::test]
|
||||
// SAFETY: single-threaded tokio runtime; lock serializes generation counter
|
||||
// mutations — cannot deadlock. See import_tests.rs for full rationale.
|
||||
#[allow(clippy::await_holding_lock)]
|
||||
async fn test_transition_held_queued_install_detects_stale_scope() {
|
||||
use crate::managed_agents::scope::{
|
||||
next_scope_generation, WorkspaceAgentScope, SCOPE_GENERATION_TEST_LOCK,
|
||||
@@ -971,6 +974,9 @@ async fn test_transition_held_queued_install_detects_stale_scope() {
|
||||
/// on the same lock. When the test body drops the guard the transition task
|
||||
/// acquires, runs `fail_if_client_mesh_active`, and finds the installed client.
|
||||
#[tokio::test]
|
||||
// SAFETY: single-threaded tokio runtime; lock serializes generation counter
|
||||
// mutations — cannot deadlock. See import_tests.rs for full rationale.
|
||||
#[allow(clippy::await_holding_lock)]
|
||||
async fn test_install_held_transition_preflight_observes_client() {
|
||||
use crate::managed_agents::scope::{
|
||||
next_scope_generation, WorkspaceAgentScope, SCOPE_GENERATION_TEST_LOCK,
|
||||
|
||||
@@ -245,6 +245,12 @@ fn setup_import_app_with_scope(
|
||||
/// The snapshot in this test has no memory entries so the memory adapter is
|
||||
/// never called; the profile-relay assertion is sufficient.
|
||||
#[tokio::test]
|
||||
// SAFETY: `#[tokio::test]` uses a single-threaded runtime by default, so
|
||||
// holding `std::sync::Mutex` across `.await` points cannot deadlock.
|
||||
// The lock serializes tests that advance the process-global scope generation
|
||||
// counter — dropping it early would let a racing test corrupt the counter
|
||||
// mid-import, causing a spurious stale-scope failure.
|
||||
#[allow(clippy::await_holding_lock)]
|
||||
async fn test_agent_switch_between_store_and_profile_finishes_captured_outbound() {
|
||||
// Serialize against the before_store rejection test to prevent the
|
||||
// concurrent next_scope_generation() bump from causing a spurious
|
||||
@@ -284,7 +290,7 @@ async fn test_agent_switch_between_store_and_profile_finishes_captured_outbound(
|
||||
|
||||
let result = confirm_agent_snapshot_import_core(
|
||||
input,
|
||||
&handle,
|
||||
handle,
|
||||
&state,
|
||||
|| {}, // before_store: no-op
|
||||
move || {
|
||||
@@ -335,6 +341,9 @@ async fn test_agent_switch_between_store_and_profile_finishes_captured_outbound(
|
||||
/// concurrent workspace switch that arrived after `capture_agent_snapshot_import_entry`
|
||||
/// returned but before Phase 3a acquired the store lock.
|
||||
#[tokio::test]
|
||||
// SAFETY: single-threaded tokio runtime; lock held to serialize generation
|
||||
// counter mutations — cannot deadlock. See sister test for full rationale.
|
||||
#[allow(clippy::await_holding_lock)]
|
||||
async fn test_agent_identity_switch_before_store_is_rejected() {
|
||||
// Serialize against the after_store test to prevent the generation bump
|
||||
// inside before_store from racing Phase 3a of the after_store test.
|
||||
@@ -360,7 +369,7 @@ async fn test_agent_identity_switch_before_store_is_rejected() {
|
||||
|
||||
let result = confirm_agent_snapshot_import_core(
|
||||
input,
|
||||
&handle,
|
||||
handle,
|
||||
&state,
|
||||
move || {
|
||||
// before_store: advance generation — simulates a workspace switch
|
||||
|
||||
@@ -498,12 +498,12 @@ pub(crate) use team_snapshot_entry::capture_team_snapshot_import_entry;
|
||||
/// If ANY generation fails, return immediately — zero writes.
|
||||
/// 3. Store — inside `managed_agents_store_lock`: write all `AgentDefinition`s
|
||||
/// + all `ManagedAgentRecord`s (with `team_id` set) + `TeamRecord`.
|
||||
/// Both store files are snapshotted (or noted absent) before the first
|
||||
/// write. On any write error the pre-import state is restored — including
|
||||
/// deleting a file that was absent, cleaning minted keyring entries, and
|
||||
/// surfacing rollback failures alongside the original error. This makes
|
||||
/// the store phase all-or-none for ordinary application errors; a process
|
||||
/// crash between atomic file commits is NOT covered.
|
||||
/// Both store files are snapshotted (or noted absent) before the first
|
||||
/// write. On any write error the pre-import state is restored — including
|
||||
/// deleting a file that was absent, cleaning minted keyring entries, and
|
||||
/// surfacing rollback failures alongside the original error. This makes
|
||||
/// the store phase all-or-none for ordinary application errors; a process
|
||||
/// crash between atomic file commits is NOT covered.
|
||||
/// 4. Profile sync — for each member, call `sync_managed_agent_profile`.
|
||||
/// Best-effort; errors are collected per member.
|
||||
/// 5. Memory restore — for each member with non-empty snapshot memory,
|
||||
|
||||
@@ -941,6 +941,9 @@ fn setup_team_import_app_with_scope(
|
||||
/// to a different relay + fresh owner inside the hook; all per-member profile
|
||||
/// adapters must receive the old captured relay URL.
|
||||
#[tokio::test]
|
||||
// SAFETY: single-threaded tokio runtime; lock held to serialize generation
|
||||
// counter mutations — cannot deadlock. See import_tests.rs for full rationale.
|
||||
#[allow(clippy::await_holding_lock)]
|
||||
async fn test_confirm_team_snapshot_import_switch_between_store_and_profile() {
|
||||
let _gen_guard = GENERATION_TEST_LOCK.lock().unwrap();
|
||||
|
||||
@@ -972,7 +975,7 @@ async fn test_confirm_team_snapshot_import_switch_between_store_and_profile() {
|
||||
|
||||
let result = confirm_team_snapshot_import_core(
|
||||
input,
|
||||
&handle,
|
||||
handle,
|
||||
&state,
|
||||
|| {},
|
||||
move || {
|
||||
@@ -1032,6 +1035,9 @@ async fn test_confirm_team_snapshot_import_switch_between_store_and_profile() {
|
||||
/// `before_store` hook advances scope generation — Phase 3 must reject BEFORE
|
||||
/// any write and BEFORE any outbound call.
|
||||
#[tokio::test]
|
||||
// SAFETY: single-threaded tokio runtime; lock held to serialize generation
|
||||
// counter mutations — cannot deadlock. See import_tests.rs for full rationale.
|
||||
#[allow(clippy::await_holding_lock)]
|
||||
async fn test_team_identity_switch_before_store_is_rejected() {
|
||||
let _gen_guard = GENERATION_TEST_LOCK.lock().unwrap();
|
||||
|
||||
@@ -1054,7 +1060,7 @@ async fn test_team_identity_switch_before_store_is_rejected() {
|
||||
|
||||
let result = confirm_team_snapshot_import_core(
|
||||
input,
|
||||
&handle,
|
||||
handle,
|
||||
&state,
|
||||
move || {
|
||||
next_scope_generation();
|
||||
|
||||
@@ -480,6 +480,7 @@ pub fn spawn_agent_child<R: tauri::Runtime>(
|
||||
/// through the entire epoch — no workspace switch can occur during this call,
|
||||
/// so the caller's captured teams (loaded from the captured definitions_dir)
|
||||
/// are the correct teams for this spawn.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) fn spawn_agent_child_at<R: tauri::Runtime>(
|
||||
app: &tauri::AppHandle<R>,
|
||||
record: &ManagedAgentRecord,
|
||||
@@ -680,7 +681,7 @@ pub(crate) fn spawn_agent_child_at<R: tauri::Runtime>(
|
||||
}
|
||||
}
|
||||
}
|
||||
let team_instructions = super::spawn_hash::effective_team_instructions(record, &teams);
|
||||
let team_instructions = super::spawn_hash::effective_team_instructions(record, teams);
|
||||
if let Some(instructions) = &team_instructions {
|
||||
command.env("BUZZ_ACP_TEAM_INSTRUCTIONS", instructions);
|
||||
} else {
|
||||
@@ -802,13 +803,8 @@ pub(crate) fn spawn_agent_child_at<R: tauri::Runtime>(
|
||||
)
|
||||
})?;
|
||||
|
||||
let spawn_config_hash = super::spawn_hash::spawn_config_hash(
|
||||
record,
|
||||
personas,
|
||||
&teams,
|
||||
&effective_relay_url,
|
||||
global,
|
||||
);
|
||||
let spawn_config_hash =
|
||||
super::spawn_hash::spawn_config_hash(record, personas, teams, &effective_relay_url, global);
|
||||
|
||||
let spawned_adapter_availability = if runtime_meta.is_some_and(|r| r.id == "codex") {
|
||||
super::adapter_availability_cached()
|
||||
|
||||
@@ -488,6 +488,7 @@ fn test_partial_drain_stop_failure_delivers_stopped_prefix_and_remaining_tail()
|
||||
// The serialization invariant (writers blocked by store lock, not transition
|
||||
// guard) is documented in the adapter and verified in the adapter-level tests.
|
||||
|
||||
#[allow(dead_code)] // helper prepared for future tests; not yet referenced
|
||||
fn make_captured_scope() -> super::super::scope::WorkspaceAgentScope {
|
||||
// Build a scope whose generation matches the current global counter.
|
||||
// Tests that need a stale scope call `next_scope_generation()` after
|
||||
@@ -858,7 +859,8 @@ fn test_compensate_drain_writer_vs_compensation_deterministic() {
|
||||
runtime: None,
|
||||
name_pool: vec![],
|
||||
};
|
||||
super::super::storage::save_managed_agents_at(&tmp_path, &[initial_record.clone()]).unwrap();
|
||||
super::super::storage::save_managed_agents_at(&tmp_path, std::slice::from_ref(&initial_record))
|
||||
.unwrap();
|
||||
|
||||
let entry1 = make_drain_entry(&pubkey1, "wss://relay.example", true);
|
||||
let stopped = vec![entry1.clone()];
|
||||
|
||||
@@ -236,6 +236,18 @@ impl WorkspaceApplyResult {
|
||||
}
|
||||
}
|
||||
|
||||
/// Process-global mutex that serializes tests touching the process-global
|
||||
/// scope generation counter.
|
||||
///
|
||||
/// Any test that (a) captures a generation and requires it to be stable
|
||||
/// through Phase 3a or (b) calls `next_scope_generation()` inside a hook
|
||||
/// must hold this guard for its entire duration. Tests across modules share
|
||||
/// the same counter so they must share the same serialization primitive.
|
||||
///
|
||||
/// Exposed only under `#[cfg(test)]` to avoid polluting the production API.
|
||||
#[cfg(test)]
|
||||
pub(crate) static SCOPE_GENERATION_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -442,15 +454,3 @@ mod tests {
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// Process-global mutex that serializes tests touching the process-global
|
||||
/// scope generation counter.
|
||||
///
|
||||
/// Any test that (a) captures a generation and requires it to be stable
|
||||
/// through Phase 3a or (b) calls `next_scope_generation()` inside a hook
|
||||
/// must hold this guard for its entire duration. Tests across modules share
|
||||
/// the same counter so they must share the same serialization primitive.
|
||||
///
|
||||
/// Exposed only under `#[cfg(test)]` to avoid polluting the production API.
|
||||
#[cfg(test)]
|
||||
pub(crate) static SCOPE_GENERATION_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
|
||||
|
||||
Reference in New Issue
Block a user