fix(desktop): clear native clippy gate and fresh retention scope init

Repair every clippy 1.95 -D warnings failure from CI run 34155454680
(Desktop Core 101846514710) without weakening a check: the test env lock
becomes a tokio Mutex (buzz-db POOL_METRICS_TEST_LOCK convention) so async
tests hold the same single lock domain across await via
lock_path_mutex_async while sync tests keep blocking_lock; the dead
spawn_agent_child wrapper is removed and test-only helpers are cfg(test)
gated with ownership notes; mesh-feature-used relay_mesh_model_id carries
a justified cfg_attr; identical placement branches merge; restore test
module moves after the last item; mechanical bool/borrow/slice fixes; two
documented ABI argument-count allows at the launch-inputs passthrough and
the Tauri invoke contract.

local_host opened the scoped retention db without ensuring the
agents/retention parent that active_retention_scope only creates lazily
at retention time, panicking every remote_credentials fixture before its
assertions and any first lifecycle message preceding retention on a fresh
install. Mirror remote_stop::connection's on-demand parent ensure with
the same contextual directory error.

All seven remote_credentials fixtures execute and pass on the authorized
no-default lane; workspace clippy -D warnings and rustfmt pass locally.

Signed-off-by: Logan Johnson <loganj@squareup.com>
This commit is contained in:
Logan Johnson
2026-09-07 16:22:12 -04:00
parent 1c4428cb82
commit 61943e15ec
12 changed files with 124 additions and 94 deletions

View File

