diff --git a/src/apps/cli/src/account.rs b/src/apps/cli/src/account.rs index 60c427bd94..b70e68e694 100644 --- a/src/apps/cli/src/account.rs +++ b/src/apps/cli/src/account.rs @@ -1076,9 +1076,8 @@ async fn handle_relay_auth_error( return; } let mut current_context = account_context.write().await; - if !current_context - .as_ref() - .is_some_and(|context| context.session.token == expected_token) + if current_context + .as_ref().is_none_or(|context| context.session.token != expected_token) { tracing::debug!("Ignoring auth error cleanup for a replaced account"); return; diff --git a/src/apps/cli/src/account_sync.rs b/src/apps/cli/src/account_sync.rs index c094e9b7b2..d27b4bc3fc 100644 --- a/src/apps/cli/src/account_sync.rs +++ b/src/apps/cli/src/account_sync.rs @@ -54,7 +54,7 @@ pub(crate) fn start_settings_sync_loop() { on_settings_pushed: Some(Arc::new(|| { crate::peer_host::notify_controllers_settings_changed(); })), - on_token_expired: Some(Arc::new(|| crate::account::mark_token_expired())), + on_token_expired: Some(Arc::new(crate::account::mark_token_expired)), ..Default::default() }; settings_sync::start_settings_sync_engine(hooks); @@ -96,18 +96,15 @@ pub(crate) async fn push_settings_after_local_change() { } #[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Default)] pub(crate) enum SyncStatus { + #[default] Idle, Syncing, Done, Failed, } -impl Default for SyncStatus { - fn default() -> Self { - Self::Idle - } -} #[derive(Debug, Clone)] pub(crate) struct SyncProgress { diff --git a/src/apps/cli/src/hook_import.rs b/src/apps/cli/src/hook_import.rs index 9eb0d9ee78..7b01deab7a 100644 --- a/src/apps/cli/src/hook_import.rs +++ b/src/apps/cli/src/hook_import.rs @@ -9,7 +9,6 @@ use bitfun_product_domains::external_sources::{ }; use clap::{Subcommand, ValueEnum}; use serde::Serialize; -use std::path::PathBuf; #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] pub(crate) enum HookImportOutputFormat { @@ -78,7 +77,7 @@ pub(crate) enum HookAction { } pub(crate) async fn run(action: Option) -> Result<()> { - let workspace = std::env::current_dir().ok().map(PathBuf::from); + let workspace = std::env::current_dir().ok(); match action.unwrap_or(HookAction::List { refresh: false, format: HookImportOutputFormat::Text, diff --git a/src/apps/cli/src/mcp_import.rs b/src/apps/cli/src/mcp_import.rs index dcc6ed632d..67d9954160 100644 --- a/src/apps/cli/src/mcp_import.rs +++ b/src/apps/cli/src/mcp_import.rs @@ -5,7 +5,6 @@ use bitfun_product_domains::external_sources::{ EXTERNAL_MCP_IMPORT_SCHEMA_V1, }; use clap::ValueEnum; -use std::path::PathBuf; #[derive(Debug, Clone, Copy, PartialEq, Eq, ValueEnum)] pub(crate) enum McpImportOutputFormat { @@ -21,7 +20,7 @@ pub(crate) struct McpImportCommand { } pub(crate) async fn execute(command: McpImportCommand) -> Result<()> { - let workspace = std::env::current_dir().ok().map(PathBuf::from); + let workspace = std::env::current_dir().ok(); let plan = bitfun_core::external_mcp_import::plan_external_mcp_import(workspace.clone()) .await .map_err(operation_error)?; diff --git a/src/apps/cli/src/modes/chat/commands.rs b/src/apps/cli/src/modes/chat/commands.rs index 7ca9fe4f6e..24ee2326ca 100644 --- a/src/apps/cli/src/modes/chat/commands.rs +++ b/src/apps/cli/src/modes/chat/commands.rs @@ -710,7 +710,7 @@ impl ChatMode { .and_then(|command| command.native_collision.as_ref()) .map(|collision| collision.conflict_key.as_str()); let expected_preference_revision = native_conflict_key - .and_then(|_| self.external_source_snapshot.as_ref()) + .and(self.external_source_snapshot.as_ref()) .map(|snapshot| snapshot.preference_revision); let expanded = tokio::task::block_in_place(|| { rt_handle.block_on(expand_external_prompt_command( diff --git a/src/apps/cli/src/modes/chat/external_review.rs b/src/apps/cli/src/modes/chat/external_review.rs index 40299c2392..b589c9e2f3 100644 --- a/src/apps/cli/src/modes/chat/external_review.rs +++ b/src/apps/cli/src/modes/chat/external_review.rs @@ -682,13 +682,10 @@ fn external_tool_pending_notice_key(snapshot: &ExternalSourceCatalogSnapshot) -> .map(|conflict| format!("conflict:{}", conflict.conflict_key)), ) .collect::>(); - decisions.extend(snapshot.diagnostics.iter().filter_map(|diagnostic| { - matches!( + decisions.extend(snapshot.diagnostics.iter().filter(|&diagnostic| matches!( diagnostic.severity, ExternalSourceDiagnosticSeverity::Warning | ExternalSourceDiagnosticSeverity::Error - ) - .then(|| { - format!( + )).map(|diagnostic| format!( "diagnostic:{:?}:{}:{}:{}", diagnostic.severity, diagnostic.code, @@ -698,9 +695,7 @@ fn external_tool_pending_notice_key(snapshot: &ExternalSourceCatalogSnapshot) -> .as_ref() .map(|source| source.stable_key()) .unwrap_or_default() - ) - }) - })); + ))); if decisions.is_empty() { return None; } diff --git a/src/apps/cli/src/modes/exec/lifecycle.rs b/src/apps/cli/src/modes/exec/lifecycle.rs index afb4110251..b757c3ee2b 100644 --- a/src/apps/cli/src/modes/exec/lifecycle.rs +++ b/src/apps/cli/src/modes/exec/lifecycle.rs @@ -996,7 +996,7 @@ impl ExecMode { Err(error) => (Vec::new(), Err(error)), }; for envelope in buffered_events { - let _ = self + self .project_exec_nonterminal_event( &envelope, &session_id, diff --git a/src/apps/cli/src/peer_host/commands/external_sources.rs b/src/apps/cli/src/peer_host/commands/external_sources.rs index 5e65687db9..c598f7f6ae 100644 --- a/src/apps/cli/src/peer_host/commands/external_sources.rs +++ b/src/apps/cli/src/peer_host/commands/external_sources.rs @@ -279,6 +279,14 @@ async fn dispatch_inner( public_snapshot(snapshot) } +fn control_request( + request: &Value, +) -> ExternalSourceOperationResult { + serde_json::from_value(request.clone()).map_err(|_| { + ExternalSourceOperationError::invalid_request("Invalid external source control request") + }) +} + #[cfg(test)] mod tests { use super::*; @@ -339,11 +347,3 @@ mod tests { )); } } - -fn control_request( - request: &Value, -) -> ExternalSourceOperationResult { - serde_json::from_value(request.clone()).map_err(|_| { - ExternalSourceOperationError::invalid_request("Invalid external source control request") - }) -} diff --git a/src/apps/cli/src/peer_host/deny.rs b/src/apps/cli/src/peer_host/deny.rs index ff4b20d943..80831adbf4 100644 --- a/src/apps/cli/src/peer_host/deny.rs +++ b/src/apps/cli/src/peer_host/deny.rs @@ -100,11 +100,11 @@ static CLI_UNSUPPORTED_EXACT: &[&str] = &[ ]; pub(crate) fn is_local_only_command(command: &str) -> bool { - LOCAL_ONLY_COMMANDS.iter().any(|denied| *denied == command) + LOCAL_ONLY_COMMANDS.contains(&command) } pub(crate) fn is_cli_unsupported_command(command: &str) -> bool { - if CLI_UNSUPPORTED_EXACT.iter().any(|c| *c == command) { + if CLI_UNSUPPORTED_EXACT.contains(&command) { return true; } let prefixes = [ diff --git a/src/apps/cli/src/peer_host/fanout.rs b/src/apps/cli/src/peer_host/fanout.rs index 72a5c19925..3fa826358e 100644 --- a/src/apps/cli/src/peer_host/fanout.rs +++ b/src/apps/cli/src/peer_host/fanout.rs @@ -660,7 +660,7 @@ async fn fanout_peer_device_event_once(queued: QueuedPeerDeviceEvent) { }; let correlation_id = uuid::Uuid::new_v4().to_string(); if let Err(error) = relay_client - .send_device_message(&target, &correlation_id, &encrypted_data, &nonce) + .send_device_message(target, &correlation_id, &encrypted_data, &nonce) .await { tracing::debug!("Peer event fanout to {target} failed: {error}"); diff --git a/src/apps/cli/src/peer_host/state.rs b/src/apps/cli/src/peer_host/state.rs index c1f5fc721c..3555c2654a 100644 --- a/src/apps/cli/src/peer_host/state.rs +++ b/src/apps/cli/src/peer_host/state.rs @@ -1069,6 +1069,20 @@ fn spawn_turn_cancellation( }); } +static PEER_HOST_STATE: OnceLock = OnceLock::new(); + +pub(crate) fn set_peer_host_state(state: PeerHostState) -> Result<(), PeerHostState> { + PEER_HOST_STATE.set(state) +} + +pub(crate) fn try_peer_host_state() -> Option<&'static PeerHostState> { + PEER_HOST_STATE.get() +} + +pub(crate) fn peer_host_state() -> Result<&'static PeerHostState, String> { + try_peer_host_state().ok_or_else(|| "CLI peer host is not initialized".to_string()) +} + #[cfg(test)] mod tests { use std::collections::HashSet; @@ -1831,17 +1845,3 @@ mod tests { assert!(tracker.register_root(root).is_err()); } } - -static PEER_HOST_STATE: OnceLock = OnceLock::new(); - -pub(crate) fn set_peer_host_state(state: PeerHostState) -> Result<(), PeerHostState> { - PEER_HOST_STATE.set(state) -} - -pub(crate) fn try_peer_host_state() -> Option<&'static PeerHostState> { - PEER_HOST_STATE.get() -} - -pub(crate) fn peer_host_state() -> Result<&'static PeerHostState, String> { - try_peer_host_state().ok_or_else(|| "CLI peer host is not initialized".to_string()) -} diff --git a/src/apps/cli/tests/acp_stdio_cli.rs b/src/apps/cli/tests/acp_stdio_cli.rs index 9acfd79be6..211d0d72c8 100644 --- a/src/apps/cli/tests/acp_stdio_cli.rs +++ b/src/apps/cli/tests/acp_stdio_cli.rs @@ -333,7 +333,7 @@ async fn acp_stdio_preserves_mode_and_history_across_restart_then_closes_active_ .find(|response| response.get("error").is_some()) .expect("one concurrent session/load must be rejected"); assert!(loaded.get("error").is_none(), "{loaded}"); - assert_eq!(current_config_value(&loaded, "mode"), Some(&json!("Plan"))); + assert_eq!(current_config_value(loaded, "mode"), Some(&json!("Plan"))); assert!( !replay_updates.is_empty(), "session/load must replay persisted history before its success response" diff --git a/src/apps/cli/tests/terminal_process_contracts.rs b/src/apps/cli/tests/terminal_process_contracts.rs index 939a97f256..2e29612103 100644 --- a/src/apps/cli/tests/terminal_process_contracts.rs +++ b/src/apps/cli/tests/terminal_process_contracts.rs @@ -302,7 +302,7 @@ fn strict_stream_json_events(output: &str) -> Vec { if !is_protocol_candidate { return None; } - let value = serde_json::from_str::(&line).unwrap_or_else(|error| { + let value = serde_json::from_str::(line).unwrap_or_else(|error| { panic!("invalid stream-json PTY line {line:?}: {error}\nfull output:\n{output}") }); assert!( diff --git a/src/apps/desktop/src/api/clipboard_file_api.rs b/src/apps/desktop/src/api/clipboard_file_api.rs index 47859d2c4b..1c130addf3 100644 --- a/src/apps/desktop/src/api/clipboard_file_api.rs +++ b/src/apps/desktop/src/api/clipboard_file_api.rs @@ -189,7 +189,7 @@ mod macos_clipboard { pub(super) fn get_clipboard_files() -> Result, String> { let output = Command::new("osascript") - .args(&[ + .args([ "-e", r#" set theFiles to {} diff --git a/src/apps/desktop/src/api/computer_use_api.rs b/src/apps/desktop/src/api/computer_use_api.rs index 051524bf7f..2ee9be5da6 100644 --- a/src/apps/desktop/src/api/computer_use_api.rs +++ b/src/apps/desktop/src/api/computer_use_api.rs @@ -75,7 +75,7 @@ pub async fn computer_use_open_system_settings( .arg(url) .status() .map_err(|e| e.to_string())?; - return Ok(()); + Ok(()) } #[cfg(target_os = "windows")] { diff --git a/src/apps/desktop/src/api/path_target.rs b/src/apps/desktop/src/api/path_target.rs index 31b2d9a86d..aa8839518f 100644 --- a/src/apps/desktop/src/api/path_target.rs +++ b/src/apps/desktop/src/api/path_target.rs @@ -216,6 +216,10 @@ pub fn stat_local_path_metadata( ) })?; + // Not collapsible: on Windows a junction/reparse point is not reported as a + // symlink by `file_type()`, so the extra attribute check is load-bearing. + // Clippy only sees the `#[cfg(not(windows))] { false }` arm on other targets. + #[allow(clippy::needless_bool)] let is_symlink = if link_metadata.file_type().is_symlink() { true } else { @@ -299,49 +303,6 @@ fn encode_remote_file_bytes(bytes: Vec, encoding: Option<&str>) -> Result bool { - LOCAL_ONLY_COMMANDS.iter().any(|denied| *denied == command) + LOCAL_ONLY_COMMANDS.contains(&command) } /// Register a controller device id to receive peer UI events. diff --git a/src/apps/desktop/src/api/remote_connect_api.rs b/src/apps/desktop/src/api/remote_connect_api.rs index 497bea7f5d..b4ea03dc8a 100644 --- a/src/apps/desktop/src/api/remote_connect_api.rs +++ b/src/apps/desktop/src/api/remote_connect_api.rs @@ -3555,7 +3555,7 @@ pub async fn account_delegate_to_paired(correlation_id: String) -> Result = std::sync::Mutex::new(()); + /// Serializes these tests against the process-global account context. + /// Async-aware so the guard may be held across each test's awaits; the + /// plain `#[test]` cases take it with `blocking_lock`, which cannot stall + /// because they run without an ambient runtime. + static ACCOUNT_CONTEXT_TEST_LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); #[test] fn relay_url_normalization_removes_all_trailing_slashes() { @@ -4472,9 +4476,7 @@ mod sync_state_tests { #[test] fn background_sync_is_fail_closed_while_login_choice_is_pending() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.blocking_lock(); set_pending_login_id(Some("pending-a".to_string())); assert!(!background_account_sync_is_allowed()); set_pending_login_id(None); @@ -4516,9 +4518,7 @@ mod sync_state_tests { #[tokio::test(flavor = "current_thread")] async fn account_transition_cancels_an_in_flight_auto_sync_future() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.lock().await; let operation_id = u64::MAX - 41; ACTIVE_ACCOUNT_AUTO_SYNC_OPERATION_ID.store(operation_id, Ordering::Release); let waiter = tokio::spawn(async move { @@ -4537,9 +4537,7 @@ mod sync_state_tests { #[tokio::test(flavor = "current_thread")] async fn external_account_reads_are_hidden_during_transition() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.lock().await; *get_account_context().write().await = Some(AccountContextState { session: AccountSession { token: "token-a".to_string(), @@ -4560,9 +4558,7 @@ mod sync_state_tests { #[tokio::test(flavor = "current_thread")] async fn login_event_probe_sees_context_before_transition_mutex_is_released() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.lock().await; let transition_guard = ACCOUNT_CONTEXT_TRANSITION_LOCK.lock().await; let transition = AccountContextTransitionPermit::begin(); let mut guard = AccountContextTransitionGuard { @@ -4582,9 +4578,7 @@ mod sync_state_tests { #[tokio::test(flavor = "current_thread")] async fn stale_pending_login_id_cannot_finalize_or_cancel_replacement() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.lock().await; *get_account_context().write().await = Some(AccountContextState { session: AccountSession { token: "token-b".to_string(), @@ -4613,9 +4607,7 @@ mod sync_state_tests { #[tokio::test(flavor = "current_thread")] async fn finalize_retry_after_commit_is_idempotent_only_for_the_same_account_owner() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.lock().await; *get_account_context().write().await = Some(AccountContextState { session: AccountSession { token: "token-a".to_string(), @@ -4643,9 +4635,7 @@ mod sync_state_tests { #[test] fn stale_routing_owner_cannot_clear_or_update_replacement() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.blocking_lock(); clear_device_routing_state(); let owner_a = DeviceRoutingOwner { account_generation: 10, @@ -4686,9 +4676,7 @@ mod sync_state_tests { #[test] fn routing_presence_is_bound_to_account_generation_and_token() { - let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _test_guard = ACCOUNT_CONTEXT_TEST_LOCK.blocking_lock(); clear_device_routing_state(); let owner = DeviceRoutingOwner { account_generation: 20, diff --git a/src/apps/desktop/src/api/ssh_api.rs b/src/apps/desktop/src/api/ssh_api.rs index d00a796630..2d72ed00eb 100644 --- a/src/apps/desktop/src/api/ssh_api.rs +++ b/src/apps/desktop/src/api/ssh_api.rs @@ -540,7 +540,7 @@ fn validate_remote_name_for_local_download(name: &str) -> Result<(), String> { fn local_download_name_key(name: &str) -> String { #[cfg(any(windows, target_os = "macos"))] { - name.trim_end_matches(|character| character == '.' || character == ' ') + name.trim_end_matches(['.', ' ']) .to_lowercase() } #[cfg(not(any(windows, target_os = "macos")))] diff --git a/src/apps/desktop/src/computer_use/macos_ax_ui.rs b/src/apps/desktop/src/computer_use/macos_ax_ui.rs index 9fe652335c..721b6a1e43 100644 --- a/src/apps/desktop/src/computer_use/macos_ax_ui.rs +++ b/src/apps/desktop/src/computer_use/macos_ax_ui.rs @@ -317,7 +317,7 @@ impl CandidateMatch { let area = self.bounds_width * self.bounds_height; if area > 0.0 && area < 50000.0 { score += 100; // Small interactive element - } else if area >= 50000.0 && area < 200000.0 { + } else if (50000.0..200000.0).contains(&area) { score += 50; // Medium element } // Very large elements (>200000 area) get no bonus -- likely containers @@ -338,7 +338,7 @@ impl CandidateMatch { } // Prefer elements with a non-empty title (more likely to be interactive) - if self.title.as_ref().map_or(false, |t| !t.is_empty()) { + if self.title.as_ref().is_some_and(|t| !t.is_empty()) { score += 20; } @@ -398,16 +398,14 @@ impl CandidateMatch { [&self.title, &self.value, &self.description, &self.help]; let mut exact = false; let mut substring = false; - for f in fields { - if let Some(s) = f { - let sl = s.trim().to_lowercase(); - if sl == n { - exact = true; - break; - } - if sl.contains(&n) { - substring = true; - } + for s in fields.into_iter().flatten() { + let sl = s.trim().to_lowercase(); + if sl == n { + exact = true; + break; + } + if sl.contains(&n) { + substring = true; } } if exact { @@ -528,7 +526,7 @@ unsafe fn is_ax_hidden(elem: AXUIElementRef) -> bool { }; // AXHidden is a CFBoolean let hidden = - val as *const c_void == core_foundation::boolean::kCFBooleanTrue as *const c_void; + std::ptr::eq(val, core_foundation::boolean::kCFBooleanTrue as *const c_void); ax_release(val); hidden } diff --git a/src/apps/desktop/src/computer_use/macos_list_apps.rs b/src/apps/desktop/src/computer_use/macos_list_apps.rs index 602d283cc1..e45f89a0cd 100644 --- a/src/apps/desktop/src/computer_use/macos_list_apps.rs +++ b/src/apps/desktop/src/computer_use/macos_list_apps.rs @@ -117,7 +117,7 @@ pub(super) fn list_running_apps(include_hidden: bool) -> BitFunResult Option { - let candidates = obs.topCandidates(TOP_CANDIDATES_MAX as usize); + let candidates = obs.topCandidates(TOP_CANDIDATES_MAX); let n = candidates.len(); let q_norm = normalize_for_match(text_query); diff --git a/src/apps/desktop/src/runtime/session_application.rs b/src/apps/desktop/src/runtime/session_application.rs index 657f6a1d28..78bafd5cd4 100644 --- a/src/apps/desktop/src/runtime/session_application.rs +++ b/src/apps/desktop/src/runtime/session_application.rs @@ -152,7 +152,7 @@ impl DesktopSessionScopeResolver { .is_some_and(|registered| { requested_remote_ssh_host .as_deref() - .map_or(true, |requested| requested.eq_ignore_ascii_case(registered)) + .is_none_or(|requested| requested.eq_ignore_ascii_case(registered)) }); let mut saved_remote_ssh_host = None; if requested_remote_ssh_host.is_none() && registered_remote_ssh_host.is_none() { @@ -717,7 +717,7 @@ fn merge_ui_owned_session_metadata( custom.insert(key.to_string(), value.clone()); } } - current.custom_metadata = (!custom.is_empty()).then(|| serde_json::Value::Object(custom)); + current.custom_metadata = (!custom.is_empty()).then_some(serde_json::Value::Object(custom)); } } diff --git a/src/apps/relay-server/src/bin/relay_admin.rs b/src/apps/relay-server/src/bin/relay_admin.rs index 18882a08ae..11263ebd1b 100644 --- a/src/apps/relay-server/src/bin/relay_admin.rs +++ b/src/apps/relay-server/src/bin/relay_admin.rs @@ -90,7 +90,7 @@ async fn main() -> Result<()> { if users.is_empty() { println!("No accounts found."); } else { - println!("{:<24} {:<38} {}", "USERNAME", "USER_ID", "CREATED"); + println!("{:<24} {:<38} CREATED", "USERNAME", "USER_ID"); println!("{}", "-".repeat(80)); for (username, user_id, created) in users { let dt = chrono::DateTime::from_timestamp(created, 0) diff --git a/src/apps/relay-server/tests/library_compat.rs b/src/apps/relay-server/tests/library_compat.rs index 8833354036..c3ee376a09 100644 --- a/src/apps/relay-server/tests/library_compat.rs +++ b/src/apps/relay-server/tests/library_compat.rs @@ -20,7 +20,9 @@ fn legacy_library_path_exposes_supported_relay_api() { let _ = RoomManager::new; let _ = relay::room::RoomManager::new; let _ = routes::api::health_check; - let _ = routes::api::server_info(); + // Pin the symbol, not a call: `server_info` is async, and `let _ =` on the + // returned future would drop it unpolled. + let _ = routes::api::server_info; let _: Option = None; let _ = std::mem::size_of::(); let _ = AppState { @@ -37,6 +39,7 @@ fn legacy_library_path_exposes_supported_relay_api() { login_rate_limiter: Arc::new(routes::auth::LoginRateLimiter::new()), device_manager: relay::DeviceManager::new(), cors_allow_origins: Arc::new(Vec::new()), + page_browser_auth: None, }; fn require_store() {} diff --git a/src/crates/adapters/agent-runtime-ipc/src/tests/local_health.rs b/src/crates/adapters/agent-runtime-ipc/src/tests/local_health.rs index 32a7e18273..5e15c0b048 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/tests/local_health.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/tests/local_health.rs @@ -395,7 +395,7 @@ async fn non_utf8_runtime_root_supports_discovery_bind_and_health() { let discovery = server.discovery_record().clone(); let server_task = tokio::spawn(server.serve()); - let mut client = RuntimeIpcClient::connect( + let client = RuntimeIpcClient::connect( &runtime_root, &discovery, "non-utf8-test", diff --git a/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs b/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs index 2bd6f44ac6..ee725d480a 100644 --- a/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs +++ b/src/crates/adapters/agent-runtime-ipc/src/tests/shared_controller.rs @@ -774,16 +774,18 @@ async fn mode_update_requires_the_controlled_idle_session() { ) .await; - let calls = handler.calls.lock().expect("calls"); - assert_eq!( + // Scoped so the guard is provably released before the awaits below. + let updates = { + let calls = handler.calls.lock().expect("calls"); calls .iter() .filter(|operation| matches!(operation, RuntimeIpcOperation::UpdateSessionMode { .. })) - .count(), - 1, + .count() + }; + assert_eq!( + updates, 1, "only the controlled idle-session update reaches the Runtime handler" ); - drop(calls); drop(client); server.finish().await; } @@ -871,16 +873,18 @@ async fn model_update_requires_the_controlled_idle_session() { ) .await; - let calls = handler.calls.lock().expect("calls"); - assert_eq!( + // Scoped so the guard is provably released before the awaits below. + let updates = { + let calls = handler.calls.lock().expect("calls"); calls .iter() .filter(|operation| matches!(operation, RuntimeIpcOperation::UpdateSessionModel { .. })) - .count(), - 1, + .count() + }; + assert_eq!( + updates, 1, "only the controlled idle-session update reaches the Runtime handler" ); - drop(calls); drop(client); server.finish().await; } diff --git a/src/crates/adapters/ai-adapters/src/subscription_auth/mod.rs b/src/crates/adapters/ai-adapters/src/subscription_auth/mod.rs index 50437fd972..c3d7ad9878 100644 --- a/src/crates/adapters/ai-adapters/src/subscription_auth/mod.rs +++ b/src/crates/adapters/ai-adapters/src/subscription_auth/mod.rs @@ -670,9 +670,12 @@ mod tests { const STALE_LOGIN_CHILD_OUTCOME_ENV: &str = "BITFUN_SUBAUTH_CAS_CHILD_OUTCOME"; /// Serializes tests that rely on the process-global store path override. - fn test_lock() -> &'static Mutex<()> { - static LOCK: OnceLock> = OnceLock::new(); - LOCK.get_or_init(|| Mutex::new(())) + /// Serializes these tests against the shared on-disk store. Async-aware so + /// the guard may be held across the awaits each test performs, matching how + /// `store_lock` above already guards the real store. + fn test_lock() -> &'static tokio::sync::Mutex<()> { + static LOCK: tokio::sync::Mutex<()> = tokio::sync::Mutex::const_new(()); + &LOCK } fn temp_store_path() -> std::path::PathBuf { @@ -707,9 +710,7 @@ mod tests { #[tokio::test] async fn store_roundtrip_in_temp_dir() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; store::set_store_path_for_test(temp_store_path()); let mut store = store::Store::new(); store.insert( @@ -758,9 +759,7 @@ mod tests { #[tokio::test] async fn legacy_plaintext_store_is_migrated_and_scrubbed() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); let legacy = serde_json::json!({ @@ -786,9 +785,7 @@ mod tests { #[tokio::test] async fn legacy_migration_retries_after_temporary_vault_unavailability() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); let legacy = serde_json::json!({ @@ -824,9 +821,7 @@ mod tests { #[tokio::test] async fn partial_chunk_write_is_durably_cleaned_after_retry() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; store::set_store_path_for_test(temp_store_path()); store::set_test_vault_write_failure_after(Some(1)); store::set_test_vault_delete_failure(true); @@ -861,9 +856,7 @@ mod tests { #[tokio::test] async fn windows_post_commit_backup_cleanup_failure_does_not_fail_commit() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); let tmp = path.with_extension("tmp-one"); let backup = path.with_extension("bak"); @@ -889,9 +882,7 @@ mod tests { #[tokio::test] async fn concurrent_provider_upserts_preserve_both_metadata_entries() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); @@ -931,9 +922,7 @@ mod tests { #[tokio::test] async fn logout_tombstone_wins_over_a_refresh_paused_after_load() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); store::upsert( @@ -1047,9 +1036,7 @@ mod tests { #[tokio::test] async fn logout_of_an_absent_provider_invalidates_a_cross_process_login() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); let parent = path.parent().unwrap(); @@ -1098,9 +1085,7 @@ mod tests { #[tokio::test] async fn v2_metadata_without_revision_map_remains_conditionally_writable() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); std::fs::write( @@ -1133,9 +1118,7 @@ mod tests { #[tokio::test] async fn repeated_upsert_replaces_existing_metadata_file() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); @@ -1188,9 +1171,7 @@ mod tests { #[tokio::test] async fn long_tokens_are_split_below_the_windows_vault_limit() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let path = temp_store_path(); store::set_store_path_for_test(path.clone()); let refresh = "r".repeat(5_000); @@ -1232,9 +1213,7 @@ mod tests { #[tokio::test] async fn unavailable_vault_is_retryable_not_missing_credential() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; store::set_store_path_for_test(temp_store_path()); store::upsert( "opencode", @@ -1248,7 +1227,7 @@ mod tests { store::set_test_vault_unavailable(true); let state = store::load_with_state().await.unwrap(); - assert!(state.credentials.get("opencode").is_none()); + assert!(!state.credentials.contains_key("opencode")); assert!(!state.requires_reauthentication.contains("opencode")); assert!(state.vault_unavailable.contains("opencode")); let error = store::load_entry("opencode").await.unwrap_err(); @@ -1261,9 +1240,7 @@ mod tests { #[tokio::test] async fn logout_clears_stored_credential() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; store::set_store_path_for_test(temp_store_path()); let mut store = store::Store::new(); store.insert( @@ -1277,14 +1254,12 @@ mod tests { logout(SubscriptionProvider::Opencode).await.unwrap(); let loaded = store::load().await.unwrap(); - assert!(loaded.get("opencode").is_none()); + assert!(!loaded.contains_key("opencode")); } #[tokio::test] async fn failed_logout_metadata_commit_preserves_usable_credential() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; store::set_store_path_for_test(temp_store_path()); store::upsert( "opencode", @@ -1312,9 +1287,7 @@ mod tests { #[tokio::test] async fn failed_logout_vault_delete_is_reported_and_retried() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; store::set_store_path_for_test(temp_store_path()); store::upsert( "opencode", @@ -1349,9 +1322,7 @@ mod tests { #[tokio::test] async fn finalize_ignores_superseded_session() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let provider = SubscriptionProvider::Codex; let stale_generation = next_generation(); let stale_session_id = test_session_id(); @@ -1475,9 +1446,7 @@ mod tests { #[tokio::test] async fn cancel_command_waits_for_the_commit_boundary() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let provider = SubscriptionProvider::Opencode; let store_guard = store_lock(provider).lock().await; let cancel = CancellationToken::new(); @@ -1520,9 +1489,7 @@ mod tests { #[tokio::test] async fn duplicate_cancel_waits_for_the_same_commit_boundary() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let provider = SubscriptionProvider::Antigravity; let store_guard = store_lock(provider).lock().await; let cancel = CancellationToken::new(); @@ -1571,9 +1538,7 @@ mod tests { #[tokio::test] async fn stale_cancel_does_not_cancel_replacement_session() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let provider = SubscriptionProvider::Opencode; let stale_session_id = test_session_id(); let current_session_id = test_session_id(); @@ -1606,9 +1571,7 @@ mod tests { #[tokio::test] async fn cancel_does_not_rewrite_authorized_terminal_state() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + let _guard = test_lock().lock().await; let provider = SubscriptionProvider::Codex; let session_id = test_session_id(); let cancel = CancellationToken::new(); @@ -1639,9 +1602,9 @@ mod tests { #[test] fn final_state_update_rechecks_generation_after_async_work() { - let _guard = test_lock() - .lock() - .unwrap_or_else(|poisoned| poisoned.into_inner()); + // Plain `#[test]`, so there is no ambient runtime for `blocking_lock` to + // stall; it still serializes against the async tests above. + let _guard = test_lock().blocking_lock(); let provider = SubscriptionProvider::Codex; let old_generation = next_generation(); let new_generation = next_generation(); diff --git a/src/crates/adapters/claude-code-adapter/src/mcp_source.rs b/src/crates/adapters/claude-code-adapter/src/mcp_source.rs index cdc48fa91f..8806386349 100644 --- a/src/crates/adapters/claude-code-adapter/src/mcp_source.rs +++ b/src/crates/adapters/claude-code-adapter/src/mcp_source.rs @@ -130,7 +130,7 @@ impl ClaudeCodeMcpProvider { let key = source_key(&layer); let allowed_root = layer.path.parent().unwrap_or(Path::new(".")); let resolved_path = resolve_bounded_regular_file(&layer.path, allowed_root) - .map_err(|error| bounded_file_error(error))?; + .map_err(bounded_file_error)?; let document = documents .entry(resolved_path.clone()) .or_insert_with(|| parse_document(&resolved_path)) diff --git a/src/crates/adapters/codex-adapter/src/mcp_source.rs b/src/crates/adapters/codex-adapter/src/mcp_source.rs index 0a431b098a..f36ac54a14 100644 --- a/src/crates/adapters/codex-adapter/src/mcp_source.rs +++ b/src/crates/adapters/codex-adapter/src/mcp_source.rs @@ -134,7 +134,7 @@ impl CodexMcpProvider { let key = source_key(&layer.path); let allowed_root = layer.path.parent().unwrap_or(Path::new(".")); let resolved_path = resolve_bounded_regular_file(&layer.path, allowed_root) - .map_err(|error| bounded_file_error(error))?; + .map_err(bounded_file_error)?; let parsed = parse_layer(&resolved_path); let mut layer_diagnostics = parsed .diagnostics diff --git a/src/crates/adapters/opencode-adapter/src/mcp_source.rs b/src/crates/adapters/opencode-adapter/src/mcp_source.rs index 2818dc1400..347cb9ce10 100644 --- a/src/crates/adapters/opencode-adapter/src/mcp_source.rs +++ b/src/crates/adapters/opencode-adapter/src/mcp_source.rs @@ -1066,7 +1066,7 @@ fn parse_config_layer( ) -> ParsedConfigLayer { match read_bounded_text(path, MAX_CONFIG_FILE_BYTES) { Ok(BoundedTextRead::TooLarge) => { - return ParsedConfigLayer { + ParsedConfigLayer { servers: BTreeMap::new(), diagnostics: vec![ExternalSourceDiagnostic::error( "opencode.mcp.config_too_large", @@ -1076,10 +1076,10 @@ fn parse_config_layer( .with_asset_kind(ExternalSourceAssetKind::Mcp)], content_version: "too-large".to_string(), fatal: true, - }; + } } Ok(BoundedTextRead::InvalidUtf8) => { - return ParsedConfigLayer { + ParsedConfigLayer { servers: BTreeMap::new(), diagnostics: vec![ExternalSourceDiagnostic::error( "opencode.mcp.config_invalid_utf8", @@ -1089,7 +1089,7 @@ fn parse_config_layer( .with_asset_kind(ExternalSourceAssetKind::Mcp)], content_version: "invalid-utf8".to_string(), fatal: true, - }; + } } Ok(BoundedTextRead::Content(content)) => { let content_version = content_version(revision_key, path, content.as_bytes()); @@ -1129,15 +1129,15 @@ fn parse_config_layer( }; } }; - return ParsedConfigLayer { + ParsedConfigLayer { servers, diagnostics: Vec::new(), content_version, fatal: false, - }; + } } Err(error) => { - return ParsedConfigLayer { + ParsedConfigLayer { servers: BTreeMap::new(), diagnostics: vec![ExternalSourceDiagnostic::error( "opencode.mcp.config_unreadable", @@ -1147,7 +1147,7 @@ fn parse_config_layer( .with_asset_kind(ExternalSourceAssetKind::Mcp)], content_version: "unreadable".to_string(), fatal: true, - }; + } } } } diff --git a/src/crates/adapters/webdriver/src/platform/capture.rs b/src/crates/adapters/webdriver/src/platform/capture.rs index 9db768a5ed..1715cae220 100644 --- a/src/crates/adapters/webdriver/src/platform/capture.rs +++ b/src/crates/adapters/webdriver/src/platform/capture.rs @@ -42,6 +42,10 @@ mod imp { ) -> Result { let (tx, rx) = oneshot::channel(); + // SAFETY: Tauri runs this callback on the main thread with a live + // platform webview, so the pointer is valid for the call and + // `MainThreadMarker::new_unchecked` states a fact the runtime upholds. + // On macOS that webview is always WKWebView-backed, making the cast sound. let result = webview.with_webview(move |platform_webview| unsafe { let wk_webview: &WKWebView = &*platform_webview.inner().cast(); let mtm = MainThreadMarker::new_unchecked(); @@ -116,6 +120,10 @@ mod imp { })?; let (tx, rx) = oneshot::channel(); + // SAFETY: Tauri runs this callback on the main thread with a live + // platform webview, so the pointer is valid for the call and + // `MainThreadMarker::new_unchecked` states a fact the runtime upholds. + // On macOS that webview is always WKWebView-backed, making the cast sound. let result = webview.with_webview(move |platform_webview| unsafe { let wk_webview: &WKWebView = &*platform_webview.inner().cast(); let mtm = MainThreadMarker::new_unchecked(); @@ -184,6 +192,8 @@ mod imp { let empty_dict: objc2::rc::Retained> = NSDictionary::new(); + // SAFETY: `bitmap_rep` and `empty_dict` are live `Retained` handles for + // the whole call; the method only reads them and returns a new object. let png_data = unsafe { bitmap_rep.representationUsingType_properties(NSBitmapImageFileType::PNG, &empty_dict) } @@ -226,6 +236,10 @@ mod imp { ) -> Result { let (tx, rx) = oneshot::channel(); + // SAFETY: Tauri runs this callback on the main thread with a live + // platform webview, so the pointer is valid for the call and + // `MainThreadMarker::new_unchecked` states a fact the runtime upholds. + // On macOS that webview is always WKWebView-backed, making the cast sound. let result = webview.with_webview(move |platform_webview| unsafe { let webview2 = match platform_webview.controller().CoreWebView2() { Ok(webview2) => webview2, @@ -293,6 +307,10 @@ mod imp { let margin_left = options.margin_left; let margin_right = options.margin_right; + // SAFETY: Tauri runs this callback on the main thread with a live + // platform webview, so the pointer is valid for the call and + // `MainThreadMarker::new_unchecked` states a fact the runtime upholds. + // On macOS that webview is always WKWebView-backed, making the cast sound. let result = webview.with_webview(move |platform_webview| unsafe { let _ = CoInitializeEx(None, COINIT_APARTMENTTHREADED); diff --git a/src/crates/adapters/webdriver/src/platform/evaluator/macos.rs b/src/crates/adapters/webdriver/src/platform/evaluator/macos.rs index e3f1a4e960..33491bbb8f 100644 --- a/src/crates/adapters/webdriver/src/platform/evaluator/macos.rs +++ b/src/crates/adapters/webdriver/src/platform/evaluator/macos.rs @@ -25,6 +25,10 @@ pub(super) async fn evaluate_script( let wrapped = script::build_native_eval_script(script, args, async_mode, frame_context); let (sender, receiver) = oneshot::channel::>(); + // SAFETY: Tauri runs this callback on the main thread with a live platform + // webview, so the pointer is valid for the call and + // `MainThreadMarker::new_unchecked` states a fact the runtime upholds. On + // macOS that webview is always WKWebView-backed, making the cast sound. let result = webview.with_webview(move |platform_webview| unsafe { let wk_webview: &WKWebView = &*platform_webview.inner().cast(); let ns_script = NSString::from_str(&wrapped); @@ -102,6 +106,9 @@ unsafe fn ns_object_to_string(obj: &AnyObject) -> Option { return None; } + // SAFETY: the class-name check above established that `obj` is an NSString, + // and `NSString` is a transparent wrapper over `AnyObject`, so the reborrow + // keeps `obj`'s lifetime and points at a live object. let ns_string: &NSString = unsafe { &*std::ptr::from_ref::(obj).cast::() }; Some(ns_string.to_string()) } diff --git a/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard.rs b/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard.rs index f52da5ddec..f19702bc14 100644 --- a/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard.rs +++ b/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard.rs @@ -788,7 +788,7 @@ fn append_jsonl(path: &Path, event: &Value) -> Result<(), String> { let mut file = OpenOptions::new() .create(true) .append(true) - .open(&path) + .open(path) .map_err(|error| format!("failed to open telemetry stream: {error}"))?; serde_json::to_writer(&mut file, event) .and_then(|_| file.write_all(b"\n").map_err(serde_json::Error::io)) @@ -894,14 +894,14 @@ pub fn check( && !edit_constraint_telemetry_enabled() && state .as_ref() - .map_or(true, |state| !state.has_enforceable_constraints()) + .is_none_or(|state| !state.has_enforceable_constraints()) { return None; } if !force_requested && state .as_ref() - .map_or(true, |state| !state.has_enforceable_constraints()) + .is_none_or(|state| !state.has_enforceable_constraints()) { if edit_constraint_telemetry_enabled() { decision_result( @@ -1047,7 +1047,7 @@ pub async fn check_write( if !force_requested && state .as_ref() - .map_or(true, |state| !state.tracks_agent_created_test_paths()) + .is_none_or(|state| !state.tracks_agent_created_test_paths()) { return check(context, tool_name, operation, file_path, false); } @@ -1104,14 +1104,14 @@ pub fn check_edit( && !edit_constraint_telemetry_enabled() && state .as_ref() - .map_or(true, |state| !state.has_enforceable_constraints()) + .is_none_or(|state| !state.has_enforceable_constraints()) { return None; } if !force_requested && state .as_ref() - .map_or(true, |state| !state.tracks_agent_created_test_paths()) + .is_none_or(|state| !state.tracks_agent_created_test_paths()) { return check(context, tool_name, operation, file_path, false); } diff --git a/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard/shell_targets.rs b/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard/shell_targets.rs index a0ab08dc92..4ed8eba605 100644 --- a/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard/shell_targets.rs +++ b/src/crates/assembly/core/src/agentic/execution/edit_constraint_guard/shell_targets.rs @@ -47,7 +47,7 @@ pub(super) fn explicit_bash_mutation_targets(command: &str) -> Vec" | ">>" | "1>" | "1>>") { if let Some(path) = words.get(index + 1) { push_bash_target(&mut targets, path, ShellMutationOperation::Write); diff --git a/src/crates/assembly/core/src/external_mcp.rs b/src/crates/assembly/core/src/external_mcp.rs index 58cfca1c0d..7f70b88cfc 100644 --- a/src/crates/assembly/core/src/external_mcp.rs +++ b/src/crates/assembly/core/src/external_mcp.rs @@ -255,19 +255,12 @@ pub(super) fn prepared_mcp_config( Ok(config) } +#[derive(Default)] struct CandidateGroup<'a> { native: Vec<&'a NativeMcpCandidate>, external: Vec<&'a ExternalMcpServerDefinition>, } -impl<'a> Default for CandidateGroup<'a> { - fn default() -> Self { - Self { - native: Vec::new(), - external: Vec::new(), - } - } -} /// Produces the source-neutral product decision for external MCP candidates. /// This function is pure: no provider preparation, process launch, credential @@ -323,7 +316,7 @@ pub(super) fn reconcile_external_mcp_catalog( .sort_by(|left, right| left.candidate_id.cmp(&right.candidate_id)); group .external - .sort_by(|left, right| left.candidate_id().cmp(&right.candidate_id())); + .sort_by_key(|left| left.candidate_id()); let active_external = group .external .iter() diff --git a/src/crates/assembly/core/src/external_sources.rs b/src/crates/assembly/core/src/external_sources.rs index 8f1a0c549e..0b2870eefc 100644 --- a/src/crates/assembly/core/src/external_sources.rs +++ b/src/crates/assembly/core/src/external_sources.rs @@ -2053,7 +2053,7 @@ impl WorkspaceExternalSourceService { &catalog, self.execution_domain_id.clone(), safe_mode, - host_capabilities.clone(), + host_capabilities, ); let mut public = ExternalSourcePublicSnapshot::from(catalog); public.host_capabilities = host_capabilities; @@ -3274,7 +3274,6 @@ fn sanitize_external_snapshot_locations( .unwrap_or(ExternalSourceScope::WorkspaceLocal); remember_location(scope, directory); } - drop(remember_location); replacements.sort_by(|left, right| right.0.len().cmp(&left.0.len())); let sanitize_message = |message: &mut String| { for (raw, safe) in &replacements { @@ -3396,7 +3395,7 @@ fn write_canonical_json(value: &serde_json::Value, output: &mut Vec) -> serd serde_json::Value::Object(values) => { output.push(b'{'); let mut entries = values.iter().collect::>(); - entries.sort_by(|(left, _), (right, _)| left.cmp(right)); + entries.sort_by_key(|(left, _)| *left); for (index, (key, value)) in entries.into_iter().enumerate() { if index > 0 { output.push(b','); @@ -4740,7 +4739,7 @@ fn project_native_prompt_command_conflicts( for command in native_commands { command .validate() - .map_err(|error| invalid_operation_error(&error.to_string()))?; + .map_err(|error| invalid_operation_error(error.to_string()))?; } let command_names = native_commands .iter() @@ -4787,14 +4786,11 @@ fn project_native_prompt_command_conflicts( .collect::>() }; let Some((_, _, source)) = external.first() else { - reconfirmations.extend(native.iter().filter_map(|command| { - conflicted_candidate_ids - .contains(&command.candidate_id) - .then(|| NativePromptCommandReconfirmationProjection { + reconfirmations.extend(native.iter().filter(|&command| conflicted_candidate_ids + .contains(&command.candidate_id)).map(|command| NativePromptCommandReconfirmationProjection { command_name: command_name.clone(), native_candidate_id: command.candidate_id.clone(), - }) - })); + })); continue; }; let execution_domain = snapshot diff --git a/src/crates/assembly/core/src/external_tools.rs b/src/crates/assembly/core/src/external_tools.rs index 797db4a45e..df8b25ed6a 100644 --- a/src/crates/assembly/core/src/external_tools.rs +++ b/src/crates/assembly/core/src/external_tools.rs @@ -1003,9 +1003,8 @@ impl ExternalToolRuntimeManager { reason: &str, ) -> bool { let mut loaded = self.loaded.lock().await; - if !loaded - .get(runtime_target_id) - .is_some_and(|target| target.load_generation == load_generation) + if loaded + .get(runtime_target_id).is_none_or(|target| target.load_generation != load_generation) { return false; } @@ -2080,6 +2079,27 @@ fn tool_diagnostic( } } +pub(super) fn merge_tool_state( + mut snapshot: bitfun_product_domains::external_sources::ExternalSourceCatalogSnapshot, + tool_snapshot: &ExternalToolCoordinatorSnapshot, + state: ExternalToolProductState, +) -> bitfun_product_domains::external_sources::ExternalSourceCatalogSnapshot { + snapshot.generation = snapshot.generation.max(tool_snapshot.generation); + snapshot.discovery_pending |= tool_snapshot.discovery_pending; + snapshot.sources.extend(tool_snapshot.sources.clone()); + snapshot + .sources + .sort_by(|left, right| left.stable_key.cmp(&right.stable_key)); + snapshot.tools = state.tools; + snapshot.tool_approval_requests = state.approval_requests; + snapshot.tool_conflicts = state.conflicts; + snapshot + .diagnostics + .extend(tool_snapshot.diagnostics.clone()); + snapshot.diagnostics.extend(state.diagnostics); + snapshot +} + #[cfg(test)] mod tests { use super::*; @@ -2671,24 +2691,3 @@ mod tests { assert!(manager.lost_targets.lock().await.is_empty()); } } - -pub(super) fn merge_tool_state( - mut snapshot: bitfun_product_domains::external_sources::ExternalSourceCatalogSnapshot, - tool_snapshot: &ExternalToolCoordinatorSnapshot, - state: ExternalToolProductState, -) -> bitfun_product_domains::external_sources::ExternalSourceCatalogSnapshot { - snapshot.generation = snapshot.generation.max(tool_snapshot.generation); - snapshot.discovery_pending |= tool_snapshot.discovery_pending; - snapshot.sources.extend(tool_snapshot.sources.clone()); - snapshot - .sources - .sort_by(|left, right| left.stable_key.cmp(&right.stable_key)); - snapshot.tools = state.tools; - snapshot.tool_approval_requests = state.approval_requests; - snapshot.tool_conflicts = state.conflicts; - snapshot - .diagnostics - .extend(tool_snapshot.diagnostics.clone()); - snapshot.diagnostics.extend(state.diagnostics); - snapshot -} diff --git a/src/crates/assembly/core/src/runtime_ownership.rs b/src/crates/assembly/core/src/runtime_ownership.rs index 1b20b43dc8..efc3210a1c 100644 --- a/src/crates/assembly/core/src/runtime_ownership.rs +++ b/src/crates/assembly/core/src/runtime_ownership.rs @@ -114,9 +114,8 @@ impl CoreRuntimeOwnership { &key, RuntimeDeployment::Shared, ) - .map_err(|error| { - log_acquisition_failure(entrypoint, RuntimeDeployment::Shared, &key, &error); - error + .inspect_err(|error| { + log_acquisition_failure(entrypoint, RuntimeDeployment::Shared, &key, error); })?; log_acquired(entrypoint, RuntimeDeployment::Shared, &key); Ok(Self { @@ -190,14 +189,13 @@ impl CoreRuntimeOwnership { &key, RuntimeDeployment::Embedded, ) - .map_err(|error| { + .inspect_err(|error| { log_acquisition_failure( self.entrypoint, RuntimeDeployment::Embedded, &key, - &error, + error, ); - error })?; log_acquired(self.entrypoint, RuntimeDeployment::Embedded, &key); leases.insert(key, lease); @@ -350,7 +348,7 @@ fn remote_scope_matches( && requested .ssh_host .as_ref() - .map_or(true, |host| known.ssh_host.as_ref() == Some(host)) + .is_none_or(|host| known.ssh_host.as_ref() == Some(host)) } fn product_identity() -> &'static str { diff --git a/src/crates/assembly/core/src/service/config/providers.rs b/src/crates/assembly/core/src/service/config/providers.rs index 61cbf7bbb9..b763cb89af 100644 --- a/src/crates/assembly/core/src/service/config/providers.rs +++ b/src/crates/assembly/core/src/service/config/providers.rs @@ -160,32 +160,6 @@ impl ConfigProvider for AIConfigProvider { } } -#[cfg(test)] -mod tests { - use super::*; - - #[tokio::test] - async fn rejects_a_model_context_window_smaller_than_32k() { - let mut config = AIConfig::default(); - config.models.push(AIModelConfig { - name: "Test model".to_string(), - provider: "openai".to_string(), - context_window: Some(MIN_MODEL_CONTEXT_WINDOW_TOKENS - 1), - ..AIModelConfig::default() - }); - let value = serde_json::to_value(config).expect("AI config should serialize"); - - let error = AIConfigProvider - .validate_config(&value) - .await - .expect_err("small context windows must be rejected"); - - assert!(error - .to_string() - .contains("context_window must be at least 32000")); - } -} - /// Theme system configuration provider (new, supports theme management). pub struct ThemesConfigProvider; @@ -609,3 +583,29 @@ impl Default for ConfigProviderRegistry { Self::new() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[tokio::test] + async fn rejects_a_model_context_window_smaller_than_32k() { + let mut config = AIConfig::default(); + config.models.push(AIModelConfig { + name: "Test model".to_string(), + provider: "openai".to_string(), + context_window: Some(MIN_MODEL_CONTEXT_WINDOW_TOKENS - 1), + ..AIModelConfig::default() + }); + let value = serde_json::to_value(config).expect("AI config should serialize"); + + let error = AIConfigProvider + .validate_config(&value) + .await + .expect_err("small context windows must be rejected"); + + assert!(error + .to_string() + .contains("context_window must be at least 32000")); + } +} diff --git a/src/crates/assembly/core/src/service/config/types.rs b/src/crates/assembly/core/src/service/config/types.rs index 134c741b44..8be7d83ff4 100644 --- a/src/crates/assembly/core/src/service/config/types.rs +++ b/src/crates/assembly/core/src/service/config/types.rs @@ -602,8 +602,10 @@ pub struct DefaultModelsConfig { /// model named `inherit` can never be interpreted as a control value. #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Eq)] #[serde(tag = "kind", rename_all = "snake_case")] +#[derive(Default)] pub enum SubagentModelSelection { Fixed { model_id: String }, + #[default] Inherit, } @@ -622,11 +624,6 @@ impl SubagentModelSelection { } } -impl Default for SubagentModelSelection { - fn default() -> Self { - Self::Inherit - } -} /// Model defaults for subagents created through user-visible delegation. #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/src/crates/assembly/core/src/service/dispatch/mod.rs b/src/crates/assembly/core/src/service/dispatch/mod.rs index 4910e41ecf..af22334274 100644 --- a/src/crates/assembly/core/src/service/dispatch/mod.rs +++ b/src/crates/assembly/core/src/service/dispatch/mod.rs @@ -572,7 +572,7 @@ fn target_workspace_path_is_absolute(path: &str) -> bool { return false; }; let mut components = unc_path - .split(|character| matches!(character, '\\' | '/')) + .split(['\\', '/']) .filter(|component| !component.is_empty()); components.next().is_some() && components.next().is_some() } diff --git a/src/crates/assembly/core/src/service/dispatch/target.rs b/src/crates/assembly/core/src/service/dispatch/target.rs index 8fce36c30d..1d0c18618d 100644 --- a/src/crates/assembly/core/src/service/dispatch/target.rs +++ b/src/crates/assembly/core/src/service/dispatch/target.rs @@ -2,7 +2,9 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "kebab-case")] +#[derive(Default)] pub enum DispatchWorkspaceDeliveryRequest { + #[default] Existing, SnapshotExact { #[serde(rename = "sourceWorkspacePath")] @@ -12,11 +14,6 @@ pub enum DispatchWorkspaceDeliveryRequest { }, } -impl Default for DispatchWorkspaceDeliveryRequest { - fn default() -> Self { - Self::Existing - } -} /// The execution location selected while a chat session is being created. /// @@ -25,7 +22,9 @@ impl Default for DispatchWorkspaceDeliveryRequest { /// owned by another BitFun process. #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(tag = "kind", rename_all = "lowercase")] +#[derive(Default)] pub enum DispatchTargetRequest { + #[default] Local, Ssh { #[serde(rename = "connectionId")] @@ -41,11 +40,6 @@ pub enum DispatchTargetRequest { }, } -impl Default for DispatchTargetRequest { - fn default() -> Self { - Self::Local - } -} impl DispatchTargetRequest { pub fn is_local(&self) -> bool { diff --git a/src/crates/assembly/core/src/service/remote_connect/mod.rs b/src/crates/assembly/core/src/service/remote_connect/mod.rs index 68acf99b9c..432441bd34 100644 --- a/src/crates/assembly/core/src/service/remote_connect/mod.rs +++ b/src/crates/assembly/core/src/service/remote_connect/mod.rs @@ -1148,7 +1148,7 @@ impl RemoteConnectService { // loop closing after reconnect must not disconnect the new room. let _room_effect = room_lifecycle.lock().await; let mut owner = active_room_owner.write().await; - if clear_room_owner_if_current(&mut *owner, &room_owner) { + if clear_room_owner_if_current(&mut owner, &room_owner) { drop(owner); *relay_arc.write().await = None; pairing_arc.write().await.disconnect().await; diff --git a/src/crates/assembly/core/src/service/worktree/mod.rs b/src/crates/assembly/core/src/service/worktree/mod.rs index e0b82a4d54..9366c7c291 100644 --- a/src/crates/assembly/core/src/service/worktree/mod.rs +++ b/src/crates/assembly/core/src/service/worktree/mod.rs @@ -1272,7 +1272,7 @@ async fn known_project_workspace_paths() -> Vec { } let mut paths = projects.into_values().collect::>(); - paths.sort_by(|left, right| path_string(left).cmp(&path_string(right))); + paths.sort_by_key(|left| path_string(left)); paths } diff --git a/src/crates/assembly/external-sources/src/lib.rs b/src/crates/assembly/external-sources/src/lib.rs index 04823cbfe1..e53bcdce8c 100644 --- a/src/crates/assembly/external-sources/src/lib.rs +++ b/src/crates/assembly/external-sources/src/lib.rs @@ -650,7 +650,7 @@ impl ExternalSourceCoordinator { let mut commands = Vec::new(); let mut command_conflicts = Vec::new(); for (command_name, mut candidates) in command_candidates_by_name { - candidates.sort_by(|left, right| left.id.stable_key().cmp(&right.id.stable_key())); + candidates.sort_by_key(|left| left.id.stable_key()); let requires_reconfirmation = candidates.len() == 1 && self .conflicted_candidate_ids diff --git a/src/crates/contracts/product-domains/src/external_integration_policy.rs b/src/crates/contracts/product-domains/src/external_integration_policy.rs index 721c97edc0..11f139c904 100644 --- a/src/crates/contracts/product-domains/src/external_integration_policy.rs +++ b/src/crates/contracts/product-domains/src/external_integration_policy.rs @@ -18,7 +18,9 @@ pub const EXTERNAL_INTEGRATION_POLICY_SCHEMA_MAJOR: u32 = 1; /// evaluation and are never projected as selectable UI options. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[non_exhaustive] +#[derive(Default)] pub enum ExternalIntegrationMode { + #[default] Recommended, DiscoverOnly, Disabled, @@ -53,11 +55,6 @@ impl ExternalIntegrationMode { } } -impl Default for ExternalIntegrationMode { - fn default() -> Self { - Self::Recommended - } -} impl Serialize for ExternalIntegrationMode { fn serialize(&self, serializer: S) -> Result @@ -82,8 +79,10 @@ impl<'de> Deserialize<'de> for ExternalIntegrationMode { /// `Disabled` on Hosts that do not understand them. #[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)] #[non_exhaustive] +#[derive(Default)] pub enum ExternalIntegrationAccess { Disabled, + #[default] DiscoverOnly, AskBeforeUse, Auto, @@ -137,11 +136,6 @@ impl ExternalIntegrationAccess { } } -impl Default for ExternalIntegrationAccess { - fn default() -> Self { - Self::DiscoverOnly - } -} impl Serialize for ExternalIntegrationAccess { fn serialize(&self, serializer: S) -> Result @@ -184,6 +178,7 @@ impl Default for ExternalEcosystemPolicy { #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(default, rename_all = "camelCase")] +#[derive(Default)] pub struct ExternalIntegrationPolicySettings { pub enabled: bool, #[serde(default, skip_serializing_if = "BTreeMap::is_empty")] @@ -193,15 +188,6 @@ pub struct ExternalIntegrationPolicySettings { pub extensions: BTreeMap, } -impl Default for ExternalIntegrationPolicySettings { - fn default() -> Self { - Self { - enabled: false, - ecosystems: BTreeMap::new(), - extensions: BTreeMap::new(), - } - } -} #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)] #[serde(default, rename_all = "camelCase")] @@ -360,7 +346,9 @@ impl From<&ExternalIntegrationPolicyOverride> for ExternalIntegrationPolicyOverr #[derive(Debug, Clone, PartialEq, Eq)] #[non_exhaustive] +#[derive(Default)] pub enum ExternalIntegrationPolicyStatus { + #[default] Compatible, IncompatibleSchema, Unknown(String), @@ -389,11 +377,6 @@ impl ExternalIntegrationPolicyStatus { } } -impl Default for ExternalIntegrationPolicyStatus { - fn default() -> Self { - Self::Compatible - } -} impl Serialize for ExternalIntegrationPolicyStatus { fn serialize(&self, serializer: S) -> Result diff --git a/src/crates/contracts/product-domains/src/miniapp/market.rs b/src/crates/contracts/product-domains/src/miniapp/market.rs index 771956a509..35fda4630d 100644 --- a/src/crates/contracts/product-domains/src/miniapp/market.rs +++ b/src/crates/contracts/product-domains/src/miniapp/market.rs @@ -49,17 +49,14 @@ impl MarketSubmissionStatus { #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "snake_case")] +#[derive(Default)] pub enum MarketSort { + #[default] Newest, Downloads, Rating, } -impl Default for MarketSort { - fn default() -> Self { - Self::Newest - } -} #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] diff --git a/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs b/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs index 8e17b22bc1..6788520f06 100644 --- a/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs +++ b/src/crates/contracts/product-domains/tests/plugin_source_contracts.rs @@ -426,10 +426,9 @@ fn activation_lifecycle_is_exact_independent_and_idempotent() { .expect("deactivate source") .is_some()); assert_eq!((store.epoch(), store.activation_epoch()), (trust_epoch, 9)); - assert!(!store + assert!(store .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, None) - .expect("repeat deactivation") - .is_some()); + .expect("repeat deactivation").is_none()); assert_eq!((store.epoch(), store.activation_epoch()), (trust_epoch, 9)); } @@ -495,10 +494,9 @@ fn stale_residual_cleanup_cannot_clear_a_newer_activation() { .expect("read current activation authority") .activation_epoch(); - assert!(!store + assert!(store .clear_activation_record(PROJECT, WORKSPACE, &package.package_id, Some(stale_epoch),) - .expect("stale cleanup is a no-op") - .is_some()); + .expect("stale cleanup is a no-op").is_none()); assert!(store.is_activated(PROJECT, WORKSPACE, &package)); assert_eq!(store.activation_epoch(), current_epoch); } diff --git a/src/crates/contracts/runtime-ports/src/lib.rs b/src/crates/contracts/runtime-ports/src/lib.rs index 627df3d5bf..73c0398a93 100644 --- a/src/crates/contracts/runtime-ports/src/lib.rs +++ b/src/crates/contracts/runtime-ports/src/lib.rs @@ -2345,13 +2345,15 @@ mod tests { AgentSessionManagementPort::set_session_archived(&provider, archive_state_request(true)) .await .expect("archive=true should delegate to the legacy provider"); - let requests = provider.archived_requests.lock().unwrap(); - assert_eq!(requests.len(), 1); - assert_eq!(requests[0].workspace_path, "/workspace/project"); - assert_eq!(requests[0].session_id, "session_1"); - assert_eq!(requests[0].remote_connection_id.as_deref(), Some("conn-1")); - assert_eq!(requests[0].remote_ssh_host.as_deref(), Some("host-1")); - drop(requests); + // Scoped so the guard is provably released before the await below. + { + let requests = provider.archived_requests.lock().unwrap(); + assert_eq!(requests.len(), 1); + assert_eq!(requests[0].workspace_path, "/workspace/project"); + assert_eq!(requests[0].session_id, "session_1"); + assert_eq!(requests[0].remote_connection_id.as_deref(), Some("conn-1")); + assert_eq!(requests[0].remote_ssh_host.as_deref(), Some("host-1")); + } let error = AgentSessionManagementPort::set_session_archived( &provider, diff --git a/src/crates/execution/tool-call-jsonrepair/tests/streaming_tests.rs b/src/crates/execution/tool-call-jsonrepair/tests/streaming_tests.rs index dff7f036ea..c2640c25db 100644 --- a/src/crates/execution/tool-call-jsonrepair/tests/streaming_tests.rs +++ b/src/crates/execution/tool-call-jsonrepair/tests/streaming_tests.rs @@ -127,7 +127,7 @@ struct FailingReader; impl Read for FailingReader { fn read(&mut self, _buf: &mut [u8]) -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "source closed")) + Err(io::Error::other("source closed")) } } @@ -135,7 +135,7 @@ struct FailingWriter; impl Write for FailingWriter { fn write(&mut self, _buf: &[u8]) -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "destination closed")) + Err(io::Error::other("destination closed")) } fn flush(&mut self) -> io::Result<()> { diff --git a/src/crates/execution/tool-call-jsonrepair/tests/writer_tests.rs b/src/crates/execution/tool-call-jsonrepair/tests/writer_tests.rs index cef8eaa116..46e07a27b1 100644 --- a/src/crates/execution/tool-call-jsonrepair/tests/writer_tests.rs +++ b/src/crates/execution/tool-call-jsonrepair/tests/writer_tests.rs @@ -27,7 +27,7 @@ struct FailingWriter; impl Write for FailingWriter { fn write(&mut self, _buf: &[u8]) -> io::Result { - Err(io::Error::new(io::ErrorKind::Other, "destination closed")) + Err(io::Error::other("destination closed")) } fn flush(&mut self) -> io::Result<()> { diff --git a/src/crates/execution/tool-execution/src/fs/mod.rs b/src/crates/execution/tool-execution/src/fs/mod.rs index 22573c9c81..28dbbb346f 100644 --- a/src/crates/execution/tool-execution/src/fs/mod.rs +++ b/src/crates/execution/tool-execution/src/fs/mod.rs @@ -34,7 +34,7 @@ pub fn path_has_multiple_hard_links(path: &std::path::Path) -> std::io::Result 1); + Ok(metadata.nlink() > 1) } #[cfg(windows)] diff --git a/src/crates/execution/tool-execution/src/search/glob_search.rs b/src/crates/execution/tool-execution/src/search/glob_search.rs index de1d3ff8c4..506e8eba15 100644 --- a/src/crates/execution/tool-execution/src/search/glob_search.rs +++ b/src/crates/execution/tool-execution/src/search/glob_search.rs @@ -179,8 +179,8 @@ fn create_command(program: &str) -> Command { #[cfg(not(windows))] fn create_command(program: &str) -> Command { - let command = Command::new(program); - command + + Command::new(program) } fn build_fallback_matcher(relative_pattern: &str) -> Result { diff --git a/src/crates/interfaces/sdk-host/tests/host_lifecycle.rs b/src/crates/interfaces/sdk-host/tests/host_lifecycle.rs index 482f24c105..747a0106c4 100644 --- a/src/crates/interfaces/sdk-host/tests/host_lifecycle.rs +++ b/src/crates/interfaces/sdk-host/tests/host_lifecycle.rs @@ -911,10 +911,12 @@ async fn cancel_close_and_shutdown_use_existing_runtime_owners() { }))) .await; while output.recv().await.unwrap()["id"] != "close-1" {} - let discard_requests = owner.discard_requests.lock().unwrap(); - assert_eq!(discard_requests.len(), 1); - assert_eq!(discard_requests[0].wait_timeout_ms, 1234); - drop(discard_requests); + // Scoped so the guard is provably released before the awaits below. + { + let discard_requests = owner.discard_requests.lock().unwrap(); + assert_eq!(discard_requests.len(), 1); + assert_eq!(discard_requests[0].wait_timeout_ms, 1234); + } let control = host .handle_request(request(serde_json::json!({ @@ -1754,11 +1756,13 @@ async fn query_submission_disables_unavailable_interactive_callbacks() { .await; assert_eq!(output.recv().await.unwrap()["result"]["accepted"], true); - let metadata = owner.dialog_metadata.lock().unwrap(); - assert_eq!(metadata.len(), 1); - assert_eq!(metadata[0]["user_input_available"], false); - assert_eq!(metadata[0]["auto_approve_ask"], false); - drop(metadata); + // Scoped so the guard is provably released before the await below. + { + let metadata = owner.dialog_metadata.lock().unwrap(); + assert_eq!(metadata.len(), 1); + assert_eq!(metadata[0]["user_input_available"], false); + assert_eq!(metadata[0]["auto_approve_ask"], false); + } host.shutdown_connection().await; } @@ -1783,12 +1787,10 @@ async fn provider_quota_and_billing_keep_distinct_wire_codes() { .as_str() .unwrap() .to_string(); - owner - .queue - .lock() - .unwrap() - .clone() - .unwrap() + // Cloned out of the guard first: the guard must not survive the + // `enqueue` await below. + let queue = owner.queue.lock().unwrap().clone().unwrap(); + queue .enqueue( AgenticEvent::DialogTurnFailed { session_id, diff --git a/src/crates/services/relay-service/src/admin.rs b/src/crates/services/relay-service/src/admin.rs index 6a7eff4b0d..1640c829da 100644 --- a/src/crates/services/relay-service/src/admin.rs +++ b/src/crates/services/relay-service/src/admin.rs @@ -117,7 +117,7 @@ pub fn provision(_username: &str, password: &str) -> Result let wrapped_master_key = format!( "{}.{}", BASE64.encode(&wrapped_ct), - BASE64.encode(&nonce_bytes) + BASE64.encode(nonce_bytes) ); // 3. Derive the server-verifiable password hash (separate salt). diff --git a/src/crates/services/relay-service/src/db.rs b/src/crates/services/relay-service/src/db.rs index 129587b5b4..6a3f4ed420 100644 --- a/src/crates/services/relay-service/src/db.rs +++ b/src/crates/services/relay-service/src/db.rs @@ -603,7 +603,7 @@ fn lockout_until(attempts: i64, now: i64) -> i64 { if attempts < 5 { return 0; } - let level = (attempts - 4).min(4) as i64; + let level = (attempts - 4).min(4); let secs = match level { 1 => 60, 2 => 300, diff --git a/src/crates/services/relay-service/src/routes/pages.rs b/src/crates/services/relay-service/src/routes/pages.rs index 4bf1491bef..7f3f35121a 100644 --- a/src/crates/services/relay-service/src/routes/pages.rs +++ b/src/crates/services/relay-service/src/routes/pages.rs @@ -4384,7 +4384,7 @@ mod tests { let resp = app .oneshot( Request::builder() - .uri(format!("/p/alice/fn/api/hello")) + .uri("/p/alice/fn/api/hello".to_string()) .body(axum::body::Body::empty()) .unwrap(), ) diff --git a/src/crates/services/services-core/src/json_store.rs b/src/crates/services/services-core/src/json_store.rs index 013afe58f2..946ef816a6 100644 --- a/src/crates/services/services-core/src/json_store.rs +++ b/src/crates/services/services-core/src/json_store.rs @@ -342,10 +342,14 @@ impl JsonFileStore { .unwrap_or_else(|| "data.json".to_string()); let lock_path = path.with_file_name(format!("{file_name}.lock")); tokio::task::spawn_blocking(move || { + // The lock file carries no payload; it exists only to hold the + // advisory `flock`. Never truncate it — another process may already + // be holding the lock on this same inode. let file = OpenOptions::new() .create(true) .read(true) .write(true) + .truncate(false) .open(&lock_path) .map_err(|source| JsonFileStoreError::CrossProcessLock { path: lock_path.clone(), diff --git a/src/crates/services/services-core/src/process_tree.rs b/src/crates/services/services-core/src/process_tree.rs index 9cc76cd09d..487ccb5668 100644 --- a/src/crates/services/services-core/src/process_tree.rs +++ b/src/crates/services/services-core/src/process_tree.rs @@ -392,9 +392,9 @@ mod tests { if std::env::var_os("BITFUN_DETACHED_FIXTURE").is_none() { return; } - // SAFETY: the fixture is a single-threaded test subprocess created - // solely to verify the documented process-group containment limit. assert!( + // SAFETY: the fixture is a single-threaded test subprocess created + // solely to verify the documented process-group containment limit. unsafe { libc::setsid() } >= 0, "fixture must create a new session" ); diff --git a/src/crates/services/services-core/src/token_usage/service.rs b/src/crates/services/services-core/src/token_usage/service.rs index 9a90e9f91e..b89083c4da 100644 --- a/src/crates/services/services-core/src/token_usage/service.rs +++ b/src/crates/services/services-core/src/token_usage/service.rs @@ -662,6 +662,37 @@ impl TokenUsageService { } } +fn records_date_key(date: DateTime) -> String { + date.format("%Y-%m-%d").to_string() +} + +fn is_token_usage_record_path(path: &Path) -> bool { + if path.extension().and_then(|value| value.to_str()) != Some("json") { + return false; + } + + path.file_stem() + .and_then(|value| value.to_str()) + .is_some_and(|stem| NaiveDate::parse_from_str(stem, "%Y-%m-%d").is_ok()) +} + +async fn read_records_batch(path: &Path) -> Result { + if !fs::try_exists(path).await.unwrap_or(false) { + return Ok(RecordsBatch { + records: Vec::new(), + }); + } + + let content = fs::read_to_string(path) + .await + .map_err(|e| format!("Failed to read token usage records: {}", e))?; + Ok( + serde_json::from_str::(&content).unwrap_or_else(|_| RecordsBatch { + records: Vec::new(), + }), + ) +} + #[cfg(test)] mod tests { use super::*; @@ -721,34 +752,3 @@ mod tests { assert_eq!(records[0].session_id, "parent-session"); } } - -fn records_date_key(date: DateTime) -> String { - date.format("%Y-%m-%d").to_string() -} - -fn is_token_usage_record_path(path: &Path) -> bool { - if path.extension().and_then(|value| value.to_str()) != Some("json") { - return false; - } - - path.file_stem() - .and_then(|value| value.to_str()) - .is_some_and(|stem| NaiveDate::parse_from_str(stem, "%Y-%m-%d").is_ok()) -} - -async fn read_records_batch(path: &Path) -> Result { - if !fs::try_exists(path).await.unwrap_or(false) { - return Ok(RecordsBatch { - records: Vec::new(), - }); - } - - let content = fs::read_to_string(path) - .await - .map_err(|e| format!("Failed to read token usage records: {}", e))?; - Ok( - serde_json::from_str::(&content).unwrap_or_else(|_| RecordsBatch { - records: Vec::new(), - }), - ) -} diff --git a/src/crates/services/services-integrations/src/browser_control/launcher.rs b/src/crates/services/services-integrations/src/browser_control/launcher.rs index abc60e5297..b639a76305 100644 --- a/src/crates/services/services-integrations/src/browser_control/launcher.rs +++ b/src/crates/services/services-integrations/src/browser_control/launcher.rs @@ -603,7 +603,7 @@ impl BrowserLauncher { return Ok(()); } let stderr = String::from_utf8_lossy(&output.stderr); - return Err(anyhow!("Failed to quit {}: {}", kind, stderr.trim())); + Err(anyhow!("Failed to quit {}: {}", kind, stderr.trim())) } #[cfg(target_os = "windows")] diff --git a/src/crates/services/services-integrations/src/hook_import.rs b/src/crates/services/services-integrations/src/hook_import.rs index ce97539773..3ab7b7b509 100644 --- a/src/crates/services/services-integrations/src/hook_import.rs +++ b/src/crates/services/services-integrations/src/hook_import.rs @@ -554,8 +554,8 @@ async fn publish_bundle( write: &HookImportWrite, content_digest: &str, ) -> Result { - if tokio::fs::symlink_metadata(final_path).await.is_ok() { - if validate_bundle_content(root, final_path, content_digest) + if tokio::fs::symlink_metadata(final_path).await.is_ok() + && validate_bundle_content(root, final_path, content_digest) .await .is_ok() { @@ -566,7 +566,6 @@ async fn publish_bundle( changed: false, }); } - } let staging = root .join(".staging") .join(format!("import-{}", uuid::Uuid::new_v4())); diff --git a/src/crates/services/services-integrations/src/mcp/protocol/client_info.rs b/src/crates/services/services-integrations/src/mcp/protocol/client_info.rs index 157ca7d3c1..b21779e36f 100644 --- a/src/crates/services/services-integrations/src/mcp/protocol/client_info.rs +++ b/src/crates/services/services-integrations/src/mcp/protocol/client_info.rs @@ -6,13 +6,17 @@ pub fn create_mcp_client_info( client_name: impl Into, client_version: impl Into, ) -> ClientInfo { - ClientInfo::new( - ClientCapabilities::builder() - .enable_roots() - .enable_sampling() - .enable_elicitation() - .build(), - Implementation::new(client_name, client_version), - ) - .with_protocol_version(ProtocolVersion::LATEST) + // SEP-2577 deprecates `roots` and `sampling`, but BitFun still advertises + // both so servers that gate features on them keep working; the handshake + // shape is pinned by `mcp_remote_client_info_declares_supported_client_capabilities`. + // Dropping them is a protocol-visible decision, not a lint cleanup — revisit + // when rmcp actually removes the builders. + #[allow(deprecated)] + let capabilities = ClientCapabilities::builder() + .enable_roots() + .enable_sampling() + .enable_elicitation() + .build(); + ClientInfo::new(capabilities, Implementation::new(client_name, client_version)) + .with_protocol_version(ProtocolVersion::LATEST) } diff --git a/src/crates/services/services-integrations/src/mcp/protocol/transport_remote.rs b/src/crates/services/services-integrations/src/mcp/protocol/transport_remote.rs index 77e984a27e..faa0a13bec 100644 --- a/src/crates/services/services-integrations/src/mcp/protocol/transport_remote.rs +++ b/src/crates/services/services-integrations/src/mcp/protocol/transport_remote.rs @@ -202,7 +202,7 @@ impl StreamableHttpClient for BitFunStreamableHttpClient { } } - let event_stream = SseStream::from_byte_stream(response.bytes_stream()).boxed(); + let event_stream = SseStream::from_bytes_stream(response.bytes_stream()).boxed(); Ok(event_stream) } @@ -303,7 +303,7 @@ impl StreamableHttpClient for BitFunStreamableHttpClient { match content_type.as_deref() { Some(ct) if ct.as_bytes().starts_with(EVENT_STREAM_MIME_TYPE.as_bytes()) => { - let event_stream = SseStream::from_byte_stream(response.bytes_stream()).boxed(); + let event_stream = SseStream::from_bytes_stream(response.bytes_stream()).boxed(); Ok(StreamableHttpPostResponse::Sse(event_stream, session_id)) } Some(ct) if ct.as_bytes().starts_with(JSON_MIME_TYPE.as_bytes()) => { diff --git a/src/crates/services/services-integrations/src/mcp/server/process.rs b/src/crates/services/services-integrations/src/mcp/server/process.rs index bb61793a9f..289d70700e 100644 --- a/src/crates/services/services-integrations/src/mcp/server/process.rs +++ b/src/crates/services/services-integrations/src/mcp/server/process.rs @@ -540,6 +540,19 @@ fn redact_sensitive_value(message: &str, sensitive_value: Option<&str>) -> Strin .unwrap_or_else(|| message.to_string()) } +#[cfg(not(windows))] +fn safe_process_environment_keys() -> &'static [&'static str] { + &[ + "PATH", "HOME", "TMPDIR", "LANG", "LC_ALL", "LC_CTYPE", "SHELL", + ] +} + +impl Drop for MCPServerProcess { + fn drop(&mut self) { + self.child.take(); + } +} + #[cfg(test)] mod tests { use super::{redact_sensitive_value, safe_process_environment_keys}; @@ -562,16 +575,3 @@ mod tests { assert!(redacted.contains("")); } } - -#[cfg(not(windows))] -fn safe_process_environment_keys() -> &'static [&'static str] { - &[ - "PATH", "HOME", "TMPDIR", "LANG", "LC_ALL", "LC_CTYPE", "SHELL", - ] -} - -impl Drop for MCPServerProcess { - fn drop(&mut self) { - self.child.take(); - } -} diff --git a/src/crates/services/services-integrations/src/plugin_source.rs b/src/crates/services/services-integrations/src/plugin_source.rs index f4bdeed29c..daa0587edf 100644 --- a/src/crates/services/services-integrations/src/plugin_source.rs +++ b/src/crates/services/services-integrations/src/plugin_source.rs @@ -2608,6 +2608,8 @@ fn open_directory_no_follow(path: &Path) -> io::Result { let path = std::ffi::CString::new(path.as_os_str().as_bytes()) .map_err(|_| io::Error::new(ErrorKind::InvalidInput, "path contains NUL"))?; + // SAFETY: `path` is a NUL-terminated CString that outlives this call, and + // the flags are a valid `open` flag set. let fd = unsafe { libc::open( path.as_ptr(), @@ -2617,6 +2619,8 @@ fn open_directory_no_follow(path: &Path) -> io::Result { if fd < 0 { return Err(io::Error::last_os_error()); } + // SAFETY: `open` just returned this descriptor and the error case returned + // above, so it is open, valid, and owned by nothing else. Ok(unsafe { std::fs::File::from_raw_fd(fd) }) } @@ -2627,6 +2631,9 @@ fn openat_directory(directory: &std::fs::File, name: &OsStr) -> io::Result io::Result io::Result } let file_name = std::ffi::CString::new(file_name.as_bytes()) .map_err(|_| io::Error::new(ErrorKind::InvalidInput, "path contains NUL"))?; + // SAFETY: `directory` is a live `File` held for this call, so its + // descriptor stays open; `file_name` is a NUL-terminated CString outliving + // the call, and the flags are a valid `openat` flag set. let fd = unsafe { libc::openat( directory.as_raw_fd(), @@ -2674,6 +2686,8 @@ fn openat_regular_file(base: &std::fs::File, relative_path: &Path) -> io::Result if fd < 0 { return Err(io::Error::last_os_error()); } + // SAFETY: `openat` just returned this descriptor and the error case + // returned above, so it is open, valid, and owned by nothing else. let file = unsafe { std::fs::File::from_raw_fd(fd) }; if !file.metadata()?.is_file() { return Err(io::Error::new( @@ -3140,7 +3154,7 @@ fn declared_parent_metadata_issue_code(kind: ErrorKind) -> PluginSourceIssueCode mod tests { use super::{ build_snapshot, charge_scanned_read, declared_parent_metadata_issue_code, - map_activation_store_error, map_load_store_error, native_path_identity, + map_activation_store_error, map_load_store_error, persist_trust_bytes_with_parent_sync, read_bounded_reader, read_scanned_file, replace_file_atomically, trust_file_identity, trust_store_issue_code, workspace_scope, ManagedPluginSourceError, ManagedPluginSourceService, OperationScanBudget, diff --git a/src/crates/services/services-integrations/src/remote_connect/page_upload.rs b/src/crates/services/services-integrations/src/remote_connect/page_upload.rs index 18b74f8112..13f518adae 100644 --- a/src/crates/services/services-integrations/src/remote_connect/page_upload.rs +++ b/src/crates/services/services-integrations/src/remote_connect/page_upload.rs @@ -529,10 +529,10 @@ pub async fn list_pages_from_relay(relay_url: &str, token: &str) -> Result String { - let quoted_pid_file = crate::remote_ssh::shell::quote_arg(&pid_file); + let quoted_pid_file = crate::remote_ssh::shell::quote_arg(pid_file); let quoted_command = crate::remote_ssh::shell::quote_arg(command); let quoted_shell = crate::remote_ssh::shell::quote_arg(&container.shell); let sweep = stale_pid_file_sweep(); @@ -5611,7 +5611,7 @@ mod tests { #[test] fn parses_container_directory_entry_with_newline_and_unit_separator() { let entries = parse_container_dir_output( - "src\n\u{1f}name\0/workspace/src\n\u{1f}name\0d\0\01720000000\0755\0", + "src\n\u{1f}name\x00/workspace/src\n\u{1f}name\x00d\x00\x001720000000\x00755\x00", ) .unwrap(); let entry = &entries[0]; diff --git a/src/crates/services/services-integrations/src/remote_ssh/transport.rs b/src/crates/services/services-integrations/src/remote_ssh/transport.rs index 6567a01604..c0729994dd 100644 --- a/src/crates/services/services-integrations/src/remote_ssh/transport.rs +++ b/src/crates/services/services-integrations/src/remote_ssh/transport.rs @@ -534,26 +534,26 @@ async fn run_local_process( let stdout_task = tokio::spawn(copy_to_duplex(child_stdout, pipes.stdout)); let stderr_task = tokio::spawn(copy_to_duplex(child_stderr, pipes.stderr)); - let exit_code = loop { - tokio::select! { - status = child.wait() => { - break status.ok().and_then(local_process_exit_code); - } - // We are the ones ending the process here, so a signal death says - // nothing the caller does not already know. Prefer a status the - // child chose for itself and otherwise report the requested intent. - signal = pipes.control_rx.recv() => { - let fallback = match signal { - Some(WorkspaceProcessSignal::Interrupt) => 130, - Some(WorkspaceProcessSignal::Kill) | None => 137, - }; - let _ = child.start_kill(); - break child.wait().await.ok().and_then(|status| status.code()).or(Some(fallback)); - } - _ = pipes.cancellation.cancelled() => { - let _ = child.start_kill(); - break child.wait().await.ok().and_then(|status| status.code()).or(Some(137)); - } + // Whichever arm wins settles the exit code; none of them can resume + // waiting, so this is a single-shot select rather than a loop. + let exit_code = tokio::select! { + status = child.wait() => { + status.ok().and_then(local_process_exit_code) + } + // We are the ones ending the process here, so a signal death says + // nothing the caller does not already know. Prefer a status the + // child chose for itself and otherwise report the requested intent. + signal = pipes.control_rx.recv() => { + let fallback = match signal { + Some(WorkspaceProcessSignal::Interrupt) => 130, + Some(WorkspaceProcessSignal::Kill) | None => 137, + }; + let _ = child.start_kill(); + child.wait().await.ok().and_then(|status| status.code()).or(Some(fallback)) + } + _ = pipes.cancellation.cancelled() => { + let _ = child.start_kill(); + child.wait().await.ok().and_then(|status| status.code()).or(Some(137)) } }; diff --git a/src/crates/services/services-integrations/src/speech/audio.rs b/src/crates/services/services-integrations/src/speech/audio.rs index 70f62c912a..822c2dac6c 100644 --- a/src/crates/services/services-integrations/src/speech/audio.rs +++ b/src/crates/services/services-integrations/src/speech/audio.rs @@ -1,7 +1,7 @@ use super::{BitFunError, BitFunResult}; pub(super) fn pcm16_le_to_f32_samples(bytes: &[u8]) -> BitFunResult> { - if bytes.len() % 2 != 0 { + if !bytes.len().is_multiple_of(2) { return Err(BitFunError::validation( "PCM16 audio payload must have an even number of bytes", )); diff --git a/src/crates/services/services-integrations/src/speech/downloader.rs b/src/crates/services/services-integrations/src/speech/downloader.rs index 601b2d1831..e93f8890c7 100644 --- a/src/crates/services/services-integrations/src/speech/downloader.rs +++ b/src/crates/services/services-integrations/src/speech/downloader.rs @@ -309,7 +309,7 @@ async fn install_artifacts_into_staging( } } - let payload_dir = find_payload_dir(&staging, &manifest.required_files).await?; + let payload_dir = find_payload_dir(staging, &manifest.required_files).await?; if final_dir.exists() { fs::remove_dir_all(&final_dir).await?; } @@ -323,7 +323,7 @@ async fn install_artifacts_into_staging( } } - store.write_install_record(manifest, &final_dir).await?; + store.write_install_record(manifest, final_dir).await?; Ok(()) } diff --git a/src/crates/services/terminal/src/exec.rs b/src/crates/services/terminal/src/exec.rs index 0b2af05a99..8c066604b7 100644 --- a/src/crates/services/terminal/src/exec.rs +++ b/src/crates/services/terminal/src/exec.rs @@ -1357,6 +1357,9 @@ fn local_pipe_exit_code(status: std::process::ExitStatus) -> Option { #[cfg(unix)] fn configure_pipe_process_group(command: &mut Command) { + // SAFETY: `pre_exec` runs between fork and exec, where only async-signal-safe + // work is allowed. The closure calls nothing but `setsid`/`setpgid` and reads + // `errno`; it allocates nothing and touches no shared state. unsafe { command.pre_exec(|| { if libc::setsid() == -1 { @@ -1512,6 +1515,9 @@ fn request_unix_pipe_control( #[cfg(unix)] fn signal_pipe_process_group_id(pgid: libc::pid_t, signal: libc::c_int) { + // SAFETY: `killpg` is a plain syscall with no memory-safety contract. A + // stale or already-reaped group id fails with ESRCH, which is ignored here + // because a group that is already gone needs no signal. unsafe { libc::killpg(pgid, signal); } @@ -1707,7 +1713,7 @@ mod tests { use crate::shell::{ShellDetector, ShellType}; use encoding_rs::GBK; use std::collections::HashMap; - use std::path::PathBuf; + use std::sync::Arc; #[cfg(windows)] diff --git a/src/crates/services/terminal/src/transcript.rs b/src/crates/services/terminal/src/transcript.rs index 58b09ee5cf..29722f4359 100644 --- a/src/crates/services/terminal/src/transcript.rs +++ b/src/crates/services/terminal/src/transcript.rs @@ -226,8 +226,7 @@ impl TranscriptRecorder { operation: impl FnOnce(&mut TranscriptStore) -> io::Result, ) -> io::Result { let mut store = self.inner.lock().map_err(|_| { - io::Error::new( - io::ErrorKind::Other, + io::Error::other( "terminal transcript recorder lock is poisoned", ) })?; @@ -685,8 +684,7 @@ impl TranscriptStore { }); let index = TranscriptIndex { sessions }; let serialized = serde_json::to_vec_pretty(&index).map_err(|error| { - io::Error::new( - io::ErrorKind::Other, + io::Error::other( format!("serialize terminal transcript index: {error}"), ) })?;