From 59de3b8018f17c18b4ce78505a5992d10be7942c Mon Sep 17 00:00:00 2001 From: leynos Date: Fri, 22 May 2026 20:35:44 +0200 Subject: [PATCH 1/4] Use mockable clock in build tool wrapper (#194) Replace the bespoke build-tool clock trait with `mockable::Clock` so production code uses the shared dependency-injection abstraction and tests use `mockable::MockClock` for deterministic duration assertions. Add `mockable` to the manifest with the clock support needed by production code and the mock support needed by the wrapper test. --- Cargo.lock | 93 +++++++++++++++++++++++-- Cargo.toml | 2 + src/tools/builder/core/wrapper.rs | 37 +++------- src/tools/builder/core/wrapper_tests.rs | 39 ++++++----- 4 files changed, 122 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 63dfc59aa..049ee8159 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2186,6 +2186,12 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" +[[package]] +name = "difflib" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" + [[package]] name = "digest" version = "0.10.7" @@ -2605,6 +2611,15 @@ dependencies = [ "miniz_oxide", ] +[[package]] +name = "float-cmp" +version = "0.9.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4" +dependencies = [ + "num-traits", +] + [[package]] name = "fluent" version = "0.17.0" @@ -3807,7 +3822,8 @@ dependencies = [ "lru", "metrics", "mime_guess", - "mockall", + "mockable", + "mockall 0.13.1", "open", "pdf-extract", "pgvector", @@ -3894,6 +3910,15 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" +[[package]] +name = "itertools" +version = "0.10.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" +dependencies = [ + "either", +] + [[package]] name = "itertools" version = "0.12.1" @@ -4464,6 +4489,32 @@ dependencies = [ "windows-sys 0.61.2", ] +[[package]] +name = "mockable" +version = "3.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "696033a748f67c97f2169b29b428fbab0d12317707cede1ca62fcb786e80b7fe" +dependencies = [ + "chrono", + "mockall 0.11.4", + "tracing", +] + +[[package]] +name = "mockall" +version = "0.11.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4c84490118f2ee2d74570d114f3d0493cbf02790df303d2707606c3e14e07c96" +dependencies = [ + "cfg-if", + "downcast", + "fragile", + "lazy_static", + "mockall_derive 0.11.4", + "predicates 2.1.5", + "predicates-tree", +] + [[package]] name = "mockall" version = "0.13.1" @@ -4473,11 +4524,23 @@ dependencies = [ "cfg-if", "downcast", "fragile", - "mockall_derive", - "predicates", + "mockall_derive 0.13.1", + "predicates 3.1.4", "predicates-tree", ] +[[package]] +name = "mockall_derive" +version = "0.11.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "22ce75669015c4f47b289fd4d4f56e894e4c96003ffdf3ac51313126f94c6cbb" +dependencies = [ + "cfg-if", + "proc-macro2", + "quote", + "syn 1.0.109", +] + [[package]] name = "mockall_derive" version = "0.13.1" @@ -4589,6 +4652,12 @@ dependencies = [ "minimal-lexical", ] +[[package]] +name = "normalize-line-endings" +version = "0.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" + [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -5303,6 +5372,20 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c" +[[package]] +name = "predicates" +version = "2.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "59230a63c37f3e18569bdb90e4a89cbf5bf8b06fea0b84e65ea10cc4df47addd" +dependencies = [ + "difflib", + "float-cmp", + "itertools 0.10.5", + "normalize-line-endings", + "predicates-core", + "regex", +] + [[package]] name = "predicates" version = "3.1.4" @@ -5433,7 +5516,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81bddcdb20abf9501610992b6759a4c888aef7d1a7247ef75e2404275ac24af1" dependencies = [ "anyhow", - "itertools", + "itertools 0.12.1", "proc-macro2", "quote", "syn 2.0.117", @@ -8681,7 +8764,7 @@ dependencies = [ "cranelift-frontend", "cranelift-native", "gimli", - "itertools", + "itertools 0.12.1", "log", "object 0.36.7", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 341e3348f..736a7b890 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -181,6 +181,7 @@ rusqlite = { version = "0.32", optional = true } json5 = { version = "0.4", optional = true } tempfile = { version = "3", optional = true } const_format = { version = "0.2.35", default-features = false } +mockable = { version = "3.0.0", default-features = false, features = ["clock"] } # macOS keychain [target.'cfg(target_os = "macos")'.dependencies] @@ -206,6 +207,7 @@ rstest-bdd = "0.5.0" rstest-bdd-macros = { version = "0.5.0", features = ["compile-time-validation"] } tempfile = "3" mockall = "0.13" +mockable = { version = "3.0.0", default-features = false, features = ["clock", "mock"] } trybuild = "1" proptest = "1.6.0" delegate = "0.13" diff --git a/src/tools/builder/core/wrapper.rs b/src/tools/builder/core/wrapper.rs index 58306df9e..c58b74119 100644 --- a/src/tools/builder/core/wrapper.rs +++ b/src/tools/builder/core/wrapper.rs @@ -8,24 +8,7 @@ //! centralize fallback and error-message logic. use super::*; - -trait Clock: Send + Sync { - fn now(&self) -> std::time::Instant; - - fn elapsed_since(&self, start: std::time::Instant) -> std::time::Duration; -} - -struct SystemClock; - -impl Clock for SystemClock { - fn now(&self) -> std::time::Instant { - std::time::Instant::now() - } - - fn elapsed_since(&self, start: std::time::Instant) -> std::time::Duration { - start.elapsed() - } -} +use mockable::{Clock, DefaultClock}; /// Tool that allows the agent to build software on demand. pub struct BuildSoftwareTool { @@ -38,15 +21,10 @@ impl BuildSoftwareTool { pub fn new(builder: Arc) -> Self { Self { builder, - clock: Arc::new(SystemClock), + clock: Arc::new(DefaultClock), } } - #[cfg(test)] - fn new_with_clock(builder: Arc, clock: Arc) -> Self { - Self { builder, clock } - } - /// Resolves an optional string override against a parse closure. /// /// Returns `fallback` when `override_str` is `None`. When @@ -156,7 +134,7 @@ impl NativeTool for BuildSoftwareTool { .and_then(|v| v.as_str()) .ok_or_else(|| ToolError::InvalidParameters("missing 'description'".into()))?; - let start = self.clock.now(); + let start = self.clock.utc(); // Analyze the requirement let mut requirement = self @@ -193,7 +171,14 @@ impl NativeTool for BuildSoftwareTool { "phases": result.logs.iter().map(|l| format!("{:?}: {}", l.phase, l.message)).collect::>() }); - Ok(ToolOutput::success(output, self.clock.elapsed_since(start))) + Ok(ToolOutput::success( + output, + self.clock + .utc() + .signed_duration_since(start) + .to_std() + .unwrap_or_default(), + )) } /// Approval is required unless the surrounding job auto-approves tools. diff --git a/src/tools/builder/core/wrapper_tests.rs b/src/tools/builder/core/wrapper_tests.rs index bb8ab573f..2a9da4c71 100644 --- a/src/tools/builder/core/wrapper_tests.rs +++ b/src/tools/builder/core/wrapper_tests.rs @@ -1,29 +1,19 @@ //! Tests for the build software native-tool wrapper. -use std::sync::{Arc, Mutex}; +use std::sync::{ + Arc, Mutex, + atomic::{AtomicUsize, Ordering}, +}; use super::super::domain::SoftwareBuilderFuture; use super::*; use insta::assert_snapshot; +use mockable::MockClock; use rstest::rstest; type AnalyzeResult = dyn Fn() -> Result + Send + Sync; type BuildResultFn = dyn Fn(&BuildRequirement) -> Result + Send + Sync; -struct FixedClock { - elapsed: std::time::Duration, -} - -impl Clock for FixedClock { - fn now(&self) -> std::time::Instant { - std::time::Instant::now() - } - - fn elapsed_since(&self, _start: std::time::Instant) -> std::time::Duration { - self.elapsed - } -} - fn assert_invalid_parameters(result: Result, expected_msg: &str) { match result.expect_err("expected invalid parameters error") { ToolError::InvalidParameters(msg) => assert_eq!(msg, expected_msg), @@ -341,10 +331,23 @@ async fn execute_success_output_matches_snapshot() { let requirement = test_requirement(); let build_result = test_build_result(requirement.clone()); let builder = FakeSoftwareBuilder::success(requirement, build_result); - let clock = Arc::new(FixedClock { - elapsed: std::time::Duration::from_millis(42), + let mut clock = MockClock::new(); + let start = Utc::now(); + let calls = Arc::new(AtomicUsize::new(0)); + clock.expect_utc().returning({ + let calls = Arc::clone(&calls); + move || { + if calls.fetch_add(1, Ordering::Relaxed) == 0 { + start + } else { + start + chrono::Duration::milliseconds(42) + } + } }); - let tool = BuildSoftwareTool::new_with_clock(Arc::new(builder), clock); + let tool = BuildSoftwareTool { + builder: Arc::new(builder), + clock: Arc::new(clock), + }; let output = tool .execute( From 35ffcf94e62fbd3e9c65a221d4eb4fdeafc1247a Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 00:50:08 +0200 Subject: [PATCH 2/4] Deduplicate build wrapper failure tests (#194) Share the execute-and-assert body for analyzer and builder failure tests so the cases keep one assertion path. --- src/tools/builder/core/wrapper_tests.rs | 43 ++++++++++++------------- 1 file changed, 20 insertions(+), 23 deletions(-) diff --git a/src/tools/builder/core/wrapper_tests.rs b/src/tools/builder/core/wrapper_tests.rs index 2a9da4c71..bf2cf5448 100644 --- a/src/tools/builder/core/wrapper_tests.rs +++ b/src/tools/builder/core/wrapper_tests.rs @@ -238,41 +238,38 @@ async fn execute_missing_description_returns_error() { assert_invalid_parameters(result, "missing 'description'"); } -#[tokio::test] -async fn execute_analyze_failure_returns_execution_failed() { - let builder = FakeSoftwareBuilder::analyze_error("analysis exploded"); - let tool = BuildSoftwareTool::new(Arc::new(builder)); - +async fn execute_failure_returns_execution_failed( + builder: Arc, + expected_msg: &str, +) { + let tool = BuildSoftwareTool::new(builder); let result = tool .execute( - serde_json::json!({ - "description": "build a test tool", - }), + serde_json::json!({ "description": "build a test tool" }), &JobContext::default(), ) .await; + assert_execution_failed(result, expected_msg); +} - assert_execution_failed( - result, +#[tokio::test] +async fn execute_analyze_failure_returns_execution_failed() { + let builder = FakeSoftwareBuilder::analyze_error("analysis exploded"); + execute_failure_returns_execution_failed( + Arc::new(builder), "Analysis failed: Tool builder failed: analysis exploded", - ); + ) + .await; } #[tokio::test] async fn execute_build_failure_returns_execution_failed() { let builder = FakeSoftwareBuilder::build_error(test_requirement(), "build exploded"); - let tool = BuildSoftwareTool::new(Arc::new(builder)); - - let result = tool - .execute( - serde_json::json!({ - "description": "build a test tool", - }), - &JobContext::default(), - ) - .await; - - assert_execution_failed(result, "Build failed: Tool builder failed: build exploded"); + execute_failure_returns_execution_failed( + Arc::new(builder), + "Build failed: Tool builder failed: build exploded", + ) + .await; } #[rstest] From 0c8003a54baf94143da76c15404eb977fdf6ed50 Mon Sep 17 00:00:00 2001 From: leynos Date: Tue, 26 May 2026 12:24:51 +0200 Subject: [PATCH 3/4] Use monotonic build tool timing (#194) Measure `BuildSoftwareTool` execution duration with `Instant` so host wall-clock adjustments cannot collapse elapsed time to zero. Remove the now-unused `mockable` dependency because its clock abstraction only exposes wall-clock time and cannot provide the required monotonic source. --- Cargo.lock | 93 ++----------------------- Cargo.toml | 2 - src/tools/builder/core/wrapper.rs | 19 ++--- src/tools/builder/core/wrapper_tests.rs | 25 +------ 4 files changed, 11 insertions(+), 128 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 049ee8159..63dfc59aa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2186,12 +2186,6 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" -[[package]] -name = "difflib" -version = "0.4.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6184e33543162437515c2e2b48714794e37845ec9851711914eec9d308f6ebe8" - [[package]] name = "digest" version = "0.10.7" @@ -2611,15 +2605,6 @@ dependencies = [ "miniz_oxide", ] -[[package]] -name = "float-cmp" -version = "0.9.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "98de4bbd547a563b716d8dfa9aad1cb19bfab00f4fa09a6a4ed21dbcf44ce9c4" -dependencies = [ - "num-traits", -] - [[package]] name = "fluent" version = "0.17.0" @@ -3822,8 +3807,7 @@ dependencies = [ "lru", "metrics", "mime_guess", - "mockable", - "mockall 0.13.1", + "mockall", "open", "pdf-extract", "pgvector", @@ -3910,15 +3894,6 @@ version = "1.70.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "a6cb138bb79a146c1bd460005623e142ef0181e3d0219cb493e02f7d08a35695" -[[package]] -name = "itertools" -version = "0.10.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0fd2260e829bddf4cb6ea802289de2f86d6a7a690192fbe91b3f46e0f2c8473" -dependencies = [ - "either", -] - [[package]] name = "itertools" version = "0.12.1" @@ -4489,32 +4464,6 @@ dependencies = [ "windows-sys 0.61.2", ] -[[package]] -name = "mockable" -version = "3.0.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "696033a748f67c97f2169b29b428fbab0d12317707cede1ca62fcb786e80b7fe" -dependencies = [ - "chrono", - "mockall 0.11.4", - "tracing", -] - -[[package]] -name = "mockall" -version = "0.11.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4c84490118f2ee2d74570d114f3d0493cbf02790df303d2707606c3e14e07c96" -dependencies = [ - "cfg-if", - "downcast", - "fragile", - "lazy_static", - "mockall_derive 0.11.4", - "predicates 2.1.5", - "predicates-tree", -] - [[package]] name = "mockall" version = "0.13.1" @@ -4524,23 +4473,11 @@ dependencies = [ "cfg-if", "downcast", "fragile", - "mockall_derive 0.13.1", - "predicates 3.1.4", + "mockall_derive", + "predicates", "predicates-tree", ] -[[package]] -name = "mockall_derive" -version = "0.11.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "22ce75669015c4f47b289fd4d4f56e894e4c96003ffdf3ac51313126f94c6cbb" -dependencies = [ - "cfg-if", - "proc-macro2", - "quote", - "syn 1.0.109", -] - [[package]] name = "mockall_derive" version = "0.13.1" @@ -4652,12 +4589,6 @@ dependencies = [ "minimal-lexical", ] -[[package]] -name = "normalize-line-endings" -version = "0.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61807f77802ff30975e01f4f071c8ba10c022052f98b3294119f3e615d13e5be" - [[package]] name = "nu-ansi-term" version = "0.50.3" @@ -5372,20 +5303,6 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "925383efa346730478fb4838dbe9137d2a47675ad789c546d150a6e1dd4ab31c" -[[package]] -name = "predicates" -version = "2.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59230a63c37f3e18569bdb90e4a89cbf5bf8b06fea0b84e65ea10cc4df47addd" -dependencies = [ - "difflib", - "float-cmp", - "itertools 0.10.5", - "normalize-line-endings", - "predicates-core", - "regex", -] - [[package]] name = "predicates" version = "3.1.4" @@ -5516,7 +5433,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "81bddcdb20abf9501610992b6759a4c888aef7d1a7247ef75e2404275ac24af1" dependencies = [ "anyhow", - "itertools 0.12.1", + "itertools", "proc-macro2", "quote", "syn 2.0.117", @@ -8764,7 +8681,7 @@ dependencies = [ "cranelift-frontend", "cranelift-native", "gimli", - "itertools 0.12.1", + "itertools", "log", "object 0.36.7", "smallvec", diff --git a/Cargo.toml b/Cargo.toml index 736a7b890..341e3348f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -181,7 +181,6 @@ rusqlite = { version = "0.32", optional = true } json5 = { version = "0.4", optional = true } tempfile = { version = "3", optional = true } const_format = { version = "0.2.35", default-features = false } -mockable = { version = "3.0.0", default-features = false, features = ["clock"] } # macOS keychain [target.'cfg(target_os = "macos")'.dependencies] @@ -207,7 +206,6 @@ rstest-bdd = "0.5.0" rstest-bdd-macros = { version = "0.5.0", features = ["compile-time-validation"] } tempfile = "3" mockall = "0.13" -mockable = { version = "3.0.0", default-features = false, features = ["clock", "mock"] } trybuild = "1" proptest = "1.6.0" delegate = "0.13" diff --git a/src/tools/builder/core/wrapper.rs b/src/tools/builder/core/wrapper.rs index c58b74119..6ce2927e0 100644 --- a/src/tools/builder/core/wrapper.rs +++ b/src/tools/builder/core/wrapper.rs @@ -8,21 +8,17 @@ //! centralize fallback and error-message logic. use super::*; -use mockable::{Clock, DefaultClock}; +use std::time::Instant; /// Tool that allows the agent to build software on demand. pub struct BuildSoftwareTool { builder: Arc, - clock: Arc, } impl BuildSoftwareTool { /// Wraps a [`SoftwareBuilder`] for use as a [`NativeTool`]. pub fn new(builder: Arc) -> Self { - Self { - builder, - clock: Arc::new(DefaultClock), - } + Self { builder } } /// Resolves an optional string override against a parse closure. @@ -134,7 +130,7 @@ impl NativeTool for BuildSoftwareTool { .and_then(|v| v.as_str()) .ok_or_else(|| ToolError::InvalidParameters("missing 'description'".into()))?; - let start = self.clock.utc(); + let start = Instant::now(); // Analyze the requirement let mut requirement = self @@ -171,14 +167,7 @@ impl NativeTool for BuildSoftwareTool { "phases": result.logs.iter().map(|l| format!("{:?}: {}", l.phase, l.message)).collect::>() }); - Ok(ToolOutput::success( - output, - self.clock - .utc() - .signed_duration_since(start) - .to_std() - .unwrap_or_default(), - )) + Ok(ToolOutput::success(output, start.elapsed())) } /// Approval is required unless the surrounding job auto-approves tools. diff --git a/src/tools/builder/core/wrapper_tests.rs b/src/tools/builder/core/wrapper_tests.rs index bf2cf5448..e2ea96bd6 100644 --- a/src/tools/builder/core/wrapper_tests.rs +++ b/src/tools/builder/core/wrapper_tests.rs @@ -1,14 +1,10 @@ //! Tests for the build software native-tool wrapper. -use std::sync::{ - Arc, Mutex, - atomic::{AtomicUsize, Ordering}, -}; +use std::sync::{Arc, Mutex}; use super::super::domain::SoftwareBuilderFuture; use super::*; use insta::assert_snapshot; -use mockable::MockClock; use rstest::rstest; type AnalyzeResult = dyn Fn() -> Result + Send + Sync; @@ -328,23 +324,7 @@ async fn execute_success_output_matches_snapshot() { let requirement = test_requirement(); let build_result = test_build_result(requirement.clone()); let builder = FakeSoftwareBuilder::success(requirement, build_result); - let mut clock = MockClock::new(); - let start = Utc::now(); - let calls = Arc::new(AtomicUsize::new(0)); - clock.expect_utc().returning({ - let calls = Arc::clone(&calls); - move || { - if calls.fetch_add(1, Ordering::Relaxed) == 0 { - start - } else { - start + chrono::Duration::milliseconds(42) - } - } - }); - let tool = BuildSoftwareTool { - builder: Arc::new(builder), - clock: Arc::new(clock), - }; + let tool = BuildSoftwareTool::new(Arc::new(builder)); let output = tool .execute( @@ -356,7 +336,6 @@ async fn execute_success_output_matches_snapshot() { .await .expect("expected execute to return successful output"); - assert_eq!(output.duration, std::time::Duration::from_millis(42)); assert_eq!(output.cost, None); assert_eq!(output.raw, None); assert_snapshot!( From 1e4d9814d6268a8f73f5c343b5e2c7a7e4a71dfd Mon Sep 17 00:00:00 2001 From: leynos Date: Sat, 30 May 2026 03:34:52 +0200 Subject: [PATCH 4/4] Inject monotonic build tool clock (#194) Add a `MonotonicClock` seam for `BuildSoftwareTool` duration measurement so tests can assert elapsed time without relying on wall time. Keep production timing monotonic by defaulting to `StdMonotonicClock`, and use a test-only fixed clock for deterministic wrapper assertions. --- src/tools/builder/core.rs | 2 ++ src/tools/builder/core/clock.rs | 47 +++++++++++++++++++++++++ src/tools/builder/core/wrapper.rs | 25 ++++++++++--- src/tools/builder/core/wrapper_tests.rs | 21 +++++++++-- 4 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 src/tools/builder/core/clock.rs diff --git a/src/tools/builder/core.rs b/src/tools/builder/core.rs index d811d33c8..3e1c58784 100644 --- a/src/tools/builder/core.rs +++ b/src/tools/builder/core.rs @@ -49,10 +49,12 @@ use crate::tools::tool::{ mod build_loop; mod builder_impl; +mod clock; mod domain; mod setup; mod wrapper; +pub(crate) use clock::{MonotonicClock, StdMonotonicClock}; pub use domain::{ BuildLog, BuildPhase, BuildRequirement, BuildResult, BuilderConfig, ExecutionCommand, Language, NativeSoftwareBuilder, ProjectName, SoftwareBuilder, SoftwareType, diff --git a/src/tools/builder/core/clock.rs b/src/tools/builder/core/clock.rs new file mode 100644 index 000000000..01cf13932 --- /dev/null +++ b/src/tools/builder/core/clock.rs @@ -0,0 +1,47 @@ +//! Monotonic clocks for measuring build tool execution duration. + +use std::time::Instant; + +/// Clock abstraction for monotonic elapsed-time measurements. +pub trait MonotonicClock: Send + Sync { + /// Returns the current monotonic instant. + fn now(&self) -> Instant; +} + +/// Monotonic clock backed by [`Instant::now`]. +pub struct StdMonotonicClock; + +impl MonotonicClock for StdMonotonicClock { + fn now(&self) -> Instant { + Instant::now() + } +} + +#[cfg(test)] +pub struct FixedMonotonicClock { + instants: std::sync::Mutex>, +} + +#[cfg(test)] +impl FixedMonotonicClock { + pub fn with_elapsed(elapsed: std::time::Duration) -> Self { + let start = Instant::now(); + Self { + instants: std::sync::Mutex::new(std::collections::VecDeque::from([ + start, + start + elapsed, + ])), + } + } +} + +#[cfg(test)] +impl MonotonicClock for FixedMonotonicClock { + fn now(&self) -> Instant { + self.instants + .lock() + .expect("fixed monotonic clock mutex should not be poisoned") + .pop_front() + .unwrap_or_else(Instant::now) + } +} diff --git a/src/tools/builder/core/wrapper.rs b/src/tools/builder/core/wrapper.rs index 6ce2927e0..58bf64eb3 100644 --- a/src/tools/builder/core/wrapper.rs +++ b/src/tools/builder/core/wrapper.rs @@ -7,18 +7,32 @@ //! `language` parameters are resolved through private helpers that //! centralize fallback and error-message logic. +#[cfg(test)] +use super::clock; use super::*; -use std::time::Instant; +use super::{MonotonicClock, StdMonotonicClock}; /// Tool that allows the agent to build software on demand. pub struct BuildSoftwareTool { builder: Arc, + clock: Arc, } impl BuildSoftwareTool { /// Wraps a [`SoftwareBuilder`] for use as a [`NativeTool`]. pub fn new(builder: Arc) -> Self { - Self { builder } + Self { + builder, + clock: Arc::new(StdMonotonicClock), + } + } + + #[cfg(test)] + pub(crate) fn new_with_clock( + builder: Arc, + clock: Arc, + ) -> Self { + Self { builder, clock } } /// Resolves an optional string override against a parse closure. @@ -130,7 +144,7 @@ impl NativeTool for BuildSoftwareTool { .and_then(|v| v.as_str()) .ok_or_else(|| ToolError::InvalidParameters("missing 'description'".into()))?; - let start = Instant::now(); + let start = self.clock.now(); // Analyze the requirement let mut requirement = self @@ -167,7 +181,10 @@ impl NativeTool for BuildSoftwareTool { "phases": result.logs.iter().map(|l| format!("{:?}: {}", l.phase, l.message)).collect::>() }); - Ok(ToolOutput::success(output, start.elapsed())) + Ok(ToolOutput::success( + output, + self.clock.now().duration_since(start), + )) } /// Approval is required unless the surrounding job auto-approves tools. diff --git a/src/tools/builder/core/wrapper_tests.rs b/src/tools/builder/core/wrapper_tests.rs index e2ea96bd6..841b2eb7c 100644 --- a/src/tools/builder/core/wrapper_tests.rs +++ b/src/tools/builder/core/wrapper_tests.rs @@ -1,8 +1,10 @@ //! Tests for the build software native-tool wrapper. use std::sync::{Arc, Mutex}; +use std::time::Duration; use super::super::domain::SoftwareBuilderFuture; +use super::clock::FixedMonotonicClock; use super::*; use insta::assert_snapshot; use rstest::rstest; @@ -297,7 +299,10 @@ async fn execute_valid_params_returns_success_output() { let requirement = test_requirement(); let build_result = test_build_result(requirement.clone()); let builder = FakeSoftwareBuilder::success(requirement, build_result); - let tool = BuildSoftwareTool::new(Arc::new(builder)); + let tool = BuildSoftwareTool::new_with_clock( + Arc::new(builder), + Arc::new(FixedMonotonicClock::with_elapsed(Duration::from_millis(1))), + ); let output = tool .execute( @@ -309,6 +314,10 @@ async fn execute_valid_params_returns_success_output() { .await .expect("expected execute to return successful output"); + assert!( + output.duration > Duration::ZERO, + "duration must be positive" + ); assert_eq!(output.result["success"], true); let captured = execute_capturing_requirement(serde_json::json!({ @@ -324,7 +333,10 @@ async fn execute_success_output_matches_snapshot() { let requirement = test_requirement(); let build_result = test_build_result(requirement.clone()); let builder = FakeSoftwareBuilder::success(requirement, build_result); - let tool = BuildSoftwareTool::new(Arc::new(builder)); + let tool = BuildSoftwareTool::new_with_clock( + Arc::new(builder), + Arc::new(FixedMonotonicClock::with_elapsed(Duration::from_millis(42))), + ); let output = tool .execute( @@ -336,6 +348,11 @@ async fn execute_success_output_matches_snapshot() { .await .expect("expected execute to return successful output"); + assert_eq!( + output.duration, + Duration::from_millis(42), + "duration must reflect the clock seam, not wall time" + ); assert_eq!(output.cost, None); assert_eq!(output.raw, None); assert_snapshot!(