From 0af80e262f7d5af594cdb4054f4ae6eb33c94568 Mon Sep 17 00:00:00 2001
From: Hunter B <hmbown@gmail.com>
Date: Mon, 1 Jun 2026 00:41:52 -0700
Subject: [PATCH] fix: prevent double-registration of web/patch tools in agent
 mode
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

with_agent_tools() unconditionally registered web_search/fetch_url/web.run
(via with_web_tools) and apply_patch (via with_patch_tools), but tool_setup.rs
conditionally registered them again based on Feature::WebSearch and
Feature::ApplyPatch flags. This caused double registration (overwritten
with a warning log on the second insert).

Changes:
- Remove with_web_tools() and with_patch_tools() from with_agent_tools()
- Move finance tool out of with_web_tools() into its own with_finance_tool()
  (finance is market data, not web search — it should not be gated behind
  the web-search feature flag)
- Add with_finance_tool() to with_agent_tools() so finance stays always
  available
- Update tests: new test for with_finance_tool(), updated web_tools test
  to verify finance is no longer in the web group
---
 crates/tui/src/tools/registry.rs | 49 ++++++++++++++++++++++++++------
 1 file changed, 41 insertions(+), 8 deletions(-)

diff --git a/crates/tui/src/tools/registry.rs b/crates/tui/src/tools/registry.rs
index b0e31844..b33c79c5 100644
--- a/crates/tui/src/tools/registry.rs
+++ b/crates/tui/src/tools/registry.rs
@@ -702,19 +702,32 @@ impl ToolRegistryBuilder {
             .with_tool(Arc::new(AutomationReadTool))
     }
 
-    /// Include web search tools.
+    /// Include web search and fetch tools.
+    ///
+    /// These are feature-gated behind `Feature::WebSearch` in `tool_setup.rs`.
+    /// `finance` is registered separately via `with_finance_tool()` and is
+    /// NOT gated behind the web-search feature.
     #[must_use]
     pub fn with_web_tools(self) -> Self {
         use super::fetch_url::FetchUrlTool;
-        use super::finance::FinanceTool;
         use super::web_run::WebRunTool;
         use super::web_search::WebSearchTool;
         self.with_tool(Arc::new(WebSearchTool))
             .with_tool(Arc::new(FetchUrlTool))
-            .with_tool(Arc::new(FinanceTool::new()))
             .with_tool(Arc::new(WebRunTool))
     }
 
+    /// Include the `finance` market-data tool.
+    ///
+    /// This tool is registered unconditionally for agent modes and is NOT
+    /// gated behind `Feature::WebSearch` (it fetches financial data, not
+    /// web search results).
+    #[must_use]
+    pub fn with_finance_tool(self) -> Self {
+        use super::finance::FinanceTool;
+        self.with_tool(Arc::new(FinanceTool::new()))
+    }
+
     /// Register the `image_analyze` vision tool.
     /// Only registered when `[vision_model]` is configured in config.toml.
     #[must_use]
@@ -882,17 +895,21 @@ impl ToolRegistryBuilder {
         self
     }
 
-    /// Include all agent tools (file tools + shell + note + search + patch).
+    /// Include all agent tools (file tools + shell + note + search).
+    ///
+    /// Web and patch tools are NOT registered here — callers must add them
+    /// via `.with_web_tools()` and `.with_patch_tools()` after checking
+    /// feature flags (see `tool_setup.rs`). This prevents double-registration
+    /// when `tool_setup.rs` conditionally registers them on top of
+    /// `with_agent_tools`.
     #[must_use]
     pub fn with_agent_tools(self, allow_shell: bool) -> Self {
         let builder = self
             .with_file_tools()
             .with_note_tool()
             .with_search_tools()
-            .with_web_tools()
             .with_user_input_tool()
             .with_parallel_tool()
-            .with_patch_tools()
             .with_git_tools()
             .with_git_history_tools()
             .with_diagnostics_tool()
@@ -905,7 +922,8 @@ impl ToolRegistryBuilder {
             .with_runtime_task_tools()
             .with_revert_turn_tool()
             .with_pandoc_tools()
-            .with_image_ocr_tools();
+            .with_image_ocr_tools()
+            .with_finance_tool();
 
         if allow_shell {
             builder.with_shell_tools().with_runtime_task_shell_tools()
@@ -1509,12 +1527,27 @@ mod tests {
     }
 
     #[test]
-    fn test_builder_with_web_tools_includes_finance() {
+    fn test_builder_with_web_tools_no_longer_includes_finance() {
         let tmp = tempdir().expect("tempdir");
         let ctx = ToolContext::new(tmp.path().to_path_buf());
 
         let registry = ToolRegistryBuilder::new().with_web_tools().build(ctx);
 
+        // finance was moved to with_finance_tool() in v0.8.49;
+        // with_web_tools() now only registers web search / fetch / web.run
+        assert!(registry.contains("web_search"));
+        assert!(registry.contains("fetch_url"));
+        assert!(registry.contains("web.run"));
+        assert!(!registry.contains("finance"));
+    }
+
+    #[test]
+    fn test_builder_with_finance_tool() {
+        let tmp = tempdir().expect("tempdir");
+        let ctx = ToolContext::new(tmp.path().to_path_buf());
+
+        let registry = ToolRegistryBuilder::new().with_finance_tool().build(ctx);
+
         assert!(registry.contains("finance"));
     }