@@ -218,6 +218,11 @@ pub(crate) enum LocalStartIntent {
Automatic,
}
// The argument tuple is the launch-inputs ABI this seam forwards verbatim to
// `start_local_agent_with_preflight_using` (which carries the same allow): the
// capture → preflight → locked-revalidate → spawn inputs. Collapsing it into
// a params struct would fork the shared `_using` boundary shape for a lint.
#[allow(clippy::too_many_arguments)]
async fn start_local_agent_with_preflight(
app: &AppHandle,
state: &AppState,

View File

@@ -17,6 +17,12 @@ use tauri::{AppHandle, Manager};
mod configurations;
// Tauri IPC command boundary: the nine parameters are the existing frontend
// invoke contract (each named argument is passed individually from
// `desktopLifecycle.ts`), so the shape is ABI-fixed here — a params struct
// would change the wire format. Same command-boundary allow as the other
// wide `#[tauri::command]` (`send_managed_agent_channel_message`).
#[allow(clippy::too_many_arguments)]
#[tauri::command]
pub fn prepare_desktop_lifecycle(
app: AppHandle,

View File

@@ -286,6 +286,11 @@ pub fn resolve_effective_model_provider_pair(
/// never spawns (see `require_resolved`), so it never needs a mesh preflight
/// either; the caller's own orphan handling downstream is unaffected, this
/// just avoids tripping mesh bootstrap for a start that will be refused.
///
/// Bound only by `effective_config/tests.rs` now — production mesh preflights
/// resolve through `PreparedLaunch`'s captured effective config — so the
/// resolver stays as a `cfg(test)` seam for the mesh-model regression tests.
#[cfg(test)]
pub fn resolve_effective_relay_mesh_model_id(
record: &ManagedAgentRecord,
definitions: &[AgentDefinition],

View File

@@ -52,27 +52,45 @@ pub(crate) use team_repair::team_persona_key;
mod teams;
mod types;
// Shared lock for tests that call `lock_path_mutex` or `lock_env_mutex`.
// Both helpers delegate here so any two tests using either helper are mutually
// exclusive with each other. Tests in other modules that maintain their own
// independent locks (app_state_tests, agent_config_tests, reader_tests) are
// NOT in this domain and are not covered by this mutex.
// Shared lock for tests that call `lock_path_mutex`, `lock_path_mutex_async`,
// or `lock_env_mutex`. All helpers delegate here so any two tests using any
// of them are mutually exclusive with each other. Tests in other modules that
// maintain their own independent locks (app_state_tests, agent_config_tests,
// reader_tests) are NOT in this domain and are not covered by this mutex.
//
// The lock is a `tokio::sync::Mutex` (same convention as buzz-db's
// `POOL_METRICS_TEST_LOCK`) so async tests can hold the serialization guard
// across `.await` points without tripping `clippy::await_holding_lock`, while
// sync tests acquire it with `blocking_lock()`. One lock domain keeps sync
// and async env-mutating tests mutually exclusive; the guard scope (the whole
// test body) is unchanged, so shared-state isolation is preserved exactly.
#[cfg(test)]
static PROCESS_ENV_MUTEX: std::sync::Mutex<()> = std::sync::Mutex::new(());
static PROCESS_ENV_MUTEX: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(());
// Acquires the shared process-env lock. Call from any test in this module that
// reads, writes, or removes a process-global environment variable (including PATH).
// Acquires the shared process-env lock from a synchronous test. Call from any
// sync test that reads, writes, or removes a process-global environment
// variable (including PATH). Panics if called from inside an async context —
// async tests must use `lock_path_mutex_async` instead.
#[cfg(test)]
pub(crate) fn lock_path_mutex() -> std::sync::MutexGuard<'static, ()> {
PROCESS_ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner())
pub(crate) fn lock_path_mutex() -> tokio::sync::MutexGuard<'static, ()> {
PROCESS_ENV_MUTEX.blocking_lock()
}
// Async twin of `lock_path_mutex`: the same single lock domain, acquired in
// `async fn` tests. Hold the returned guard for the whole test body, across
// `.await` points, exactly where the sync guard was previously held — the
// guard keeps every env-mutating test (sync or async) mutually exclusive.
#[cfg(test)]
pub(crate) async fn lock_path_mutex_async() -> tokio::sync::MutexGuard<'static, ()> {
PROCESS_ENV_MUTEX.lock().await
}
// Delegates to the same lock as `lock_path_mutex`. Tests using either helper
// are mutually exclusive with each other; PATH and env-key mutations that go
// through these helpers cannot race.
#[cfg(test)]
pub(crate) fn lock_env_mutex() -> std::sync::MutexGuard<'static, ()> {
PROCESS_ENV_MUTEX.lock().unwrap_or_else(|e| e.into_inner())
pub(crate) fn lock_env_mutex() -> tokio::sync::MutexGuard<'static, ()> {
PROCESS_ENV_MUTEX.blocking_lock()
}
pub use backend::*;

View File

