v0.9.14 slices: #6213 T4/T5, #6244, #6235

crates/tui/src/core/engine/tool_catalog.rs

@@ -734,13 +734,56 @@ pub(crate) fn active_tools_for_request(
     Some(tools)
 }
 
-fn tool_search_haystack(tool: &Tool) -> String {
-    format!(
-        "{}\n{}\n{}",
-        tool.name.to_lowercase(),
-        tool.description.to_lowercase(),
-        tool.input_schema.to_string().to_lowercase()
-    )
+/// Reusable scratch for one `tool_search` catalog scan.
+///
+/// Each deferred tool needs a lowercased `name\ndescription\ninput_schema` blob
+/// that is compared once and dropped. Building it with `format!` also copied all
+/// three pieces a second time into the concatenation, and the bm25 scorer then
+/// re-lowered `tool.name` once per query term for a value that does not vary
+/// across terms. Reusing one set of buffers across the scan removes the
+/// concatenation copy and the per-term lowering, and keeps the buffers' capacity
+/// instead of reallocating per tool (#6213 T5).
+///
+/// This is the same precomputed-index idiom `CachedFallback` already uses for
+/// the static core-action fallbacks in this file; it is not a new pattern.
+///
+/// Lowercasing deliberately stays `str::to_lowercase`, matching the original
+/// exactly. A per-`char` fold would allocate less but is not the same function —
+/// it differs on Greek final sigma — and this path runs a handful of times per
+/// turn beside a multi-second provider call, so it is not worth a semantic
+/// change.
+#[derive(Default)]
+struct ToolSearchScratch {
+    /// `tool.name`, lowercased. Loop-invariant across query terms, so the bm25
+    /// scorer reads this instead of re-lowering the name once per term.
+    name_lower: String,
+    /// Compact JSON of `tool.input_schema`, before lowercasing.
+    schema_json: String,
+    /// The match target: `name\ndescription\nschema`, all lowercased.
+    hay: String,
+}
+
+impl ToolSearchScratch {
+    fn load(&mut self, tool: &Tool) {
+        use std::fmt::Write as _;
+
+        self.name_lower.clear();
+        self.name_lower.push_str(&tool.name.to_lowercase());
+
+        self.schema_json.clear();
+        // `Value`'s `Display` is what `to_string()` calls, so this is the same
+        // text without materializing an owned copy first. Infallible for a
+        // `String` sink; a formatting error could only shorten the schema,
+        // which weakens matching and never breaks correctness.
+        let _ = write!(self.schema_json, "{}", tool.input_schema);
+
+        self.hay.clear();
+        self.hay.push_str(&self.name_lower);
+        self.hay.push('\n');
+        self.hay.push_str(&tool.description.to_lowercase());
+        self.hay.push('\n');
+        self.hay.push_str(&self.schema_json.to_lowercase());
+    }
 }
 
 fn catalog_contains_tool(catalog: &[Tool], name: &str) -> bool {
@@ -820,6 +863,7 @@ fn discover_tools_with_regex(
         .map_err(|err| ToolError::invalid_input(format!("Invalid regex query: {err}")))?;
 
     let mut matches = Vec::new();
+    let mut scratch = ToolSearchScratch::default();
     for tool in catalog {
         // tool_search loads definitions omitted from the current request. An
         // eager tool is already present, so returning it as a cache candidate
@@ -828,8 +872,8 @@ fn discover_tools_with_regex(
         if !tool.defer_loading.unwrap_or(false) || is_tool_search_tool(&tool.name) {
             continue;
         }
-        let hay = tool_search_haystack(tool);
-        if regex.is_match(&hay) {
+        scratch.load(tool);
+        if regex.is_match(&scratch.hay) {
             matches.push(tool.name.clone());
         }
         if matches.len() >= max_results {
@@ -850,17 +894,19 @@ fn discover_tools_with_bm25_like(catalog: &[Tool], query: &str, max_results: usi
     }
 
     let mut scored: Vec<(i64, String)> = Vec::new();
+    let mut scratch = ToolSearchScratch::default();
     for tool in catalog {
         if !tool.defer_loading.unwrap_or(false) || is_tool_search_tool(&tool.name) {
             continue;
         }
-        let hay = tool_search_haystack(tool);
+        scratch.load(tool);
         let mut score = 0i64;
         for term in &terms {
-            if hay.contains(term) {
+            if scratch.hay.contains(term) {
                 score += 1;
             }
-            if tool.name.to_lowercase().contains(term) {
+            // Loop-invariant: lowered once by `load`, not once per term.
+            if scratch.name_lower.contains(term) {
                 score += 2;
             }
         }

crates/tui/src/core/engine/turn_loop.rs

@@ -5118,20 +5118,13 @@ impl Engine {
                                     tool_state.name, partial_json, tool_state.input_buffer
                                 ));
                             }
-                            // Mid-stream mirror of a partial buffer. The
-                            // argument text is *expected* to be incomplete
-                            // here, so `structure_synthesized` is ignored on
-                            // purpose; ContentBlockStop below is where an
-                            // unfinished argument becomes an error.
-                            if let Some(parsed) = parse_tool_input(&tool_state.input_buffer) {
-                                tool_state.input = parsed.value.clone();
-                                if crate::logging::is_verbose() {
-                                    crate::logging::info(format!(
-                                        "Tool '{}' input parsed: {:?}",
-                                        tool_state.name, parsed.value
-                                    ));
-                                }
-                            }
+                            // The buffer is the only mid-stream state: nothing
+                            // reads `tool_state.input` before finalization, so
+                            // there is no mirror parse here. Running the
+                            // `arg_repair` ladder per delta re-scanned the whole
+                            // accumulated buffer O(n²) times per tool call to
+                            // produce a value that `finalize_streamed_tool_input`
+                            // unconditionally overwrote (#6213 T4).
                         }
                     }
                 },
@@ -5174,8 +5167,8 @@ impl Engine {
                         && let Some(tool_state) = tool_uses.get_mut(tool_idx)
                     {
                         crate::logging::info(format!(
-                            "Tool '{}' block stop. Buffer: '{}', Current input: {:?}",
-                            tool_state.name, tool_state.input_buffer, tool_state.input
+                            "Tool '{}' block stop. Buffer: '{}'",
+                            tool_state.name, tool_state.input_buffer
                         ));
                         self.finalize_streamed_tool_input(tool_state).await;
 
@@ -5232,14 +5225,12 @@ impl Engine {
             }
         }
         // A stream cut at the provider's output limit ends without the
-        // closing ContentBlockStop for whatever block was in flight. Those
-        // blocks' inputs still hold the mid-stream mirror's best-effort
-        // parse, which ignores `structure_synthesized` by design — left
-        // as-is, a truncated tool call reaches dispatch through
-        // `tool.input` and executes (#5986). Every block that never
-        // stopped goes through the same finalization gate a normal
-        // ContentBlockStop applies, and is announced with the same
-        // finalized input.
+        // closing ContentBlockStop for whatever block was in flight. Before
+        // this drain existed a truncated tool call reached dispatch through
+        // `tool.input` and executed (#5986). Every block that never stopped
+        // goes through the same finalization gate a normal ContentBlockStop
+        // applies, and is announced with the same finalized input — which is
+        // also why no mid-stream parse is needed (#6213 T4).
         for tool_idx in std::mem::take(&mut current_tool_indices).into_values() {
             let Some(tool_state) = tool_uses.get_mut(tool_idx) else {
                 continue;
@@ -5284,9 +5275,9 @@ impl Engine {
     /// execute a truncated tool call (#5986). Called for a tool block that
     /// closes normally (`ContentBlockStop`) and again after the stream ends
     /// for blocks whose Stop never arrived — a provider cutting the stream
-    /// at its output limit omits the closing event, while the mid-stream
-    /// mirror deliberately ignores `structure_synthesized` because partial
-    /// text is the normal state mid-stream.
+    /// at its output limit omits the closing event. This is the only place
+    /// the accumulated buffer is parsed, and the only place
+    /// `structure_synthesized` is rejected.
     async fn finalize_streamed_tool_input(&self, tool_state: &mut ToolUseState) {
         if tool_state.input_buffer.trim().is_empty() {
             crate::logging::warn(format!(

crates/tui/src/fleet/identity.rs

@@ -280,7 +280,7 @@ pub enum FleetSelectorError {
         path: String,
     },
     #[error(
-        "Fleet member selector `{selector}` is ambiguous; choose one member explicitly: {candidates}"
+        "Fleet member selector `{selector}` is ambiguous; choose one member explicitly by passing `profile` as one of: {candidates}"
     )]
     Ambiguous {
         selector: String,

crates/tui/src/fleet/worker_runtime.rs

@@ -23,7 +23,7 @@ use codewhale_protocol::fleet::{
 };
 use serde::{Deserialize, Serialize};
 
-use super::identity::resolve_member_in_profiles;
+use super::identity::{FleetSelectorError, resolve_member_in_profiles};
 use super::profile::{
     AgentProfile, FleetDelegationHints, FleetLoadout, FleetProfile, FleetProfilePermissions,
     FleetRole as FleetProfileRole, FleetSlot, ProfileOrigin, canonical_public_role_name,
@@ -935,7 +935,7 @@ pub(crate) fn append_agent_profile_prompt(prompt: &mut String, agent_profile: &A
 pub(crate) fn resolve_pinned_role_profile(
     agent_profiles: &[AgentProfile],
     role: &str,
-) -> Result<Option<AgentProfile>> {
+) -> Result<Option<AgentProfile>, FleetSelectorError> {
     let pinned = agent_profiles
         .iter()
         .filter(|profile| {
@@ -949,11 +949,11 @@ pub(crate) fn resolve_pinned_role_profile(
         })
         .cloned()
         .collect::<Vec<_>>();
-    Ok(resolve_member_in_profiles(
+    resolve_member_in_profiles(
         &pinned,
         &format!("role:{}", canonical_public_role_name(role)),
-    )?
-    .cloned())
+    )
+    .map(|member| member.cloned())
 }
 
 /// Compare only the known route pair; never infer a provider from a wire id's

crates/tui/src/tools/subagent/mod.rs

@@ -14927,13 +14927,28 @@ fn resolve_spawn_route_profile(
     if member.is_some() || configured_manual_spawn_model(runtime, request)?.is_some() {
         return Ok(member);
     }
-    let role = request
-        .assignment
-        .role
-        .as_deref()
-        .unwrap_or_else(|| request.agent_type.as_str());
-    let member = crate::fleet::worker_runtime::resolve_pinned_role_profile(roster.members(), role)
-        .map_err(|error| ToolError::invalid_input(error.to_string()))?;
+    // A role the caller never wrote is Codewhale's own default
+    // (`FleetRole::Worker` -> "general"), not a request. Refusing an ambiguous
+    // pin is right when the caller named a role, type or profile — it stops us
+    // silently choosing one of several providers for them. But for a
+    // prompt-only `agent(action=start, ...)` it turned our default into a hard
+    // failure whenever two members happened to share role `general`, blocking
+    // the spawn at the tool boundary over a selector the caller never asked
+    // for (#6244). In that case an ambiguous pin means "no usable pin": fall
+    // through to the session/operator route the spawn would have taken anyway.
+    let requested_role = request.assignment.role.as_deref();
+    let role = requested_role.unwrap_or_else(|| request.agent_type.as_str());
+    let role_was_requested = requested_role.is_some() || request.agent_type_explicit;
+    let member =
+        match crate::fleet::worker_runtime::resolve_pinned_role_profile(roster.members(), role) {
+            Ok(member) => member,
+            Err(crate::fleet::identity::FleetSelectorError::Ambiguous { .. })
+                if !role_was_requested =>
+            {
+                None
+            }
+            Err(error) => return Err(ToolError::invalid_input(error.to_string())),
+        };
     let Some(member) = member else {
         return Ok(None);
     };

crates/tui/src/tools/subagent/tests.rs

@@ -4825,6 +4825,64 @@ fn saved_role_ambiguity_fails_unless_a_higher_priority_pin_selects_the_route() {
     assert_eq!(explicit.agent_type, FleetRole::Reviewer);
 }
 
+/// A prompt-only spawn must not be refused over a role the caller never wrote.
+///
+/// `request.agent_type` defaults to `FleetRole::Worker`, whose `as_str()` is
+/// `"general"`, so the route lookup synthesized `role:general` for a call that
+/// named no type, role or profile. Two saved members sharing role `general` then
+/// made every bare `agent(action=start, ...)` fail at the tool boundary (#6244).
+/// The refusal is still correct when the caller *did* ask — that half is pinned
+/// by `saved_role_ambiguity_fails_unless_a_higher_priority_pin_selects_the_route`.
+#[test]
+fn prompt_only_spawn_survives_an_ambiguous_default_role() {
+    let root = tempdir().unwrap();
+    for id in ["general-a", "general-b"] {
+        std::fs::write(
+            root.path().join(format!("{id}.toml")),
+            format!(
+                "id = '{id}'\nbase_role = 'general'\nprovider = 'deepseek'\nmodel = 'deepseek-v4-flash'\n",
+            ),
+        )
+        .unwrap();
+    }
+    let profiles = crate::fleet::profile::load_agent_profiles_from_dir(root.path()).unwrap();
+    assert_eq!(profiles.len(), 2);
+    let roster = FleetRoster::from_members(profiles);
+    let runtime = stub_runtime();
+
+    let mut request = parse_spawn_request(&json!({"prompt":"do the thing"})).unwrap();
+    assert!(
+        !request.agent_type_explicit,
+        "a prompt-only spawn must not report an explicit type"
+    );
+    assert_eq!(request.assignment.role, None);
+
+    let member = resolve_spawn_route_profile(&runtime, &mut request, &roster)
+        .expect("an ambiguous default role must not block a prompt-only spawn");
+    assert!(
+        member.is_none(),
+        "no member is pinned, so the spawn falls through to the session route"
+    );
+    assert_eq!(
+        request.profile, None,
+        "an unusable pin must not stamp a member"
+    );
+
+    // The same roster still refuses when the caller actually named the role.
+    let mut asked = parse_spawn_request(&json!({"prompt":"do the thing", "type":"general"}))
+        .expect("an explicit general type parses");
+    if asked.agent_type_explicit {
+        let error = resolve_spawn_route_profile(&runtime, &mut asked, &roster)
+            .expect_err("an explicitly requested ambiguous role must still refuse");
+        let message = error.to_string();
+        assert!(message.contains("ambiguous"), "{message}");
+        assert!(
+            message.contains("profile"),
+            "the refusal must say how to choose one: {message}"
+        );
+    }
+}
+
 #[test]
 fn providerless_spawn_model_gate_rejects_known_foreign_route_before_spawn() {
     let runtime = stub_runtime_for_provider("moonshot");