From 1048534e2cbc02b3babb7d64a5bd162736d7299a Mon Sep 17 00:00:00 2001 From: Bryan Helmkamp Date: Thu, 16 Apr 2026 18:59:11 -0400 Subject: [PATCH] ci: switch clippy to pinned nightly, clean up workspace lints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - rust.yml: move clippy to nightly-2026-04-14 (was stable); also pin fmt to the same nightly date for consistency. Both jobs now use the dated nightly and the run-step uses `cargo +nightly-2026-04-14 ...`. - AGENTS.md: update developer commands to match CI. - Duration constructors: replace `Duration::from_secs(N * 60)` / `Duration::from_millis(N * 1000)` with `from_mins` / `from_secs` / `from_hours` across the workspace to satisfy clippy's new `duration_suboptimal_units` lint. std::time::Duration only — custom `settings::duration::Duration` sites kept on `from_secs`. - map/unwrap_or cleanup: `.map(f).unwrap_or(v)` → `.map_or(v, f)`, `.map(f).unwrap_or(false)` on Result → `.is_ok_and(f)`, per `clippy::map_unwrap_or`. - Misc lints: collapse nested `if` into match guard in handler/llm/api.rs and run_state.rs; replace `columns.len() > 0` with `!columns.is_empty()`; switch a pair of `sort_by` calls to `sort_by_key`. Co-Authored-By: Claude Opus 4.7 (1M context) --- .github/workflows/rust.yml | 9 +++++--- AGENTS.md | 6 ++--- lib/crates/fabro-agent/src/mcp_integration.rs | 2 +- lib/crates/fabro-cli/src/commands/install.rs | 3 +-- .../fabro-cli/src/commands/run/attach.rs | 6 ++--- lib/crates/fabro-cli/src/commands/run/wait.rs | 3 +-- .../fabro-cli/src/commands/uninstall.rs | 2 +- lib/crates/fabro-cli/tests/it/cmd/exec.rs | 10 ++++----- .../fabro-cli/tests/it/scenario/exec.rs | 2 +- lib/crates/fabro-cli/tests/it/scenario/mod.rs | 4 ++-- lib/crates/fabro-cli/tests/it/workflow/mod.rs | 4 ++-- lib/crates/fabro-config/src/envfile.rs | 2 +- lib/crates/fabro-core/src/executor.rs | 6 ++--- .../fabro-graphviz/src/parser/semantic.rs | 12 +++++----- lib/crates/fabro-interview/src/console.rs | 2 +- lib/crates/fabro-interview/src/lib.rs | 2 +- lib/crates/fabro-llm/src/retry.rs | 4 ++-- lib/crates/fabro-llm/src/types.rs | 6 ++--- lib/crates/fabro-retro/src/retro_agent.rs | 2 +- lib/crates/fabro-sandbox/src/daytona/mod.rs | 2 +- lib/crates/fabro-server/src/server.rs | 14 +++++------- lib/crates/fabro-server/src/web_auth.rs | 3 +-- lib/crates/fabro-store/src/run_state.rs | 18 +++++---------- lib/crates/fabro-store/src/slate/mod.rs | 2 +- lib/crates/fabro-telemetry/src/buffer.rs | 8 +++---- lib/crates/fabro-types/src/graph.rs | 4 ++-- lib/crates/fabro-util/src/backoff.rs | 4 ++-- lib/crates/fabro-workflow/src/condition.rs | 4 +--- lib/crates/fabro-workflow/src/event.rs | 2 +- .../fabro-workflow/src/handler/command.rs | 2 +- .../fabro-workflow/src/handler/llm/api.rs | 8 +++---- .../src/handler/manager_loop.rs | 2 +- .../fabro-workflow/src/lifecycle/artifact.rs | 3 +-- .../fabro-workflow/src/operations/create.rs | 3 +-- .../fabro-workflow/src/operations/start.rs | 3 +-- .../fabro-workflow/src/pipeline/initialize.rs | 2 +- lib/crates/fabro-workflow/src/retry.rs | 8 +++---- lib/crates/fabro-workflow/src/run_options.rs | 3 +-- .../tests/it/daytona_integration.rs | 4 ++-- .../fabro-workflow/tests/it/integration.rs | 22 +++++++++---------- test/twin/openai/tests/debug_ui.rs | 3 +-- 41 files changed, 95 insertions(+), 116 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 574389ae4..143047768 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -45,9 +45,9 @@ jobs: persist-credentials: false - uses: dtolnay/rust-toolchain@631a55b12751854ce901bb631d5902ceb48146f7 # stable with: - toolchain: nightly + toolchain: nightly-2026-04-14 components: rustfmt - - run: cargo +nightly fmt --check --all + - run: cargo +nightly-2026-04-14 fmt --check --all clippy: name: Clippy @@ -57,10 +57,13 @@ jobs: with: persist-credentials: false - uses: dtolnay/rust-toolchain@631a55b12751854ce901bb631d5902ceb48146f7 # stable + with: + toolchain: nightly-2026-04-14 + components: clippy - uses: Swatinem/rust-cache@779680da715d629ac1d338a641029a2f4372abb5 # v2 with: cache-on-failure: true - - run: cargo clippy --workspace --all-targets -- -D warnings + - run: cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings test: name: Test (Linux) diff --git a/AGENTS.md b/AGENTS.md index 9412edeef..e58b31bdf 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -11,9 +11,9 @@ This file provides guidance to Claude Code (claude.ai/code) when working with co - `cargo nextest run -p fabro-workflow -- test_name` — run a single test - `set -a && source .env && set +a && cargo nextest run --workspace --profile e2e --run-ignored only` — run all E2E live tests (requires credentials in `.env`, see `.env.example`) - `set -a && source .env && set +a && cargo nextest run -p fabro-llm --profile e2e --run-ignored only` — run E2E tests for a single crate -- `cargo +nightly fmt --check --all` — check formatting (nightly required for rustfmt config) -- `cargo +nightly fmt --all` — auto-format -- `cargo clippy --workspace -- -D warnings` — lint +- `cargo +nightly-2026-04-14 fmt --check --all` — check formatting (pinned nightly required for rustfmt config; CI uses the same date) +- `cargo +nightly-2026-04-14 fmt --all` — auto-format +- `cargo +nightly-2026-04-14 clippy --workspace --all-targets -- -D warnings` — lint (CI runs nightly clippy to match; install with `rustup toolchain install nightly-2026-04-14 --profile minimal --component clippy,rustfmt`) macOS note: if `cargo nextest run` fails with `Too many open files (os error 24)` / `EMFILE`, raise the shell's soft FD limit before running tests, for example `ulimit -n 4096 && cargo nextest run --workspace`. Some terminals and inherited agent sessions start with `ulimit -n 256`, which is too low for the shared CLI test daemon under parallel nextest load. diff --git a/lib/crates/fabro-agent/src/mcp_integration.rs b/lib/crates/fabro-agent/src/mcp_integration.rs index 65d8a3fef..da6154cbc 100644 --- a/lib/crates/fabro-agent/src/mcp_integration.rs +++ b/lib/crates/fabro-agent/src/mcp_integration.rs @@ -14,7 +14,7 @@ pub fn make_mcp_tools(manager: &Arc) -> Vec bool { .stderr(Stdio::null()) .status() .await - .map(|s| s.success()) - .unwrap_or(false) + .is_ok_and(|s| s.success()) } // --------------------------------------------------------------------------- diff --git a/lib/crates/fabro-cli/src/commands/run/attach.rs b/lib/crates/fabro-cli/src/commands/run/attach.rs index 49ab984cf..432f59295 100644 --- a/lib/crates/fabro-cli/src/commands/run/attach.rs +++ b/lib/crates/fabro-cli/src/commands/run/attach.rs @@ -75,13 +75,11 @@ pub(crate) async fn attach_run_with_client( let state = client.get_run_state(run_id).await?; let auto_approve = state.run.as_ref().is_some_and(|record| { fabro_config::resolve_run_from_file(&record.settings) - .map(|settings| settings.execution.approval == ApprovalMode::Auto) - .unwrap_or(false) + .is_ok_and(|settings| settings.execution.approval == ApprovalMode::Auto) }); let verbose = state.run.as_ref().is_some_and(|record| { fabro_config::resolve_cli_from_file(&record.settings) - .map(|settings| settings.output.verbosity == OutputVerbosity::Verbose) - .unwrap_or(false) + .is_ok_and(|settings| settings.output.verbosity == OutputVerbosity::Verbose) }); let events = client.list_run_events(run_id, None, None).await?; let replay_events = events.clone(); diff --git a/lib/crates/fabro-cli/src/commands/run/wait.rs b/lib/crates/fabro-cli/src/commands/run/wait.rs index 33bfaf18a..5ef38e105 100644 --- a/lib/crates/fabro-cli/src/commands/run/wait.rs +++ b/lib/crates/fabro-cli/src/commands/run/wait.rs @@ -294,8 +294,7 @@ mod tests { let status = match std::fs::read_to_string(std::path::Path::new("/nonexistent/status.json")) { Ok(data) => serde_json::from_str::(&data) - .map(|record| record.status) - .unwrap_or(RunStatus::Dead), + .map_or(RunStatus::Dead, |record| record.status), Err(_) => RunStatus::Dead, }; assert_eq!(status, RunStatus::Dead); diff --git a/lib/crates/fabro-cli/src/commands/uninstall.rs b/lib/crates/fabro-cli/src/commands/uninstall.rs index c1d56b49f..5e85a62fb 100644 --- a/lib/crates/fabro-cli/src/commands/uninstall.rs +++ b/lib/crates/fabro-cli/src/commands/uninstall.rs @@ -94,7 +94,7 @@ fn dir_size(path: &Path) -> u64 { if ft.is_dir() { total += dir_size(&entry.path()); } else { - total += entry.metadata().map(|m| m.len()).unwrap_or(0); + total += entry.metadata().map_or(0, |m| m.len()); } } } diff --git a/lib/crates/fabro-cli/tests/it/cmd/exec.rs b/lib/crates/fabro-cli/tests/it/cmd/exec.rs index 4cc2ab30b..57818b031 100644 --- a/lib/crates/fabro-cli/tests/it/cmd/exec.rs +++ b/lib/crates/fabro-cli/tests/it/cmd/exec.rs @@ -264,7 +264,7 @@ fn exec_creates_file() { "claude-haiku-4-5", "Create a file called hello.txt containing exactly 'Hello'", ]) - .timeout(std::time::Duration::from_secs(120)) + .timeout(std::time::Duration::from_mins(2)) .assert() .success(); let path = context.temp_dir.join("hello.txt"); @@ -291,7 +291,7 @@ fn exec_shell_command() { "claude-haiku-4-5", "Run the shell command `echo arc_test_marker_42` and tell me what it printed", ]) - .timeout(std::time::Duration::from_secs(120)) + .timeout(std::time::Duration::from_mins(2)) .assert() .success(); } @@ -311,7 +311,7 @@ fn exec_read_only_blocks_write() { "claude-haiku-4-5", "Create a file called forbidden.txt containing 'should not exist'", ]) - .timeout(std::time::Duration::from_secs(120)) + .timeout(std::time::Duration::from_mins(2)) .assert() .success(); assert!( @@ -337,7 +337,7 @@ fn exec_json_output_format() { "claude-haiku-4-5", "Create a file called test.txt containing 'test'", ]) - .timeout(std::time::Duration::from_secs(120)) + .timeout(std::time::Duration::from_mins(2)) .assert() .success() .get_output() @@ -372,7 +372,7 @@ fn exec_read_and_edit() { "claude-haiku-4-5", "Read data.txt then replace its entire content with 'new content'", ]) - .timeout(std::time::Duration::from_secs(120)) + .timeout(std::time::Duration::from_mins(2)) .assert() .success(); let content = diff --git a/lib/crates/fabro-cli/tests/it/scenario/exec.rs b/lib/crates/fabro-cli/tests/it/scenario/exec.rs index fba7ca76c..ba64536e3 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/exec.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/exec.rs @@ -17,7 +17,7 @@ fn test_exec_creates_file() { "claude-haiku-4-5", "Create a file called hello.txt containing exactly 'Hello from exec scenario'", ]); - cmd.timeout(Duration::from_secs(120)); + cmd.timeout(Duration::from_mins(2)); cmd.assert().success(); let hello = context.temp_dir.join("hello.txt"); diff --git a/lib/crates/fabro-cli/tests/it/scenario/mod.rs b/lib/crates/fabro-cli/tests/it/scenario/mod.rs index ebaf1c0f7..7792bbc65 100644 --- a/lib/crates/fabro-cli/tests/it/scenario/mod.rs +++ b/lib/crates/fabro-cli/tests/it/scenario/mod.rs @@ -66,7 +66,7 @@ pub(super) fn run_state(run_dir: &Path) -> RunProjection { pub(super) fn timeout_for(sandbox: &str) -> Duration { match sandbox { - "daytona" => Duration::from_secs(600), - _ => Duration::from_secs(180), + "daytona" => Duration::from_mins(10), + _ => Duration::from_mins(3), } } diff --git a/lib/crates/fabro-cli/tests/it/workflow/mod.rs b/lib/crates/fabro-cli/tests/it/workflow/mod.rs index efbcc79e1..3f50091e8 100644 --- a/lib/crates/fabro-cli/tests/it/workflow/mod.rs +++ b/lib/crates/fabro-cli/tests/it/workflow/mod.rs @@ -167,7 +167,7 @@ pub(super) use sandbox_tests; pub(super) fn timeout_for(sandbox: &str) -> Duration { match sandbox { - "daytona" => Duration::from_secs(600), - _ => Duration::from_secs(180), + "daytona" => Duration::from_mins(10), + _ => Duration::from_mins(3), } } diff --git a/lib/crates/fabro-config/src/envfile.rs b/lib/crates/fabro-config/src/envfile.rs index 67a02bdf7..90f1671d9 100644 --- a/lib/crates/fabro-config/src/envfile.rs +++ b/lib/crates/fabro-config/src/envfile.rs @@ -67,7 +67,7 @@ pub fn write_env_file(path: &Path, entries: &HashMap) -> io::Res let tmp_path = parent.join(format!(".{file_name}.tmp-{}", ulid::Ulid::new())); let mut data = entries.iter().collect::>(); - data.sort_by(|(left, _), (right, _)| left.cmp(right)); + data.sort_by_key(|(left, _)| *left); let contents = data .into_iter() .map(|(key, value)| format!("{key}={}", encode_value(value))) diff --git a/lib/crates/fabro-core/src/executor.rs b/lib/crates/fabro-core/src/executor.rs index 60053930b..0b02f003f 100644 --- a/lib/crates/fabro-core/src/executor.rs +++ b/lib/crates/fabro-core/src/executor.rs @@ -1281,7 +1281,7 @@ mod tests { backoff: BackoffPolicy { initial_delay: Duration::from_secs(5), factor: 2.0, - max_delay: Duration::from_secs(60), + max_delay: Duration::from_mins(1), jitter: false, }, }), @@ -2049,9 +2049,9 @@ mod tests { RetryPolicy { max_attempts: 3, backoff: BackoffPolicy { - initial_delay: Duration::from_secs(60), + initial_delay: Duration::from_mins(1), factor: 1.0, - max_delay: Duration::from_secs(60), + max_delay: Duration::from_mins(1), jitter: false, }, } diff --git a/lib/crates/fabro-graphviz/src/parser/semantic.rs b/lib/crates/fabro-graphviz/src/parser/semantic.rs index 64d7b6954..6c52fc691 100644 --- a/lib/crates/fabro-graphviz/src/parser/semantic.rs +++ b/lib/crates/fabro-graphviz/src/parser/semantic.rs @@ -283,7 +283,7 @@ mod tests { fn convert_ast_duration_str() { assert_eq!( convert_value(&AstValue::Str("900s".into())), - AttrValue::Duration(Duration::from_secs(900)) + AttrValue::Duration(Duration::from_mins(15)) ); assert_eq!( convert_value(&AstValue::Str("250ms".into())), @@ -291,15 +291,15 @@ mod tests { ); assert_eq!( convert_value(&AstValue::Str("15m".into())), - AttrValue::Duration(Duration::from_secs(900)) + AttrValue::Duration(Duration::from_mins(15)) ); assert_eq!( convert_value(&AstValue::Str("2h".into())), - AttrValue::Duration(Duration::from_secs(7200)) + AttrValue::Duration(Duration::from_hours(2)) ); assert_eq!( convert_value(&AstValue::Str("1d".into())), - AttrValue::Duration(Duration::from_secs(86400)) + AttrValue::Duration(Duration::from_hours(24)) ); } @@ -413,7 +413,7 @@ mod tests { ); assert_eq!( plan.attrs.get("timeout").and_then(AttrValue::as_duration), - Some(Duration::from_secs(900)) + Some(Duration::from_mins(15)) ); let implement = &graph.nodes["implement"]; @@ -422,7 +422,7 @@ mod tests { .attrs .get("timeout") .and_then(AttrValue::as_duration), - Some(Duration::from_secs(1800)) + Some(Duration::from_mins(30)) ); } diff --git a/lib/crates/fabro-interview/src/console.rs b/lib/crates/fabro-interview/src/console.rs index e3217c3b5..311cc2f85 100644 --- a/lib/crates/fabro-interview/src/console.rs +++ b/lib/crates/fabro-interview/src/console.rs @@ -236,7 +236,7 @@ impl Interviewer for ConsoleInterviewer { // Non-TTY fallback: line-based stdin reading let s = self.styles; - eprintln!("{} {}", s.bold_cyan.apply_to("?"), question.text,); + eprintln!("{} {}", s.bold_cyan.apply_to("?"), question.text); match question.question_type { QuestionType::MultipleChoice | QuestionType::MultiSelect => { diff --git a/lib/crates/fabro-interview/src/lib.rs b/lib/crates/fabro-interview/src/lib.rs index c057b2fc4..c311a5218 100644 --- a/lib/crates/fabro-interview/src/lib.rs +++ b/lib/crates/fabro-interview/src/lib.rs @@ -357,7 +357,7 @@ mod tests { #[async_trait] impl Interviewer for SlowInterviewer { async fn ask(&self, _question: Question) -> Answer { - time::sleep(std::time::Duration::from_secs(60)).await; + time::sleep(std::time::Duration::from_mins(1)).await; Answer::yes() } } diff --git a/lib/crates/fabro-llm/src/retry.rs b/lib/crates/fabro-llm/src/retry.rs index 0feddc07a..ef8d049ae 100644 --- a/lib/crates/fabro-llm/src/retry.rs +++ b/lib/crates/fabro-llm/src/retry.rs @@ -80,7 +80,7 @@ mod tests { BackoffPolicy { initial_delay: Duration::from_micros(1), factor: 2.0, - max_delay: Duration::from_secs(60), + max_delay: Duration::from_mins(1), jitter: false, } } @@ -249,7 +249,7 @@ mod tests { backoff: BackoffPolicy { initial_delay: Duration::from_secs(10), // high, but retry_after is low factor: 2.0, - max_delay: Duration::from_secs(60), + max_delay: Duration::from_mins(1), jitter: false, }, ..Default::default() diff --git a/lib/crates/fabro-llm/src/types.rs b/lib/crates/fabro-llm/src/types.rs index 89255b90b..08fc9be6e 100644 --- a/lib/crates/fabro-llm/src/types.rs +++ b/lib/crates/fabro-llm/src/types.rs @@ -724,7 +724,7 @@ impl Default for RetryPolicy { backoff: BackoffPolicy { initial_delay: std::time::Duration::from_secs(1), factor: 2.0, - max_delay: std::time::Duration::from_secs(60), + max_delay: std::time::Duration::from_mins(1), jitter: true, }, on_retry: None, @@ -1112,7 +1112,7 @@ mod tests { backoff: BackoffPolicy { initial_delay: Duration::from_secs(1), factor: 2.0, - max_delay: Duration::from_secs(60), + max_delay: Duration::from_mins(1), jitter: false, }, ..Default::default() @@ -1148,7 +1148,7 @@ mod tests { backoff: BackoffPolicy { initial_delay: Duration::from_secs(1), factor: 2.0, - max_delay: Duration::from_secs(60), + max_delay: Duration::from_mins(1), jitter: true, }, ..Default::default() diff --git a/lib/crates/fabro-retro/src/retro_agent.rs b/lib/crates/fabro-retro/src/retro_agent.rs index e0a08e27b..2529c436a 100644 --- a/lib/crates/fabro-retro/src/retro_agent.rs +++ b/lib/crates/fabro-retro/src/retro_agent.rs @@ -177,7 +177,7 @@ pub async fn run_retro_agent( let config = SessionOptions { max_tool_rounds_per_input: 20, - wall_clock_timeout: Some(Duration::from_secs(180)), + wall_clock_timeout: Some(Duration::from_mins(3)), // Disable features not needed for retro analysis enable_context_compaction: false, skill_dirs: Some(vec![]), diff --git a/lib/crates/fabro-sandbox/src/daytona/mod.rs b/lib/crates/fabro-sandbox/src/daytona/mod.rs index 6f0f8f004..b9637b652 100644 --- a/lib/crates/fabro-sandbox/src/daytona/mod.rs +++ b/lib/crates/fabro-sandbox/src/daytona/mod.rs @@ -281,7 +281,7 @@ impl DaytonaSandbox { use daytona_api_client::models::SnapshotState; let mut delay = std::time::Duration::from_secs(2); let max_delay = std::time::Duration::from_secs(30); - let deadline = Instant::now() + std::time::Duration::from_secs(600); + let deadline = Instant::now() + std::time::Duration::from_mins(10); while Instant::now() < deadline { time::sleep(delay).await; diff --git a/lib/crates/fabro-server/src/server.rs b/lib/crates/fabro-server/src/server.rs index 86f0cc6ab..8ee6f5311 100644 --- a/lib/crates/fabro-server/src/server.rs +++ b/lib/crates/fabro-server/src/server.rs @@ -687,8 +687,7 @@ impl AppState { fn issue_artifact_upload_token(&self, run_id: &RunId) -> Result { let now = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) - .map(|duration| duration.as_secs()) - .unwrap_or(0); + .map_or(0, |duration| duration.as_secs()); let claims = ArtifactUploadClaims { iss: ARTIFACT_UPLOAD_TOKEN_ISSUER.to_string(), iat: now, @@ -1242,12 +1241,9 @@ async fn get_system_info( } fn system_features(settings: &SettingsLayer) -> SystemFeatures { - let session_sandboxes = fabro_config::resolve_features_from_file(settings) - .map(|s| s.session_sandboxes) - .unwrap_or(false); - let retros = fabro_config::resolve_run_from_file(settings) - .map(|s| s.execution.retros) - .unwrap_or(false); + let session_sandboxes = + fabro_config::resolve_features_from_file(settings).is_ok_and(|s| s.session_sandboxes); + let retros = fabro_config::resolve_run_from_file(settings).is_ok_and(|s| s.execution.retros); SystemFeatures { session_sandboxes: Some(session_sandboxes), retros: Some(retros), @@ -9238,7 +9234,7 @@ timeout = "30s" // Verify columns are included in the response let columns = body["columns"].as_array().expect("columns should be array"); - assert!(columns.len() > 0); + assert!(!columns.is_empty()); assert!(columns.iter().any(|c| c["id"].as_str() == Some("waiting"))); assert!( columns diff --git a/lib/crates/fabro-server/src/web_auth.rs b/lib/crates/fabro-server/src/web_auth.rs index ed999c482..f159f3c0a 100644 --- a/lib/crates/fabro-server/src/web_auth.rs +++ b/lib/crates/fabro-server/src/web_auth.rs @@ -218,8 +218,7 @@ fn session_cookie_secure(state: &AppState) -> bool { .web .url .resolve(|name| std::env::var(name).ok()) - .map(|resolved| resolved.value.starts_with("https://")) - .unwrap_or(false) + .is_ok_and(|resolved| resolved.value.starts_with("https://")) } async fn login_dev_token( diff --git a/lib/crates/fabro-store/src/run_state.rs b/lib/crates/fabro-store/src/run_state.rs index cb0be518d..34d6d958c 100644 --- a/lib/crates/fabro-store/src/run_state.rs +++ b/lib/crates/fabro-store/src/run_state.rs @@ -223,20 +223,14 @@ impl RunProjection { started_at: Some(ts), }); } - EventBody::InterviewCompleted(props) => { - if !props.question_id.is_empty() { - self.pending_interviews.remove(&props.question_id); - } + EventBody::InterviewCompleted(props) if !props.question_id.is_empty() => { + self.pending_interviews.remove(&props.question_id); } - EventBody::InterviewTimeout(props) => { - if !props.question_id.is_empty() { - self.pending_interviews.remove(&props.question_id); - } + EventBody::InterviewTimeout(props) if !props.question_id.is_empty() => { + self.pending_interviews.remove(&props.question_id); } - EventBody::InterviewInterrupted(props) => { - if !props.question_id.is_empty() { - self.pending_interviews.remove(&props.question_id); - } + EventBody::InterviewInterrupted(props) if !props.question_id.is_empty() => { + self.pending_interviews.remove(&props.question_id); } EventBody::StagePrompt(props) => { let Some(node_id) = stored.node_id.as_deref() else { diff --git a/lib/crates/fabro-store/src/slate/mod.rs b/lib/crates/fabro-store/src/slate/mod.rs index d6801631e..0cc2a557b 100644 --- a/lib/crates/fabro-store/src/slate/mod.rs +++ b/lib/crates/fabro-store/src/slate/mod.rs @@ -170,7 +170,7 @@ impl Database { } summaries.push(RunDatabase::build_summary(&db, &run_id).await?); } - summaries.sort_by(|a, b| b.run_id.created_at().cmp(&a.run_id.created_at())); + summaries.sort_by_key(|b| std::cmp::Reverse(b.run_id.created_at())); Ok(summaries) } diff --git a/lib/crates/fabro-telemetry/src/buffer.rs b/lib/crates/fabro-telemetry/src/buffer.rs index 5997159e9..92eb38226 100644 --- a/lib/crates/fabro-telemetry/src/buffer.rs +++ b/lib/crates/fabro-telemetry/src/buffer.rs @@ -13,7 +13,7 @@ impl Default for BufferPolicy { fn default() -> Self { Self { count_threshold: 20, - time_threshold: Duration::from_secs(60), + time_threshold: Duration::from_mins(1), } } } @@ -98,7 +98,7 @@ mod tests { &rx, BufferPolicy { count_threshold: 2, - time_threshold: Duration::from_secs(60), + time_threshold: Duration::from_mins(1), }, move |tracks| { let events: Vec = tracks.iter().map(|t| t.event.clone()).collect(); @@ -133,7 +133,7 @@ mod tests { &rx, BufferPolicy { count_threshold: 2, - time_threshold: Duration::from_secs(60), + time_threshold: Duration::from_mins(1), }, move |_| { *mid.lock().unwrap() = true; @@ -210,7 +210,7 @@ mod tests { &rx, BufferPolicy { count_threshold: 100, // won't trigger - time_threshold: Duration::from_secs(60), + time_threshold: Duration::from_mins(1), }, move |tracks| { let events: Vec = tracks.iter().map(|t| t.event.clone()).collect(); diff --git a/lib/crates/fabro-types/src/graph.rs b/lib/crates/fabro-types/src/graph.rs index 9caa03376..a21e92252 100644 --- a/lib/crates/fabro-types/src/graph.rs +++ b/lib/crates/fabro-types/src/graph.rs @@ -451,7 +451,7 @@ impl Graph { { Some(d) if d.is_zero() => None, Some(d) => Some(d), - None => Some(Duration::from_secs(1800)), + None => Some(Duration::from_mins(30)), } } @@ -763,7 +763,7 @@ mod tests { #[test] fn graph_stall_timeout_default() { let g = Graph::new("empty"); - assert_eq!(g.stall_timeout(), Some(Duration::from_secs(1800))); + assert_eq!(g.stall_timeout(), Some(Duration::from_mins(30))); } #[test] diff --git a/lib/crates/fabro-util/src/backoff.rs b/lib/crates/fabro-util/src/backoff.rs index 25bad37e7..2a78f2b0c 100644 --- a/lib/crates/fabro-util/src/backoff.rs +++ b/lib/crates/fabro-util/src/backoff.rs @@ -87,12 +87,12 @@ mod tests { #[test] fn delay_with_jitter_within_range() { let b = BackoffPolicy { - initial_delay: Duration::from_millis(1000), + initial_delay: Duration::from_secs(1), factor: 1.0, max_delay: Duration::from_secs(10), jitter: true, }; - let base = Duration::from_millis(1000); + let base = Duration::from_secs(1); let min = base.mul_f64(0.5); let max = base.mul_f64(1.5); diff --git a/lib/crates/fabro-workflow/src/condition.rs b/lib/crates/fabro-workflow/src/condition.rs index 66272d7d1..d7c01cf31 100644 --- a/lib/crates/fabro-workflow/src/condition.rs +++ b/lib/crates/fabro-workflow/src/condition.rs @@ -130,9 +130,7 @@ fn eval_clause(clause: &Clause, outcome: &Outcome, context: &Context) -> bool { Op::Matches => { let resolved = resolve_key(&clause.key, outcome, context); // Regex was validated at parse time, so unwrap is safe - regex::Regex::new(&clause.value) - .map(|re| re.is_match(&resolved)) - .unwrap_or(false) + regex::Regex::new(&clause.value).is_ok_and(|re| re.is_match(&resolved)) } } } diff --git a/lib/crates/fabro-workflow/src/event.rs b/lib/crates/fabro-workflow/src/event.rs index a11bff16c..72eb8ae7a 100644 --- a/lib/crates/fabro-workflow/src/event.rs +++ b/lib/crates/fabro-workflow/src/event.rs @@ -2771,7 +2771,7 @@ pub struct Emitter { impl std::fmt::Debug for Emitter { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let count = self.listeners.lock().map(|l| l.len()).unwrap_or(0); + let count = self.listeners.lock().map_or(0, |l| l.len()); f.debug_struct("Emitter") .field("run_id", &self.run_id) .field("listener_count", &count) diff --git a/lib/crates/fabro-workflow/src/handler/command.rs b/lib/crates/fabro-workflow/src/handler/command.rs index c88e43b68..11f2cd3ec 100644 --- a/lib/crates/fabro-workflow/src/handler/command.rs +++ b/lib/crates/fabro-workflow/src/handler/command.rs @@ -397,7 +397,7 @@ mod tests { ); node.attrs.insert( "timeout".to_string(), - AttrValue::Duration(Duration::from_millis(5000)), + AttrValue::Duration(Duration::from_secs(5)), ); let context = Context::new(); let graph = Graph::new("test"); diff --git a/lib/crates/fabro-workflow/src/handler/llm/api.rs b/lib/crates/fabro-workflow/src/handler/llm/api.rs index 27ad86ae8..c4bb57ec7 100644 --- a/lib/crates/fabro-workflow/src/handler/llm/api.rs +++ b/lib/crates/fabro-workflow/src/handler/llm/api.rs @@ -110,11 +110,9 @@ fn track_file_event(event: &AgentEvent, state: &mut FileTracking) { tool_name, tool_call_id, arguments, - } => { - if tool_name == "write_file" || tool_name == "edit_file" { - if let Some(path) = arguments.get("file_path").and_then(|v| v.as_str()) { - state.pending.insert(tool_call_id.clone(), path.to_string()); - } + } if tool_name == "write_file" || tool_name == "edit_file" => { + if let Some(path) = arguments.get("file_path").and_then(|v| v.as_str()) { + state.pending.insert(tool_call_id.clone(), path.to_string()); } } AgentEvent::ToolCallCompleted { diff --git a/lib/crates/fabro-workflow/src/handler/manager_loop.rs b/lib/crates/fabro-workflow/src/handler/manager_loop.rs index 87150a7f6..7eeca7018 100644 --- a/lib/crates/fabro-workflow/src/handler/manager_loop.rs +++ b/lib/crates/fabro-workflow/src/handler/manager_loop.rs @@ -787,7 +787,7 @@ mod tests { #[test] fn parse_duration_str_minutes() { - assert_eq!(parse_duration_str("5m"), Duration::from_secs(300)); + assert_eq!(parse_duration_str("5m"), Duration::from_mins(5)); } #[test] diff --git a/lib/crates/fabro-workflow/src/lifecycle/artifact.rs b/lib/crates/fabro-workflow/src/lifecycle/artifact.rs index 5cab268e7..7f0e83ffb 100644 --- a/lib/crates/fabro-workflow/src/lifecycle/artifact.rs +++ b/lib/crates/fabro-workflow/src/lifecycle/artifact.rs @@ -85,8 +85,7 @@ impl RunLifecycle for ArtifactLifecycle { // Record epoch seconds (floored to integer for macOS stat mtime parity) let epoch = std::time::SystemTime::now() .duration_since(std::time::UNIX_EPOCH) - .map(|d| d.as_secs() as f64) - .unwrap_or(0.0); + .map_or(0.0, |d| d.as_secs() as f64); *self.attempt_start_epoch.lock().unwrap() = Some(epoch); Ok(NodeDecision::Continue) } diff --git a/lib/crates/fabro-workflow/src/operations/create.rs b/lib/crates/fabro-workflow/src/operations/create.rs index 6fbda6340..925ff67f2 100644 --- a/lib/crates/fabro-workflow/src/operations/create.rs +++ b/lib/crates/fabro-workflow/src/operations/create.rs @@ -75,8 +75,7 @@ pub async fn create(store: &Database, request: CreateRunInput) -> Result Result<(), Error> { .sandbox .apply_devcontainer_snapshot(devcontainer_to_snapshot_config(&config)); - let timeout = std::time::Duration::from_millis(300_000); + let timeout = std::time::Duration::from_mins(5); for command in &config.initialize_commands { let shell_commands = match command { fabro_devcontainer::Command::Shell(shell) => vec![shell.clone()], diff --git a/lib/crates/fabro-workflow/src/retry.rs b/lib/crates/fabro-workflow/src/retry.rs index 3a92925b6..87ca75b16 100644 --- a/lib/crates/fabro-workflow/src/retry.rs +++ b/lib/crates/fabro-workflow/src/retry.rs @@ -4,9 +4,9 @@ use fabro_core::retry::{BackoffPolicy, RetryPolicy}; use fabro_graphviz::graph::types::{Graph as GvGraph, Node as GvNode}; const DEFAULT_BACKOFF: BackoffPolicy = BackoffPolicy { - initial_delay: Duration::from_millis(5_000), + initial_delay: Duration::from_secs(5), factor: 2.0, - max_delay: Duration::from_millis(60_000), + max_delay: Duration::from_mins(1), jitter: true, }; @@ -59,7 +59,7 @@ fn preset_retry_policy(preset: &str) -> Option { "patient" => Some(RetryPolicy { max_attempts: 3, backoff: BackoffPolicy { - initial_delay: Duration::from_millis(2_000), + initial_delay: Duration::from_secs(2), factor: 3.0, ..DEFAULT_BACKOFF }, @@ -126,7 +126,7 @@ mod tests { let graph = Graph::new("test"); let policy = build_retry_policy(&node, &graph); assert_eq!(policy.max_attempts, 4); - assert_eq!(policy.backoff.initial_delay, Duration::from_millis(5_000)); + assert_eq!(policy.backoff.initial_delay, Duration::from_secs(5)); } #[test] diff --git a/lib/crates/fabro-workflow/src/run_options.rs b/lib/crates/fabro-workflow/src/run_options.rs index 66beb3518..28057beb1 100644 --- a/lib/crates/fabro-workflow/src/run_options.rs +++ b/lib/crates/fabro-workflow/src/run_options.rs @@ -45,8 +45,7 @@ pub struct RunOptions { impl RunOptions { pub fn dry_run_enabled(&self) -> bool { fabro_config::resolve_run_from_file(&self.settings) - .map(|settings| settings.execution.mode == RunMode::DryRun) - .unwrap_or(false) + .is_ok_and(|settings| settings.execution.mode == RunMode::DryRun) } pub fn checkpoint_exclude_globs(&self) -> Vec { diff --git a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs index dc41f9e49..8f3f94d25 100644 --- a/lib/crates/fabro-workflow/tests/it/daytona_integration.rs +++ b/lib/crates/fabro-workflow/tests/it/daytona_integration.rs @@ -295,7 +295,7 @@ async fn daytona_exec_command_local_timeout() { // local timeout (duration ~2100ms). Both are valid success conditions for // the system as a whole avoiding a stall. assert!( - duration < std::time::Duration::from_millis(3000), + duration < std::time::Duration::from_secs(3), "Command stalled for longer than the local timeout mechanism" ); assert!(result.exit_code != 0); @@ -2209,7 +2209,7 @@ async fn daytona_playwright_mcp_sandbox_transport() { .call_tool( install_tool, serde_json::json!({}), - std::time::Duration::from_secs(120), + std::time::Duration::from_mins(2), ) .await; match &install_result { diff --git a/lib/crates/fabro-workflow/tests/it/integration.rs b/lib/crates/fabro-workflow/tests/it/integration.rs index 5a30a0a4c..46655db4e 100644 --- a/lib/crates/fabro-workflow/tests/it/integration.rs +++ b/lib/crates/fabro-workflow/tests/it/integration.rs @@ -6254,7 +6254,7 @@ mod real_llm { git: None, }; let (outcome, state) = tokio::time::timeout( - std::time::Duration::from_secs(120), + std::time::Duration::from_mins(2), engine.run_with_state(&graph, &run_options), ) .await @@ -6362,7 +6362,7 @@ mod real_llm { git: None, }; let outcome = tokio::time::timeout( - std::time::Duration::from_secs(120), + std::time::Duration::from_mins(2), engine.run(&graph, &run_options), ) .await @@ -6494,7 +6494,7 @@ mod real_llm { git: None, }; let outcome = tokio::time::timeout( - std::time::Duration::from_secs(120), + std::time::Duration::from_mins(2), engine.run(&graph, &run_options), ) .await @@ -7214,14 +7214,14 @@ fn subgraph_node_defaults_scoped_to_subgraph() { // Plan inherits both thread_id and timeout from subgraph defaults let plan = &graph.nodes["plan"]; assert_eq!(plan.thread_id(), Some("loop-a")); - assert_eq!(plan.timeout(), Some(std::time::Duration::from_secs(900))); + assert_eq!(plan.timeout(), Some(std::time::Duration::from_mins(15))); // Implement inherits thread_id but overrides timeout let implement = &graph.nodes["implement"]; assert_eq!(implement.thread_id(), Some("loop-a")); assert_eq!( implement.timeout(), - Some(std::time::Duration::from_secs(1800)) + Some(std::time::Duration::from_mins(30)) ); // Outside node should NOT have subgraph defaults @@ -7302,11 +7302,11 @@ fn subgraph_scoping_does_not_leak_to_outer_scope() { // Inner node gets the subgraph-scoped timeout of 900s let inner = &graph.nodes["inner_node"]; - assert_eq!(inner.timeout(), Some(std::time::Duration::from_secs(900))); + assert_eq!(inner.timeout(), Some(std::time::Duration::from_mins(15))); // Outer node gets the graph-level default of 300s, not the subgraph's 900s let outer = &graph.nodes["outer_node"]; - assert_eq!(outer.timeout(), Some(std::time::Duration::from_secs(300))); + assert_eq!(outer.timeout(), Some(std::time::Duration::from_mins(5))); } #[test] @@ -7331,13 +7331,13 @@ fn subgraph_global_defaults_plus_subgraph_defaults() { let step = &graph.nodes["step"]; assert_eq!(step.shape(), "box"); assert_eq!(step.thread_id(), Some("loop-thread")); - assert_eq!(step.timeout(), Some(std::time::Duration::from_secs(300))); + assert_eq!(step.timeout(), Some(std::time::Duration::from_mins(5))); // Plain should have the global defaults but no thread_id let plain = &graph.nodes["plain"]; assert_eq!(plain.shape(), "box"); assert_eq!(plain.thread_id(), None); - assert_eq!(plain.timeout(), Some(std::time::Duration::from_secs(300))); + assert_eq!(plain.timeout(), Some(std::time::Duration::from_mins(5))); } #[test] @@ -7377,7 +7377,7 @@ fn subgraph_without_label_no_class_derived() { let worker = &graph.nodes["worker"]; assert!(worker.classes.is_empty()); // But the default should still apply - assert_eq!(worker.timeout(), Some(std::time::Duration::from_secs(600))); + assert_eq!(worker.timeout(), Some(std::time::Duration::from_mins(10))); } // --------------------------------------------------------------------------- @@ -12239,7 +12239,7 @@ impl Handler for HangingHandler { _run_dir: &Path, _services: &fabro_workflow::handler::EngineServices, ) -> Result { - tokio::time::sleep(std::time::Duration::from_secs(60)).await; + tokio::time::sleep(std::time::Duration::from_mins(1)).await; Ok(Outcome::success()) } } diff --git a/test/twin/openai/tests/debug_ui.rs b/test/twin/openai/tests/debug_ui.rs index a748ecda1..8b0890bd8 100644 --- a/test/twin/openai/tests/debug_ui.rs +++ b/test/twin/openai/tests/debug_ui.rs @@ -269,8 +269,7 @@ async fn debug_page_renders_in_headless_chrome() { std::process::Command::new("which") .arg(name) .output() - .map(|o| o.status.success()) - .unwrap_or(false) + .is_ok_and(|o| o.status.success()) }); let Some(chrome_binary) = chrome_binary.copied() else {