@@ -202,10 +202,8 @@ pub(crate) fn receive(
|| (request.action == Action::Start
&& desired(conn, &request.target.agent)?.map(|(_, id)| id)
!= Some(event.id.to_hex()))))
{
(Outcome::Unknown, None)
} else if request.action != Action::Status
&& nostr::Timestamp::now().as_secs() >= event.created_at.as_secs().saturating_add(30)
|| (request.action != Action::Status
&& nostr::Timestamp::now().as_secs() >= event.created_at.as_secs().saturating_add(30))
{
(Outcome::Unknown, None)
} else {

View File

@@ -598,22 +598,6 @@ pub(crate) fn spawn_pending_profile_reconciliations(app: &tauri::AppHandle, work
}
}
#[cfg(test)]
mod profile_reconcile_tests {
use super::profile_reconcile_completed;
use crate::commands::ProfileReconcileOutcome;
#[test]
fn skipped_reconciliation_never_retires_pending_work() {
assert!(profile_reconcile_completed(
ProfileReconcileOutcome::Reconciled
));
assert!(!profile_reconcile_completed(
ProfileReconcileOutcome::SkippedDisabled
));
}
}
fn persist_restore_error<R: tauri::Runtime>(
app: &tauri::AppHandle<R>,
state: &AppState,
@@ -630,3 +614,19 @@ fn persist_restore_error<R: tauri::Runtime>(
record.last_error = Some(error);
save_managed_agents(app, &records)
}
#[cfg(test)]
mod profile_reconcile_tests {
use super::profile_reconcile_completed;
use crate::commands::ProfileReconcileOutcome;
#[test]
fn skipped_reconciliation_never_retires_pending_work() {
assert!(profile_reconcile_completed(
ProfileReconcileOutcome::Reconciled
));
assert!(!profile_reconcile_completed(
ProfileReconcileOutcome::SkippedDisabled
));
}
}

View File

@@ -333,8 +333,8 @@ pub fn build_managed_agent_summary<R: tauri::Runtime>(
last_error_code: record.last_error_code,
start_on_app_launch: record.start_on_app_launch,
auto_restart_on_config_change: record.auto_restart_on_config_change
&& !pair_runtime
.is_some_and(|runtime| runtime.spawn_config.runtime_configuration.is_some())
&& pair_runtime
.is_none_or(|runtime| runtime.spawn_config.runtime_configuration.is_none())
&& app
.state::<crate::app_state::AppState>()
.signing_keys()
@@ -422,11 +422,12 @@ pub(crate) fn configure_runtime_cli(
#[must_use]
pub(crate) struct EffortApplied(());
/// Apply effort env to an agent spawn command. Called by `spawn_agent_child`
/// (production) and `effort_cmd_tests` (test seam). Inner-seam: removing
/// `apply_spawn_effort_env` below turns the production-sequence tests RED.
/// Outer-seam: the returned token is consumed by `spawn_with_effort_proof`;
/// deleting this call leaves `effort` undefined at the spawn site.
/// Apply effort env to an agent spawn command. Called by
/// `spawn_agent_child_prepared` (production) and `effort_cmd_tests` (test
/// seam). Inner-seam: removing `apply_spawn_effort_env` below turns the
/// production-sequence tests RED. Outer-seam: the returned token is consumed
/// by `spawn_with_effort_proof`; deleting this call leaves `effort` undefined
/// at the spawn site.
pub(crate) fn apply_effort_to_spawn_command(
cmd: &mut std::process::Command,
record: &crate::managed_agents::types::ManagedAgentRecord,
@@ -443,8 +444,9 @@ pub(crate) fn apply_effort_to_spawn_command(
}
/// Spawn the agent command, consuming the `EffortApplied` proof token.
/// Deleting `apply_effort_to_spawn_command` from `spawn_agent_child` leaves
/// `effort` undefined here — a compile error CI catches before any test runs.
/// Deleting `apply_effort_to_spawn_command` from `spawn_agent_child_prepared`
/// leaves `effort` undefined here — a compile error CI catches before any
/// test runs.
pub(crate) fn spawn_with_effort_proof(
cmd: &mut std::process::Command,
_effort: EffortApplied,
@@ -464,27 +466,10 @@ pub(crate) fn spawn_with_effort_proof(
/// publishes the triggering message before this spawn and passes its send
/// timestamp here so the harness's first REQ replays past that message no
/// matter how long the spawn takes. buzz-acp clamps stale floors to ~15 min.
pub fn spawn_agent_child<R: tauri::Runtime>(
app: &AppHandle<R>,
record: &ManagedAgentRecord,
relay_url: &str,
lazy: bool,
owner_hex: Option<&str>,
replay_floor_unix: Option<u64>,
resume: Option<&super::remote_stop::ResumeTicket>,
) -> Result<crate::managed_agents::ManagedAgentProcess, String> {
spawn_agent_child_prepared(
app,
record,
relay_url,
lazy,
owner_hex,
replay_floor_unix,
resume,
None,
)
}
///
/// `prepared`: the captured, preflighted launch plan. Production callers pass
/// a `PreparedLaunch`; tests may pass `None` to exercise the refusal paths
/// that reject an un-preflighted spawn.
#[allow(clippy::too_many_arguments)]
pub(crate) fn spawn_agent_child_prepared<R: tauri::Runtime>(
app: &AppHandle<R>,
@@ -646,7 +631,7 @@ pub(crate) fn spawn_agent_child_prepared<R: tauri::Runtime>(
// ── Readiness check: set setup-payload if agent is not ready ─────────────
// `spawned_setup_mode` is stamped on `ManagedAgentProcess` below.
let spawned_setup_mode =
apply_setup_payload_env(&mut command, record, &descriptor, runtime_meta);
apply_setup_payload_env(&mut command, record, descriptor, runtime_meta);
// Emit BUZZ_ACP_IDLE_TIMEOUT only when explicitly set; the harness
// DEFAULT_IDLE_TIMEOUT_SECS is the single source of truth. The deprecated
// BUZZ_ACP_TURN_TIMEOUT pinned agents to a stale default (320s).
@@ -883,7 +868,7 @@ pub(crate) fn spawn_agent_child_prepared<R: tauri::Runtime>(
.transpose()?;
super::runtime_configurations::apply_required_model_env(&mut command, required_model)?;
if let Some(model) = required_model {
if !runtime_meta.is_some_and(|runtime| runtime.id == "claude") {
if runtime_meta.is_none_or(|runtime| runtime.id != "claude") {
command.env("BUZZ_ACP_MODEL", model);
}
}
@@ -935,12 +920,13 @@ pub(crate) fn spawn_agent_child_prepared<R: tauri::Runtime>(
})
}
/// Spawn (or adopt) the runtime pair for `record` on the caller's bound
/// workspace relay. `workspace_relay` can only be produced by
/// `bind_expected_relay_scope`, so this spawn consumes — by construction — the
/// exact workspace-relay read the caller's scope assertion passed on; it never
/// re-reads the mutable override (see `relay::scope`). The key comes from
/// [`bound_runtime_key`] — the seam the spawn-key regressions exercise.
/// Test-only seam over [`start_managed_agent_process_prepared`] with no
/// captured launch: the runtime-authority regressions in
/// `runtime/authority_tests.rs` use it to prove a spawn without a preflighted
/// plan is refused before any side effect. Production callers resolve a
/// `PreparedLaunch` and go through `start_managed_agent_process_prepared`
/// directly.
#[cfg(test)]
pub fn start_managed_agent_process<R: tauri::Runtime>(
app: &AppHandle<R>,
record: &mut ManagedAgentRecord,

View File

@@ -88,9 +88,6 @@ impl PreparedLaunch {
Ok(())
}
pub(crate) fn record(&self) -> &ManagedAgentRecord {
&self.record
}
pub(crate) fn configuration(&self) -> Option<RuntimeConfigurationRef> {
selected(&self.record)
.ok()
@@ -407,7 +404,7 @@ fn verify_owner(record: &ManagedAgentRecord, owner: &str) -> Result<(), String>
buzz_sdk_pkg::nip_oa::verify_auth_tag(tag, &agent).ok()
});
if record.backend != super::BackendKind::Local
|| !verified.is_some_and(|key| key.to_hex() == owner)
|| verified.is_none_or(|key| key.to_hex() != owner)
{
return Err("Agent ownership is unavailable in this scope".into());
}
@@ -460,6 +457,17 @@ fn local_host<R: tauri::Runtime>(
relay_url: community.into(),
owner_keys: keys,
};
// `managed_agents_base_dir` only creates `agents/`; the scoped
// `retention/` parent is normally ensured lazily by
// `active_retention_scope` during event retention, which a first
// lifecycle message can precede on a fresh install (and callers that
// bypass app startup, like tests, never trigger). Same on-demand parent
// ensure as `remote_stop::connection` so opening the scoped identity db
// is safe for every fresh-state caller.
if let Some(parent) = scope.db_path.parent() {
std::fs::create_dir_all(parent)
.map_err(|error| format!("failed to create retention scope directory: {error}"))?;
}
crate::commands::desktop_stop::local_id(
&mut super::retention::open_retention_db(&scope.db_path)?,
&scope,

View File

@@ -67,7 +67,7 @@ impl Fixture {
save(&mut self.record, &self.owner, relay, entry)
}
fn persist(&self) {
agents::save_managed_agents(self.app.handle(), &[self.record.clone()]).unwrap();
agents::save_managed_agents(self.app.handle(), std::slice::from_ref(&self.record)).unwrap();
}
fn finish_children(&self) -> Vec<(String, Option<RuntimeConfigurationRef>)> {
let state = self.app.state::<crate::app_state::AppState>();
@@ -132,7 +132,7 @@ impl Drop for Fixture {
#[tokio::test]
async fn ordinary_default_start_cannot_launch_new_selection_after_preflight() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let mut fixture = Fixture::new();
fixture.persist();
let app = fixture.app.handle().clone();
@@ -168,7 +168,7 @@ async fn ordinary_default_start_cannot_launch_new_selection_after_preflight() {
#[tokio::test]
async fn restore_preflights_selected_provider_in_both_directions() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for (default_provider, named_provider, expected) in [
("relay-mesh", "openai", None),
("openai", "relay-mesh", Some("selected-model")),
@@ -215,7 +215,7 @@ async fn restore_preflights_selected_provider_in_both_directions() {
#[tokio::test]
async fn restore_refuses_provider_preflight_failure_before_spawn() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let mut fixture = Fixture::new();
fixture.named(ONE, "relay-mesh", "offline-model");
fixture.persist();
@@ -242,7 +242,7 @@ async fn restore_refuses_provider_preflight_failure_before_spawn() {
#[tokio::test]
async fn bulk_restart_preflights_every_captured_community_and_revalidates_before_spawn() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
// The unchanged case proves a real lazy pair spawn and receipts. The other
// cases edit while the first provider awaits, after BOTH plans were captured.
for mutation in ["unchanged", "revision", "workspace", "selection"] {
@@ -340,7 +340,7 @@ async fn bulk_restart_preflights_every_captured_community_and_revalidates_before
#[tokio::test]
async fn restore_selection_fence_survives_suspension_in_both_directions() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for starts_named in [false, true] {
let mut fixture = Fixture::new();
let named = fixture.named(ONE, "relay-mesh", "named-model");
@@ -407,7 +407,7 @@ async fn restore_selection_fence_survives_suspension_in_both_directions() {
#[tokio::test]
async fn ordinary_default_revalidates_effective_inputs_not_only_selection() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let mut fixture = Fixture::new();
fixture.persist();
let app = fixture.app.handle().clone();
@@ -466,7 +466,7 @@ impl Fixture {
#[tokio::test]
async fn direct_restart_preflights_target_and_preserves_old_child_on_refusal() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for relay in [ONE, TWO] {
for mutation in [
"unchanged",
@@ -565,7 +565,7 @@ async fn direct_restart_preflights_target_and_preserves_old_child_on_refusal() {
#[tokio::test]
async fn direct_start_and_reconcile_continuation_refuse_selection_change_and_stop() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for explicit in [true, false] {
for mutation in ["selection", "stop"] {
let mut fixture = Fixture::new();
@@ -609,7 +609,7 @@ async fn direct_start_and_reconcile_continuation_refuse_selection_change_and_sto
#[cfg(feature = "mesh-llm")]
#[tokio::test]
async fn recovery_uses_running_pair_snapshots_not_default_or_next_selection() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for default_mesh in [false, true] {
let mut fixture = Fixture::new();
fixture.hold_children();
@@ -664,7 +664,7 @@ async fn recovery_uses_running_pair_snapshots_not_default_or_next_selection() {
#[tokio::test]
async fn reconcile_keeps_authorization_inputs_without_cross_pair_timestamp_failure() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let mut fixture = Fixture::new();
fixture.hold_children();
fixture.named(ONE, "relay-mesh", "mesh-one");
@@ -693,7 +693,7 @@ async fn reconcile_keeps_authorization_inputs_without_cross_pair_timestamp_failu
#[tokio::test]
async fn bulk_stop_during_preflight_cannot_be_resumed_by_automatic_start() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let mut fixture = Fixture::new();
fixture.named(ONE, "relay-mesh", "mesh-one");
fixture.named(TWO, "relay-mesh", "mesh-two");

View File

@@ -227,7 +227,7 @@ fn malformed_or_stale_reference_is_rejected_before_launch_resolution() {
async fn shared_spawn_registers_exact_plan_and_rejects_edits_or_lost_identity() {
use std::os::unix::fs::PermissionsExt;
use tauri::Manager;
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let temp = tempfile::tempdir().unwrap();
struct Restore(Vec<(&'static str, Option<std::ffi::OsString>)>);
impl Drop for Restore {

View File

@@ -220,7 +220,7 @@ impl Drop for Fixture {
#[tokio::test]
async fn provisioned_destination_catalog_start_and_same_host_switch_use_shared_launch() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let mut fixture = Fixture::new();
let catalog = fixture.action(Action::Catalog, None).await;
assert_eq!(catalog.outcome, Outcome::Ready);
@@ -285,7 +285,7 @@ async fn provisioned_destination_catalog_start_and_same_host_switch_use_shared_l
#[tokio::test]
async fn unavailable_identity_or_runtime_excludes_catalog_and_refuses_start() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for missing in ["absent", "wrong", "runtime"] {
let mut fixture = Fixture::new();
match missing {
@@ -324,7 +324,7 @@ async fn unavailable_identity_or_runtime_excludes_catalog_and_refuses_start() {
#[tokio::test]
async fn revoked_key_keeps_existing_process_visible_and_restart_refuses_without_teardown() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let mut fixture = Fixture::new();
assert_eq!(
fixture.action(Action::Start, None).await.outcome,
@@ -350,7 +350,7 @@ async fn revoked_key_keeps_existing_process_visible_and_restart_refuses_without_
#[tokio::test]
async fn key_loss_during_preflight_is_rechecked_for_start_and_catalog() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for action in [Action::Start, Action::Catalog] {
let fixture = Fixture::new();
let result = fixture
@@ -382,7 +382,7 @@ async fn key_loss_during_preflight_is_rechecked_for_start_and_catalog() {
#[tokio::test]
async fn failed_destination_preflight_never_tears_down_existing_source() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let fixture = Fixture::new();
assert_eq!(
fixture.action(Action::Start, None).await.outcome,
@@ -436,7 +436,7 @@ fn assert_failed_without_child(fixture: &Fixture) -> ManagedAgentRecord {
#[tokio::test]
async fn post_stop_key_loss_or_executable_loss_persists_failed_without_restoring_credentials() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
for revoke_key in [true, false] {
let fixture = Fixture::new();
let store_path = agents::storage::managed_agents_store_path(fixture.app.handle()).unwrap();
@@ -480,7 +480,7 @@ async fn post_stop_key_loss_or_executable_loss_persists_failed_without_restoring
#[tokio::test]
async fn post_stop_expiry_persists_truthful_failed_without_second_spawn() {
let _guard = agents::lock_path_mutex();
let _guard = agents::lock_path_mutex_async().await;
let fixture = Fixture::new();
// TERM is ignored only by this bounded synthetic process; ordinary Stop
// takes its one-second grace then SIGKILL, crossing the captured deadline.

View File

@@ -181,6 +181,10 @@ pub(crate) fn effective_effort(descriptor: &EffectiveHarnessDescriptor) -> Optio
impl SpawnConfigSnapshot {
/// Assemble the snapshot from values a spawn has already resolved.
/// Read by `mesh_llm::recovery` to collect the mesh consumer model ids of
/// live pairs; dead only in default-feature builds, where `mesh-llm` is
/// off and that consumer is compiled out.
#[cfg_attr(not(feature = "mesh-llm"), allow(dead_code))]
pub(crate) fn relay_mesh_model_id(&self) -> Option<String> {
super::resolved_relay_mesh_model_id(self.provider.as_deref(), self.model.as_deref())
}