fix: MCP tools not surfacing after Sync to Nodes, update Anthropic to… (#272)

* fix: MCP tools not surfacing after Sync to Nodes, update Anthropic tool search Three fixes: 1. session_factory closure captured mcp_client=None when no --mcp-config was passed at startup. internal_mcp_reload created a new MCPClientManager on app.state but the factory never saw it. New workstreams got 0 MCP tools. Fix: mutable _mcp_ref list shared between factory and reload handler. 2. Anthropic dropped the date suffix from tool_search_tool_bm25_20251119 and now requires name == type. Updated constant and tool definition. 3. Add diagnostic logging around API errors (provider, model, base_url, message counts, full exception chain) and workstream resume (pre/post provider state, alias resolution warnings). Also adds Node.js 24 LTS to Dockerfile via multi-stage copy for npx-based MCP servers. * fix: address Copilot review — set_storage on reload, sanitize log output - Call mcp_mgr.set_storage(storage) when internal_mcp_reload creates a new MCPClientManager so prompt sync works for post-startup servers - Strip query params from base_url before logging (may contain API keys in some vLLM deployments) - Split API error logging: concise warning (type names only) + separate debug with exc_info=True for full traceback when needed * chore: remove DDG MCP sidecar, web_search uses built-in ddgs client The DuckDuckGo MCP server container is redundant — the built-in DuckDuckGoClient (via ddgs package, included in all extras) auto-detects when no Tavily key is configured. Removes the ddg-search service, ddgCluster profile, and mcp-ddg.json config file.
2026-08-12 23:12:23 -06:00 · 2026-04-01 12:42:09 -07:00
parent e901e859c7
commit 651c4d98cd
8 changed files with 85 additions and 56 deletions
@@ -18,6 +18,12 @@ RUN apt-get update && apt-get upgrade -y && apt-get install -y --no-install-reco
    libpq5 git curl jq man-db manpages procps file \
    && rm -rf /var/lib/apt/lists/*

+# Node.js LTS (for npx-based MCP servers like @modelcontextprotocol/server-github)
+COPY --from=node:24-slim /usr/local/bin/node /usr/local/bin/node
+COPY --from=node:24-slim /usr/local/lib/node_modules /usr/local/lib/node_modules
+RUN ln -s ../lib/node_modules/npm/bin/npm-cli.js /usr/local/bin/npm \
+    && ln -s ../lib/node_modules/npm/bin/npx-cli.js /usr/local/bin/npx
+
 # Non-root user
 RUN useradd --create-home --shell /bin/bash turnstone

@@ -6,7 +6,6 @@
 #   Single node:          docker compose --profile production up
 #   Production (PG):      DB_BACKEND=postgresql docker compose --profile production up
 #   10-node cluster:      docker compose --profile cluster up
-#   Cluster + DDG:        docker compose --profile ddgCluster up
 # =============================================================================

 name: turnstone
@@ -28,7 +27,6 @@ services:
    profiles:
      - production
      - cluster
-      - ddgCluster
    command:
      - postgres
      - -c
@@ -82,7 +80,6 @@ services:
      - "${SERVER_PORT:-8080}:8080"
    volumes:
      - turnstone-data:/data
-      - ./docker/mcp-ddg.json:/etc/turnstone/mcp-ddg.json:ro
    environment:
      - LLM_BASE_URL=${LLM_BASE_URL:-http://host.docker.internal:8000/v1}
      - OPENAI_API_KEY=${OPENAI_API_KEY:-dummy}
@@ -105,9 +102,6 @@ services:
      postgres:
        condition: service_healthy
        required: false
-      ddg-search:
-        condition: service_healthy
-        required: false
    healthcheck:
      test: ["CMD", "python", "/usr/local/bin/healthcheck.py", "http://127.0.0.1:8080/health"]
      interval: 10s
@@ -157,7 +151,6 @@ services:
    profiles:
      - production
      - cluster
-      - ddgCluster
    command:
      - sh
      - -c
@@ -181,39 +174,6 @@ services:
        required: false
    restart: unless-stopped

-  # -------------------------------------------------------------------
-  # ddg-search — DuckDuckGo Search MCP server (HTTP transport)
-  # Provides web search + content fetch tools to turnstone via MCP.
-  # No API key required.
-  #
-  # Start with: MCP_CONFIG=/etc/turnstone/mcp-ddg.json \
-  #             docker compose --profile ddgCluster up
-  # -------------------------------------------------------------------
-  ddg-search:
-    image: python:3.14-slim
-    profiles:
-      - ddgCluster
-    command:
-      - sh
-      - -c
-      - >-
-        pip install --no-cache-dir duckduckgo-mcp-server &&
-        python -c "from mcp.server.transport_security import TransportSecuritySettings; import duckduckgo_mcp_server.server as s; s.safe_search=s.SafeSearchMode.OFF; s.mcp.settings.host='0.0.0.0'; s.mcp.settings.port=3000; s.mcp.settings.transport_security=TransportSecuritySettings(enable_dns_rebinding_protection=False); s.mcp.run(transport='streamable-http')"
-    networks:
-      - turnstone-net
-    healthcheck:
-      test: ["CMD-SHELL", "python -c \"import socket; s=socket.create_connection(('0.0.0.0',3000),2); s.close()\""]
-      interval: 10s
-      timeout: 5s
-      retries: 3
-      start_period: 30s
-    deploy:
-      resources:
-        limits:
-          memory: 256M
-          cpus: '0.25'
-    restart: unless-stopped
-
  # ===================================================================
  # 10-node cluster (profile: cluster)
  #
@@ -228,7 +188,7 @@ services:
  server-1: &cluster-server
    image: turnstone:local
    build: { context: ., dockerfile: Dockerfile }
-    profiles: [cluster, ddgCluster]
+    profiles: [cluster]
    command:
      - sh
      - -c
@@ -243,7 +203,6 @@ services:
        $${MCP_CONFIG:+--mcp-config $$MCP_CONFIG}
    volumes:
      - turnstone-data:/data
-      - ./docker/mcp-ddg.json:/etc/turnstone/mcp-ddg.json:ro
    environment: &cluster-server-env
      LLM_BASE_URL: ${LLM_BASE_URL:-http://host.docker.internal:8000/v1}
      OPENAI_API_KEY: ${OPENAI_API_KEY:-dummy}
@@ -262,7 +221,6 @@ services:
    networks: [turnstone-net]
    depends_on:
      postgres: { condition: service_healthy }
-      ddg-search: { condition: service_healthy, required: false }
    healthcheck:
      test: ["CMD", "python", "/usr/local/bin/healthcheck.py", "http://127.0.0.1:8080/health"]
      interval: 10s
@@ -1,7 +0,0 @@
-{
-  "mcpServers": {
-    "ddg": {
-      "url": "http://ddg-search:3000/mcp"
-    }
-  }
-}
@@ -593,7 +593,7 @@ current turn and letting it search for them on demand.
 Tool search uses the best available mechanism for each provider:

 1. **Anthropic (native)** -- Models that support it receive `defer_loading: true`
-   on deferred tool definitions plus the `tool_search_tool_bm25_20251119` server-side
+   on deferred tool definitions plus the `tool_search_tool_bm25` server-side
   search tool. Anthropic's API handles search and expansion transparently.

 2. **OpenAI GPT-5.4+ (native)** -- Models with hosted tool search receive
@@ -2303,8 +2303,8 @@ class TestAnthropicToolSearch:
        # MCP tool should be deferred
        assert result[1]["defer_loading"] is True
        # Search tool should be appended
-        assert result[-1]["type"] == "tool_search_tool_bm25_20251119"
-        assert result[-1]["name"] == "tool_search"
+        assert result[-1]["type"] == "tool_search_tool_bm25"
+        assert result[-1]["name"] == "tool_search_tool_bm25"

    def test_inject_tool_search_no_op_without_deferred(self, provider):
        caps = provider.get_capabilities("claude-opus-4-6-20260101")
@@ -69,7 +69,7 @@ def _merge_consecutive(messages: list[dict[str, Any]]) -> list[dict[str, Any]]:
 _WEB_SEARCH_TOOL_TYPE = "web_search_20250305"

 # Tool search: server-side BM25 tool discovery for deferred tools
-_TOOL_SEARCH_TOOL_TYPE = "tool_search_tool_bm25_20251119"
+_TOOL_SEARCH_TOOL_TYPE = "tool_search_tool_bm25"

 # -- model capabilities -------------------------------------------------------

@@ -205,7 +205,7 @@ class AnthropicProvider:
                result.append({**tool, "defer_loading": True})
            else:
                result.append(tool)
-        result.append({"type": _TOOL_SEARCH_TOOL_TYPE, "name": "tool_search"})
+        result.append({"type": _TOOL_SEARCH_TOOL_TYPE, "name": _TOOL_SEARCH_TOOL_TYPE})
        return result

    # -- shared param logic --------------------------------------------------
@@ -980,6 +980,13 @@ class ChatSession:
        self._msg_tokens = [
            max(1, int(self._msg_char_count(m) / self._chars_per_token)) for m in self.messages
        ]
+        log.info(
+            "Resuming ws=%s: %d messages, provider=%s, model=%s",
+            ws_id,
+            len(messages),
+            type(self._provider).__name__,
+            self.model,
+        )
        # Restore persisted config
        config = load_workstream_config(ws_id)
        if config:
@@ -997,11 +1004,24 @@ class ChatSession:
                self.context_window = cfg.context_window
                if not self._manual_tool_truncation:
                    self.tool_truncation = int(cfg.context_window * self._chars_per_token * 0.5)
+                log.info(
+                    "Resume: resolved alias=%s → provider=%s, model=%s, ctx=%d",
+                    saved_alias,
+                    type(self._provider).__name__,
+                    model_name,
+                    cfg.context_window,
+                )
            elif saved_model and saved_model != self.model:
                # No alias or alias no longer in registry — at least set the model name
                self.model = saved_model
                self._model_alias = None
                self._cached_capabilities = None
+                log.warning(
+                    "Resume: alias %r not in registry, keeping default provider=%s for model=%s",
+                    saved_alias,
+                    type(self._provider).__name__,
+                    saved_model,
+                )
            if "temperature" in config:
                self.temperature = float(config["temperature"])
            if "reasoning_effort" in config:
@@ -1368,6 +1388,21 @@ class ChatSession:
    ) -> Iterator[StreamChunk]:
        """Attempt a streaming API call with retries on transient errors."""
        prov = provider or self._provider
+        raw_url = str(getattr(client, "base_url", getattr(client, "_base_url", "?")))
+        safe_url = raw_url.split("?")[0]  # strip query params (may contain keys)
+        msg_count = len(msgs)
+        role_counts: dict[str, int] = {}
+        for m in msgs:
+            r = m.get("role", "?")
+            role_counts[r] = role_counts.get(r, 0) + 1
+        log.debug(
+            "API call: provider=%s model=%s base_url=%s msgs=%d roles=%s",
+            type(prov).__name__,
+            model,
+            safe_url,
+            msg_count,
+            role_counts,
+        )
        last_err: Exception | None = None
        for attempt in range(self._MAX_RETRIES + 1):
            self._check_cancelled()
@@ -1387,6 +1422,29 @@ class ChatSession:
                )
            except Exception as e:
                ename = type(e).__name__
+                cause_name = (
+                    type(e.__cause__).__name__
+                    if e.__cause__
+                    else (type(e.__context__).__name__ if e.__context__ else "None")
+                )
+                log.warning(
+                    "API error (attempt %d/%d): %s (cause=%s) "
+                    "provider=%s model=%s base_url=%s msgs=%d",
+                    attempt + 1,
+                    self._MAX_RETRIES + 1,
+                    ename,
+                    cause_name,
+                    type(prov).__name__,
+                    model,
+                    safe_url,
+                    msg_count,
+                )
+                log.debug(
+                    "API error details (attempt %d/%d)",
+                    attempt + 1,
+                    self._MAX_RETRIES + 1,
+                    exc_info=True,
+                )
                if ename not in prov.retryable_error_names or attempt == self._MAX_RETRIES:
                    raise
                last_err = e
@@ -2101,7 +2101,12 @@ def internal_mcp_reload(request: Request) -> JSONResponse:

        mcp_mgr = MCPClientManager({})
        mcp_mgr.start()
+        mcp_mgr.set_storage(storage)
        request.app.state.mcp_client = mcp_mgr
+        # Update shared ref so session_factory sees the new client
+        mcp_ref = getattr(request.app.state, "mcp_ref", None)
+        if mcp_ref is not None:
+            mcp_ref[0] = mcp_mgr

    result = mcp_mgr.reconcile_sync(storage)
    return JSONResponse({"status": "ok", **result})
@@ -2450,6 +2455,7 @@ def create_app(
    health_monitor: Any = None,
    rate_limiter: Any = None,
    mcp_client: Any = None,
+    mcp_ref: list[Any] | None = None,
    registry: Any = None,
    idle_timeout: int = 0,
    node_id: str = "",
@@ -2530,6 +2536,7 @@ def create_app(
    app.state.health_monitor = health_monitor
    app.state.rate_limiter = rate_limiter
    app.state.mcp_client = mcp_client
+    app.state.mcp_ref = mcp_ref
    app.state.registry = registry
    app.state.idle_timeout = idle_timeout
    app.state.node_id = node_id
@@ -2756,6 +2763,9 @@ def main() -> None:
        refresh_interval=config_store.get("mcp.refresh_interval"),
        storage=_get_storage(),
    )
+    # Mutable ref so session_factory always sees the latest MCP client,
+    # including ones created by internal_mcp_reload after startup.
+    _mcp_ref: list[Any] = [mcp_client]

    # Backend health monitor with circuit breaker
    from turnstone.core.healthcheck import BackendHealthMonitor
@@ -2866,6 +2876,9 @@ def main() -> None:
    ) -> ChatSession:
        assert ui is not None
        r_client, r_model, r_cfg = registry.resolve(model_alias)
+        # Read MCP client from shared ref — may have been replaced after startup
+        # by internal_mcp_reload (Sync to Nodes) when no --mcp-config was passed.
+        live_mcp_client = _mcp_ref[0]
        uid = getattr(ui, "_user_id", "") or ""

        # Resolve username from user_id for system message context
@@ -2900,7 +2913,7 @@ def main() -> None:
            auto_compact_pct=config_store.get("session.auto_compact_pct"),
            agent_max_turns=config_store.get("tools.agent_max_turns"),
            tool_truncation=config_store.get("tools.truncation"),
-            mcp_client=mcp_client,
+            mcp_client=live_mcp_client,
            registry=registry,
            model_alias=model_alias or registry.default,
            health_monitor=health_monitor,
@@ -3031,6 +3044,7 @@ def main() -> None:
        health_monitor=health_monitor,
        rate_limiter=rate_limiter,
        mcp_client=mcp_client,
+        mcp_ref=_mcp_ref,
        registry=registry,
        idle_timeout=config_store.get("server.workstream_idle_timeout"),
        node_id=_node_id,