From bdb5a1093f1ef513c45b4336724082c61d9d1719 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 26 Sep 2026 11:09:50 -0400 Subject: [PATCH] Agent: fail loud on cpu past full; group tools in grouped() SandboxStatus::cpu asserts its share lies in 0..=1 and hands it to Meter unclamped. ToolRegistry gathers kept tools by source through grouped(), tested for first-seen order and repeated sources. --- TASKS.md | 1 + src/agent/environment.rs | 12 ++++------ src/agent/registry.rs | 42 ++++++++++++++++++++++++++-------- src/agent/tests/environment.rs | 6 +++++ 4 files changed, 44 insertions(+), 17 deletions(-) diff --git a/TASKS.md b/TASKS.md index 2fb740f..7153856 100644 --- a/TASKS.md +++ b/TASKS.md @@ -282,3 +282,4 @@ Per-component lines live in `tasks/`. Tags there: | T24d | 1 | FAIL | at 280px eight thumbnails ran past the browser card; the artifact panel lost its title and its controls ran out of the box | | T24d | 2 | FAIL | Tab to a later thumbnail left the strip where it was, the focused frame and its ring clipped | | T24d | 3 | FAIL | reviewed by Claude Fable 5.1 max after codex ran out of quota: the shown thumbnail wore the focus color, so focus and choice looked alike; frames were forced to 16:10 and cropped, and a pointer's share missed its target. Fixed after the cap: the shown frame takes accent, and BrowserPreview and ComputerUseViewer take the host's ratio (test a_screen_takes_its_pictures_shape); no fourth review | +| T24e | 1 | FAIL | cpu() clamped a share past 1 in silence; the registry's grouping by source had no test | diff --git a/src/agent/environment.rs b/src/agent/environment.rs index bb4d1e5..75615ea 100644 --- a/src/agent/environment.rs +++ b/src/agent/environment.rs @@ -92,6 +92,7 @@ impl SandboxStatus { /// The share of its processors in use, 0 to 1. pub fn cpu(mut self, share: f32) -> Self { + assert!((0.0..=1.0).contains(&share), "cpu {share} of 1"); self.cpu = Some(share); self } @@ -165,13 +166,10 @@ impl RenderOnce for SandboxStatus { })), ), ) - .children(self.cpu.map(|share| { - Meter::new( - (self.id.clone(), "cpu"), - "Processors", - share.clamp(0.0, 1.0), - ) - })) + .children( + self.cpu + .map(|share| Meter::new((self.id.clone(), "cpu"), "Processors", share)), + ) .children(self.memory.map(|(used, limit)| { Meter::new( (self.id.clone(), "memory"), diff --git a/src/agent/registry.rs b/src/agent/registry.rs index 7e3a439..1064d94 100644 --- a/src/agent/registry.rs +++ b/src/agent/registry.rs @@ -55,6 +55,19 @@ pub(crate) fn found(tools: &[RegisteredTool], query: &str) -> Vec { .collect() } +/// The kept tools gathered by source, sources in the order they first appear. +pub(crate) fn grouped(tools: &[RegisteredTool], kept: &[usize]) -> Vec<(SharedString, Vec)> { + let mut groups: Vec<(SharedString, Vec)> = Vec::new(); + for &ix in kept { + let source = &tools[ix].source; + match groups.iter_mut().find(|(name, _)| name == source) { + Some((_, items)) => items.push(ix), + None => groups.push((source.clone(), vec![ix])), + } + } + groups +} + type OnAccess = Rc; /// Every tool the agent may call, grouped by where it comes from and found by the owner's query: each allowed, asked for first, or denied. @@ -96,15 +109,7 @@ impl RenderOnce for ToolRegistry { fn render(self, _: &mut Window, cx: &mut App) -> impl IntoElement { let theme = cx.theme(); let colors = theme.colors.clone(); - let kept = found(&self.tools, &self.query); - let mut groups: Vec<(SharedString, Vec)> = Vec::new(); - for ix in kept { - let source = &self.tools[ix].source; - match groups.iter_mut().find(|(name, _)| name == source) { - Some((_, items)) => items.push(ix), - None => groups.push((source.clone(), vec![ix])), - } - } + let groups = grouped(&self.tools, &found(&self.tools, &self.query)); let empty = groups.is_empty(); div() .w_full() @@ -183,7 +188,9 @@ impl RenderOnce for ToolRegistry { #[cfg(test)] mod tests { - use super::{RegisteredTool, ToolAccess, found}; + use gpui::SharedString; + + use super::{RegisteredTool, ToolAccess, found, grouped}; fn tool(name: &str, description: &str) -> RegisteredTool { RegisteredTool { @@ -206,4 +213,19 @@ mod tests { assert_eq!(found(&tools, "writes file"), [1]); assert_eq!(found(&tools, "web file"), Vec::::new()); } + + #[test] + fn kept_tools_gather_by_source_in_first_seen_order() { + let from = |source: &str| RegisteredTool { + source: source.to_string().into(), + ..tool("tool", "") + }; + let tools = [from("A"), from("B"), from("A")]; + let (a, b) = (SharedString::from("A"), SharedString::from("B")); + assert_eq!( + grouped(&tools, &[0, 1, 2]), + [(a.clone(), vec![0, 2]), (b.clone(), vec![1])] + ); + assert_eq!(grouped(&tools, &[1, 2]), [(b, vec![1]), (a, vec![2])]); + } } diff --git a/src/agent/tests/environment.rs b/src/agent/tests/environment.rs index 237c08b..4404289 100644 --- a/src/agent/tests/environment.rs +++ b/src/agent/tests/environment.rs @@ -168,3 +168,9 @@ fn every_checkpoint_but_the_current_rewinds(cx: &mut TestAppContext) { fn memory_past_its_limit_fails_loud() { let _ = SandboxStatus::new("box", "python", SandboxState::Ready).memory(5, 4); } + +#[test] +#[should_panic(expected = "cpu 1.8 of 1")] +fn cpu_past_full_fails_loud() { + let _ = SandboxStatus::new("box", "python", SandboxState::Ready).cpu(1.8); +}