From f2729d22ef27ce46cdefd2aa5e5955ff478da5be Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Mon, 30 Mar 2026 11:27:25 -0400 Subject: [PATCH] Clean up workspace clippy warnings --- lib/crates/fabro-agent/src/compaction.rs | 9 +- lib/crates/fabro-agent/src/event.rs | 5 +- lib/crates/fabro-agent/src/history.rs | 10 +- lib/crates/fabro-agent/src/mcp_integration.rs | 9 +- .../fabro-agent/src/profiles/anthropic.rs | 8 +- lib/crates/fabro-agent/src/profiles/gemini.rs | 8 +- lib/crates/fabro-agent/src/profiles/openai.rs | 9 +- lib/crates/fabro-agent/src/session.rs | 68 +++++----- lib/crates/fabro-agent/src/skills.rs | 21 +-- lib/crates/fabro-agent/src/subagent.rs | 5 +- lib/crates/fabro-agent/src/test_support.rs | 27 ++-- lib/crates/fabro-agent/src/tool_execution.rs | 32 ++--- lib/crates/fabro-agent/src/tool_registry.rs | 6 +- lib/crates/fabro-agent/src/tools.rs | 31 ++--- lib/crates/fabro-agent/src/types.rs | 8 +- lib/crates/fabro-agent/src/v4a_patch.rs | 4 +- lib/crates/fabro-agent/tests/it/guardrails.rs | 8 +- .../fabro-agent/tests/it/parity_matrix.rs | 12 +- lib/crates/fabro-cli/src/commands/doctor.rs | 5 +- lib/crates/fabro-cli/src/commands/install.rs | 5 +- .../fabro-cli/src/commands/run/attach.rs | 2 +- .../fabro-cli/src/commands/run/command.rs | 2 +- .../fabro-cli/src/commands/run/launcher.rs | 8 +- .../fabro-cli/src/commands/run/resume.rs | 24 ++-- .../src/commands/run/run_progress.rs | 12 +- .../fabro-cli/src/commands/store/dump.rs | 4 +- lib/crates/fabro-cli/src/commands/upgrade.rs | 8 +- .../fabro-cli/src/shared/provider_auth.rs | 2 +- lib/crates/fabro-cli/tests/it/cli.rs | 12 +- lib/crates/fabro-cli/tests/it/scenario.rs | 37 +++--- lib/crates/fabro-config/src/project.rs | 14 +- lib/crates/fabro-config/src/user.rs | 1 + lib/crates/fabro-core/src/error.rs | 2 +- lib/crates/fabro-core/src/executor.rs | 19 +-- lib/crates/fabro-core/src/lifecycle.rs | 2 + lib/crates/fabro-core/src/stall.rs | 13 +- lib/crates/fabro-core/src/test_fixtures.rs | 7 + lib/crates/fabro-devcontainer/src/compose.rs | 47 ++++--- lib/crates/fabro-devcontainer/src/jsonc.rs | 4 +- .../fabro-git-storage/src/branchstore.rs | 3 +- lib/crates/fabro-github/src/lib.rs | 34 +++-- .../fabro-graphviz/src/parser/semantic.rs | 35 ++--- lib/crates/fabro-hooks/src/bridge.rs | 2 +- lib/crates/fabro-hooks/src/executor.rs | 5 +- lib/crates/fabro-interview/src/file.rs | 34 +++-- lib/crates/fabro-interview/src/lib.rs | 3 +- lib/crates/fabro-interview/src/web.rs | 12 +- lib/crates/fabro-llm/src/cli.rs | 2 +- lib/crates/fabro-llm/src/error.rs | 2 +- lib/crates/fabro-llm/src/generate.rs | 31 +++-- lib/crates/fabro-llm/src/provider.rs | 4 +- .../fabro-llm/src/providers/anthropic.rs | 25 ++-- .../fabro-llm/src/providers/fabro_server.rs | 10 +- lib/crates/fabro-llm/src/providers/gemini.rs | 9 +- lib/crates/fabro-llm/src/providers/openai.rs | 10 +- .../src/providers/openai_compatible.rs | 9 +- lib/crates/fabro-llm/src/retry.rs | 40 +++--- lib/crates/fabro-llm/src/types.rs | 7 +- lib/crates/fabro-llm/tests/integration.rs | 8 +- .../fabro-mcp/tests/stdio_integration.rs | 4 +- lib/crates/fabro-model/src/catalog.rs | 6 +- lib/crates/fabro-model/src/model_ref.rs | 11 +- lib/crates/fabro-retro/src/retro_agent.rs | 5 +- lib/crates/fabro-sandbox/src/docker.rs | 10 +- lib/crates/fabro-sandbox/src/exe/mod.rs | 121 +++++++++--------- .../fabro-sandbox/src/exe/openssh_runner.rs | 8 +- lib/crates/fabro-sandbox/src/local.rs | 8 +- lib/crates/fabro-sandbox/src/reconnect.rs | 12 +- lib/crates/fabro-sandbox/src/sandbox_spec.rs | 6 +- lib/crates/fabro-sandbox/src/ssh/mod.rs | 8 +- lib/crates/fabro-sandbox/src/test_support.rs | 13 +- lib/crates/fabro-sandbox/src/worktree.rs | 7 +- lib/crates/fabro-server/src/lib.rs | 5 + lib/crates/fabro-server/src/server.rs | 15 ++- lib/crates/fabro-server/src/sessions.rs | 4 +- lib/crates/fabro-server/tests/it/api.rs | 2 + .../tests/it/openapi_conformance.rs | 7 + .../fabro-server/tests/it/pagination.rs | 2 + lib/crates/fabro-slack/src/connection.rs | 2 +- lib/crates/fabro-store/src/memory.rs | 6 +- lib/crates/fabro-store/src/slate/catalog.rs | 2 +- lib/crates/fabro-store/src/slate/mod.rs | 19 +-- lib/crates/fabro-telemetry/src/lib.rs | 8 +- lib/crates/fabro-telemetry/src/sanitize.rs | 2 +- lib/crates/fabro-telemetry/src/sender.rs | 3 +- lib/crates/fabro-tracker/src/linear.rs | 2 +- lib/crates/fabro-util/src/backoff.rs | 5 +- lib/crates/fabro-util/src/redact/entropy.rs | 4 +- lib/crates/fabro-workflows/src/event.rs | 2 +- .../fabro-workflows/src/graph/routing.rs | 2 +- .../fabro-workflows/src/handler/parallel.rs | 7 +- lib/crates/fabro-workflows/src/lib.rs | 16 +++ .../fabro-workflows/src/lifecycle/event.rs | 2 +- .../fabro-workflows/src/lifecycle/hook.rs | 34 ++--- .../fabro-workflows/src/lifecycle/mod.rs | 10 +- .../fabro-workflows/src/operations/fork.rs | 4 +- .../fabro-workflows/src/operations/start.rs | 18 +-- .../fabro-workflows/src/pipeline/execute.rs | 2 +- .../src/pipeline/execute/tests.rs | 4 +- .../fabro-workflows/src/pipeline/finalize.rs | 2 +- .../src/pipeline/initialize.rs | 6 +- lib/crates/fabro-workflows/src/run_options.rs | 6 +- .../fabro-workflows/src/transforms/import.rs | 8 +- .../tests/it/attractor_compat.rs | 2 + .../tests/it/cp_integration.rs | 2 + .../tests/it/daytona_integration.rs | 8 ++ .../fabro-workflows/tests/it/integration.rs | 12 ++ 107 files changed, 664 insertions(+), 585 deletions(-) diff --git a/lib/crates/fabro-agent/src/compaction.rs b/lib/crates/fabro-agent/src/compaction.rs index a632a4644..14d630aaa 100644 --- a/lib/crates/fabro-agent/src/compaction.rs +++ b/lib/crates/fabro-agent/src/compaction.rs @@ -253,6 +253,8 @@ mod tests { use super::*; use crate::event::EventEmitter; use crate::history::History; + use crate::test_support::TestProfile; + use crate::tool_registry::ToolRegistry; use crate::types::Turn; use fabro_llm::types::{ToolCall, ToolResult, Usage}; use std::time::SystemTime; @@ -330,7 +332,7 @@ mod tests { fn check_context_usage_below_threshold() { let history = History::default(); let emitter = EventEmitter::new(); - let profile = crate::test_support::TestProfile::new(); + let profile = TestProfile::new(); // Empty history, huge context window => well below threshold let over = check_context_usage("short", &history, &profile, 80, &emitter, "sess"); assert!(!over); @@ -347,10 +349,7 @@ mod tests { let emitter = EventEmitter::new(); let mut rx = emitter.subscribe(); // TestProfile has context_window=200_000 by default; use a small one - let profile = crate::test_support::TestProfile::with_context_window( - crate::tool_registry::ToolRegistry::new(), - 100, - ); + let profile = TestProfile::with_context_window(ToolRegistry::new(), 100); let over = check_context_usage("prompt", &history, &profile, 80, &emitter, "sess"); assert!(over); diff --git a/lib/crates/fabro-agent/src/event.rs b/lib/crates/fabro-agent/src/event.rs index 348df9b67..95a00931d 100644 --- a/lib/crates/fabro-agent/src/event.rs +++ b/lib/crates/fabro-agent/src/event.rs @@ -40,6 +40,7 @@ impl Default for EventEmitter { #[cfg(test)] mod tests { use super::*; + use crate::error::AgentError; #[tokio::test] async fn emit_and_receive_event() { @@ -61,7 +62,7 @@ mod tests { emitter.emit( "sess-2".into(), AgentEvent::Error { - error: crate::error::AgentError::ToolExecution("something went wrong".into()), + error: AgentError::ToolExecution("something went wrong".into()), }, ); @@ -93,7 +94,7 @@ mod tests { emitter.emit( "sess-4".into(), AgentEvent::Error { - error: crate::error::AgentError::ToolExecution("test".into()), + error: AgentError::ToolExecution("test".into()), }, ); } diff --git a/lib/crates/fabro-agent/src/history.rs b/lib/crates/fabro-agent/src/history.rs index b9de2706f..7ed9f928f 100644 --- a/lib/crates/fabro-agent/src/history.rs +++ b/lib/crates/fabro-agent/src/history.rs @@ -136,7 +136,7 @@ fn extract_recent_user_messages(discarded: Vec, token_budget: usize) -> Ve #[cfg(test)] mod tests { use super::*; - use fabro_llm::types::{ToolCall, ToolResult, Usage}; + use fabro_llm::types::{ThinkingData, ToolCall, ToolResult, Usage}; use std::time::SystemTime; #[test] @@ -266,7 +266,7 @@ mod tests { #[test] fn assistant_turn_with_reasoning_in_provider_parts() { let mut history = History::default(); - let thinking = ContentPart::Thinking(fabro_llm::types::ThinkingData { + let thinking = ContentPart::Thinking(ThinkingData { text: "Let me think about this...".into(), signature: None, redacted: false, @@ -291,7 +291,7 @@ mod tests { #[test] fn thinking_with_signature_preserved_via_provider_parts() { let mut history = History::default(); - let thinking = ContentPart::Thinking(fabro_llm::types::ThinkingData { + let thinking = ContentPart::Thinking(ThinkingData { text: "Let me think...".into(), signature: Some("sig_abc123".into()), redacted: false, @@ -418,7 +418,7 @@ mod tests { "shell", serde_json::json!({"cmd": "ls"}), )], - provider_parts: vec![ContentPart::Thinking(fabro_llm::types::ThinkingData { + provider_parts: vec![ContentPart::Thinking(ThinkingData { text: "thinking...".into(), signature: None, redacted: false, @@ -505,7 +505,7 @@ mod tests { content: "recent msg".into(), timestamp: SystemTime::now(), }); - let thinking = ContentPart::Thinking(fabro_llm::types::ThinkingData { + let thinking = ContentPart::Thinking(ThinkingData { text: "deep thought".into(), signature: Some("sig_xyz".into()), redacted: false, diff --git a/lib/crates/fabro-agent/src/mcp_integration.rs b/lib/crates/fabro-agent/src/mcp_integration.rs index 375d1d1d1..33c97d173 100644 --- a/lib/crates/fabro-agent/src/mcp_integration.rs +++ b/lib/crates/fabro-agent/src/mcp_integration.rs @@ -41,9 +41,13 @@ pub fn make_mcp_tools(manager: &Arc) -> Vec McpServerConfig { let test_server = format!( @@ -82,11 +86,6 @@ mod tests { let tools = make_mcp_tools(&Arc::new(mgr)); let tool = &tools[0]; - use crate::sandbox::Sandbox; - use crate::test_support::MockSandbox; - use crate::tool_registry::ToolContext; - use tokio_util::sync::CancellationToken; - let env: Arc = Arc::new(MockSandbox::default()); let result = (tool.executor)( serde_json::json!({"message": "test message"}), diff --git a/lib/crates/fabro-agent/src/profiles/anthropic.rs b/lib/crates/fabro-agent/src/profiles/anthropic.rs index ddb0152a5..3f44aa01c 100644 --- a/lib/crates/fabro-agent/src/profiles/anthropic.rs +++ b/lib/crates/fabro-agent/src/profiles/anthropic.rs @@ -167,7 +167,10 @@ in the project. Keep changes minimal and focused on the task."; #[cfg(test)] mod tests { use super::*; + use crate::subagent::{SessionFactory, SubAgentManager}; use crate::test_support::MockSandbox; + use std::sync::Arc; + use tokio::sync::Mutex as AsyncMutex; #[test] fn anthropic_profile_identity() { @@ -291,13 +294,10 @@ mod tests { #[test] fn anthropic_register_subagent_tools() { - use crate::subagent::{SessionFactory, SubAgentManager}; - use std::sync::Arc; - let mut profile = AnthropicProfile::new("claude-sonnet-4-20250514"); assert_eq!(profile.tool_registry().names().len(), 8); - let manager = Arc::new(tokio::sync::Mutex::new(SubAgentManager::new(3))); + let manager = Arc::new(AsyncMutex::new(SubAgentManager::new(3))); let factory: SessionFactory = Arc::new(|| { panic!("should not be called in test"); }); diff --git a/lib/crates/fabro-agent/src/profiles/gemini.rs b/lib/crates/fabro-agent/src/profiles/gemini.rs index 0dd2e652e..91691d402 100644 --- a/lib/crates/fabro-agent/src/profiles/gemini.rs +++ b/lib/crates/fabro-agent/src/profiles/gemini.rs @@ -202,8 +202,10 @@ in the project."; #[cfg(test)] mod tests { use super::*; + use crate::subagent::{SessionFactory, SubAgentManager}; use crate::test_support::MockSandbox; use std::sync::Arc; + use tokio::sync::Mutex as AsyncMutex; #[test] fn gemini_profile_identity() { @@ -296,10 +298,8 @@ mod tests { #[test] fn gemini_subagent_tools_registered() { let mut profile = GeminiProfile::new("gemini-2.0-flash"); - let manager = Arc::new(tokio::sync::Mutex::new( - crate::subagent::SubAgentManager::new(3), - )); - let factory: crate::subagent::SessionFactory = Arc::new(|| { + let manager = Arc::new(AsyncMutex::new(SubAgentManager::new(3))); + let factory: SessionFactory = Arc::new(|| { panic!("should not be called"); }); profile.register_subagent_tools(manager, factory, 0); diff --git a/lib/crates/fabro-agent/src/profiles/openai.rs b/lib/crates/fabro-agent/src/profiles/openai.rs index 8113ea4ea..c3f71f2a7 100644 --- a/lib/crates/fabro-agent/src/profiles/openai.rs +++ b/lib/crates/fabro-agent/src/profiles/openai.rs @@ -200,7 +200,10 @@ in the project."); #[cfg(test)] mod tests { use super::*; + use crate::subagent::{SessionFactory, SubAgentManager}; use crate::test_support::MockSandbox; + use std::sync::Arc; + use tokio::sync::Mutex as AsyncMutex; #[test] fn openai_profile_identity() { @@ -271,14 +274,10 @@ mod tests { #[test] fn openai_subagent_tools_registered() { - use crate::subagent::SessionFactory; - use crate::subagent::SubAgentManager; - use std::sync::Arc; - let mut profile = OpenAiProfile::new("o3-mini"); assert_eq!(profile.tool_registry().names().len(), 8); - let manager = Arc::new(tokio::sync::Mutex::new(SubAgentManager::new(3))); + let manager = Arc::new(AsyncMutex::new(SubAgentManager::new(3))); let factory: SessionFactory = Arc::new(|| panic!("should not be called in test")); profile.register_subagent_tools(manager, factory, 0); assert_eq!(profile.tool_registry().names().len(), 12); diff --git a/lib/crates/fabro-agent/src/session.rs b/lib/crates/fabro-agent/src/session.rs index 934c7b15e..16b773fa2 100644 --- a/lib/crates/fabro-agent/src/session.rs +++ b/lib/crates/fabro-agent/src/session.rs @@ -981,17 +981,22 @@ const fn is_auth_error(err: &SdkError) -> bool { #[cfg(test)] mod tests { use super::*; + use crate::config::ToolApprovalAdapter; + use crate::subagent::SubAgentStatus; use crate::test_support::*; use crate::tool_registry::{RegisteredTool, ToolRegistry}; use fabro_llm::error::{ProviderErrorDetail, ProviderErrorKind}; use fabro_llm::provider::{ProviderAdapter, StreamEventStream}; - use fabro_llm::types::{Request, Response, Role, StreamEvent, ToolDefinition}; + use fabro_llm::types::{ + ContentPart, ReasoningEffort, Request, Response, Role, StreamEvent, ToolDefinition, + }; + use futures::stream; use std::sync::Arc; use std::sync::atomic::{AtomicUsize, Ordering}; #[derive(Clone)] enum ScriptedStreamCall { - Response(Response), + Response(Box), Events(Vec>), Error(SdkError), } @@ -1021,7 +1026,7 @@ mod tests { } for part in &response.message.content { - if let fabro_llm::types::ContentPart::ToolCall(tool_call) = part { + if let ContentPart::ToolCall(tool_call) = part { events.push(Ok(StreamEvent::ToolCallEnd { tool_call: tool_call.clone(), })); @@ -1059,10 +1064,10 @@ mod tests { }; match scripted { - ScriptedStreamCall::Response(response) => Ok(Box::pin(futures::stream::iter( - Self::events_for_response(response), - ))), - ScriptedStreamCall::Events(events) => Ok(Box::pin(futures::stream::iter(events))), + ScriptedStreamCall::Response(response) => { + Ok(Box::pin(stream::iter(Self::events_for_response(*response)))) + } + ScriptedStreamCall::Events(events) => Ok(Box::pin(stream::iter(events))), ScriptedStreamCall::Error(err) => Err(err), } } @@ -1074,7 +1079,7 @@ mod tests { async fn make_session_with_provider_and_manager( provider: Arc, - subagent_manager: Option>>, + subagent_manager: Option>>, ) -> Session { let client = make_client(provider).await; let profile = Arc::new(TestProfile::new()); @@ -1643,17 +1648,14 @@ mod tests { let mut session = Session::new(client, profile, env, SessionConfig::default(), None); // Default reasoning_effort is None - session.set_reasoning_effort(Some(fabro_llm::types::ReasoningEffort::High)); + session.set_reasoning_effort(Some(ReasoningEffort::High)); session.process_input("test").await.unwrap(); let captured = provider_ref.captured_request.lock().unwrap(); let request = captured .as_ref() .expect("request should have been captured"); - assert_eq!( - request.reasoning_effort, - Some(fabro_llm::types::ReasoningEffort::High) - ); + assert_eq!(request.reasoning_effort, Some(ReasoningEffort::High)); } #[tokio::test] @@ -1856,9 +1858,9 @@ mod tests { ]; let config = SessionConfig { - tool_hooks: Some(Arc::new(crate::config::ToolApprovalAdapter(Arc::new( - |_name, _args| Err("denied by policy".to_string()), - )))), + tool_hooks: Some(Arc::new(ToolApprovalAdapter(Arc::new(|_name, _args| { + Err("denied by policy".to_string()) + })))), ..Default::default() }; @@ -1897,9 +1899,9 @@ mod tests { ]; let config = SessionConfig { - tool_hooks: Some(Arc::new(crate::config::ToolApprovalAdapter(Arc::new( - |_name, _args| Ok(()), - )))), + tool_hooks: Some(Arc::new(ToolApprovalAdapter(Arc::new(|_name, _args| { + Ok(()) + })))), ..Default::default() }; @@ -1933,7 +1935,7 @@ mod tests { ]; let config = SessionConfig { - tool_hooks: Some(Arc::new(crate::config::ToolApprovalAdapter(Arc::new( + tool_hooks: Some(Arc::new(ToolApprovalAdapter(Arc::new( move |name, args| { *captured_clone.lock().unwrap() = Some((name.to_string(), args.clone())); Ok(()) @@ -1995,9 +1997,9 @@ mod tests { ]; let config = SessionConfig { - tool_hooks: Some(Arc::new(crate::config::ToolApprovalAdapter(Arc::new( - |_name, _args| Err("not allowed".to_string()), - )))), + tool_hooks: Some(Arc::new(ToolApprovalAdapter(Arc::new(|_name, _args| { + Err("not allowed".to_string()) + })))), ..Default::default() }; @@ -2068,7 +2070,7 @@ mod tests { async fn stream_retries_when_stream_ends_without_finish_before_any_deltas() { let provider = Arc::new(ScriptedStreamProvider::new(vec![ ScriptedStreamCall::Events(vec![]), - ScriptedStreamCall::Response(text_response("Recovered")), + ScriptedStreamCall::Response(Box::new(text_response("Recovered"))), ])); let mut session = make_session_with_provider(provider.clone()).await; let mut rx = session.subscribe(); @@ -2106,7 +2108,7 @@ mod tests { async fn stream_retries_with_output_replace_after_partial_text() { let provider = Arc::new(ScriptedStreamProvider::new(vec![ ScriptedStreamCall::Events(vec![Ok(StreamEvent::text_delta("Hel", None))]), - ScriptedStreamCall::Response(text_response("Hello")), + ScriptedStreamCall::Response(Box::new(text_response("Hello"))), ])); let mut session = make_session_with_provider(provider.clone()).await; let mut rx = session.subscribe(); @@ -2329,7 +2331,7 @@ mod tests { events.push(Ok(StreamEvent::text_delta(text, None))); } for part in &response.message.content { - if let fabro_llm::types::ContentPart::ToolCall(tc) = part { + if let ContentPart::ToolCall(tc) = part { events.push(Ok(StreamEvent::ToolCallEnd { tool_call: tc.clone(), })); @@ -2340,7 +2342,7 @@ mod tests { response.usage.clone(), response, ))); - Ok(Box::pin(futures::stream::iter(events))) + Ok(Box::pin(stream::iter(events))) } } @@ -2414,7 +2416,7 @@ mod tests { } else { self.stream_responses[self.stream_responses.len() - 1].clone() }; - Ok(crate::test_support::response_to_stream(response)) + Ok(response_to_stream(response)) } } @@ -2556,8 +2558,8 @@ mod tests { let provider = Arc::new(MockLlmProvider::new(responses)); let client = make_client(provider).await; - let profile: Arc = Arc::new(TestProfile::new()); - let env: Arc = Arc::new(MockSandbox::default()); + let profile: Arc = Arc::new(TestProfile::new()); + let env: Arc = Arc::new(MockSandbox::default()); let mut session = Session::new(client, profile, env, config, None); // Subscribe to events before initialize @@ -2706,10 +2708,10 @@ mod tests { async fn close_cleans_up_subagents_before_emitting_session_ended() { use crate::subagent::SubAgentManager; - let manager = Arc::new(tokio::sync::Mutex::new(SubAgentManager::new(3))); + let manager = Arc::new(AsyncMutex::new(SubAgentManager::new(3))); let provider = Arc::new(ScriptedStreamProvider::new(vec![ - ScriptedStreamCall::Response(text_response("done")), + ScriptedStreamCall::Response(Box::new(text_response("done"))), ])); let mut session = make_session_with_provider_and_manager(provider, Some(manager.clone())).await; @@ -2731,7 +2733,7 @@ mod tests { // The subagent should have been closed assert!(matches!( manager.lock().await.status(&agent_id), - Some(crate::subagent::SubAgentStatus::Closed) + Some(SubAgentStatus::Closed) )); // Verify event ordering: SubAgentClosed before SessionEnded diff --git a/lib/crates/fabro-agent/src/skills.rs b/lib/crates/fabro-agent/src/skills.rs index e8fe1c257..062140721 100644 --- a/lib/crates/fabro-agent/src/skills.rs +++ b/lib/crates/fabro-agent/src/skills.rs @@ -248,8 +248,11 @@ pub async fn discover_skills(env: &dyn Sandbox, dirs: &[String]) -> Vec { #[cfg(test)] mod tests { use super::*; + use crate::sandbox::Sandbox; use crate::test_support::MockSandbox; + use crate::tool_registry::ToolContext; use std::collections::HashMap; + use tokio_util::sync::CancellationToken; // --- parse_skill tests --- @@ -538,11 +541,11 @@ name: trimmed let skills = Arc::new(test_skills()); let tool = make_use_skill_tool(skills); - let env: Arc = Arc::new(MockSandbox::default()); + let env: Arc = Arc::new(MockSandbox::default()); let args = serde_json::json!({"skill_name": "commit"}); - let ctx = crate::tool_registry::ToolContext { + let ctx = ToolContext { env, - cancel: tokio_util::sync::CancellationToken::new(), + cancel: CancellationToken::new(), tool_env: None, }; let result = (tool.executor)(args, ctx).await; @@ -557,11 +560,11 @@ name: trimmed let skills = Arc::new(test_skills()); let tool = make_use_skill_tool(skills); - let env: Arc = Arc::new(MockSandbox::default()); + let env: Arc = Arc::new(MockSandbox::default()); let args = serde_json::json!({"skill_name": "nonexistent"}); - let ctx = crate::tool_registry::ToolContext { + let ctx = ToolContext { env, - cancel: tokio_util::sync::CancellationToken::new(), + cancel: CancellationToken::new(), tool_env: None, }; let result = (tool.executor)(args, ctx).await; @@ -574,11 +577,11 @@ name: trimmed let skills = Arc::new(test_skills()); let tool = make_use_skill_tool(skills); - let env: Arc = Arc::new(MockSandbox::default()); + let env: Arc = Arc::new(MockSandbox::default()); let args = serde_json::json!({}); - let ctx = crate::tool_registry::ToolContext { + let ctx = ToolContext { env, - cancel: tokio_util::sync::CancellationToken::new(), + cancel: CancellationToken::new(), tool_env: None, }; let result = (tool.executor)(args, ctx).await; diff --git a/lib/crates/fabro-agent/src/subagent.rs b/lib/crates/fabro-agent/src/subagent.rs index cab3e6669..b3562b06c 100644 --- a/lib/crates/fabro-agent/src/subagent.rs +++ b/lib/crates/fabro-agent/src/subagent.rs @@ -463,6 +463,7 @@ mod tests { use crate::test_support::*; use fabro_llm::provider::ProviderAdapter; use fabro_llm::types::Role; + use tokio::time; // --- Tests --- @@ -579,7 +580,7 @@ mod tests { #[test] fn tool_definitions_correct() { - let manager = Arc::new(tokio::sync::Mutex::new(SubAgentManager::new(3))); + let manager = Arc::new(AsyncMutex::new(SubAgentManager::new(3))); let factory: SessionFactory = Arc::new(|| { panic!("should not be called"); }); @@ -696,7 +697,7 @@ mod tests { let _result = manager.wait(&agent_id).await.unwrap(); // Give the forwarding task a moment to process remaining events - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + time::sleep(std::time::Duration::from_millis(50)).await; let captured = events.lock().unwrap(); let forwarded_count = captured diff --git a/lib/crates/fabro-agent/src/test_support.rs b/lib/crates/fabro-agent/src/test_support.rs index 62b814bcd..95d2f832a 100644 --- a/lib/crates/fabro-agent/src/test_support.rs +++ b/lib/crates/fabro-agent/src/test_support.rs @@ -5,14 +5,15 @@ use crate::config::SessionConfig; use crate::profiles::EnvContext; use crate::sandbox::*; use crate::session::Session; -use crate::skills::Skill; -use crate::tool_registry::ToolRegistry; +use crate::skills::{Skill, format_skills_prompt_section}; +use crate::tool_registry::{RegisteredTool, ToolRegistry}; use async_trait::async_trait; use fabro_llm::client::Client; use fabro_llm::error::SdkError; use fabro_llm::provider::{ProviderAdapter, StreamEventStream}; use fabro_llm::types::{ContentPart, FinishReason, Message, Request, Response, StreamEvent, Usage}; use fabro_model::Provider; +use futures::stream; use std::collections::HashMap; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; @@ -72,7 +73,7 @@ impl AgentProfile for TestProfile { user_instructions: Option<&str>, skills: &[Skill], ) -> String { - let skills_section = crate::skills::format_skills_prompt_section(skills); + let skills_section = format_skills_prompt_section(skills); let skills_part = if skills_section.is_empty() { String::new() } else { @@ -159,7 +160,7 @@ pub fn response_to_stream(response: Response) -> StreamEventStream { response, ))); - Box::pin(futures::stream::iter(events)) + Box::pin(stream::iter(events)) } // --- Helper functions --- @@ -259,9 +260,9 @@ pub fn tool_call_response( } } -pub fn make_echo_tool() -> crate::tool_registry::RegisteredTool { +pub fn make_echo_tool() -> RegisteredTool { use fabro_llm::types::ToolDefinition; - crate::tool_registry::RegisteredTool { + RegisteredTool { definition: ToolDefinition { name: "echo".into(), description: "Echoes the input".into(), @@ -279,9 +280,9 @@ pub fn make_echo_tool() -> crate::tool_registry::RegisteredTool { } } -pub fn make_error_tool() -> crate::tool_registry::RegisteredTool { +pub fn make_error_tool() -> RegisteredTool { use fabro_llm::types::ToolDefinition; - crate::tool_registry::RegisteredTool { + RegisteredTool { definition: ToolDefinition { name: "fail_tool".into(), description: "Always fails".into(), @@ -375,18 +376,18 @@ impl ProviderAdapter for MockMidStreamErrorProvider { Ok(StreamEvent::text_delta(self.partial_text.clone(), None)), Err(self.error.clone()), ]; - Ok(Box::pin(futures::stream::iter(events))) + Ok(Box::pin(stream::iter(events))) } } pub fn multi_tool_call_response(calls: Vec<(&str, &str, serde_json::Value)>) -> Response { use fabro_llm::types::{ContentPart, Role, ToolCall}; let mut content = vec![ContentPart::text("Let me use multiple tools.")]; - for (tool_name, tool_call_id, args) in &calls { + for (tool_name, tool_call_id, args) in calls { content.push(ContentPart::ToolCall(ToolCall::new( - *tool_call_id, - *tool_name, - args.clone(), + tool_call_id, + tool_name, + args, ))); } Response { diff --git a/lib/crates/fabro-agent/src/tool_execution.rs b/lib/crates/fabro-agent/src/tool_execution.rs index 13de03cdb..b79184625 100644 --- a/lib/crates/fabro-agent/src/tool_execution.rs +++ b/lib/crates/fabro-agent/src/tool_execution.rs @@ -346,7 +346,13 @@ mod tests { use super::*; use crate::config::{ToolHookCallback, ToolHookDecision}; use crate::event::EventEmitter; + use crate::local_sandbox::LocalSandbox; + use crate::read_before_write_sandbox::ReadBeforeWriteSandbox; + use crate::test_support::MutableMockSandbox; use crate::tool_registry::{RegisteredTool, ToolContext, ToolRegistry}; + use crate::tools::{ + make_edit_file_tool, make_grep_tool, make_read_file_tool, make_write_file_tool, + }; use fabro_llm::types::{ToolCall, ToolDefinition}; use std::sync::Mutex; @@ -440,9 +446,7 @@ mod tests { } fn make_sandbox() -> Arc { - Arc::new(crate::local_sandbox::LocalSandbox::new( - std::env::current_dir().unwrap(), - )) + Arc::new(LocalSandbox::new(std::env::current_dir().unwrap())) } #[tokio::test] @@ -607,17 +611,15 @@ mod tests { // --- ReadBeforeWriteSandbox e2e tests --- fn make_guarded_sandbox(files: HashMap) -> Arc { - Arc::new( - crate::read_before_write_sandbox::ReadBeforeWriteSandbox::new(Arc::new( - crate::test_support::MutableMockSandbox::new(files), - )), - ) + Arc::new(ReadBeforeWriteSandbox::new(Arc::new( + MutableMockSandbox::new(files), + ))) } #[tokio::test] async fn write_to_unread_file_blocked() { let mut registry = ToolRegistry::new(); - registry.register(crate::tools::make_write_file_tool()); + registry.register(make_write_file_tool()); let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); let tc = make_tool_call( @@ -648,8 +650,8 @@ mod tests { #[tokio::test] async fn read_then_write_succeeds() { let mut registry = ToolRegistry::new(); - registry.register(crate::tools::make_read_file_tool()); - registry.register(crate::tools::make_write_file_tool()); + registry.register(make_read_file_tool()); + registry.register(make_write_file_tool()); let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); let emitter = EventEmitter::new(); @@ -700,8 +702,8 @@ mod tests { #[tokio::test] async fn grep_then_write_succeeds() { let mut registry = ToolRegistry::new(); - registry.register(crate::tools::make_grep_tool()); - registry.register(crate::tools::make_write_file_tool()); + registry.register(make_grep_tool()); + registry.register(make_write_file_tool()); let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); let emitter = EventEmitter::new(); @@ -748,7 +750,7 @@ mod tests { #[tokio::test] async fn edit_unread_file_blocked() { let mut registry = ToolRegistry::new(); - registry.register(crate::tools::make_edit_file_tool()); + registry.register(make_edit_file_tool()); let sandbox = make_guarded_sandbox(HashMap::from([("a.ts".into(), "content".into())])); let tc = make_tool_call( @@ -779,7 +781,7 @@ mod tests { #[tokio::test] async fn write_new_file_succeeds() { let mut registry = ToolRegistry::new(); - registry.register(crate::tools::make_write_file_tool()); + registry.register(make_write_file_tool()); let sandbox = make_guarded_sandbox(HashMap::new()); let tc = make_tool_call( diff --git a/lib/crates/fabro-agent/src/tool_registry.rs b/lib/crates/fabro-agent/src/tool_registry.rs index 843639e4b..79809a406 100644 --- a/lib/crates/fabro-agent/src/tool_registry.rs +++ b/lib/crates/fabro-agent/src/tool_registry.rs @@ -72,6 +72,8 @@ impl Default for ToolRegistry { #[cfg(test)] mod tests { use super::*; + use crate::sandbox::Sandbox; + use crate::test_support::MockSandbox; fn make_tool(name: &str) -> RegisteredTool { RegisteredTool { @@ -171,10 +173,6 @@ mod tests { let tool = registry.get("echo").unwrap(); - use super::ToolContext; - use crate::sandbox::Sandbox; - use crate::test_support::MockSandbox; - let env: Arc = Arc::new(MockSandbox::default()); let ctx = ToolContext { env, diff --git a/lib/crates/fabro-agent/src/tools.rs b/lib/crates/fabro-agent/src/tools.rs index ecdbf067c..acc885ea2 100644 --- a/lib/crates/fabro-agent/src/tools.rs +++ b/lib/crates/fabro-agent/src/tools.rs @@ -620,6 +620,7 @@ mod tests { use crate::sandbox::*; use crate::test_support::MockSandbox; use crate::tool_registry::ToolContext; + use fabro_llm::provider::ProviderAdapter; use std::collections::HashMap; use tokio_util::sync::CancellationToken; @@ -1169,7 +1170,7 @@ mod tests { let env = Arc::new(MockSandbox::default()); let env_clone: Arc = env.clone(); let _result = (tool.executor)( - serde_json::json!({"url": "https://example.com", "timeout_ms": 120000}), + serde_json::json!({"url": "https://example.com", "timeout_ms": 120_000}), ToolContext { env: env_clone, cancel: CancellationToken::new(), @@ -1329,20 +1330,20 @@ mod tests { use fabro_llm::error::{ProviderErrorDetail, ProviderErrorKind, SdkError}; // "other_provider" is the default — it rejects all requests. - let default_provider: Arc = - Arc::new(MockErrorProvider { - error: SdkError::Provider { - kind: ProviderErrorKind::NotFound, - detail: Box::new(ProviderErrorDetail::new( - "model not found", - "other_provider", - )), - }, - }); + let default_provider: Arc = Arc::new(MockErrorProvider { + error: SdkError::Provider { + kind: ProviderErrorKind::NotFound, + detail: Box::new(ProviderErrorDetail::new( + "model not found", + "other_provider", + )), + }, + }); // "anthropic" provider has the model we actually want. - let target_provider: Arc = Arc::new( - MockLlmProvider::new(vec![text_response("summarized content")]), - ); + let target_provider: Arc = + Arc::new(MockLlmProvider::new(vec![text_response( + "summarized content", + )])); let mut providers = HashMap::new(); providers.insert("other_provider".to_string(), default_provider); @@ -1414,7 +1415,7 @@ mod tests { } #[tokio::test] - #[ignore] // Requires BRAVE_SEARCH_API_KEY env var + #[ignore = "requires BRAVE_SEARCH_API_KEY env var"] async fn web_search_returns_results() { let api_key = std::env::var("BRAVE_SEARCH_API_KEY") .expect("BRAVE_SEARCH_API_KEY must be set to run this test"); diff --git a/lib/crates/fabro-agent/src/types.rs b/lib/crates/fabro-agent/src/types.rs index 9c8c6de74..17a2b49d9 100644 --- a/lib/crates/fabro-agent/src/types.rs +++ b/lib/crates/fabro-agent/src/types.rs @@ -561,8 +561,8 @@ mod tests { assert!(json.contains("sess_42")); assert!(json.contains("SessionStarted")); // Timestamp should be ISO-8601 - assert!(json.contains("T")); - assert!(json.contains("Z")); + assert!(json.contains('T')); + assert!(json.contains('Z')); let deserialized: SessionEvent = serde_json::from_str(&json).unwrap(); assert_eq!(deserialized.session_id, "sess_42"); @@ -672,7 +672,7 @@ mod tests { #[test] fn error_event_serde_roundtrip_with_agent_error() { let event = AgentEvent::Error { - error: AgentError::Llm(fabro_llm::error::SdkError::Network { + error: AgentError::Llm(SdkError::Network { message: "refused".into(), source: None, }), @@ -695,7 +695,7 @@ mod tests { model: "gpt-4".into(), attempt: 1, delay_secs: 2.0, - error: fabro_llm::error::SdkError::Provider { + error: SdkError::Provider { kind: ProviderErrorKind::RateLimit, detail: Box::new(ProviderErrorDetail { message: "too fast".into(), diff --git a/lib/crates/fabro-agent/src/v4a_patch.rs b/lib/crates/fabro-agent/src/v4a_patch.rs index f38e6098e..167c6a7ba 100644 --- a/lib/crates/fabro-agent/src/v4a_patch.rs +++ b/lib/crates/fabro-agent/src/v4a_patch.rs @@ -842,9 +842,7 @@ mod tests { #[test] fn format_patch_error_truncates_large_files() { - let lines: Vec = (1..=1_000) - .map(|i| format!("line number {:04}", i)) - .collect(); + let lines: Vec = (1..=1_000).map(|i| format!("line number {i:04}")).collect(); let content = lines.join("\n"); let result = format_patch_error("some error", "big.txt", &content); assert!(result.len() < 10_000); diff --git a/lib/crates/fabro-agent/tests/it/guardrails.rs b/lib/crates/fabro-agent/tests/it/guardrails.rs index b676128b4..5c2c89723 100644 --- a/lib/crates/fabro-agent/tests/it/guardrails.rs +++ b/lib/crates/fabro-agent/tests/it/guardrails.rs @@ -7,8 +7,10 @@ fn profile_context_window_matches_catalog_for_default_models() { let catalog_info = Catalog::builtin() .default_for_provider(provider) .cloned() - .unwrap_or_else(|| panic!("no default model for {:?} in catalog", provider)); + .unwrap_or_else(|| panic!("no default model for {provider:?} in catalog")); let model = &catalog_info.id; + let context_window = usize::try_from(catalog_info.context_window()) + .expect("catalog context window should be non-negative and fit in usize"); let profile: Box = match provider { Provider::OpenAi => Box::new(OpenAiProfile::new(model)), @@ -25,12 +27,12 @@ fn profile_context_window_matches_catalog_for_default_models() { assert_eq!( profile.context_window_size(), - catalog_info.context_window() as usize, + context_window, "context_window_size mismatch for {:?} model '{}': profile={} catalog={}", provider, model, profile.context_window_size(), - catalog_info.context_window() as usize + context_window ); } } diff --git a/lib/crates/fabro-agent/tests/it/parity_matrix.rs b/lib/crates/fabro-agent/tests/it/parity_matrix.rs index 270a78ca2..06ee6aca7 100644 --- a/lib/crates/fabro-agent/tests/it/parity_matrix.rs +++ b/lib/crates/fabro-agent/tests/it/parity_matrix.rs @@ -1,6 +1,8 @@ +use std::fmt::Write as _; use std::path::Path; use std::sync::Arc; +use fabro_agent::subagent::SessionFactory; use fabro_agent::{ AgentProfile, AnthropicProfile, GeminiProfile, LocalSandbox, OpenAiProfile, Session, SessionConfig, SubAgentManager, WebFetchSummarizer, @@ -8,6 +10,7 @@ use fabro_agent::{ use fabro_llm::client::Client; use fabro_llm::provider::Provider; use fabro_model::ModelRef; +use tokio::sync::Mutex as AsyncMutex; fn summarizer_model_id(provider: Provider) -> ModelRef { match provider { @@ -61,11 +64,11 @@ async fn make_session(provider: Provider, model: &str, cwd: &Path) -> Session { let env = Arc::new(LocalSandbox::new(cwd.to_path_buf())); // Register subagent tools so spawn_agent / wait / send_input / close_agent are available - let manager = Arc::new(tokio::sync::Mutex::new(SubAgentManager::new(3))); + let manager = Arc::new(AsyncMutex::new(SubAgentManager::new(3))); let factory_client = client.clone(); let factory_model: String = model.to_string(); let factory_cwd = cwd.to_path_buf(); - let factory: fabro_agent::subagent::SessionFactory = Arc::new(move || { + let factory: SessionFactory = Arc::new(move || { let sub_profile: Arc = { let summarizer = Some(build_summarizer(provider, &factory_client)); match provider { @@ -317,7 +320,10 @@ async fn scenario_multi_step_read_analyze_edit(session: &mut Session, dir: &Path // Scenario 8: tool_output_truncation // --------------------------------------------------------------------------- async fn scenario_tool_output_truncation(session: &mut Session, dir: &Path) { - let lines: String = (1..=10_000).map(|n| format!("line {n}\n")).collect(); + let lines = (1..=10_000).fold(String::new(), |mut acc, n| { + let _ = writeln!(acc, "line {n}"); + acc + }); std::fs::write(dir.join("big.txt"), lines).expect("failed to write big.txt"); session .process_input("Read the file big.txt and tell me how many lines it has") diff --git a/lib/crates/fabro-cli/src/commands/doctor.rs b/lib/crates/fabro-cli/src/commands/doctor.rs index c9a8a7adc..3b3f38f3a 100644 --- a/lib/crates/fabro-cli/src/commands/doctor.rs +++ b/lib/crates/fabro-cli/src/commands/doctor.rs @@ -8,6 +8,7 @@ use base64::Engine as _; use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; #[cfg(feature = "server")] use fabro_config::server::{ApiAuthStrategy, AuthProvider}; +use fabro_config::user::{default_user_config_path, legacy_user_config_path}; use fabro_llm::client::Client as LlmClient; use fabro_llm::types::{Message, Request}; use fabro_model::{Catalog, Provider}; @@ -963,9 +964,9 @@ pub(crate) async fn run_doctor(verbose: bool, live: bool) -> i32 { // Gather state let cli_settings = load_user_settings().unwrap_or_default(); - let user_config_path = fabro_config::user::default_user_config_path(); + let user_config_path = default_user_config_path(); let user_config_exists = user_config_path.as_ref().is_some_and(|p| p.exists()); - let legacy_config_path = fabro_config::user::legacy_user_config_path(); + let legacy_config_path = legacy_user_config_path(); let legacy_config_exists = legacy_config_path.as_ref().is_some_and(|p| p.exists()); let llm_statuses: Vec<(Provider, bool)> = Provider::ALL diff --git a/lib/crates/fabro-cli/src/commands/install.rs b/lib/crates/fabro-cli/src/commands/install.rs index 292bdcf71..231f762ab 100644 --- a/lib/crates/fabro-cli/src/commands/install.rs +++ b/lib/crates/fabro-cli/src/commands/install.rs @@ -13,6 +13,7 @@ use base64::engine::general_purpose::STANDARD as BASE64_STANDARD; use dialoguer::console::Term; use dialoguer::theme::ColorfulTheme; use dialoguer::{MultiSelect, Select}; +use fabro_config::user::USER_CONFIG_FILENAME; use fabro_model::Provider; use fabro_util::terminal::Styles; use rand::Rng; @@ -440,7 +441,7 @@ async fn setup_github_app( .to_string(); // Write non-secret config to user.toml - let user_toml_path = arc_dir.join(fabro_config::user::USER_CONFIG_FILENAME); + let user_toml_path = arc_dir.join(USER_CONFIG_FILENAME); let existing = std::fs::read_to_string(&user_toml_path).unwrap_or_default(); let mut doc: toml::Value = if existing.is_empty() { toml::Value::Table(toml::Table::default()) @@ -652,7 +653,7 @@ pub(crate) async fn run_install(web_url: &str) -> Result<()> { if setup_github { let github_env_pairs = setup_github_app(&arc_dir, &s, web_url).await?; let slug = { - let user_toml_path = arc_dir.join(fabro_config::user::USER_CONFIG_FILENAME); + let user_toml_path = arc_dir.join(USER_CONFIG_FILENAME); let toml_content = std::fs::read_to_string(&user_toml_path).unwrap_or_default(); let doc: toml::Value = toml::from_str(&toml_content) .unwrap_or(toml::Value::Table(toml::Table::default())); diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 391c54cd5..dafcb2bf5 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -852,7 +852,7 @@ mod tests { // Clean up #[cfg(unix)] unsafe { - libc::kill(pid as i32, libc::SIGKILL); + libc::kill(i32::try_from(pid).unwrap(), libc::SIGKILL); } } diff --git a/lib/crates/fabro-cli/src/commands/run/command.rs b/lib/crates/fabro-cli/src/commands/run/command.rs index 9964ae088..bd800872b 100644 --- a/lib/crates/fabro-cli/src/commands/run/command.rs +++ b/lib/crates/fabro-cli/src/commands/run/command.rs @@ -27,7 +27,7 @@ pub(crate) async fn execute(mut args: RunArgs, globals: &GlobalArgs) -> Result<( } else { let exit_code = super::attach::attach_run(&run_dir, Some(&run_id), true, styles, Some(child)).await?; - super::output::print_run_summary(&run_dir, &run_id, styles); + super::output::print_run_summary(&run_dir, run_id, styles); if exit_code != std::process::ExitCode::SUCCESS { std::process::exit(1); } diff --git a/lib/crates/fabro-cli/src/commands/run/launcher.rs b/lib/crates/fabro-cli/src/commands/run/launcher.rs index 80831292a..10be140aa 100644 --- a/lib/crates/fabro-cli/src/commands/run/launcher.rs +++ b/lib/crates/fabro-cli/src/commands/run/launcher.rs @@ -7,6 +7,9 @@ use fabro_types::RunId; use fabro_workflows::records::{RunRecord, RunRecordExt}; use serde::{Deserialize, Serialize}; +#[cfg(test)] +use crate::commands::run::short_run_id; + #[derive(Debug, Clone, Serialize, Deserialize)] pub(crate) struct LauncherRecord { pub run_id: RunId, @@ -191,10 +194,7 @@ mod tests { assert!(command_matches_launcher( &record, - &format!( - "fabro: {} plan", - crate::commands::run::short_run_id(&record.run_id.to_string()) - ) + &format!("fabro: {} plan", short_run_id(&record.run_id.to_string())) )); } } diff --git a/lib/crates/fabro-cli/src/commands/run/resume.rs b/lib/crates/fabro-cli/src/commands/run/resume.rs index ab9fe1444..d2c185163 100644 --- a/lib/crates/fabro-cli/src/commands/run/resume.rs +++ b/lib/crates/fabro-cli/src/commands/run/resume.rs @@ -41,7 +41,7 @@ pub(crate) async fn resume_command( } else { let exit_code = super::attach::attach_run(&run_dir, Some(&run_id), true, styles, Some(child)).await?; - super::output::print_run_summary(&run_dir, &run_id, styles); + super::output::print_run_summary(&run_dir, run_id, styles); if exit_code != std::process::ExitCode::SUCCESS { std::process::exit(1); } @@ -54,17 +54,6 @@ fn launcher_pid_alive(run_dir: &std::path::Path) -> bool { .is_some_and(|record| process_alive(record.pid)) } -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn launcher_pid_alive_returns_false_for_missing_record() { - let dir = tempfile::tempdir().unwrap(); - assert!(!launcher_pid_alive(dir.path())); - } -} - #[allow(unsafe_code)] fn process_alive(pid: u32) -> bool { #[cfg(unix)] @@ -77,3 +66,14 @@ fn process_alive(pid: u32) -> bool { true } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn launcher_pid_alive_returns_false_for_missing_record() { + let dir = tempfile::tempdir().unwrap(); + assert!(!launcher_pid_alive(dir.path())); + } +} diff --git a/lib/crates/fabro-cli/src/commands/run/run_progress.rs b/lib/crates/fabro-cli/src/commands/run/run_progress.rs index 57cfbf066..49b4019c4 100644 --- a/lib/crates/fabro-cli/src/commands/run/run_progress.rs +++ b/lib/crates/fabro-cli/src/commands/run/run_progress.rs @@ -1805,7 +1805,7 @@ mod tests { branch: "security".into(), index: 0, }); - let stage = ui.active_stages.get("fork1").unwrap(); + let stage = &ui.active_stages["fork1"]; assert_eq!(stage.tool_calls.len(), 1); assert_eq!(stage.tool_calls[0].tool_call_id, "security"); assert!(matches!( @@ -1820,7 +1820,7 @@ mod tests { duration_ms: 2000, status: "success".into(), }); - let stage = ui.active_stages.get("fork1").unwrap(); + let stage = &ui.active_stages["fork1"]; assert!(matches!( stage.tool_calls[0].status, ToolCallStatus::Succeeded @@ -1837,7 +1837,7 @@ mod tests { duration_ms: 3000, status: "success".into(), }); - let stage = ui.active_stages.get("fork1").unwrap(); + let stage = &ui.active_stages["fork1"]; assert_eq!(stage.tool_calls.len(), 2); // Parallel completed → clears parent @@ -1863,7 +1863,7 @@ mod tests { index: 0, }); - let stage = ui.active_stages.get("fork1").unwrap(); + let stage = &ui.active_stages["fork1"]; let bar = &stage.tool_calls[0].bar; let msg = bar.message(); assert!( @@ -1892,7 +1892,7 @@ mod tests { status: "fail".into(), }); - let stage = ui.active_stages.get("fork1").unwrap(); + let stage = &ui.active_stages["fork1"]; assert!(matches!(stage.tool_calls[0].status, ToolCallStatus::Failed)); } @@ -2116,7 +2116,7 @@ mod tests { ui.handle_json_line(branch); // Branch should have been registered as a tool_call entry on the parent - let parent_stage = ui.active_stages.get("fork").unwrap(); + let parent_stage = &ui.active_stages["fork"]; assert!( !parent_stage.tool_calls.is_empty(), "parallel branch should be registered using node_id field" diff --git a/lib/crates/fabro-cli/src/commands/store/dump.rs b/lib/crates/fabro-cli/src/commands/store/dump.rs index c84b7b7ac..68e566ee3 100644 --- a/lib/crates/fabro-cli/src/commands/store/dump.rs +++ b/lib/crates/fabro-cli/src/commands/store/dump.rs @@ -6,6 +6,8 @@ use fabro_config::FabroSettingsExt; use fabro_store::{NodeVisitRef, RunSnapshot, RunStore}; use fabro_workflows::run_lookup::{resolve_run_combined, runs_base}; use serde::Serialize; +#[cfg(test)] +use serde::de::DeserializeOwned; use crate::args::{GlobalArgs, StoreDumpArgs}; use crate::store; @@ -494,7 +496,7 @@ mod tests { .unwrap() } - fn read_json(path: &Path) -> T { + fn read_json(path: &Path) -> T { serde_json::from_slice(&std::fs::read(path).unwrap()).unwrap() } diff --git a/lib/crates/fabro-cli/src/commands/upgrade.rs b/lib/crates/fabro-cli/src/commands/upgrade.rs index 4ae724902..7e054ad51 100644 --- a/lib/crates/fabro-cli/src/commands/upgrade.rs +++ b/lib/crates/fabro-cli/src/commands/upgrade.rs @@ -484,12 +484,12 @@ mod tests { #[test] fn upgrade_check_state_roundtrip() { let state = UpgradeCheckState { - checked_at: 1710000000, + checked_at: 1_710_000_000, latest_version: "0.5.0".to_string(), }; let json = serde_json::to_string(&state).unwrap(); let parsed: UpgradeCheckState = serde_json::from_str(&json).unwrap(); - assert_eq!(parsed.checked_at, 1710000000); + assert_eq!(parsed.checked_at, 1_710_000_000); assert_eq!(parsed.latest_version, "0.5.0"); } @@ -520,12 +520,12 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let path = dir.path().join("state.json"); let state = UpgradeCheckState { - checked_at: 1710000000, + checked_at: 1_710_000_000, latest_version: "0.5.0".to_string(), }; state.save(&path).unwrap(); let loaded = UpgradeCheckState::load(&path).unwrap(); - assert_eq!(loaded.checked_at, 1710000000); + assert_eq!(loaded.checked_at, 1_710_000_000); assert_eq!(loaded.latest_version, "0.5.0"); } diff --git a/lib/crates/fabro-cli/src/shared/provider_auth.rs b/lib/crates/fabro-cli/src/shared/provider_auth.rs index 4e8a51528..3d13ceedf 100644 --- a/lib/crates/fabro-cli/src/shared/provider_auth.rs +++ b/lib/crates/fabro-cli/src/shared/provider_auth.rs @@ -261,7 +261,7 @@ mod tests { // -- API key validation -- #[tokio::test] - #[ignore] // hits live Anthropic API + #[ignore = "hits live Anthropic API"] async fn validate_api_key_rejects_invalid_key() { let result = validate_api_key(Provider::Anthropic, "sk-invalid-key-12345").await; assert!(result.is_err(), "expected invalid key to be rejected"); diff --git a/lib/crates/fabro-cli/tests/it/cli.rs b/lib/crates/fabro-cli/tests/it/cli.rs index 313f5414c..e72714ae5 100644 --- a/lib/crates/fabro-cli/tests/it/cli.rs +++ b/lib/crates/fabro-cli/tests/it/cli.rs @@ -783,7 +783,7 @@ fn dry_run_writes_jsonl_and_live_json() { assert!(runs_base.exists(), "runs/ directory should exist"); let entries: Vec<_> = std::fs::read_dir(&runs_base) .unwrap() - .filter_map(|e| e.ok()) + .filter_map(Result::ok) .collect(); assert_eq!(entries.len(), 1, "should have exactly one run directory"); let run_dir = entries[0].path(); @@ -921,7 +921,7 @@ fn detach_creates_run_dir_with_detach_log() { assert!(runs_base.exists(), "runs/ directory should exist"); let entries: Vec<_> = std::fs::read_dir(&runs_base) .unwrap() - .filter_map(|e| e.ok()) + .filter_map(Result::ok) .collect(); assert_eq!(entries.len(), 1, "should have exactly one run directory"); let run_dir = entries[0].path(); @@ -1099,15 +1099,13 @@ fn setup_run_dir( overrides .get(key) .and_then(|v| v.as_str()) - .map(|s| serde_json::json!(s)) - .unwrap_or_else(|| serde_json::json!(default)) + .map_or_else(|| serde_json::json!(default), |s| serde_json::json!(s)) }; let get_bool = |key: &str, default: bool| -> serde_json::Value { overrides .get(key) - .and_then(|v| v.as_bool()) - .map(|b| serde_json::json!(b)) - .unwrap_or_else(|| serde_json::json!(default)) + .and_then(serde_json::Value::as_bool) + .map_or_else(|| serde_json::json!(default), |b| serde_json::json!(b)) }; // run.json (RunRecord) for resolve_run and run_engine_entrypoint diff --git a/lib/crates/fabro-cli/tests/it/scenario.rs b/lib/crates/fabro-cli/tests/it/scenario.rs index 634418cc2..2613de466 100644 --- a/lib/crates/fabro-cli/tests/it/scenario.rs +++ b/lib/crates/fabro-cli/tests/it/scenario.rs @@ -2,8 +2,9 @@ use std::path::{Path, PathBuf}; use std::time::Duration; use assert_cmd::Command; +use assert_cmd::assert::Assert; use fabro_store::RuntimeState; -use predicates; +use predicates::str::contains; use serde_json::Value; // --------------------------------------------------------------------------- @@ -49,7 +50,7 @@ fn find_run_dir(storage_dir: &Path) -> PathBuf { let runs_base = storage_dir.join("runs"); let entries: Vec<_> = std::fs::read_dir(&runs_base) .unwrap_or_else(|e| panic!("failed to read {}: {e}", runs_base.display())) - .filter_map(|e| e.ok()) + .filter_map(Result::ok) .filter(|e| e.path().is_dir()) .collect(); assert_eq!( @@ -485,7 +486,7 @@ fn test_repo_deinit_fails_when_not_initialized() { .current_dir(tmp.path()) .assert() .failure() - .stderr(predicates::str::contains("not initialized")); + .stderr(contains("not initialized")); } // --------------------------------------------------------------------------- @@ -530,9 +531,7 @@ fn test_repo_init_help_does_not_show_skill() { fn test_secret_lifecycle() { let tmp = tempfile::tempdir().unwrap(); - let secret = |args: &[&str]| -> assert_cmd::assert::Assert { - fabro().env("HOME", tmp.path()).args(args).assert() - }; + let secret = |args: &[&str]| -> Assert { fabro().env("HOME", tmp.path()).args(args).assert() }; // 1. set FOO=bar secret(&["secret", "set", "FOO", "bar"]).success(); @@ -543,7 +542,7 @@ fn test_secret_lifecycle() { // 3. list → contains FOO secret(&["secret", "list"]) .success() - .stdout(predicates::str::contains("FOO")); + .stdout(contains("FOO")); // 4. update FOO secret(&["secret", "set", "FOO", "updated"]).success(); @@ -564,9 +563,7 @@ fn test_secret_lifecycle() { fn test_secret_list_show_values() { let tmp = tempfile::tempdir().unwrap(); - let secret = |args: &[&str]| -> assert_cmd::assert::Assert { - fabro().env("HOME", tmp.path()).args(args).assert() - }; + let secret = |args: &[&str]| -> Assert { fabro().env("HOME", tmp.path()).args(args).assert() }; secret(&["secret", "set", "A", "1"]).success(); secret(&["secret", "set", "B", "2"]).success(); @@ -574,15 +571,15 @@ fn test_secret_list_show_values() { // Without --show-values: just keys let out = secret(&["secret", "list"]).success(); let stdout = String::from_utf8(out.get_output().stdout.clone()).unwrap(); - assert!(stdout.contains("A")); - assert!(stdout.contains("B")); + assert!(stdout.contains('A')); + assert!(stdout.contains('B')); assert!(!stdout.contains("A=1")); // With --show-values: KEY=VALUE secret(&["secret", "list", "--show-values"]) .success() - .stdout(predicates::str::contains("A=1")) - .stdout(predicates::str::contains("B=2")); + .stdout(contains("A=1")) + .stdout(contains("B=2")); } #[test] @@ -600,7 +597,7 @@ fn test_secret_list_alias_ls() { .args(["secret", "ls"]) .assert() .success() - .stdout(predicates::str::contains("X")); + .stdout(contains("X")); } #[test] @@ -612,7 +609,7 @@ fn test_secret_get_missing_key() { .args(["secret", "get", "NOPE"]) .assert() .failure() - .stderr(predicates::str::contains("secret not found")); + .stderr(contains("secret not found")); } #[test] @@ -624,7 +621,7 @@ fn test_secret_rm_missing_key() { .args(["secret", "rm", "NOPE"]) .assert() .failure() - .stderr(predicates::str::contains("secret not found")); + .stderr(contains("secret not found")); } #[test] @@ -663,7 +660,7 @@ fn test_model_list() { .args(["model", "list"]) .assert() .success() - .stdout(predicates::str::contains("claude-haiku")); + .stdout(contains("claude-haiku")); } #[test] @@ -686,7 +683,7 @@ fn test_workflow_list() { .assert() .success() // workflow list prints to stderr - .stderr(predicates::str::contains("my_test_wf")); + .stderr(contains("my_test_wf")); } #[test] @@ -706,7 +703,7 @@ fn local_run_lifecycle() { dotenvy::dotenv().ok(); let tmp = tempfile::tempdir().unwrap(); - let fabro_home = |args: &[&str]| -> assert_cmd::assert::Assert { + let fabro_home = |args: &[&str]| -> Assert { fabro() .env("HOME", tmp.path()) .args(args) diff --git a/lib/crates/fabro-config/src/project.rs b/lib/crates/fabro-config/src/project.rs index 2a3a87603..f4f3551b8 100644 --- a/lib/crates/fabro-config/src/project.rs +++ b/lib/crates/fabro-config/src/project.rs @@ -413,6 +413,7 @@ pub fn resolve_fabro_root(config_path: &Path, config: &ConfigLayer) -> PathBuf { #[cfg(test)] mod tests { use super::*; + use crate::run::{LlmConfig, PullRequestConfig}; use std::fs; use tempfile::TempDir; @@ -432,13 +433,12 @@ mod tests { #[test] fn parse_retros_default_false() { let config = parse_project_config("version = 1\n").unwrap(); - assert_eq!( - config + assert!( + !config .features .as_ref() .and_then(|f| f.retros) - .unwrap_or(false), - false, + .unwrap_or(false) ); } @@ -464,7 +464,7 @@ mod tests { .unwrap(); assert_eq!( config.pull_request, - Some(crate::run::PullRequestConfig { + Some(PullRequestConfig { enabled: Some(true), draft: Some(false), auto_merge: None, @@ -561,7 +561,6 @@ model = "claude-sonnet-4-6" version: Some(1), fabro: Some(ProjectConfig { root: Some("fabro/".to_string()), - ..Default::default() }), ..Default::default() }; @@ -578,7 +577,6 @@ model = "claude-sonnet-4-6" version: Some(1), fabro: Some(ProjectConfig { root: Some(".".to_string()), - ..Default::default() }), ..Default::default() }; @@ -645,7 +643,7 @@ model = "claude-sonnet-4-6" let cli_defaults = ConfigLayer { verbose: Some(false), - llm: Some(crate::run::LlmConfig { + llm: Some(LlmConfig { model: Some("cli-model".to_string()), provider: None, fallbacks: None, diff --git a/lib/crates/fabro-config/src/user.rs b/lib/crates/fabro-config/src/user.rs index 5d24b3493..0dedf2522 100644 --- a/lib/crates/fabro-config/src/user.rs +++ b/lib/crates/fabro-config/src/user.rs @@ -98,6 +98,7 @@ fn should_warn_about_legacy_user_config(path: &Path) -> bool { /// Load user config from an explicit path or `~/.fabro/user.toml`, returning defaults if the /// default file doesn't exist. An explicit path that doesn't exist is an error. +#[allow(clippy::print_stderr)] pub fn load_user_config(path: Option<&Path>) -> anyhow::Result { if let Some(explicit) = path { return crate::load_config_file(Some(explicit), USER_CONFIG_FILENAME); diff --git a/lib/crates/fabro-core/src/error.rs b/lib/crates/fabro-core/src/error.rs index f29bee80f..21ed4f286 100644 --- a/lib/crates/fabro-core/src/error.rs +++ b/lib/crates/fabro-core/src/error.rs @@ -166,7 +166,7 @@ mod tests { category: Some(FailureCategory::TransientInfra), signature: Some("sig123".into()), }); - let outcome: crate::outcome::Outcome = err.to_fail_outcome(); + let outcome: Outcome = err.to_fail_outcome(); assert_eq!(outcome.status, StageStatus::Fail); let failure = outcome.failure.unwrap(); assert_eq!(failure.message, "api down"); diff --git a/lib/crates/fabro-core/src/executor.rs b/lib/crates/fabro-core/src/executor.rs index 651044e28..92cb9e47f 100644 --- a/lib/crates/fabro-core/src/executor.rs +++ b/lib/crates/fabro-core/src/executor.rs @@ -410,6 +410,8 @@ impl Executor { #[cfg(test)] mod tests { + #![allow(clippy::items_after_statements)] + use std::sync::atomic::AtomicU32; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -422,6 +424,9 @@ mod tests { use crate::lifecycle::RunLifecycle; use crate::retry::{BackoffPolicy, RetryPolicy}; use crate::test_fixtures::*; + use tokio::time::{self, Instant}; + + type NextNodeLog = Arc)>>>; // Helper to build and run an executor with default settings async fn run_linear( @@ -1202,7 +1207,7 @@ mod tests { #[tokio::test] async fn executor_retry_backoff_delay() { - tokio::time::pause(); + time::pause(); let handler = Arc::new( CountingHandler::new(vec![ Ok(Outcome { @@ -1221,7 +1226,7 @@ mod tests { }, }), ); - let start = tokio::time::Instant::now(); + let start = Instant::now(); let result = run_linear( &["start", "end"], handler as Arc>, @@ -1520,7 +1525,7 @@ mod tests { async fn executor_checkpoint_called_after_edge_selection() { // Verify on_checkpoint receives the resolved next_node_id let log = Arc::new(Mutex::new(Vec::<(String, Option)>::new())); - struct NextNodeTracker(Arc)>>>); + struct NextNodeTracker(NextNodeLog); #[async_trait] impl RunLifecycle for NextNodeTracker { async fn on_checkpoint( @@ -1895,7 +1900,7 @@ mod tests { Err(CoreError::StallTimeout { ref node_id }) => { assert_eq!(node_id, "start"); } - other => panic!("expected StallTimeout, got {:?}", other), + other => panic!("expected StallTimeout, got {other:?}"), } } @@ -1955,8 +1960,7 @@ mod tests { let result = executor.run(&g, state).await; assert!( matches!(result, Err(CoreError::StallTimeout { .. })), - "expected StallTimeout, got {:?}", - result + "expected StallTimeout, got {result:?}" ); } @@ -1990,8 +1994,7 @@ mod tests { let result = executor.run(&g, state).await; assert!( matches!(result, Err(CoreError::StallTimeout { .. })), - "expected StallTimeout, got {:?}", - result + "expected StallTimeout, got {result:?}" ); } } diff --git a/lib/crates/fabro-core/src/lifecycle.rs b/lib/crates/fabro-core/src/lifecycle.rs index 667bae801..eca75a030 100644 --- a/lib/crates/fabro-core/src/lifecycle.rs +++ b/lib/crates/fabro-core/src/lifecycle.rs @@ -262,6 +262,8 @@ impl RunLifecycle for CompositeLifecycle { #[cfg(test)] mod tests { + #![allow(clippy::items_after_statements)] + use std::sync::atomic::{AtomicU32, Ordering}; use std::sync::{Arc, Mutex}; diff --git a/lib/crates/fabro-core/src/stall.rs b/lib/crates/fabro-core/src/stall.rs index 74c28fa4a..981a1cc9a 100644 --- a/lib/crates/fabro-core/src/stall.rs +++ b/lib/crates/fabro-core/src/stall.rs @@ -108,6 +108,7 @@ impl Drop for StallGuard { mod tests { use super::*; use std::sync::atomic::AtomicU32; + use tokio::time::sleep; struct TestMonitor { stall_count: AtomicU32, @@ -140,7 +141,7 @@ mod tests { let _guard = watchdog.start(); // Wait for timeout to fire - tokio::time::sleep(Duration::from_millis(100)).await; + sleep(Duration::from_millis(100)).await; assert!(cancel.load(Ordering::Relaxed)); assert_eq!(monitor.stalls(), 1); @@ -155,15 +156,15 @@ mod tests { let guard = watchdog.start(); // Report activity before timeout - tokio::time::sleep(Duration::from_millis(50)).await; + sleep(Duration::from_millis(50)).await; guard.report_activity(); // After another 50ms (100ms total, but only 50ms since activity), should not have timed out - tokio::time::sleep(Duration::from_millis(50)).await; + sleep(Duration::from_millis(50)).await; assert!(!cancel.load(Ordering::Relaxed)); // Wait long enough for timeout after last activity (80ms + margin) - tokio::time::sleep(Duration::from_millis(60)).await; + sleep(Duration::from_millis(60)).await; assert!(cancel.load(Ordering::Relaxed)); assert_eq!(monitor.stalls(), 1); } @@ -180,7 +181,7 @@ mod tests { drop(guard); // Wait past timeout - tokio::time::sleep(Duration::from_millis(100)).await; + sleep(Duration::from_millis(100)).await; // Should NOT have triggered assert!(!cancel.load(Ordering::Relaxed)); @@ -199,7 +200,7 @@ mod tests { drop(guard); // Wait well past timeout - tokio::time::sleep(Duration::from_millis(150)).await; + sleep(Duration::from_millis(150)).await; // Cancel should not be set assert!(!cancel.load(Ordering::Relaxed)); diff --git a/lib/crates/fabro-core/src/test_fixtures.rs b/lib/crates/fabro-core/src/test_fixtures.rs index 1e85074f4..dc8c42f8c 100644 --- a/lib/crates/fabro-core/src/test_fixtures.rs +++ b/lib/crates/fabro-core/src/test_fixtures.rs @@ -40,11 +40,13 @@ impl TestNode { } } + #[must_use] pub fn with_max_visits(mut self, max: usize) -> Self { self.max_visits = Some(max); self } + #[must_use] pub fn with_goal_gate(mut self, node_id: &str, required_status: StageStatus) -> Self { self.goal_gate = Some((node_id.to_string(), required_status)); self @@ -85,11 +87,13 @@ impl TestEdge { } } + #[must_use] pub fn with_label(mut self, label: &str) -> Self { self.label = Some(label.to_string()); self } + #[must_use] pub fn with_loop_restart(mut self) -> Self { self.loop_restart = true; self @@ -130,6 +134,7 @@ impl TestGraph { } } + #[must_use] pub fn with_retry_target(mut self, from: &str, to: &str) -> Self { self.retry_targets.insert(from.to_string(), to.to_string()); self @@ -296,6 +301,7 @@ impl CountingHandler { } } + #[must_use] pub fn with_retry_policy(mut self, policy: RetryPolicy) -> Self { self.retry_policy = policy; self @@ -342,6 +348,7 @@ impl DispatchHandler { } } + #[must_use] pub fn with_handler(mut self, node_id: &str, handler: Arc>) -> Self { self.handlers.insert(node_id.to_string(), handler); self diff --git a/lib/crates/fabro-devcontainer/src/compose.rs b/lib/crates/fabro-devcontainer/src/compose.rs index 8733d19df..a7ccbfc60 100644 --- a/lib/crates/fabro-devcontainer/src/compose.rs +++ b/lib/crates/fabro-devcontainer/src/compose.rs @@ -220,11 +220,11 @@ mod tests { #[test] fn service_with_image_only() { let f = write_compose( - r#" + r" services: web: image: nginx:latest -"#, +", ); let cfg = parse_compose(f.path(), "web").unwrap(); assert_eq!(cfg.image.as_deref(), Some("nginx:latest")); @@ -237,11 +237,11 @@ services: #[test] fn service_with_build_string() { let f = write_compose( - r#" + r" services: app: build: ./src -"#, +", ); let cfg = parse_compose(f.path(), "app").unwrap(); let build = cfg.build.unwrap(); @@ -252,13 +252,13 @@ services: #[test] fn service_with_build_object() { let f = write_compose( - r#" + r" services: app: build: context: ./app dockerfile: Dockerfile.dev -"#, +", ); let cfg = parse_compose(f.path(), "app").unwrap(); let build = cfg.build.unwrap(); @@ -298,39 +298,36 @@ services: ); let cfg = parse_compose(f.path(), "app").unwrap(); assert_eq!(cfg.environment.len(), 2); - assert_eq!( - cfg.environment.get("DATABASE_URL").unwrap(), - "postgres://localhost/db" - ); - assert_eq!(cfg.environment.get("DEBUG").unwrap(), "true"); + assert_eq!(cfg.environment["DATABASE_URL"], "postgres://localhost/db"); + assert_eq!(cfg.environment["DEBUG"], "true"); } #[test] fn environment_as_object() { let f = write_compose( - r#" + r" services: app: image: myapp environment: RAILS_ENV: production PORT: 3000 -"#, +", ); let cfg = parse_compose(f.path(), "app").unwrap(); assert_eq!(cfg.environment.len(), 2); - assert_eq!(cfg.environment.get("RAILS_ENV").unwrap(), "production"); - assert_eq!(cfg.environment.get("PORT").unwrap(), "3000"); + assert_eq!(cfg.environment["RAILS_ENV"], "production"); + assert_eq!(cfg.environment["PORT"], "3000"); } #[test] fn service_not_found() { let f = write_compose( - r#" + r" services: web: image: nginx -"#, +", ); let err = parse_compose(f.path(), "missing").unwrap_err(); assert!(err.contains("service 'missing' not found")); @@ -385,18 +382,18 @@ services: let cfg = parse_compose_multi(&paths, "app").unwrap(); assert_eq!(cfg.image.as_deref(), Some("node:22")); assert_eq!(cfg.ports, vec![3000, 9229]); - assert_eq!(cfg.environment.get("NODE_ENV").unwrap(), "development"); - assert_eq!(cfg.environment.get("DEBUG").unwrap(), "true"); + assert_eq!(cfg.environment["NODE_ENV"], "development"); + assert_eq!(cfg.environment["DEBUG"], "true"); } #[test] fn multi_compose_service_not_found() { let f = write_compose( - r#" + r" services: web: image: nginx -"#, +", ); let paths = vec![f.path().to_path_buf()]; let err = parse_compose_multi(&paths, "missing").unwrap_err(); @@ -406,18 +403,18 @@ services: #[test] fn multi_compose_skips_file_without_service() { let base = write_compose( - r#" + r" services: db: image: postgres:15 -"#, +", ); let over = write_compose( - r#" + r" services: app: image: node:22 -"#, +", ); let paths = vec![base.path().to_path_buf(), over.path().to_path_buf()]; let cfg = parse_compose_multi(&paths, "app").unwrap(); diff --git a/lib/crates/fabro-devcontainer/src/jsonc.rs b/lib/crates/fabro-devcontainer/src/jsonc.rs index c1e0689b2..e219bc61c 100644 --- a/lib/crates/fabro-devcontainer/src/jsonc.rs +++ b/lib/crates/fabro-devcontainer/src/jsonc.rs @@ -179,8 +179,8 @@ mod tests { #[test] fn strip_trailing_comma_before_bracket() { - let input = r#"[1, 2, 3,]"#; - let expected = r#"[1, 2, 3]"#; + let input = r"[1, 2, 3,]"; + let expected = r"[1, 2, 3]"; assert_eq!(strip_jsonc(input), expected); } diff --git a/lib/crates/fabro-git-storage/src/branchstore.rs b/lib/crates/fabro-git-storage/src/branchstore.rs index e0eaae4ab..408cb040f 100644 --- a/lib/crates/fabro-git-storage/src/branchstore.rs +++ b/lib/crates/fabro-git-storage/src/branchstore.rs @@ -219,6 +219,7 @@ pub fn sharded_path(id: &str, prefix_len: usize) -> String { #[cfg(test)] mod tests { use super::*; + use crate::gitobj::FileMode; use git2::Repository; fn temp_repo() -> (tempfile::TempDir, Store) { @@ -331,7 +332,7 @@ mod tests { let blob_oid = store.write_blob(b"custom content").unwrap(); bs.write_with("custom write", |entries| { - entries.set("custom.txt", blob_oid, crate::gitobj::FileMode::Blob); + entries.set("custom.txt", blob_oid, FileMode::Blob); Ok(()) }) .unwrap(); diff --git a/lib/crates/fabro-github/src/lib.rs b/lib/crates/fabro-github/src/lib.rs index bd2672ff3..240239ba5 100644 --- a/lib/crates/fabro-github/src/lib.rs +++ b/lib/crates/fabro-github/src/lib.rs @@ -1,3 +1,5 @@ +use base64::Engine; +use base64::engine::general_purpose::STANDARD; use serde::Deserialize; pub const GITHUB_API_BASE_URL: &str = "https://api.github.com"; @@ -79,7 +81,8 @@ fn decode_pem_env(name: &str, raw: &str) -> Result { if raw.starts_with("-----") { return Ok(raw.to_string()); } - let pem_bytes = base64::Engine::decode(&base64::engine::general_purpose::STANDARD, raw) + let pem_bytes = STANDARD + .decode(raw) .map_err(|err| format!("{name} is not valid PEM or base64: {err}"))?; String::from_utf8(pem_bytes) .map_err(|err| format!("{name} base64 decoded to invalid UTF-8: {err}")) @@ -973,6 +976,7 @@ pub async fn create_installation_access_token_for_projects( #[cfg(test)] mod tests { use super::*; + use base64::engine::general_purpose::URL_SAFE_NO_PAD; #[test] fn decode_pem_env_accepts_raw_pem() { @@ -983,7 +987,7 @@ mod tests { #[test] fn decode_pem_env_accepts_base64_pem() { let pem = "-----BEGIN TEST KEY-----\nabc\n-----END TEST KEY-----"; - let encoded = base64::Engine::encode(&base64::engine::general_purpose::STANDARD, pem); + let encoded = STANDARD.encode(pem); assert_eq!( decode_pem_env("GITHUB_APP_PRIVATE_KEY", &encoded).unwrap(), pem @@ -1114,11 +1118,7 @@ mod tests { let pem = test_rsa_key(); let jwt = sign_app_jwt("12345", pem).unwrap(); let header_b64 = jwt.split('.').next().unwrap(); - let header_json = base64::Engine::decode( - &base64::engine::general_purpose::URL_SAFE_NO_PAD, - header_b64, - ) - .unwrap(); + let header_json = URL_SAFE_NO_PAD.decode(header_b64).unwrap(); let header: serde_json::Value = serde_json::from_slice(&header_json).unwrap(); assert_eq!(header["alg"], "RS256"); } @@ -1128,11 +1128,7 @@ mod tests { let pem = test_rsa_key(); let jwt = sign_app_jwt("99999", pem).unwrap(); let payload_b64 = jwt.split('.').nth(1).unwrap(); - let payload_json = base64::Engine::decode( - &base64::engine::general_purpose::URL_SAFE_NO_PAD, - payload_b64, - ) - .unwrap(); + let payload_json = URL_SAFE_NO_PAD.decode(payload_b64).unwrap(); let claims: serde_json::Value = serde_json::from_slice(&payload_json).unwrap(); assert_eq!(claims["iss"], "99999"); @@ -1413,7 +1409,7 @@ mod tests { }; let result = branch_exists_with_client(&mock, &creds, "owner", "repo", "my-branch", "").await; - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[tokio::test] @@ -1445,7 +1441,7 @@ mod tests { }; let result = branch_exists_with_client(&mock, &creds, "owner", "repo", "no-such-branch", "").await; - assert_eq!(result.unwrap(), false); + assert!(!result.unwrap()); } #[tokio::test] @@ -1495,7 +1491,7 @@ mod tests { .with_req_header("Authorization", "Bearer test-jwt"); let result = check_app_installed(&mock, "test-jwt", "owner", "repo", "").await; - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[tokio::test] @@ -1504,7 +1500,7 @@ mod tests { MockHttpClient::new().on(HttpMethod::Get, "/repos/owner/repo/installation", 404, ""); let result = check_app_installed(&mock, "test-jwt", "owner", "repo", "").await; - assert_eq!(result.unwrap(), false); + assert!(!result.unwrap()); } #[tokio::test] @@ -1566,7 +1562,7 @@ mod tests { ); let result = is_app_public(&mock, "my-fabro-app", "").await; - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } #[tokio::test] @@ -1574,7 +1570,7 @@ mod tests { let mock = MockHttpClient::new().on(HttpMethod::Get, "/apps/my-private-app", 404, ""); let result = is_app_public(&mock, "my-private-app", "").await; - assert_eq!(result.unwrap(), false); + assert!(!result.unwrap()); } #[tokio::test] @@ -1589,7 +1585,7 @@ mod tests { .with_req_header_missing("Authorization"); let result = is_app_public(&mock, "my-app", "").await; - assert_eq!(result.unwrap(), true); + assert!(result.unwrap()); } // ----------------------------------------------------------------------- diff --git a/lib/crates/fabro-graphviz/src/parser/semantic.rs b/lib/crates/fabro-graphviz/src/parser/semantic.rs index 2756f81ea..e7065c243 100644 --- a/lib/crates/fabro-graphviz/src/parser/semantic.rs +++ b/lib/crates/fabro-graphviz/src/parser/semantic.rs @@ -269,6 +269,7 @@ pub fn ast_to_graph(dot: &DotGraph) -> Result { #[cfg(test)] mod tests { use super::*; + use crate::parser::ast::SubgraphStmt; #[test] fn convert_ast_str_to_string() { @@ -345,25 +346,25 @@ mod tests { statements: vec![ Statement::GraphAttr(vec![("goal".into(), AstValue::Str("Run tests".into()))]), Statement::GraphAttrDecl("rankdir".into(), AstValue::Ident("LR".into())), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "start".into(), attrs: Some(vec![ ("shape".into(), AstValue::Ident("Mdiamond".into())), ("label".into(), AstValue::Str("Start".into())), ]), }), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "exit".into(), attrs: Some(vec![ ("shape".into(), AstValue::Ident("Msquare".into())), ("label".into(), AstValue::Str("Exit".into())), ]), }), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "run_tests".into(), attrs: Some(vec![("label".into(), AstValue::Str("Run Tests".into()))]), }), - Statement::Edge(crate::parser::ast::EdgeStmt { + Statement::Edge(EdgeStmt { nodes: vec!["start".into(), "run_tests".into(), "exit".into()], attrs: None, }), @@ -390,11 +391,11 @@ mod tests { ("shape".into(), AstValue::Ident("box".into())), ("timeout".into(), AstValue::Str("900s".into())), ]), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "plan".into(), attrs: Some(vec![("label".into(), AstValue::Str("Plan".into()))]), }), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "implement".into(), attrs: Some(vec![ ("label".into(), AstValue::Str("Implement".into())), @@ -429,11 +430,11 @@ mod tests { fn ast_to_graph_subgraph_class_derivation() { let dot = DotGraph { name: "SubgraphTest".into(), - statements: vec![Statement::Subgraph(crate::parser::ast::SubgraphStmt { + statements: vec![Statement::Subgraph(SubgraphStmt { name: Some("cluster_loop".into()), statements: vec![ Statement::GraphAttrDecl("label".into(), AstValue::Str("Loop A".into())), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "plan".into(), attrs: None, }), @@ -450,14 +451,14 @@ mod tests { fn ast_to_graph_subgraph_class_from_graph_attr_block() { let dot = DotGraph { name: "SubgraphAttrBlock".into(), - statements: vec![Statement::Subgraph(crate::parser::ast::SubgraphStmt { + statements: vec![Statement::Subgraph(SubgraphStmt { name: Some("cluster_review".into()), statements: vec![ Statement::GraphAttr(vec![( "label".into(), AstValue::Str("Code Review".into()), )]), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "reviewer".into(), attrs: None, }), @@ -476,7 +477,7 @@ mod tests { name: "EdgeDefaults".into(), statements: vec![ Statement::EdgeDefaults(vec![("weight".into(), AstValue::Int(5))]), - Statement::Edge(crate::parser::ast::EdgeStmt { + Statement::Edge(EdgeStmt { nodes: vec!["a".into(), "b".into()], attrs: None, }), @@ -491,7 +492,7 @@ mod tests { fn ast_to_graph_chained_edges_with_attrs() { let dot = DotGraph { name: "Chained".into(), - statements: vec![Statement::Edge(crate::parser::ast::EdgeStmt { + statements: vec![Statement::Edge(EdgeStmt { nodes: vec!["a".into(), "b".into(), "c".into()], attrs: Some(vec![("label".into(), AstValue::Str("next".into()))]), })], @@ -507,7 +508,7 @@ mod tests { fn ast_to_graph_class_attr_parsed() { let dot = DotGraph { name: "ClassTest".into(), - statements: vec![Statement::Node(crate::parser::ast::NodeStmt { + statements: vec![Statement::Node(NodeStmt { id: "review".into(), attrs: Some(vec![( "class".into(), @@ -526,7 +527,7 @@ mod tests { fn ast_to_graph_implicit_nodes_from_edges() { let dot = DotGraph { name: "Implicit".into(), - statements: vec![Statement::Edge(crate::parser::ast::EdgeStmt { + statements: vec![Statement::Edge(EdgeStmt { nodes: vec!["a".into(), "b".into()], attrs: None, })], @@ -542,14 +543,14 @@ mod tests { let dot = DotGraph { name: "Legacy".into(), statements: vec![ - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "classify".into(), attrs: Some(vec![( "codergen_mode".into(), AstValue::Str("one_shot".into()), )]), }), - Statement::Node(crate::parser::ast::NodeStmt { + Statement::Node(NodeStmt { id: "work".into(), attrs: Some(vec![( "codergen_mode".into(), @@ -580,7 +581,7 @@ mod tests { fn codergen_mode_does_not_override_explicit_type() { let dot = DotGraph { name: "ExplicitType".into(), - statements: vec![Statement::Node(crate::parser::ast::NodeStmt { + statements: vec![Statement::Node(NodeStmt { id: "gate".into(), attrs: Some(vec![ ("type".into(), AstValue::Str("human".into())), diff --git a/lib/crates/fabro-hooks/src/bridge.rs b/lib/crates/fabro-hooks/src/bridge.rs index a50d221fb..50aa286be 100644 --- a/lib/crates/fabro-hooks/src/bridge.rs +++ b/lib/crates/fabro-hooks/src/bridge.rs @@ -22,7 +22,7 @@ pub struct WorkflowToolHookCallback { impl WorkflowToolHookCallback { fn base_context(&self, event: HookEvent, tool_name: &str) -> HookContext { - let mut ctx = HookContext::new(event, self.run_id.clone(), self.workflow_name.clone()); + let mut ctx = HookContext::new(event, self.run_id, self.workflow_name.clone()); ctx.node_id = Some(self.node_id.clone()); ctx.tool_name = Some(tool_name.to_string()); ctx diff --git a/lib/crates/fabro-hooks/src/executor.rs b/lib/crates/fabro-hooks/src/executor.rs index 0add15b1a..fdca2747d 100644 --- a/lib/crates/fabro-hooks/src/executor.rs +++ b/lib/crates/fabro-hooks/src/executor.rs @@ -604,6 +604,7 @@ mod tests { use crate::config::HookType; use crate::types::HookEvent; use fabro_types::fixtures; + use fabro_util::env::TestEnv; fn make_context() -> HookContext { HookContext::new(HookEvent::StageStart, fixtures::RUN_1, "test-wf".into()) @@ -840,8 +841,8 @@ mod tests { // --- interpolate_env_vars tests --- - fn test_env(vars: &[(&str, &str)]) -> fabro_util::env::TestEnv { - fabro_util::env::TestEnv( + fn test_env(vars: &[(&str, &str)]) -> TestEnv { + TestEnv( vars.iter() .map(|(k, v)| (k.to_string(), v.to_string())) .collect(), diff --git a/lib/crates/fabro-interview/src/file.rs b/lib/crates/fabro-interview/src/file.rs index 99f9f295c..b425c5a35 100644 --- a/lib/crates/fabro-interview/src/file.rs +++ b/lib/crates/fabro-interview/src/file.rs @@ -177,21 +177,19 @@ mod tests { if request_path.exists() { break; } - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + time::sleep(Duration::from_millis(50)).await; } assert!(request_path.exists(), "interview_request.json should exist"); // Verify the request contains valid Question JSON - let request_data = tokio::fs::read_to_string(&request_path).await.unwrap(); + let request_data = fs::read_to_string(&request_path).await.unwrap(); let parsed: Question = serde_json::from_str(&request_data).unwrap(); assert_eq!(parsed.text, "approve?"); // Write a response let answer = Answer::yes(); let response_json = serde_json::to_string_pretty(&answer).unwrap(); - tokio::fs::write(&response_path, response_json) - .await - .unwrap(); + fs::write(&response_path, response_json).await.unwrap(); // Wait for the ask to complete let result = ask_handle.await.unwrap(); @@ -234,7 +232,7 @@ mod tests { if request_path.exists() { break; } - tokio::time::sleep(Duration::from_millis(50)).await; + time::sleep(Duration::from_millis(50)).await; } assert!(request_path.exists()); @@ -242,14 +240,14 @@ mod tests { std::fs::write(&claim_path, "12345\n").unwrap(); // Let the poll loop see the claim - tokio::time::sleep(Duration::from_millis(150)).await; + time::sleep(Duration::from_millis(150)).await; // Simulate attacher departing (deletes claim without writing response) std::fs::remove_file(&claim_path).unwrap(); // Should return timeout within REATTACH_WINDOW - let started = tokio::time::Instant::now(); - let answer = tokio::time::timeout(Duration::from_secs(2), ask_handle) + let started = time::Instant::now(); + let answer = time::timeout(Duration::from_secs(2), ask_handle) .await .expect("should complete within 2s") .unwrap(); @@ -279,16 +277,16 @@ mod tests { if request_path.exists() { break; } - tokio::time::sleep(Duration::from_millis(50)).await; + time::sleep(Duration::from_millis(50)).await; } assert!(request_path.exists()); // Simulate attacher creating then deleting claim std::fs::write(&claim_path, "12345\n").unwrap(); - tokio::time::sleep(Duration::from_millis(150)).await; + time::sleep(Duration::from_millis(150)).await; std::fs::remove_file(&claim_path).unwrap(); - let answer = tokio::time::timeout(Duration::from_secs(2), ask_handle) + let answer = time::timeout(Duration::from_secs(2), ask_handle) .await .expect("should complete within 2s") .unwrap(); @@ -316,26 +314,24 @@ mod tests { if request_path.exists() { break; } - tokio::time::sleep(Duration::from_millis(50)).await; + time::sleep(Duration::from_millis(50)).await; } assert!(request_path.exists()); // First attacher creates then releases claim std::fs::write(&claim_path, "12345\n").unwrap(); - tokio::time::sleep(Duration::from_millis(150)).await; + time::sleep(Duration::from_millis(150)).await; std::fs::remove_file(&claim_path).unwrap(); // Second attacher picks up and answers before reattach window expires - tokio::time::sleep(Duration::from_millis(50)).await; + time::sleep(Duration::from_millis(50)).await; std::fs::write(&claim_path, "12346\n").unwrap(); let answer = Answer::yes(); let response_json = serde_json::to_string_pretty(&answer).unwrap(); - tokio::fs::write(response_path, response_json) - .await - .unwrap(); + fs::write(response_path, response_json).await.unwrap(); - let result = tokio::time::timeout(Duration::from_secs(2), ask_handle) + let result = time::timeout(Duration::from_secs(2), ask_handle) .await .expect("should complete within 2s") .unwrap(); diff --git a/lib/crates/fabro-interview/src/lib.rs b/lib/crates/fabro-interview/src/lib.rs index 280e912aa..513bc6ce9 100644 --- a/lib/crates/fabro-interview/src/lib.rs +++ b/lib/crates/fabro-interview/src/lib.rs @@ -215,6 +215,7 @@ pub use web::{PendingQuestion, WebInterviewer}; #[cfg(test)] mod tests { use super::*; + use tokio::time; #[test] fn question_type_display() { @@ -328,7 +329,7 @@ mod tests { #[async_trait] impl Interviewer for SlowInterviewer { async fn ask(&self, _question: Question) -> Answer { - tokio::time::sleep(std::time::Duration::from_secs(60)).await; + time::sleep(std::time::Duration::from_secs(60)).await; Answer::yes() } } diff --git a/lib/crates/fabro-interview/src/web.rs b/lib/crates/fabro-interview/src/web.rs index 40afb220e..474025ff5 100644 --- a/lib/crates/fabro-interview/src/web.rs +++ b/lib/crates/fabro-interview/src/web.rs @@ -111,6 +111,8 @@ mod tests { use super::*; use crate::{AnswerValue, QuestionType}; use std::sync::Arc; + use std::time::Duration; + use tokio::time::sleep; #[tokio::test] async fn ask_blocks_until_answer_submitted() { @@ -123,7 +125,7 @@ mod tests { }); // Give the ask task a moment to register the question - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + sleep(Duration::from_millis(50)).await; // Question should be pending let pending = interviewer.pending_questions(); @@ -149,7 +151,7 @@ mod tests { interviewer_clone.ask(q).await }); - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + sleep(Duration::from_millis(50)).await; let pending = interviewer.pending_questions(); assert_eq!(pending.len(), 1); @@ -190,7 +192,7 @@ mod tests { i2.ask(q).await }); - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + sleep(Duration::from_millis(50)).await; let pending = interviewer.pending_questions(); assert_eq!(pending.len(), 2); @@ -243,7 +245,7 @@ mod tests { i_clone.ask(q).await }); - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + sleep(Duration::from_millis(50)).await; let pending = interviewer.pending_questions(); assert_eq!(pending.len(), 1); @@ -264,7 +266,7 @@ mod tests { interviewer_clone.ask(q).await }); - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + sleep(Duration::from_millis(50)).await; { let mut inner = interviewer diff --git a/lib/crates/fabro-llm/src/cli.rs b/lib/crates/fabro-llm/src/cli.rs index c8d6b76fd..82bad65f7 100644 --- a/lib/crates/fabro-llm/src/cli.rs +++ b/lib/crates/fabro-llm/src/cli.rs @@ -1446,7 +1446,7 @@ mod tests { "provider": "anthropic", "family": "test", "display_name": "Test Model", - "limits": { "context_window": 128000, "max_output": 4096 }, + "limits": { "context_window": 128_000, "max_output": 4096 }, "training": null, "features": { "tools": true, "vision": false, "reasoning": false }, "costs": { "input_cost_per_mtok": 1.0, "output_cost_per_mtok": 2.0, "cache_input_cost_per_mtok": null }, diff --git a/lib/crates/fabro-llm/src/error.rs b/lib/crates/fabro-llm/src/error.rs index dc22bd39a..ac4d4a2fd 100644 --- a/lib/crates/fabro-llm/src/error.rs +++ b/lib/crates/fabro-llm/src/error.rs @@ -1196,7 +1196,7 @@ mod tests { #[test] fn sdk_error_serde_roundtrip_without_source() { - let io_err = std::io::Error::new(std::io::ErrorKind::Other, "boom"); + let io_err = std::io::Error::other("boom"); let err = SdkError::network("network failed", io_err); let json = serde_json::to_string(&err).unwrap(); let deserialized: SdkError = serde_json::from_str(&json).unwrap(); diff --git a/lib/crates/fabro-llm/src/generate.rs b/lib/crates/fabro-llm/src/generate.rs index 19021e09b..0ddc4c362 100644 --- a/lib/crates/fabro-llm/src/generate.rs +++ b/lib/crates/fabro-llm/src/generate.rs @@ -1109,12 +1109,14 @@ pub async fn stream_object( mod tests { use super::*; use crate::client::Client; + use crate::error::{ProviderErrorDetail, ProviderErrorKind}; use crate::provider::ProviderAdapter; - use crate::types::{ContentPart, Role}; + use crate::types::{ContentPart, Role, ToolResult}; use futures::StreamExt; use futures::stream; use std::collections::HashMap; use std::sync::atomic::{AtomicU32, Ordering}; + use tokio::time::sleep; /// Mock provider that returns configurable responses. struct MockProvider { @@ -1781,10 +1783,6 @@ mod tests { #[tokio::test] async fn generate_abort_signal_between_tool_rounds() { - let call_count = Arc::new(AtomicU32::new(0)); - let token = CancellationToken::new(); - let token_clone = token.clone(); - // Provider that always returns tool calls struct AlwaysToolCallProvider { call_count: Arc, @@ -1830,6 +1828,10 @@ mod tests { } } + let call_count = Arc::new(AtomicU32::new(0)); + let token = CancellationToken::new(); + let token_clone = token.clone(); + let provider: Arc = Arc::new(AlwaysToolCallProvider { call_count: call_count.clone(), cancel_token: token_clone, @@ -2180,10 +2182,7 @@ mod tests { serde_json::json!({"city": "SF"}), )]; - let tool_results = vec![crate::types::ToolResult::success( - "call_1", - serde_json::json!("72F"), - )]; + let tool_results = vec![ToolResult::success("call_1", serde_json::json!("72F"))]; // Processing StepFinish should not panic and should not set the final response acc.process(&StreamEvent::step_finish( @@ -2348,10 +2347,10 @@ mod tests { if count < self.failures { return Err(SdkError::Provider { - kind: crate::error::ProviderErrorKind::Server, - detail: Box::new(crate::error::ProviderErrorDetail { + kind: ProviderErrorKind::Server, + detail: Box::new(ProviderErrorDetail { status_code: Some(500), - ..crate::error::ProviderErrorDetail::new("server error", "mock") + ..ProviderErrorDetail::new("server error", "mock") }), }); } @@ -2459,7 +2458,7 @@ mod tests { } async fn stream(&self, _request: &Request) -> Result { - tokio::time::sleep(self.delay).await; + sleep(self.delay).await; let text = "Slow response"; let response = Response { id: "resp_1".into(), @@ -2531,8 +2530,6 @@ mod tests { async fn stream_total_timeout() { // Use a streaming tool call provider with a slow tool to trigger total timeout // across multiple rounds - let call_count = Arc::new(AtomicU32::new(0)); - /// Provider that always returns tool calls with a delay on the second stream struct SlowToolCallStreamProvider { call_count: Arc, @@ -2592,7 +2589,7 @@ mod tests { Ok(Box::pin(stream::iter(events))) } else { // Second stream: delay long enough to exceed total timeout - tokio::time::sleep(std::time::Duration::from_secs(5)).await; + sleep(std::time::Duration::from_secs(5)).await; let text = "Should not arrive"; let response = Response { id: "resp_2".into(), @@ -2618,6 +2615,8 @@ mod tests { } } + let call_count = Arc::new(AtomicU32::new(0)); + let provider: Arc = Arc::new(SlowToolCallStreamProvider { call_count: call_count.clone(), }); diff --git a/lib/crates/fabro-llm/src/provider.rs b/lib/crates/fabro-llm/src/provider.rs index 174491ae4..4a30c7b7b 100644 --- a/lib/crates/fabro-llm/src/provider.rs +++ b/lib/crates/fabro-llm/src/provider.rs @@ -74,7 +74,7 @@ mod tests { #[async_trait::async_trait] impl ProviderAdapter for MockAdapter { - fn name(&self) -> &str { + fn name(&self) -> &'static str { "mock" } async fn complete(&self, _request: &Request) -> Result { @@ -90,7 +90,7 @@ mod tests { #[async_trait::async_trait] impl ProviderAdapter for RestrictedAdapter { - fn name(&self) -> &str { + fn name(&self) -> &'static str { "restricted" } async fn complete(&self, _request: &Request) -> Result { diff --git a/lib/crates/fabro-llm/src/providers/anthropic.rs b/lib/crates/fabro-llm/src/providers/anthropic.rs index d0caac494..e541a9b5a 100644 --- a/lib/crates/fabro-llm/src/providers/anthropic.rs +++ b/lib/crates/fabro-llm/src/providers/anthropic.rs @@ -1375,6 +1375,7 @@ impl ProviderAdapter for Adapter { #[cfg(test)] mod tests { use super::*; + use crate::types::{AudioData, DocumentData, ReasoningEffort, ResponseFormat}; #[test] fn adapter_with_name() { @@ -1721,7 +1722,7 @@ mod tests { } } - fn make_request_with_format(format: crate::types::ResponseFormat) -> Request { + fn make_request_with_format(format: ResponseFormat) -> Request { Request { provider: None, response_format: Some(format), @@ -1737,7 +1738,7 @@ mod tests { "properties": {"name": {"type": "string"}}, "required": ["name"] }); - let request = make_request_with_format(crate::types::ResponseFormat { + let request = make_request_with_format(ResponseFormat { kind: ResponseFormatType::JsonSchema, json_schema: Some(schema.clone()), strict: false, @@ -1765,7 +1766,7 @@ mod tests { #[test] fn response_format_json_schema_appends_to_existing_tools() { let schema = serde_json::json!({"type": "object"}); - let mut request = make_request_with_format(crate::types::ResponseFormat { + let mut request = make_request_with_format(ResponseFormat { kind: ResponseFormatType::JsonSchema, json_schema: Some(schema), strict: false, @@ -1791,7 +1792,7 @@ mod tests { #[test] fn response_format_json_object_appends_to_string_system() { - let request = make_request_with_format(crate::types::ResponseFormat { + let request = make_request_with_format(ResponseFormat { kind: ResponseFormatType::JsonObject, json_schema: None, strict: false, @@ -1815,7 +1816,7 @@ mod tests { #[test] fn response_format_json_object_sets_system_when_none() { - let request = make_request_with_format(crate::types::ResponseFormat { + let request = make_request_with_format(ResponseFormat { kind: ResponseFormatType::JsonObject, json_schema: None, strict: false, @@ -1834,7 +1835,7 @@ mod tests { #[test] fn response_format_json_object_appends_to_array_system() { - let request = make_request_with_format(crate::types::ResponseFormat { + let request = make_request_with_format(ResponseFormat { kind: ResponseFormatType::JsonObject, json_schema: None, strict: false, @@ -1855,7 +1856,7 @@ mod tests { #[test] fn response_format_text_is_noop() { - let request = make_request_with_format(crate::types::ResponseFormat { + let request = make_request_with_format(ResponseFormat { kind: ResponseFormatType::Text, json_schema: None, strict: false, @@ -1983,7 +1984,7 @@ mod tests { #[test] fn document_url_translates_to_url_source() { - let part = ContentPart::Document(crate::types::DocumentData { + let part = ContentPart::Document(DocumentData { url: Some("https://example.com/doc.pdf".to_string()), data: None, media_type: None, @@ -1997,7 +1998,7 @@ mod tests { #[test] fn document_base64_data_translates_to_base64_source() { - let part = ContentPart::Document(crate::types::DocumentData { + let part = ContentPart::Document(DocumentData { url: None, data: Some(vec![0x25, 0x50, 0x44, 0x46]), media_type: Some("application/pdf".to_string()), @@ -2012,7 +2013,7 @@ mod tests { #[test] fn document_base64_defaults_to_pdf_mime() { - let part = ContentPart::Document(crate::types::DocumentData { + let part = ContentPart::Document(DocumentData { url: None, data: Some(vec![1, 2, 3]), media_type: None, @@ -2130,7 +2131,7 @@ mod tests { #[test] fn audio_produces_text_fallback() { - let part = ContentPart::Audio(crate::types::AudioData { + let part = ContentPart::Audio(AudioData { url: Some("https://example.com/audio.wav".to_string()), data: None, media_type: None, @@ -2147,7 +2148,7 @@ mod tests { fn build_api_request_maps_reasoning_effort_to_output_config() { let adapter = Adapter::new("test-key"); let request = Request { - reasoning_effort: Some(crate::types::ReasoningEffort::Medium), + reasoning_effort: Some(ReasoningEffort::Medium), ..make_base_request() }; diff --git a/lib/crates/fabro-llm/src/providers/fabro_server.rs b/lib/crates/fabro-llm/src/providers/fabro_server.rs index 30f3e005d..da26c8a2e 100644 --- a/lib/crates/fabro-llm/src/providers/fabro_server.rs +++ b/lib/crates/fabro-llm/src/providers/fabro_server.rs @@ -224,6 +224,7 @@ fn parse_sse_block(block: &str) -> Option<(String, String)> { #[cfg(test)] mod tests { use super::*; + use crate::error::ProviderErrorKind; use crate::types::Message; use futures::StreamExt; use httpmock::prelude::*; @@ -350,7 +351,7 @@ data: {\"type\":\"text_delta\",\"delta\":\" world\",\"text_id\":null}\n\ let err = adapter.complete(&make_request()).await.unwrap_err(); match &err { SdkError::Provider { kind, detail } => { - assert_eq!(*kind, crate::error::ProviderErrorKind::Server); + assert_eq!(*kind, ProviderErrorKind::Server); assert_eq!(detail.status_code, Some(502)); } other => panic!("expected Provider error, got {other:?}"), @@ -369,13 +370,12 @@ data: {\"type\":\"text_delta\",\"delta\":\" world\",\"text_id\":null}\n\ let adapter = Adapter::new(reqwest::Client::new(), server.base_url(), "test-provider"); let result = adapter.stream(&make_request()).await; - let err = match result { - Err(e) => e, - Ok(_) => panic!("expected error"), + let Err(err) = result else { + panic!("expected error"); }; match &err { SdkError::Provider { kind, detail } => { - assert_eq!(*kind, crate::error::ProviderErrorKind::Server); + assert_eq!(*kind, ProviderErrorKind::Server); assert_eq!(detail.status_code, Some(502)); } other => panic!("expected Provider error, got {other:?}"), diff --git a/lib/crates/fabro-llm/src/providers/gemini.rs b/lib/crates/fabro-llm/src/providers/gemini.rs index 0029bf040..3b8feb10c 100644 --- a/lib/crates/fabro-llm/src/providers/gemini.rs +++ b/lib/crates/fabro-llm/src/providers/gemini.rs @@ -982,6 +982,7 @@ impl ProviderAdapter for Adapter { #[cfg(test)] mod tests { use super::*; + use crate::types::{AudioData, DocumentData}; fn minimal_request() -> Request { Request { @@ -1131,7 +1132,7 @@ mod tests { fn audio_url_translates_to_file_data() { let msg = Message { role: Role::User, - content: vec![ContentPart::Audio(crate::types::AudioData { + content: vec![ContentPart::Audio(AudioData { url: Some("https://example.com/audio.wav".to_string()), data: None, media_type: Some("audio/wav".to_string()), @@ -1150,7 +1151,7 @@ mod tests { fn audio_base64_translates_to_inline_data() { let msg = Message { role: Role::User, - content: vec![ContentPart::Audio(crate::types::AudioData { + content: vec![ContentPart::Audio(AudioData { url: None, data: Some(vec![0xFF, 0xFB, 0x90]), media_type: None, @@ -1168,7 +1169,7 @@ mod tests { fn document_url_translates_to_file_data() { let msg = Message { role: Role::User, - content: vec![ContentPart::Document(crate::types::DocumentData { + content: vec![ContentPart::Document(DocumentData { url: Some("https://example.com/doc.pdf".to_string()), data: None, media_type: Some("application/pdf".to_string()), @@ -1187,7 +1188,7 @@ mod tests { fn document_base64_translates_to_inline_data() { let msg = Message { role: Role::User, - content: vec![ContentPart::Document(crate::types::DocumentData { + content: vec![ContentPart::Document(DocumentData { url: None, data: Some(vec![0x25, 0x50, 0x44, 0x46]), media_type: None, diff --git a/lib/crates/fabro-llm/src/providers/openai.rs b/lib/crates/fabro-llm/src/providers/openai.rs index 144acccbf..13473cad8 100644 --- a/lib/crates/fabro-llm/src/providers/openai.rs +++ b/lib/crates/fabro-llm/src/providers/openai.rs @@ -1066,6 +1066,8 @@ impl ProviderAdapter for Adapter { #[cfg(test)] mod tests { use super::*; + use crate::providers::common::LineReader; + use crate::types::{AudioData, DocumentData}; use std::collections::HashMap; fn minimal_request() -> Request { @@ -1226,7 +1228,7 @@ mod tests { fn audio_content_produces_text_fallback() { let msg = Message { role: Role::User, - content: vec![ContentPart::Audio(crate::types::AudioData { + content: vec![ContentPart::Audio(AudioData { url: Some("https://example.com/audio.wav".to_string()), data: None, media_type: None, @@ -1249,7 +1251,7 @@ mod tests { fn document_content_produces_text_fallback_with_filename() { let msg = Message { role: Role::User, - content: vec![ContentPart::Document(crate::types::DocumentData { + content: vec![ContentPart::Document(DocumentData { url: Some("https://example.com/doc.pdf".to_string()), data: None, media_type: None, @@ -1273,7 +1275,7 @@ mod tests { fn document_content_produces_text_fallback_without_filename() { let msg = Message { role: Role::User, - content: vec![ContentPart::Document(crate::types::DocumentData { + content: vec![ContentPart::Document(DocumentData { url: None, data: Some(vec![1, 2, 3]), media_type: None, @@ -1584,7 +1586,7 @@ mod tests { let http_resp = http::Response::builder().status(200).body("").unwrap(); let response = reqwest::Response::from(http_resp); SseStreamState { - line_reader: crate::providers::common::LineReader::new(response, None), + line_reader: LineReader::new(response, None), model: String::new(), response_id: String::new(), response_model: String::new(), diff --git a/lib/crates/fabro-llm/src/providers/openai_compatible.rs b/lib/crates/fabro-llm/src/providers/openai_compatible.rs index dc2a4d2ab..5590eb64a 100644 --- a/lib/crates/fabro-llm/src/providers/openai_compatible.rs +++ b/lib/crates/fabro-llm/src/providers/openai_compatible.rs @@ -911,6 +911,7 @@ impl StreamState { #[cfg(test)] mod tests { use super::*; + use crate::types::{AudioData, DocumentData}; #[test] fn stream_chunk_minimax_format() { @@ -1395,7 +1396,7 @@ mod tests { fn audio_content_produces_text_fallback() { let msg = Message { role: Role::User, - content: vec![ContentPart::Audio(crate::types::AudioData { + content: vec![ContentPart::Audio(AudioData { url: Some("https://example.com/audio.wav".to_string()), data: None, media_type: None, @@ -1414,7 +1415,7 @@ mod tests { fn document_content_produces_text_fallback_with_filename() { let msg = Message { role: Role::User, - content: vec![ContentPart::Document(crate::types::DocumentData { + content: vec![ContentPart::Document(DocumentData { url: Some("https://example.com/doc.pdf".to_string()), data: None, media_type: None, @@ -1434,7 +1435,7 @@ mod tests { fn document_content_produces_text_fallback_without_filename() { let msg = Message { role: Role::User, - content: vec![ContentPart::Document(crate::types::DocumentData { + content: vec![ContentPart::Document(DocumentData { url: None, data: Some(vec![1, 2, 3]), media_type: None, @@ -1456,7 +1457,7 @@ mod tests { role: Role::User, content: vec![ ContentPart::text("Check this: "), - ContentPart::Audio(crate::types::AudioData { + ContentPart::Audio(AudioData { url: None, data: Some(vec![1, 2]), media_type: None, diff --git a/lib/crates/fabro-llm/src/retry.rs b/lib/crates/fabro-llm/src/retry.rs index 7d899aa8f..8aac82c99 100644 --- a/lib/crates/fabro-llm/src/retry.rs +++ b/lib/crates/fabro-llm/src/retry.rs @@ -63,10 +63,12 @@ where #[cfg(test)] mod tests { use super::*; + use crate::error::{ProviderErrorDetail, ProviderErrorKind}; use crate::types::RetryPolicy; use fabro_util::backoff::BackoffPolicy; use std::sync::Arc; use std::sync::atomic::{AtomicU32, Ordering}; + use tokio::time::Instant; fn fast_backoff() -> BackoffPolicy { BackoffPolicy { @@ -121,10 +123,10 @@ mod tests { let count = cc.fetch_add(1, Ordering::SeqCst); if count < 2 { Err(SdkError::Provider { - kind: crate::error::ProviderErrorKind::Server, - detail: Box::new(crate::error::ProviderErrorDetail { + kind: ProviderErrorKind::Server, + detail: Box::new(ProviderErrorDetail { status_code: Some(500), - ..crate::error::ProviderErrorDetail::new("error", "test") + ..ProviderErrorDetail::new("error", "test") }), }) } else { @@ -154,10 +156,10 @@ mod tests { async move { cc.fetch_add(1, Ordering::SeqCst); Err::(SdkError::Provider { - kind: crate::error::ProviderErrorKind::Server, - detail: Box::new(crate::error::ProviderErrorDetail { + kind: ProviderErrorKind::Server, + detail: Box::new(ProviderErrorDetail { status_code: Some(500), - ..crate::error::ProviderErrorDetail::new("error", "test") + ..ProviderErrorDetail::new("error", "test") }), }) } @@ -184,10 +186,10 @@ mod tests { async move { cc.fetch_add(1, Ordering::SeqCst); Err::(SdkError::Provider { - kind: crate::error::ProviderErrorKind::Authentication, - detail: Box::new(crate::error::ProviderErrorDetail { + kind: ProviderErrorKind::Authentication, + detail: Box::new(ProviderErrorDetail { status_code: Some(401), - ..crate::error::ProviderErrorDetail::new("bad key", "test") + ..ProviderErrorDetail::new("bad key", "test") }), }) } @@ -219,11 +221,11 @@ mod tests { async move { cc.fetch_add(1, Ordering::SeqCst); Err::(SdkError::Provider { - kind: crate::error::ProviderErrorKind::RateLimit, - detail: Box::new(crate::error::ProviderErrorDetail { + kind: ProviderErrorKind::RateLimit, + detail: Box::new(ProviderErrorDetail { status_code: Some(429), retry_after: Some(100.0), // Way beyond max_delay - ..crate::error::ProviderErrorDetail::new("rate limited", "test") + ..ProviderErrorDetail::new("rate limited", "test") }), }) } @@ -250,18 +252,18 @@ mod tests { let call_count = Arc::new(AtomicU32::new(0)); let cc = call_count.clone(); - let start = tokio::time::Instant::now(); + let start = Instant::now(); let result = retry(&policy, || { let cc = cc.clone(); async move { let count = cc.fetch_add(1, Ordering::SeqCst); if count < 1 { Err(SdkError::Provider { - kind: crate::error::ProviderErrorKind::RateLimit, - detail: Box::new(crate::error::ProviderErrorDetail { + kind: ProviderErrorKind::RateLimit, + detail: Box::new(ProviderErrorDetail { status_code: Some(429), retry_after: Some(0.01), - ..crate::error::ProviderErrorDetail::new("rate limited", "test") + ..ProviderErrorDetail::new("rate limited", "test") }), }) } else { @@ -300,10 +302,10 @@ mod tests { let count = cc.fetch_add(1, Ordering::SeqCst); if count < 2 { Err(SdkError::Provider { - kind: crate::error::ProviderErrorKind::Server, - detail: Box::new(crate::error::ProviderErrorDetail { + kind: ProviderErrorKind::Server, + detail: Box::new(ProviderErrorDetail { status_code: Some(500), - ..crate::error::ProviderErrorDetail::new("error", "test") + ..ProviderErrorDetail::new("error", "test") }), }) } else { diff --git a/lib/crates/fabro-llm/src/types.rs b/lib/crates/fabro-llm/src/types.rs index 5e57f4a76..c42a1f9da 100644 --- a/lib/crates/fabro-llm/src/types.rs +++ b/lib/crates/fabro-llm/src/types.rs @@ -815,6 +815,7 @@ impl std::ops::Deref for StepResult { #[cfg(test)] mod tests { use super::*; + use fabro_util::backoff::BackoffPolicy; #[test] fn message_system_constructor() { @@ -1163,7 +1164,7 @@ mod tests { use std::time::Duration; let policy = RetryPolicy { max_retries: 3, - backoff: fabro_util::backoff::BackoffPolicy { + backoff: BackoffPolicy { initial_delay: Duration::from_secs(1), factor: 2.0, max_delay: Duration::from_secs(60), @@ -1183,7 +1184,7 @@ mod tests { use std::time::Duration; let policy = RetryPolicy { max_retries: 10, - backoff: fabro_util::backoff::BackoffPolicy { + backoff: BackoffPolicy { initial_delay: Duration::from_secs(1), factor: 2.0, max_delay: Duration::from_secs(5), @@ -1199,7 +1200,7 @@ mod tests { use std::time::Duration; let policy = RetryPolicy { max_retries: 3, - backoff: fabro_util::backoff::BackoffPolicy { + backoff: BackoffPolicy { initial_delay: Duration::from_secs(1), factor: 2.0, max_delay: Duration::from_secs(60), diff --git a/lib/crates/fabro-llm/tests/integration.rs b/lib/crates/fabro-llm/tests/integration.rs index a81118c3f..64de86961 100644 --- a/lib/crates/fabro-llm/tests/integration.rs +++ b/lib/crates/fabro-llm/tests/integration.rs @@ -1,6 +1,6 @@ use fabro_llm::provider::ProviderAdapter; use fabro_llm::providers::{AnthropicAdapter, GeminiAdapter, OpenAiAdapter}; -use fabro_llm::types::{Message, Request}; +use fabro_llm::types::{FinishReason, Message, Request}; fn make_request(model: &str) -> Request { Request { @@ -34,7 +34,7 @@ async fn anthropic_complete() { !response.text().is_empty(), "response text should not be empty" ); - assert_eq!(response.finish_reason, fabro_llm::types::FinishReason::Stop); + assert_eq!(response.finish_reason, FinishReason::Stop); assert!(response.usage.input_tokens > 0); assert!(response.usage.output_tokens > 0); assert_eq!(response.provider, "anthropic"); @@ -53,7 +53,7 @@ async fn openai_complete() { !response.text().is_empty(), "response text should not be empty" ); - assert_eq!(response.finish_reason, fabro_llm::types::FinishReason::Stop); + assert_eq!(response.finish_reason, FinishReason::Stop); assert!(response.usage.input_tokens > 0); assert!(response.usage.output_tokens > 0); assert_eq!(response.provider, "openai"); @@ -89,7 +89,7 @@ async fn gemini_complete() { !response.text().is_empty(), "response text should not be empty" ); - assert_eq!(response.finish_reason, fabro_llm::types::FinishReason::Stop); + assert_eq!(response.finish_reason, FinishReason::Stop); assert!(response.usage.input_tokens > 0); assert!(response.usage.output_tokens > 0); assert_eq!(response.provider, "gemini"); diff --git a/lib/crates/fabro-mcp/tests/stdio_integration.rs b/lib/crates/fabro-mcp/tests/stdio_integration.rs index bd1d9319c..8821966b8 100644 --- a/lib/crates/fabro-mcp/tests/stdio_integration.rs +++ b/lib/crates/fabro-mcp/tests/stdio_integration.rs @@ -1,6 +1,8 @@ use std::collections::HashMap; use std::time::Duration; +use fabro_mcp::connection_manager::McpConnectionManager; + use fabro_mcp::client::McpClient; use fabro_mcp::config::{McpServerConfig, McpTransport}; use fabro_mcp::connection_manager::call_result_to_string; @@ -52,7 +54,7 @@ async fn stdio_client_call_tool_echo() { #[tokio::test] async fn connection_manager_stdio_roundtrip() { let config = test_server_config(); - let mut mgr = fabro_mcp::connection_manager::McpConnectionManager::new(); + let mut mgr = McpConnectionManager::new(); let results = mgr.start_servers(&[config]).await; assert_eq!(results.len(), 1); diff --git a/lib/crates/fabro-model/src/catalog.rs b/lib/crates/fabro-model/src/catalog.rs index 277018747..91f11d5c6 100644 --- a/lib/crates/fabro-model/src/catalog.rs +++ b/lib/crates/fabro-model/src/catalog.rs @@ -377,8 +377,7 @@ mod tests { let models = Catalog::builtin().list(Some(provider)); assert!( !models.is_empty(), - "Provider {:?} has no models in catalog", - provider + "Provider {provider:?} has no models in catalog" ); } } @@ -423,8 +422,7 @@ mod tests { assert_eq!( roundtripped, Ok(provider), - "Provider::{:?}.as_str() does not round-trip through from_str", - provider + "Provider::{provider:?}.as_str() does not round-trip through from_str" ); } } diff --git a/lib/crates/fabro-model/src/model_ref.rs b/lib/crates/fabro-model/src/model_ref.rs index 2390e86bf..d9e26d6d2 100644 --- a/lib/crates/fabro-model/src/model_ref.rs +++ b/lib/crates/fabro-model/src/model_ref.rs @@ -56,6 +56,7 @@ impl fmt::Debug for ModelRef { #[cfg(test)] mod tests { use super::*; + use crate::catalog::Catalog; #[test] fn by_name_display() { @@ -78,20 +79,14 @@ mod tests { #[test] fn resolved_display() { - let info = crate::catalog::Catalog::builtin() - .get("claude-opus-4-6") - .unwrap() - .clone(); + let info = Catalog::builtin().get("claude-opus-4-6").unwrap().clone(); let r = ModelRef::Resolved(Arc::new(info)); assert_eq!(r.to_string(), "anthropic:claude-opus-4-6"); } #[test] fn resolved_accessors() { - let info = crate::catalog::Catalog::builtin() - .get("gpt-5.4") - .unwrap() - .clone(); + let info = Catalog::builtin().get("gpt-5.4").unwrap().clone(); let r = ModelRef::Resolved(Arc::new(info)); assert_eq!(r.model_id(), "gpt-5.4"); assert_eq!(r.provider(), Provider::OpenAi); diff --git a/lib/crates/fabro-retro/src/retro_agent.rs b/lib/crates/fabro-retro/src/retro_agent.rs index 67d3e1fbf..84cbefb2b 100644 --- a/lib/crates/fabro-retro/src/retro_agent.rs +++ b/lib/crates/fabro-retro/src/retro_agent.rs @@ -515,6 +515,7 @@ mod tests { use super::*; use fabro_agent::AgentEvent; use std::time::SystemTime; + use tokio::sync::broadcast; #[test] fn submit_retro_schema_is_valid_json() { @@ -612,7 +613,7 @@ mod tests { let parsed: serde_json::Value = serde_json::from_str(&content).unwrap(); assert_eq!(parsed["outcome"], "success"); assert!(parsed["failure_reason"].is_null()); - assert!(parsed["timestamp"].as_str().unwrap().contains("T")); + assert!(parsed["timestamp"].as_str().unwrap().contains('T')); } #[test] @@ -641,7 +642,7 @@ mod tests { let dir = tempfile::tempdir().unwrap(); let jsonl_path = dir.path().join("retro_session.jsonl"); - let (tx, rx) = tokio::sync::broadcast::channel::(16); + let (tx, rx) = broadcast::channel::(16); let handle = spawn_retro_event_writer(rx, jsonl_path.clone()); tx.send(SessionEvent { diff --git a/lib/crates/fabro-sandbox/src/docker.rs b/lib/crates/fabro-sandbox/src/docker.rs index 86284a2a6..f76d68833 100644 --- a/lib/crates/fabro-sandbox/src/docker.rs +++ b/lib/crates/fabro-sandbox/src/docker.rs @@ -800,7 +800,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "requires local Docker daemon"] async fn full_lifecycle() { let _docker = require_docker(); let host_dir = @@ -865,7 +865,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "requires local Docker daemon"] async fn timeout_handling() { let _docker = require_docker(); let host_dir = @@ -888,7 +888,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "requires local Docker daemon"] async fn special_characters_in_write() { let _docker = require_docker(); let host_dir = @@ -914,7 +914,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "requires local Docker daemon"] async fn path_resolution() { let _docker = require_docker(); let host_dir = @@ -941,7 +941,7 @@ mod tests { } #[tokio::test] - #[ignore] + #[ignore = "requires local Docker daemon"] async fn cleanup_idempotent() { let _docker = require_docker(); let host_dir = diff --git a/lib/crates/fabro-sandbox/src/exe/mod.rs b/lib/crates/fabro-sandbox/src/exe/mod.rs index 5eb82355a..02015634b 100644 --- a/lib/crates/fabro-sandbox/src/exe/mod.rs +++ b/lib/crates/fabro-sandbox/src/exe/mod.rs @@ -1,9 +1,11 @@ mod openssh_runner; use std::collections::HashMap; +use std::fmt::Write as _; use std::path::Path; use std::time::Instant; +use crate::sandbox::resolve_path; use crate::shell_quote; use crate::ssh_common; use crate::{ @@ -11,7 +13,10 @@ use crate::{ format_lines_numbered, }; use async_trait::async_trait; +use fabro_github::GitHubAppCredentials; use fabro_types::RunId; +use tokio::fs; +use tokio::sync::OnceCell; use tokio_util::sync::CancellationToken; pub use crate::ssh_common::{GitCloneParams, SshOutput, SshRunner}; @@ -63,10 +68,10 @@ type DataSshFactory = Box< /// - Data plane (`ssh .exe.xyz`) for command execution and file I/O pub struct ExeSandbox { mgmt_ssh: Box, - data_ssh: tokio::sync::OnceCell>, - vm_name: tokio::sync::OnceCell, - data_host: tokio::sync::OnceCell, - rg_available: tokio::sync::OnceCell, + data_ssh: OnceCell>, + vm_name: OnceCell, + data_host: OnceCell, + rg_available: OnceCell, event_callback: Option, /// Factory for creating data-plane SSH runners, used during initialize(). /// In production, this connects to the VM host via OpensshRunner. @@ -75,8 +80,8 @@ pub struct ExeSandbox { config: ExeConfig, clone_params: Option, run_id: Option, - origin_url: tokio::sync::OnceCell, - github_app: Option, + origin_url: OnceCell, + github_app: Option, } impl ExeSandbox { @@ -86,14 +91,14 @@ impl ExeSandbox { config: ExeConfig, clone_params: Option, run_id: Option, - github_app: Option, + github_app: Option, ) -> Self { Self { mgmt_ssh, - data_ssh: tokio::sync::OnceCell::new(), - vm_name: tokio::sync::OnceCell::new(), - data_host: tokio::sync::OnceCell::new(), - rg_available: tokio::sync::OnceCell::const_new(), + data_ssh: OnceCell::new(), + vm_name: OnceCell::new(), + data_host: OnceCell::new(), + rg_available: OnceCell::const_new(), event_callback: None, data_ssh_factory: Box::new(|host: &str| { let host = host.to_string(); @@ -106,7 +111,7 @@ impl ExeSandbox { config, clone_params, run_id, - origin_url: tokio::sync::OnceCell::new(), + origin_url: OnceCell::new(), github_app, } } @@ -114,14 +119,14 @@ impl ExeSandbox { /// Create an `ExeSandbox` from a pre-connected data-plane SSH runner. /// Used for reconnection (e.g. `fabro cp`) when the VM already exists. pub fn from_existing(data_ssh: Box) -> Self { - let data_cell = tokio::sync::OnceCell::new(); + let data_cell = OnceCell::new(); let _ = data_cell.set(data_ssh); Self { mgmt_ssh: Box::new(NoopSshRunner), data_ssh: data_cell, - vm_name: tokio::sync::OnceCell::new(), - data_host: tokio::sync::OnceCell::new(), - rg_available: tokio::sync::OnceCell::const_new(), + vm_name: OnceCell::new(), + data_host: OnceCell::new(), + rg_available: OnceCell::const_new(), event_callback: None, data_ssh_factory: Box::new(|_: &str| { Box::pin(async { @@ -131,7 +136,7 @@ impl ExeSandbox { config: ExeConfig::default(), clone_params: None, run_id: None, - origin_url: tokio::sync::OnceCell::new(), + origin_url: OnceCell::new(), github_app: None, } } @@ -166,7 +171,7 @@ impl ExeSandbox { fn data_ssh(&self) -> Result<&dyn SshRunner, String> { self.data_ssh .get() - .map(|b| b.as_ref()) + .map(std::convert::AsRef::as_ref) .ok_or_else(|| "Exe sandbox not initialized — call initialize() first".to_string()) } @@ -190,8 +195,8 @@ impl ExeSandbox { .await } - fn resolve_path(&self, path: &str) -> String { - crate::sandbox::resolve_path(path, WORKING_DIRECTORY) + fn resolve_path(path: &str) -> String { + resolve_path(path, WORKING_DIRECTORY) } } @@ -206,7 +211,7 @@ impl Sandbox for ExeSandbox { // Create a new VM via the management plane let mut cmd = "new --json".to_string(); if let Some(ref image) = self.config.image { - cmd.push_str(&format!(" --image {}", shell_quote(image))); + let _ = write!(cmd, " --image {}", shell_quote(image)); } let output = self.mgmt_ssh.run_command(&cmd).await.map_err(|e| { let err = format!("Failed to create exe.dev VM: {e}"); @@ -338,19 +343,15 @@ impl Sandbox for ExeSandbox { if let Some(vars) = env_vars { for (key, value) in vars { - script.push_str(&format!( - "export {}={}\n", - shell_quote(key), - shell_quote(value) - )); + let _ = writeln!(script, "export {}={}", shell_quote(key), shell_quote(value)); } } let dir = match working_dir { - Some(dir) => self.resolve_path(dir), + Some(dir) => Self::resolve_path(dir), None => WORKING_DIRECTORY.to_string(), }; - script.push_str(&format!("cd {} && {command}", shell_quote(&dir))); + let _ = write!(script, "cd {} && {command}", shell_quote(&dir)); let full_cmd = ssh_common::wrap_bash_command(&script); @@ -398,7 +399,7 @@ impl Sandbox for ExeSandbox { limit: Option, ) -> Result { let ssh = self.data_ssh()?; - let resolved = self.resolve_path(path); + let resolved = Self::resolve_path(path); let output = ssh .run_command(&format!("cat {}", shell_quote(&resolved))) @@ -417,7 +418,7 @@ impl Sandbox for ExeSandbox { async fn write_file(&self, path: &str, content: &str) -> Result<(), String> { let ssh = self.data_ssh()?; - let resolved = self.resolve_path(path); + let resolved = Self::resolve_path(path); // Ensure parent directory exists if let Some(parent) = Path::new(&resolved).parent() { @@ -433,7 +434,7 @@ impl Sandbox for ExeSandbox { async fn delete_file(&self, path: &str) -> Result<(), String> { let ssh = self.data_ssh()?; - let resolved = self.resolve_path(path); + let resolved = Self::resolve_path(path); let output = ssh .run_command(&format!("rm -f {}", shell_quote(&resolved))) @@ -448,7 +449,7 @@ impl Sandbox for ExeSandbox { async fn file_exists(&self, path: &str) -> Result { let ssh = self.data_ssh()?; - let resolved = self.resolve_path(path); + let resolved = Self::resolve_path(path); let output = ssh .run_command(&format!("test -e {}", shell_quote(&resolved))) @@ -462,7 +463,7 @@ impl Sandbox for ExeSandbox { path: &str, depth: Option, ) -> Result, String> { - let resolved = self.resolve_path(path); + let resolved = Self::resolve_path(path); let max_depth = depth.unwrap_or(1); let cmd = format!( @@ -511,7 +512,7 @@ impl Sandbox for ExeSandbox { path: &str, options: &GrepOptions, ) -> Result, String> { - let resolved = self.resolve_path(path); + let resolved = Self::resolve_path(path); // Detect ripgrep availability (cached) let use_rg = *self @@ -530,16 +531,17 @@ impl Sandbox for ExeSandbox { cmd.push_str(" -i"); } if let Some(ref glob_filter) = options.glob_filter { - cmd.push_str(&format!(" --glob {}", shell_quote(glob_filter))); + let _ = write!(cmd, " --glob {}", shell_quote(glob_filter)); } if let Some(max) = options.max_results { - cmd.push_str(&format!(" --max-count {max}")); + let _ = write!(cmd, " --max-count {max}"); } - cmd.push_str(&format!( + let _ = write!( + cmd, " -- {} {}", shell_quote(pattern), shell_quote(&resolved) - )); + ); cmd } else { let mut cmd = "grep -rn".to_string(); @@ -547,16 +549,17 @@ impl Sandbox for ExeSandbox { cmd.push_str(" -i"); } if let Some(ref glob_filter) = options.glob_filter { - cmd.push_str(&format!(" --include {}", shell_quote(glob_filter))); + let _ = write!(cmd, " --include {}", shell_quote(glob_filter)); } if let Some(max) = options.max_results { - cmd.push_str(&format!(" -m {max}")); + let _ = write!(cmd, " -m {max}"); } - cmd.push_str(&format!( + let _ = write!( + cmd, " -- {} {}", shell_quote(pattern), shell_quote(&resolved) - )); + ); cmd }; @@ -576,9 +579,7 @@ impl Sandbox for ExeSandbox { } async fn glob(&self, pattern: &str, path: Option<&str>) -> Result, String> { - let base = path - .map(|p| self.resolve_path(p)) - .unwrap_or_else(|| WORKING_DIRECTORY.to_string()); + let base = path.map_or_else(|| WORKING_DIRECTORY.to_string(), Self::resolve_path); let cmd = format!( "find {} -name {} -type f | sort", @@ -609,16 +610,16 @@ impl Sandbox for ExeSandbox { local_path: &Path, ) -> Result<(), String> { let ssh = self.data_ssh()?; - let resolved = self.resolve_path(remote_path); + let resolved = Self::resolve_path(remote_path); let bytes = ssh.download_file(&resolved).await?; if let Some(parent) = local_path.parent() { - tokio::fs::create_dir_all(parent) + fs::create_dir_all(parent) .await .map_err(|e| format!("Failed to create parent dirs: {e}"))?; } - tokio::fs::write(local_path, &bytes) + fs::write(local_path, &bytes) .await .map_err(|e| format!("Failed to write {}: {e}", local_path.display()))?; @@ -631,9 +632,9 @@ impl Sandbox for ExeSandbox { remote_path: &str, ) -> Result<(), String> { let ssh = self.data_ssh()?; - let resolved = self.resolve_path(remote_path); + let resolved = Self::resolve_path(remote_path); - let bytes = tokio::fs::read(local_path) + let bytes = fs::read(local_path) .await .map_err(|e| format!("Failed to read {}: {e}", local_path.display()))?; @@ -648,7 +649,7 @@ impl Sandbox for ExeSandbox { WORKING_DIRECTORY } - fn platform(&self) -> &str { + fn platform(&self) -> &'static str { "linux" } @@ -665,13 +666,11 @@ impl Sandbox for ExeSandbox { } async fn refresh_push_credentials(&self) -> Result<(), String> { - let origin_url = match self.origin_url() { - Some(url) => url, - None => return Ok(()), + let Some(origin_url) = self.origin_url() else { + return Ok(()); }; - let creds = match &self.github_app { - Some(c) => c, - None => return Ok(()), + let Some(creds) = &self.github_app else { + return Ok(()); }; let auth_url = fabro_github::resolve_authenticated_url(creds, origin_url) @@ -736,7 +735,9 @@ impl Sandbox for ExeSandbox { mod tests { use super::*; use base64::Engine; + use base64::engine::general_purpose::STANDARD; use std::sync::{Arc, Mutex}; + use tokio::fs; /// A recorded command sent to the mock SSH runner. #[derive(Debug, Clone)] @@ -875,9 +876,7 @@ mod tests { let start = wrapped.find("echo '").expect("missing echo prefix") + 6; let end = wrapped[start..].find('\'').expect("missing closing quote") + start; let encoded = &wrapped[start..end]; - let bytes = base64::engine::general_purpose::STANDARD - .decode(encoded) - .expect("invalid base64"); + let bytes = STANDARD.decode(encoded).expect("invalid base64"); String::from_utf8(bytes).expect("invalid utf8") } @@ -1287,7 +1286,7 @@ mod tests { .await .unwrap(); - let bytes = tokio::fs::read(&local).await.unwrap(); + let bytes = fs::read(&local).await.unwrap(); assert_eq!(bytes, b"binary content"); } diff --git a/lib/crates/fabro-sandbox/src/exe/openssh_runner.rs b/lib/crates/fabro-sandbox/src/exe/openssh_runner.rs index 59ed7bfd1..1585dfb6e 100644 --- a/lib/crates/fabro-sandbox/src/exe/openssh_runner.rs +++ b/lib/crates/fabro-sandbox/src/exe/openssh_runner.rs @@ -1,5 +1,8 @@ use async_trait::async_trait; +use base64::Engine; +use base64::engine::general_purpose::STANDARD; use openssh::{KnownHosts, Session}; +use tokio::time::timeout as tokio_timeout; use super::{SshOutput, SshRunner}; use crate::shell_quote; @@ -72,7 +75,7 @@ impl SshRunner for OpensshRunner { let mut child = self.build_command(command); let fut = child.output(); - match tokio::time::timeout(timeout, fut).await { + match tokio_timeout(timeout, fut).await { Ok(Ok(output)) => { let exit_code = output.status.code().unwrap_or(-1); Ok(SshOutput { @@ -87,8 +90,7 @@ impl SshRunner for OpensshRunner { } async fn upload_file(&self, path: &str, content: &[u8]) -> Result<(), String> { - use base64::Engine; - let encoded = base64::engine::general_purpose::STANDARD.encode(content); + let encoded = STANDARD.encode(content); let cmd = format!("echo '{}' | base64 -d > {}", encoded, shell_quote(path),); let output = self .build_command(&cmd) diff --git a/lib/crates/fabro-sandbox/src/local.rs b/lib/crates/fabro-sandbox/src/local.rs index 9366758d2..376f2f347 100644 --- a/lib/crates/fabro-sandbox/src/local.rs +++ b/lib/crates/fabro-sandbox/src/local.rs @@ -534,7 +534,13 @@ mod tests { #[tokio::test] async fn read_file_line_number_padding() { let dir = temp_dir(); - let content: String = (1..=12).map(|i| format!("line {i}\n")).collect(); + let content = + (1..=12) + .map(|i| format!("line {i}\n")) + .fold(String::new(), |mut acc, line| { + acc.push_str(&line); + acc + }); std::fs::write(dir.join("padded.txt"), content.trim_end()).unwrap(); let env = LocalSandbox::new(dir.clone()); diff --git a/lib/crates/fabro-sandbox/src/reconnect.rs b/lib/crates/fabro-sandbox/src/reconnect.rs index b12113cf0..3510ac9cf 100644 --- a/lib/crates/fabro-sandbox/src/reconnect.rs +++ b/lib/crates/fabro-sandbox/src/reconnect.rs @@ -7,6 +7,8 @@ use anyhow::{Context, Result, bail}; use crate::daytona::DaytonaSandbox; #[cfg(feature = "docker")] use crate::docker::{DockerSandbox, DockerSandboxConfig}; +#[cfg(feature = "exe")] +use crate::exe::{ExeSandbox, OpensshRunner as ExeOpensshRunner}; use crate::local::LocalSandbox; use crate::sandbox_record::SandboxRecord; #[cfg(feature = "ssh")] @@ -60,13 +62,11 @@ pub async fn reconnect(record: &SandboxRecord) -> Result .as_deref() .context("Exe sandbox record missing data_host")?; - let data_ssh = crate::exe::OpensshRunner::connect(data_host) - .await - .map_err(|e| { - anyhow::anyhow!("Failed to connect to exe sandbox '{data_host}': {e}") - })?; + let data_ssh = ExeOpensshRunner::connect(data_host).await.map_err(|e| { + anyhow::anyhow!("Failed to connect to exe sandbox '{data_host}': {e}") + })?; - let sandbox = crate::exe::ExeSandbox::from_existing(Box::new(data_ssh)); + let sandbox = ExeSandbox::from_existing(Box::new(data_ssh)); Ok(Box::new(sandbox)) } #[cfg(feature = "ssh")] diff --git a/lib/crates/fabro-sandbox/src/sandbox_spec.rs b/lib/crates/fabro-sandbox/src/sandbox_spec.rs index dc5a9c9f9..165f7a600 100644 --- a/lib/crates/fabro-sandbox/src/sandbox_spec.rs +++ b/lib/crates/fabro-sandbox/src/sandbox_spec.rs @@ -227,7 +227,7 @@ impl SandboxSpec { let mut sandbox = DaytonaSandbox::new( config.clone(), github_app.clone(), - run_id.clone(), + *run_id, clone_branch.clone(), ) .await @@ -252,7 +252,7 @@ impl SandboxSpec { Box::new(mgmt_ssh), config.clone(), clone_params.clone(), - run_id.clone(), + *run_id, github_app.clone(), ); if let Some(callback) = event_callback { @@ -270,7 +270,7 @@ impl SandboxSpec { let mut sandbox = SshSandbox::new( config.clone(), clone_params.clone(), - run_id.clone(), + *run_id, github_app.clone(), ); if let Some(callback) = event_callback { diff --git a/lib/crates/fabro-sandbox/src/ssh/mod.rs b/lib/crates/fabro-sandbox/src/ssh/mod.rs index 97a1975ed..a45d80a99 100644 --- a/lib/crates/fabro-sandbox/src/ssh/mod.rs +++ b/lib/crates/fabro-sandbox/src/ssh/mod.rs @@ -618,7 +618,9 @@ impl Sandbox for SshSandbox { mod tests { use super::*; use base64::Engine; + use base64::engine::general_purpose::STANDARD; use std::sync::{Arc, Mutex}; + use tokio::fs; /// A recorded command sent to the mock SSH runner. #[derive(Debug, Clone)] @@ -757,9 +759,7 @@ mod tests { let start = wrapped.find("echo '").expect("missing echo prefix") + 6; let end = wrapped[start..].find('\'').expect("missing closing quote") + start; let encoded = &wrapped[start..end]; - let bytes = base64::engine::general_purpose::STANDARD - .decode(encoded) - .expect("invalid base64"); + let bytes = STANDARD.decode(encoded).expect("invalid base64"); String::from_utf8(bytes).expect("invalid utf8") } @@ -1190,7 +1190,7 @@ mod tests { .await .unwrap(); - let bytes = tokio::fs::read(&local).await.unwrap(); + let bytes = fs::read(&local).await.unwrap(); assert_eq!(bytes, b"binary content"); } diff --git a/lib/crates/fabro-sandbox/src/test_support.rs b/lib/crates/fabro-sandbox/src/test_support.rs index 79cbb65d0..e4ca5cbf2 100644 --- a/lib/crates/fabro-sandbox/src/test_support.rs +++ b/lib/crates/fabro-sandbox/src/test_support.rs @@ -1,7 +1,8 @@ -use crate::*; +use crate::{DirEntry, ExecResult, GrepOptions, Sandbox, SandboxEvent, SandboxEventCallback}; use async_trait::async_trait; use std::collections::HashMap; use std::sync::Mutex; +use tokio::fs; use tokio_util::sync::CancellationToken; // --- MockSandbox --- @@ -182,11 +183,11 @@ impl Sandbox for MockSandbox { .get(remote_path) .ok_or_else(|| format!("File not found: {remote_path}"))?; if let Some(parent) = local_path.parent() { - tokio::fs::create_dir_all(parent) + fs::create_dir_all(parent) .await .map_err(|e| format!("Failed to create parent dirs: {e}"))?; } - tokio::fs::write(local_path, content.as_bytes()) + fs::write(local_path, content.as_bytes()) .await .map_err(|e| format!("Failed to write {}: {e}", local_path.display()))?; Ok(()) @@ -355,11 +356,11 @@ impl Sandbox for MutableMockSandbox { .cloned() .ok_or_else(|| format!("File not found: {remote_path}"))?; if let Some(parent) = local_path.parent() { - tokio::fs::create_dir_all(parent) + fs::create_dir_all(parent) .await .map_err(|e| format!("Failed to create parent dirs: {e}"))?; } - tokio::fs::write(local_path, content.as_bytes()) + fs::write(local_path, content.as_bytes()) .await .map_err(|e| format!("Failed to write {}: {e}", local_path.display()))?; Ok(()) @@ -370,7 +371,7 @@ impl Sandbox for MutableMockSandbox { local_path: &std::path::Path, remote_path: &str, ) -> Result<(), String> { - let content = tokio::fs::read_to_string(local_path) + let content = fs::read_to_string(local_path) .await .map_err(|e| format!("Failed to read {}: {e}", local_path.display()))?; self.files diff --git a/lib/crates/fabro-sandbox/src/worktree.rs b/lib/crates/fabro-sandbox/src/worktree.rs index 25619cad4..d40c8fb26 100644 --- a/lib/crates/fabro-sandbox/src/worktree.rs +++ b/lib/crates/fabro-sandbox/src/worktree.rs @@ -348,6 +348,7 @@ impl Sandbox for WorktreeSandbox { #[cfg(test)] mod tests { use super::*; + use crate::local::LocalSandbox; use crate::test_support::MockSandbox; use std::sync::Mutex; @@ -642,7 +643,7 @@ mod tests { // Put a marker file ONLY in the worktree directory std::fs::write(worktree.join("marker.txt"), "UNIQUE_WORKTREE_MARKER").unwrap(); - let inner: Arc = Arc::new(crate::local::LocalSandbox::new(original.clone())); + let inner: Arc = Arc::new(LocalSandbox::new(original.clone())); let config = WorktreeConfig { branch_name: "test-branch".into(), base_sha: "abc123".into(), @@ -683,7 +684,7 @@ mod tests { // Put a file ONLY in the worktree directory std::fs::write(worktree.join("worktree_only.txt"), "content").unwrap(); - let inner: Arc = Arc::new(crate::local::LocalSandbox::new(original.clone())); + let inner: Arc = Arc::new(LocalSandbox::new(original.clone())); let config = WorktreeConfig { branch_name: "test-branch".into(), base_sha: "abc123".into(), @@ -717,7 +718,7 @@ mod tests { // Put the file ONLY in the worktree directory std::fs::write(worktree.join("only_in_worktree.txt"), "worktree content").unwrap(); - let inner: Arc = Arc::new(crate::local::LocalSandbox::new(original.clone())); + let inner: Arc = Arc::new(LocalSandbox::new(original.clone())); let config = WorktreeConfig { branch_name: "test-branch".into(), base_sha: "abc123".into(), diff --git a/lib/crates/fabro-server/src/lib.rs b/lib/crates/fabro-server/src/lib.rs index 55010be9c..9e94c1cd0 100644 --- a/lib/crates/fabro-server/src/lib.rs +++ b/lib/crates/fabro-server/src/lib.rs @@ -1,3 +1,8 @@ +#![cfg_attr( + test, + allow(clippy::absolute_paths, clippy::await_holding_lock, clippy::float_cmp) +)] + #[allow(clippy::wildcard_imports, clippy::absolute_paths)] mod demo; pub mod error; diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 0cab82a94..9d63efc56 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -3,6 +3,8 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex, RwLock}; use std::time::Duration; +#[cfg(test)] +use axum::body::to_bytes; use axum::extract::{self as axum_extract, Path, Query, State}; use axum::http::StatusCode; use axum::response::sse::{Event, KeepAlive, Sse}; @@ -16,7 +18,7 @@ use fabro_llm::types::{ ContentPart, FinishReason, Message as LlmMessage, Request as LlmRequest, Response as LlmResponse, Role, StreamEvent, ToolChoice, ToolDefinition, Usage, }; -use fabro_retro::retro::Retro; +use fabro_retro::retro::{Retro, extract_stage_durations}; use fabro_store::{InMemoryStore, Store}; use fabro_types::RunId; use fabro_util::redact::redact_jsonl_line; @@ -492,6 +494,7 @@ fn compute_queue_positions(runs: &HashMap) -> HashMap Result { id.parse::() .map_err(|_| ApiError::bad_request("Invalid run ID.").into_response()) @@ -728,7 +731,7 @@ async fn execute_run(state: Arc, run_id: RunId) { Ok(events) => fabro_workflows::extract_stage_durations_from_events(&events), Err(err) => { tracing::warn!(run_id = %run_id, error = %err, "Failed to load run events from store"); - fabro_retro::retro::extract_stage_durations(&run_dir) + extract_stage_durations(&run_dir) } }; let mut agg = state @@ -816,7 +819,7 @@ pub fn spawn_scheduler(state: Arc) { runs.iter() .filter(|(_, r)| r.status == RunStatus::Queued) .min_by_key(|(_, r)| r.created_at) - .map(|(id, _)| id.clone()) + .map(|(id, _)| *id) }; match run_to_start { Some(id) => { @@ -1643,7 +1646,7 @@ mod tests { } async fn body_json(body: Body) -> serde_json::Value { - let bytes = axum::body::to_bytes(body, usize::MAX).await.unwrap(); + let bytes = to_bytes(body, usize::MAX).await.unwrap(); serde_json::from_slice(&bytes).unwrap() } @@ -2127,9 +2130,7 @@ mod tests { .unwrap(); assert_eq!(content_type, "image/svg+xml"); - let bytes = axum::body::to_bytes(response.into_body(), usize::MAX) - .await - .unwrap(); + let bytes = to_bytes(response.into_body(), usize::MAX).await.unwrap(); let svg = String::from_utf8_lossy(&bytes); assert!( svg.contains(" serde_json::Value { - let bytes = axum::body::to_bytes(body, usize::MAX).await.unwrap(); + let bytes = to_bytes(body, usize::MAX).await.unwrap(); serde_json::from_slice(&bytes).unwrap() } diff --git a/lib/crates/fabro-server/tests/it/api.rs b/lib/crates/fabro-server/tests/it/api.rs index f16b3b521..5c107c792 100644 --- a/lib/crates/fabro-server/tests/it/api.rs +++ b/lib/crates/fabro-server/tests/it/api.rs @@ -2,6 +2,8 @@ // mTLS end-to-end tests // =========================================================================== +#![allow(clippy::absolute_paths)] + // Skip on macOS: LibreSSL generates certs with extensions rustls rejects #[cfg(target_os = "linux")] mod mtls_e2e { diff --git a/lib/crates/fabro-server/tests/it/openapi_conformance.rs b/lib/crates/fabro-server/tests/it/openapi_conformance.rs index a9028b198..b21b95df9 100644 --- a/lib/crates/fabro-server/tests/it/openapi_conformance.rs +++ b/lib/crates/fabro-server/tests/it/openapi_conformance.rs @@ -1,5 +1,12 @@ //! Conformance tests: spec ↔ router ↔ Rust struct consistency. +#![allow( + clippy::absolute_paths, + clippy::default_trait_access, + clippy::manual_assert, + clippy::manual_let_else +)] + use super::helpers::test_db; use std::collections::BTreeSet; diff --git a/lib/crates/fabro-server/tests/it/pagination.rs b/lib/crates/fabro-server/tests/it/pagination.rs index a87bd1f2c..f370274b3 100644 --- a/lib/crates/fabro-server/tests/it/pagination.rs +++ b/lib/crates/fabro-server/tests/it/pagination.rs @@ -1,5 +1,7 @@ //! Tests that paginated list endpoints return `{ data, meta: { has_more } }`. +#![allow(clippy::absolute_paths)] + use super::helpers::test_db; use axum::body::Body; use axum::http::{Request, StatusCode}; diff --git a/lib/crates/fabro-slack/src/connection.rs b/lib/crates/fabro-slack/src/connection.rs index beb022f6b..9e63dde92 100644 --- a/lib/crates/fabro-slack/src/connection.rs +++ b/lib/crates/fabro-slack/src/connection.rs @@ -309,7 +309,7 @@ mod tests { i_clone.ask(q).await }); - tokio::time::sleep(std::time::Duration::from_millis(50)).await; + sleep(std::time::Duration::from_millis(50)).await; let pending = interviewer.pending_questions(); assert_eq!(pending.len(), 1); diff --git a/lib/crates/fabro-store/src/memory.rs b/lib/crates/fabro-store/src/memory.rs index 3953e2aa2..18367c0cf 100644 --- a/lib/crates/fabro-store/src/memory.rs +++ b/lib/crates/fabro-store/src/memory.rs @@ -652,6 +652,8 @@ mod tests { use fabro_types::{ AttrValue, FabroSettings, Graph, RunId, RunStatus, StageStatus, StatusReason, fixtures, }; + use tokio::time::timeout; + fn dt(rfc3339: &str) -> DateTime { DateTime::parse_from_rfc3339(rfc3339) .unwrap() @@ -1041,7 +1043,7 @@ mod tests { run.append_event(&first).await.unwrap(); let mut stream = run.watch_events_from(1).await.unwrap(); - let existing = tokio::time::timeout( + let existing = timeout( Duration::from_secs(1), futures::StreamExt::next(&mut stream), ) @@ -1052,7 +1054,7 @@ mod tests { assert_eq!(existing.seq, 1); run.append_event(&second).await.unwrap(); - let live = tokio::time::timeout( + let live = timeout( Duration::from_secs(1), futures::StreamExt::next(&mut stream), ) diff --git a/lib/crates/fabro-store/src/slate/catalog.rs b/lib/crates/fabro-store/src/slate/catalog.rs index 7925b59ff..436defc53 100644 --- a/lib/crates/fabro-store/src/slate/catalog.rs +++ b/lib/crates/fabro-store/src/slate/catalog.rs @@ -82,7 +82,7 @@ pub(super) async fn repair_catalog(store: Arc, base_prefix: &st let mut canonical = HashMap::new(); for meta in by_id_metas { if let Some(record) = read_catalog_path(store.clone(), meta.location).await? { - canonical.insert(record.run_id.clone(), record); + canonical.insert(record.run_id, record); } } diff --git a/lib/crates/fabro-store/src/slate/mod.rs b/lib/crates/fabro-store/src/slate/mod.rs index 3a19dafa2..6ac67d6f6 100644 --- a/lib/crates/fabro-store/src/slate/mod.rs +++ b/lib/crates/fabro-store/src/slate/mod.rs @@ -387,7 +387,9 @@ mod tests { RunRecord, RunStatus, RunStatusRecord, StageStatus, StartRecord, StatusReason, fixtures, }; use object_store::memory::InMemory; + use slatedb::config::Settings; use slatedb::{CloseReason, ErrorKind}; + use tokio::time::timeout; use crate::{EventPayload, NodeVisitRef}; @@ -524,9 +526,9 @@ mod tests { include_init: bool, ) -> slatedb::Db { let db = slatedb::Db::builder(record.db_prefix.clone(), object_store) - .with_settings(slatedb::config::Settings { + .with_settings(Settings { flush_interval: Some(Duration::from_millis(5)), - ..slatedb::config::Settings::default() + ..Settings::default() }) .build() .await @@ -813,7 +815,7 @@ mod tests { .await .unwrap(); - let event = tokio::time::timeout( + let event = timeout( Duration::from_secs(2), futures::StreamExt::next(&mut stream), ) @@ -953,9 +955,9 @@ mod tests { let created_at = dt("2026-03-27T12:00:00Z"); let db_prefix = catalog::db_prefix("runs/", created_at, &test_run_id("run-1")); let db = slatedb::Db::builder(db_prefix.clone(), object_store) - .with_settings(slatedb::config::Settings { + .with_settings(Settings { flush_interval: Some(Duration::from_millis(5)), - ..slatedb::config::Settings::default() + ..Settings::default() }) .build() .await @@ -971,12 +973,11 @@ mod tests { .unwrap(); db.close().await.unwrap(); - let err = match store + let Err(err) = store .create_run(&test_run_id("run-1"), created_at, None) .await - { - Ok(_) => panic!("expected create_run to reject mismatched _init.json"), - Err(err) => err, + else { + panic!("expected create_run to reject mismatched _init.json"); }; assert!(matches!( err, diff --git a/lib/crates/fabro-telemetry/src/lib.rs b/lib/crates/fabro-telemetry/src/lib.rs index 6e87e9880..1b9a5a756 100644 --- a/lib/crates/fabro-telemetry/src/lib.rs +++ b/lib/crates/fabro-telemetry/src/lib.rs @@ -134,7 +134,7 @@ fn should_track_for_level(level: TelemetryLevel, is_error: bool) -> bool { /// Internal function called by the `track!` macro. Do not call directly. #[doc(hidden)] -pub fn _track_inner(event: &str, properties: Value, is_error: bool) { +pub fn track_inner(event: &str, properties: Value, is_error: bool) { let Some(global) = GLOBAL.get() else { return; }; @@ -176,10 +176,10 @@ pub fn _track_inner(event: &str, properties: Value, is_error: bool) { #[macro_export] macro_rules! track { ($event:expr, { $($tt:tt)* }) => { - $crate::_track_inner($event, ::serde_json::json!({ $($tt)* }), false) + $crate::track_inner($event, ::serde_json::json!({ $($tt)* }), false) }; ($event:expr, { $($tt:tt)* }, error) => { - $crate::_track_inner($event, ::serde_json::json!({ $($tt)* }), true) + $crate::track_inner($event, ::serde_json::json!({ $($tt)* }), true) }; } @@ -239,6 +239,6 @@ mod tests { #[test] fn track_inner_noop_when_not_initialized() { // GLOBAL is not set in unit tests, so this should silently return - _track_inner("Test Event", serde_json::json!({"key": "value"}), false); + track!("Test Event", { "key": "value" }); } } diff --git a/lib/crates/fabro-telemetry/src/sanitize.rs b/lib/crates/fabro-telemetry/src/sanitize.rs index 6bfe35f4d..66cd4c0d0 100644 --- a/lib/crates/fabro-telemetry/src/sanitize.rs +++ b/lib/crates/fabro-telemetry/src/sanitize.rs @@ -74,7 +74,7 @@ mod tests { use super::*; fn args(strs: &[&str]) -> Vec { - strs.iter().map(|s| s.to_string()).collect() + strs.iter().map(std::string::ToString::to_string).collect() } #[test] diff --git a/lib/crates/fabro-telemetry/src/sender.rs b/lib/crates/fabro-telemetry/src/sender.rs index 48127ad2a..6b7d46d9d 100644 --- a/lib/crates/fabro-telemetry/src/sender.rs +++ b/lib/crates/fabro-telemetry/src/sender.rs @@ -147,6 +147,7 @@ mod tests { use super::*; use crate::event::User; use serde_json::json; + use tokio::runtime::Runtime; // -- Step 1: build_segment_batch tests -- @@ -278,7 +279,7 @@ mod tests { #[test] fn upload_noops_without_write_key() { // SEGMENT_WRITE_KEY is not set at compile time in tests, so this should error. - let rt = tokio::runtime::Runtime::new().unwrap(); + let rt = Runtime::new().unwrap(); let result = rt.block_on(upload(Path::new("/nonexistent"))); assert!(result.is_err()); let err_msg = result.unwrap_err().to_string(); diff --git a/lib/crates/fabro-tracker/src/linear.rs b/lib/crates/fabro-tracker/src/linear.rs index 05222d68c..cf9d0f97b 100644 --- a/lib/crates/fabro-tracker/src/linear.rs +++ b/lib/crates/fabro-tracker/src/linear.rs @@ -1018,7 +1018,7 @@ mod tests { // Create 51 IDs to trigger 2 batches let ids: Vec = (0..51).map(|i| format!("id-{i}")).collect(); - let id_refs: Vec<&str> = ids.iter().map(|s| s.as_str()).collect(); + let id_refs: Vec<&str> = ids.iter().map(String::as_str).collect(); // First batch (ids 0..50) let batch1_nodes: Vec = (0..50) diff --git a/lib/crates/fabro-util/src/backoff.rs b/lib/crates/fabro-util/src/backoff.rs index 61b291d01..7784dc6ed 100644 --- a/lib/crates/fabro-util/src/backoff.rs +++ b/lib/crates/fabro-util/src/backoff.rs @@ -100,10 +100,7 @@ mod tests { let delay = b.delay_for_attempt(1); assert!( delay >= min && delay <= max, - "delay {:?} out of range [{:?}, {:?}]", - delay, - min, - max, + "delay {delay:?} out of range [{min:?}, {max:?}]", ); } } diff --git a/lib/crates/fabro-util/src/redact/entropy.rs b/lib/crates/fabro-util/src/redact/entropy.rs index 7f2b1d9cb..f6e82e458 100644 --- a/lib/crates/fabro-util/src/redact/entropy.rs +++ b/lib/crates/fabro-util/src/redact/entropy.rs @@ -69,12 +69,12 @@ mod tests { #[test] fn entropy_empty_string() { - assert_eq!(shannon_entropy(""), 0.0); + assert!(shannon_entropy("").abs() < f64::EPSILON); } #[test] fn entropy_single_char_repeated() { - assert_eq!(shannon_entropy("aaaa"), 0.0); + assert!(shannon_entropy("aaaa").abs() < f64::EPSILON); } #[test] diff --git a/lib/crates/fabro-workflows/src/event.rs b/lib/crates/fabro-workflows/src/event.rs index 7f1c5495c..d6f19bed7 100644 --- a/lib/crates/fabro-workflows/src/event.rs +++ b/lib/crates/fabro-workflows/src/event.rs @@ -943,7 +943,7 @@ impl StoreProgressLogger { (*run_id.lock().unwrap()).clone_from(started_run_id); } - let run_id = run_id.lock().unwrap().clone(); + let run_id = *run_id.lock().unwrap(); match build_redacted_event_payload(event, &run_id) { Ok(payload) => { if tx.send(StoreProgressCommand::Event(payload)).is_err() { diff --git a/lib/crates/fabro-workflows/src/graph/routing.rs b/lib/crates/fabro-workflows/src/graph/routing.rs index 2cdf5dde2..700ff3df4 100644 --- a/lib/crates/fabro-workflows/src/graph/routing.rs +++ b/lib/crates/fabro-workflows/src/graph/routing.rs @@ -345,7 +345,7 @@ mod tests { heavy_count += 1; } } - let ratio = heavy_count as f64 / 500.0; + let ratio = f64::from(heavy_count) / 500.0; assert!( ratio > 0.90, "expected heavy edge to win >90% of the time, got {ratio:.2}" diff --git a/lib/crates/fabro-workflows/src/handler/parallel.rs b/lib/crates/fabro-workflows/src/handler/parallel.rs index 8627564ba..8168ab053 100644 --- a/lib/crates/fabro-workflows/src/handler/parallel.rs +++ b/lib/crates/fabro-workflows/src/handler/parallel.rs @@ -274,7 +274,7 @@ impl Handler for ParallelHandler { let run_dir = run_dir.to_path_buf(); let sem = Arc::clone(&semaphore); let has_git = git_state.is_some(); - let run_id = git_state.as_ref().map(|gs| gs.run_id.clone()); + let run_id = git_state.as_ref().map(|gs| gs.run_id); let git_author = git_state .as_ref() .map(|gs| gs.git_author.clone()) @@ -334,9 +334,8 @@ impl Handler for ParallelHandler { // Checkpoint commit after branch execution (capture head_sha) let head_sha = if has_git { - let rid = run_id - .map(|run_id| run_id.to_string()) - .unwrap_or_else(|| "unknown".to_string()); + let rid = + run_id.map_or_else(|| "unknown".to_string(), |run_id| run_id.to_string()); let nid = &setup.target_id; let status_str = outcome.status.to_string(); // Use exec_command to commit and capture HEAD in the branch worktree diff --git a/lib/crates/fabro-workflows/src/lib.rs b/lib/crates/fabro-workflows/src/lib.rs index c3e19e617..745cf4e7c 100644 --- a/lib/crates/fabro-workflows/src/lib.rs +++ b/lib/crates/fabro-workflows/src/lib.rs @@ -1,3 +1,19 @@ +#![cfg_attr( + test, + allow( + clippy::absolute_paths, + clippy::get_unwrap, + clippy::large_futures, + clippy::needless_borrows_for_generic_args, + clippy::option_option, + clippy::ptr_as_ptr, + clippy::ref_as_ptr, + clippy::cast_ptr_alignment, + clippy::uninlined_format_args, + clippy::unnecessary_literal_bound + ) +)] + use std::collections::HashMap; use std::sync::Arc; diff --git a/lib/crates/fabro-workflows/src/lifecycle/event.rs b/lib/crates/fabro-workflows/src/lifecycle/event.rs index 2eaa003e6..942bc6d30 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/event.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/event.rs @@ -73,7 +73,7 @@ impl RunLifecycle for EventLifecycle { // Emit WorkflowRunStarted self.emitter.emit(&WorkflowRunEvent::WorkflowRunStarted { name: self.graph_name.clone(), - run_id: self.run_id.clone(), + run_id: self.run_id, base_branch: self.base_branch.clone(), base_sha: self.base_sha.clone(), run_branch: self.run_branch.clone(), diff --git a/lib/crates/fabro-workflows/src/lifecycle/hook.rs b/lib/crates/fabro-workflows/src/lifecycle/hook.rs index e8372c89e..66ed8822e 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/hook.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/hook.rs @@ -49,11 +49,7 @@ impl HookLifecycle { #[async_trait] impl RunLifecycle for HookLifecycle { async fn on_run_start(&self, _graph: &WorkflowGraph, _state: &WfRunState) -> CoreResult<()> { - let hook_ctx = HookContext::new( - HookEvent::RunStart, - self.run_id.clone(), - self.graph_name.clone(), - ); + let hook_ctx = HookContext::new(HookEvent::RunStart, self.run_id, self.graph_name.clone()); let decision = self.run_hook(&hook_ctx).await; if let HookDecision::Block { reason } = decision { let msg = reason.unwrap_or_else(|| "blocked by RunStart hook".into()); @@ -68,11 +64,8 @@ impl RunLifecycle for HookLifecycle { _state: &WfRunState, ) -> CoreResult { let gv = ctx.node.inner(); - let mut hook_ctx = HookContext::new( - HookEvent::StageStart, - self.run_id.clone(), - self.graph_name.clone(), - ); + let mut hook_ctx = + HookContext::new(HookEvent::StageStart, self.run_id, self.graph_name.clone()); hook_ctx.cwd = self .hook_work_dir .as_ref() @@ -110,8 +103,7 @@ impl RunLifecycle for HookLifecycle { } else { HookEvent::StageComplete }; - let mut hook_ctx = - HookContext::new(hook_event, self.run_id.clone(), self.graph_name.clone()); + let mut hook_ctx = HookContext::new(hook_event, self.run_id, self.graph_name.clone()); set_hook_node(&mut hook_ctx, node.inner()); hook_ctx.status = Some(outcome.status.to_string()); hook_ctx.failure_reason = outcome.failure_reason().map(String::from); @@ -126,7 +118,7 @@ impl RunLifecycle for HookLifecycle { ) -> CoreResult { let mut hook_ctx = HookContext::new( HookEvent::EdgeSelected, - self.run_id.clone(), + self.run_id, self.graph_name.clone(), ); hook_ctx.edge_from = Some(ctx.from.to_string()); @@ -155,7 +147,7 @@ impl RunLifecycle for HookLifecycle { ) -> CoreResult<()> { let mut hook_ctx = HookContext::new( HookEvent::CheckpointSaved, - self.run_id.clone(), + self.run_id, self.graph_name.clone(), ); hook_ctx.node_id = Some(node.inner().id.clone()); @@ -168,22 +160,16 @@ impl RunLifecycle for HookLifecycle { return; } if outcome.status == StageStatus::Success || outcome.status == StageStatus::PartialSuccess { - let hook_ctx = HookContext::new( - HookEvent::RunComplete, - self.run_id.clone(), - self.graph_name.clone(), - ); + let hook_ctx = + HookContext::new(HookEvent::RunComplete, self.run_id, self.graph_name.clone()); let _ = self.run_hook(&hook_ctx).await; } else { let error_msg = outcome .failure .as_ref() .map_or_else(|| "run failed".to_string(), |f| f.message.clone()); - let mut hook_ctx = HookContext::new( - HookEvent::RunFailed, - self.run_id.clone(), - self.graph_name.clone(), - ); + let mut hook_ctx = + HookContext::new(HookEvent::RunFailed, self.run_id, self.graph_name.clone()); hook_ctx.failure_reason = Some(error_msg); let _ = self.run_hook(&hook_ctx).await; } diff --git a/lib/crates/fabro-workflows/src/lifecycle/mod.rs b/lib/crates/fabro-workflows/src/lifecycle/mod.rs index 0ecfcbf30..cbbca5aa0 100644 --- a/lib/crates/fabro-workflows/src/lifecycle/mod.rs +++ b/lib/crates/fabro-workflows/src/lifecycle/mod.rs @@ -117,7 +117,7 @@ impl WorkflowLifecycle { let event = EventLifecycle { emitter: Arc::clone(emitter), graph_name: graph.name.clone(), - run_id: run_options.run_id.clone(), + run_id: run_options.run_id, run_start: Mutex::new(Instant::now()), restarted_from: Arc::clone(&restarted_from), base_branch: run_options.base_branch.clone(), @@ -134,7 +134,7 @@ impl WorkflowLifecycle { hook_runner, sandbox: Arc::clone(sandbox), hook_work_dir: working_directory.clone().map(PathBuf::from), - run_id: run_options.run_id.clone(), + run_id: run_options.run_id, graph_name: graph.name.clone(), }; @@ -142,7 +142,7 @@ impl WorkflowLifecycle { let disk = DiskLifecycle { run_dir: run_dir.clone(), - run_id: run_options.run_id.clone(), + run_id: run_options.run_id, run_store: Arc::clone(&run_store), graph: Arc::clone(&graph), run_options: Arc::clone(run_options), @@ -158,7 +158,7 @@ impl WorkflowLifecycle { artifact_store: Arc::clone(&artifact_store), emitter: Arc::clone(emitter), run_dir: run_dir.clone(), - run_id: run_options.run_id.clone(), + run_id: run_options.run_id, run_store, run_options: Arc::clone(run_options), start_node_id, @@ -189,7 +189,7 @@ impl WorkflowLifecycle { checkpoint_git_result, is_initial_resume: AtomicBool::new(is_resume), graph, - run_id: run_options.run_id.clone(), + run_id: run_options.run_id, working_directory, } } diff --git a/lib/crates/fabro-workflows/src/operations/fork.rs b/lib/crates/fabro-workflows/src/operations/fork.rs index d75f89724..c4c325caa 100644 --- a/lib/crates/fabro-workflows/src/operations/fork.rs +++ b/lib/crates/fabro-workflows/src/operations/fork.rs @@ -205,7 +205,7 @@ mod tests { fn setup_source_run(store: &Store, run_id: &RunId, nodes: &[&str]) -> Vec { let sig = test_sig(); - let run_branch = format!("{}{run_id}", RUN_BRANCH_PREFIX); + let run_branch = format!("{RUN_BRANCH_PREFIX}{run_id}"); let empty_tree = store.write_empty_tree().unwrap(); let mut run_oids = Vec::new(); let mut parent: Option = None; @@ -265,7 +265,7 @@ mod tests { ) .unwrap(); - let new_run_branch = format!("{}{new_run_id}", RUN_BRANCH_PREFIX); + let new_run_branch = format!("{RUN_BRANCH_PREFIX}{new_run_id}"); let new_meta_branch = MetadataStore::branch_name(&new_run_id.to_string()); assert!(store.resolve_ref(&new_run_branch).unwrap().is_some()); diff --git a/lib/crates/fabro-workflows/src/operations/start.rs b/lib/crates/fabro-workflows/src/operations/start.rs index 298a197ee..f66dcd219 100644 --- a/lib/crates/fabro-workflows/src/operations/start.rs +++ b/lib/crates/fabro-workflows/src/operations/start.rs @@ -317,14 +317,14 @@ impl RunSession { SandboxProvider::Daytona => SandboxSpec::Daytona { config: resolve_daytona_config(&settings).unwrap_or_default(), github_app: services.github_app.clone(), - run_id: Some(record.run_id.clone()), + run_id: Some(record.run_id), clone_branch: detected_base_branch.or_else(|| record.base_branch.clone()), }, #[cfg(feature = "exedev")] SandboxProvider::Exe => SandboxSpec::Exe { config: resolve_exe_config(&settings).unwrap_or_default(), clone_params: detect_clone_params(&working_directory), - run_id: Some(record.run_id.clone()), + run_id: Some(record.run_id), github_app: services.github_app.clone(), mgmt_destination: "exe.dev".to_string(), }, @@ -341,7 +341,7 @@ impl RunSession { ) })?, clone_params: detect_clone_params(&working_directory), - run_id: Some(record.run_id.clone()), + run_id: Some(record.run_id), github_app: services.github_app.clone(), }, }; @@ -479,7 +479,7 @@ impl RunSession { settings: record.settings.clone(), run_dir: persisted.run_dir().to_path_buf(), cancel_token: self.cancel_token, - run_id: record.run_id.clone(), + run_id: record.run_id, labels: record.labels.clone(), workflow_slug: record.workflow_slug.clone(), github_app: self.github_app.clone(), @@ -509,11 +509,11 @@ impl RunSession { } let store_progress_logger = - StoreProgressLogger::new(Arc::clone(&self.run_store), record.run_id.clone()); + StoreProgressLogger::new(Arc::clone(&self.run_store), record.run_id); store_progress_logger.register(self.emitter.as_ref()); let init_options = InitOptions { - run_id: record.run_id.clone(), + run_id: record.run_id, run_store: Arc::clone(&self.run_store), dry_run: run_options.dry_run_enabled(), emitter: self.emitter, @@ -555,7 +555,7 @@ impl RunSession { ); let retro_opts = RetroOptions { - run_id: executed.run_options.run_id.clone(), + run_id: executed.run_options.run_id, run_store: Arc::clone(&executed.run_store), workflow_name: executed.graph.name.clone(), goal: executed.graph.goal().to_string(), @@ -576,7 +576,7 @@ impl RunSession { let finalize_opts = FinalizeOptions { run_dir: retroed.run_options.run_dir.clone(), - run_id: retroed.run_options.run_id.clone(), + run_id: retroed.run_options.run_id, run_store: Arc::clone(&retroed.run_store), workflow_name: retroed.graph.name.clone(), hook_runner: retroed.hook_runner.clone(), @@ -740,7 +740,7 @@ impl Drop for DetachedRunCompletionGuard { ); } let run_store = Arc::clone(&self.run_store); - let run_id = self.run_id.clone(); + let run_id = self.run_id; if let Ok(handle) = Handle::try_current() { handle.spawn(async move { let _ = run_store diff --git a/lib/crates/fabro-workflows/src/pipeline/execute.rs b/lib/crates/fabro-workflows/src/pipeline/execute.rs index 67496f92a..98f95de77 100644 --- a/lib/crates/fabro-workflows/src/pipeline/execute.rs +++ b/lib/crates/fabro-workflows/src/pipeline/execute.rs @@ -60,7 +60,7 @@ pub async fn execute(init: Initialized) -> Executed { let git_state = run_options.git.as_ref().and_then(|git| { let base_sha = git.base_sha.clone()?; Some(Arc::new(GitState { - run_id: run_options.run_id.clone(), + run_id: run_options.run_id, base_sha, run_branch: git.run_branch.clone(), meta_branch: git.meta_branch.clone(), diff --git a/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs b/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs index 02f68ce32..0d7890d8c 100644 --- a/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs +++ b/lib/crates/fabro-workflows/src/pipeline/execute/tests.rs @@ -1,3 +1,5 @@ +#![allow(clippy::absolute_paths, clippy::large_futures)] + use std::collections::HashMap; use std::path::{Path, PathBuf}; use std::sync::Arc; @@ -227,7 +229,7 @@ async fn run_with_lifecycle( ) -> Result { std::fs::create_dir_all(&run_options.run_dir).unwrap(); let run_dir = run_options.run_dir.clone(); - let run_id = run_options.run_id.clone(); + let run_id = run_options.run_id; let initialized = initialize( persisted_workflow(graph.clone(), String::new(), &run_dir, run_id), InitOptions { diff --git a/lib/crates/fabro-workflows/src/pipeline/finalize.rs b/lib/crates/fabro-workflows/src/pipeline/finalize.rs index 40be8ce20..a37474e08 100644 --- a/lib/crates/fabro-workflows/src/pipeline/finalize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/finalize.rs @@ -422,7 +422,7 @@ pub async fn finalize( } Ok(Concluded { - run_id: run_options.run_id.clone(), + run_id: run_options.run_id, outcome, conclusion, pushed_branch: run_options.git.as_ref().and_then(|g| g.run_branch.clone()), diff --git a/lib/crates/fabro-workflows/src/pipeline/initialize.rs b/lib/crates/fabro-workflows/src/pipeline/initialize.rs index 41cba104d..640738518 100644 --- a/lib/crates/fabro-workflows/src/pipeline/initialize.rs +++ b/lib/crates/fabro-workflows/src/pipeline/initialize.rs @@ -476,7 +476,7 @@ pub async fn initialize( let hook_ctx = HookContext::new( HookEvent::SandboxReady, - options.run_options.run_id.clone(), + options.run_options.run_id, graph.name.clone(), ); let decision = run_hooks( @@ -670,11 +670,11 @@ mod tests { } fn simple_graph() -> (Graph, String) { - let source = r#"digraph test { + let source = r"digraph test { start [shape=Mdiamond]; exit [shape=Msquare]; start -> exit; -}"# +}" .to_string(); let mut graph = Graph::new("test"); let mut start = Node::new("start"); diff --git a/lib/crates/fabro-workflows/src/run_options.rs b/lib/crates/fabro-workflows/src/run_options.rs index 4c8f1cb93..a39133aef 100644 --- a/lib/crates/fabro-workflows/src/run_options.rs +++ b/lib/crates/fabro-workflows/src/run_options.rs @@ -7,6 +7,8 @@ use fabro_config::FabroSettings; use fabro_config::run::PullRequestSettings; use fabro_types::RunId; +use crate::git::{GitAuthor, git_author_from_settings}; + /// Git checkpoint options for a workflow run. #[derive(Clone)] pub struct GitCheckpointOptions { @@ -48,8 +50,8 @@ impl RunOptions { &self.settings.checkpoint.exclude_globs } - pub fn git_author(&self) -> crate::git::GitAuthor { - crate::git::git_author_from_settings(&self.settings) + pub fn git_author(&self) -> GitAuthor { + git_author_from_settings(&self.settings) } /// PR config (already normalized — disabled entries stripped at construction). diff --git a/lib/crates/fabro-workflows/src/transforms/import.rs b/lib/crates/fabro-workflows/src/transforms/import.rs index c73ccecc4..0fd9fe2d8 100644 --- a/lib/crates/fabro-workflows/src/transforms/import.rs +++ b/lib/crates/fabro-workflows/src/transforms/import.rs @@ -1080,11 +1080,11 @@ mod tests { let dir = tempfile::tempdir().unwrap(); write_file( &dir.path().join("noop.fabro"), - r#"digraph noop { + r"digraph noop { start [shape=Mdiamond] exit [shape=Msquare] start -> exit - }"#, + }", ); let graph = apply_import( @@ -1114,11 +1114,11 @@ mod tests { let dir = tempfile::tempdir().unwrap(); write_file( &dir.path().join("noop.fabro"), - r#"digraph noop { + r"digraph noop { start [shape=Mdiamond] exit [shape=Msquare] start -> exit - }"#, + }", ); let graph = apply_import( diff --git a/lib/crates/fabro-workflows/tests/it/attractor_compat.rs b/lib/crates/fabro-workflows/tests/it/attractor_compat.rs index 14e94edee..8498b30be 100644 --- a/lib/crates/fabro-workflows/tests/it/attractor_compat.rs +++ b/lib/crates/fabro-workflows/tests/it/attractor_compat.rs @@ -1,3 +1,5 @@ +#![allow(clippy::absolute_paths)] + use std::path::Path; use fabro_graphviz::parser::parse; diff --git a/lib/crates/fabro-workflows/tests/it/cp_integration.rs b/lib/crates/fabro-workflows/tests/it/cp_integration.rs index a8929cab9..50620a274 100644 --- a/lib/crates/fabro-workflows/tests/it/cp_integration.rs +++ b/lib/crates/fabro-workflows/tests/it/cp_integration.rs @@ -4,6 +4,8 @@ //! Docker tests require a Docker daemon and are marked `#[ignore]`. //! Run Docker tests with: `cargo test --package arc-workflows --test cp_integration -- --ignored` +#![allow(clippy::ignore_without_reason)] + use fabro_sandbox::reconnect::reconnect; use fabro_sandbox::{SandboxRecord, SandboxRecordExt}; diff --git a/lib/crates/fabro-workflows/tests/it/daytona_integration.rs b/lib/crates/fabro-workflows/tests/it/daytona_integration.rs index 5c7b45930..95553f590 100644 --- a/lib/crates/fabro-workflows/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflows/tests/it/daytona_integration.rs @@ -3,6 +3,14 @@ //! These tests require a `DAYTONA_API_KEY` environment variable and network access. //! Run with: `cargo test --package arc-workflows -- --ignored daytona` +#![allow( + clippy::absolute_paths, + clippy::format_push_string, + clippy::ignore_without_reason, + clippy::items_after_statements, + clippy::print_stderr +)] + use std::collections::HashMap; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher}; diff --git a/lib/crates/fabro-workflows/tests/it/integration.rs b/lib/crates/fabro-workflows/tests/it/integration.rs index 45708fffc..b851eeebc 100644 --- a/lib/crates/fabro-workflows/tests/it/integration.rs +++ b/lib/crates/fabro-workflows/tests/it/integration.rs @@ -1,3 +1,15 @@ +#![allow( + clippy::absolute_paths, + clippy::get_unwrap, + clippy::ignore_without_reason, + clippy::items_after_statements, + clippy::manual_let_else, + clippy::print_stderr, + clippy::unnecessary_box_returns, + clippy::unnecessary_literal_bound, + clippy::unreadable_literal +)] + use std::collections::VecDeque; use std::collections::hash_map::DefaultHasher; use std::hash::{Hash, Hasher};