Tool use and MCP fixes

[ScriptSmith]

No description provided.

[greptile-apps]

Greptile Summary

This PR substantially improves MCP (Model Context Protocol) reliability and UX, and fixes several tool-use correctness issues in the streaming pipeline.

Key changes:

• MCP client resilience — MCPClient now manages SSE reconnection itself with exponential backoff (up to 5 attempts) instead of relying on the browser's native auto-reconnect, and detects session expiry (404/410) to trigger a full re-initialization. callMCPTool in the store retries once after a session-expiry error.
• MCP auto-connect & lifecycle — servers now connect automatically on add/enable; listener subscriptions are tracked and cleaned up on disconnect; notifications/tools/list_changed triggers automatic tool re-discovery.
• MCP config modal — dedicated bearer-token field with show/hide toggle; "Test" button now correctly includes auth headers (fixing the previous thread concern); expandable tool items show parameter schemas; enable and connect are merged into a single toggle.
• Tool execution correctness — the ensureConnected call before building the tool list prevents tool calls on a server that hasn't finished connecting. The multi-round continuation now includes the assistant's output message items alongside function calls, giving the next round full context. Streamed text deltas are now treated as authoritative (the response.output_text.done text is used only as a fallback when no deltas arrived).
• max_content_chars (Rust) — ExaTextOptions struct carries maxCharacters to Exa's API; the truncate closure is now also applied to Tavily results (both gaps from the previous review are addressed).
• Rendering unification — MultiModelResponse synthesises completedRounds for committed messages that predate the field, and guards streaming-state selectors behind an empty key for non-streaming responses to prevent state leakage across messages.
• Default tool-iteration cap raised from 5 → 25 (now also configurable per-conversation via a slider in Advanced settings).
• CSP broadened — connect-src now allows https: http: wss: ws: to support user-configured MCP servers at arbitrary URLs; the http: scheme is a deliberate trade-off noted in comments.

Confidence Score: 4/5

• Safe to merge after addressing the stale test-result UX bug and the content fallback ordering in conversationStore.
• The PR resolves several known issues from the previous review round (Tavily truncation, bearer token in test connection, redundant max_chars re-declaration). The MCP resilience and rendering unification changes are well-scoped and include cleanup paths. Two minor issues remain: bearerToken is missing from the watch list that resets stale test results (misleading UX), and the content ?? "" fallback in replaceAssistantMessage is silently overridden by the ...updates spread. Neither is a blocking regression, but both are straightforward one-line fixes.
• ui/src/components/MCPConfigModal/MCPConfigModal.tsx (bearerToken watch gap), ui/src/stores/conversationStore.ts (content fallback spread ordering)

Important Files Changed

Filename	Overview
ui/src/services/mcp/client.ts	Adds manual SSE reconnection with exponential backoff (replacing browser auto-reconnect), session-expiry detection on 404/410, and helper methods closeEventSource/clearReconnectTimer for clean teardown.
ui/src/stores/mcpStore.ts	Adds ensureConnected, auto-connect on server add/enable, listener cleanup tracking, notifications/tools/list_changed handler for dynamic tool discovery, and auto-reconnect on session expiry in callMCPTool.
ui/src/components/MCPConfigModal/MCPConfigModal.tsx	Major redesign: dedicated bearer-token field with show/hide toggle, "Test" button that now correctly includes auth headers, expandable ToolItem with schema display, and unified enable+connect toggle. Minor: bearerToken is missing from the useEffect watch that resets stale test results.
ui/src/pages/chat/useChat.ts	Raises default tool-iteration cap from 5 → 25 (now configurable), calls ensureConnected before building MCP tool list, includes assistant output-message items in multi-round continuations, and fixes content accumulation by treating streamed deltas as authoritative over the response.output_text.done payload.
ui/src/stores/conversationStore.ts	Refactors replaceAssistantMessage to accept a Partial<ChatMessage> updates object; when inserting a new message the content ?? "" fallback is shadowed by the trailing ...updates spread, which could set content to undefined if a future caller omits it.
src/config/server.rs	Broadens default CSP connect-src from a tight CDN allowlist to https: http: wss: ws: to support user-configured MCP servers. The http: scheme in particular allows the browser UI to make plaintext connections to arbitrary hosts, which is a meaningful security trade-off worth reviewing.
ui/src/pages/chat/utils/toolExecutors.ts	Enriches the MCP tool executor with input/output artifact generation (JSON args, text output, images, resource text) for timeline display, covering error and exception paths alike.
src/routes/api/tools.rs	Adds ExaTextOptions struct to pass max_characters to Exa's API; applies the truncate closure to Tavily results (fixing the previously noted bug); max_content_chars now works for both providers.
ui/src/components/MultiModelResponse/MultiModelResponse.tsx	Unifies rendering through completedRounds for all response types, synthesizing rounds for committed messages that predate the field; fixes streaming-state leakage into committed messages by using an empty model key when not streaming.
ui/src/stores/streamingStore.ts	Adds artifact deduplication by ID in appendArtifacts (prevents duplicate entries on re-render) and a new useRunningExecutionStatusMessage selector for showing per-tool status messages.

Sequence Diagram

sequenceDiagram
    participant UI as ChatPage / useChat
    participant Store as mcpStore
    participant Client as MCPClient
    participant Server as MCP Server

    UI->>Store: ensureConnected()
    Store->>Store: filter enabled & not connected
    Store->>Client: connectServer(id)
    Client->>Server: POST (initialize)
    Server-->>Client: session ID + capabilities
    Client->>Server: GET SSE stream
    Server-->>Client: open SSE connection
    Client-->>Store: onStatusChange("connected")
    Store->>Client: listAllTools()
    Client-->>Store: tool definitions
    Store-->>UI: MCP tools available

    UI->>Store: callMCPTool(serverId, toolName, args)
    Store->>Client: client.callTool(toolName, args)
    Client->>Server: POST (tool call)
    alt Success
        Server-->>Client: tool result
        Client-->>Store: result
        Store-->>UI: { success: true, artifacts }
    else Session expired (404/410)
        Server-->>Client: 404/410
        Client-->>Store: throw "session expired"
        Store->>Client: connectServer(id) [reconnect]
        Client->>Server: POST (re-initialize)
        Server-->>Client: new session
        Store->>Client: retry callTool
        Client-->>Store: result
        Store-->>UI: { success: true, artifacts }
    end

    Note over Client,Server: SSE error → exponential backoff reconnect (max 5 attempts)

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: ui/src/stores/conversationStore.ts
Line: 251-257

Comment:
**`content` fallback silently overridden by `...updates` spread**

`content: updates.content ?? ""` is set on line 253, but then `...updates` on line 257 re-spreads the same key — if `updates.content` is `undefined`, the spread wins and the field ends up as `undefined`, defeating the `?? ""` fallback.

All current callers always provide a string `content`, so this is harmless today, but it is fragile. Moving the spread before the explicit fields makes the intent clear and guarantees the fallback applies:

```suggestion
        messages.splice(insertIndex, 0, {
          id: crypto.randomUUID(),
          role: "assistant",
          ...updates,
          content: updates.content ?? "",
          model,
          timestamp: new Date(),
        });
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/config/server.rs
Line: 477-479

Comment:
**CSP `http:` scheme allows arbitrary insecure connections**

The new `connect-src 'self' https: http: wss: ws:` is much broader than the previous explicit allowlist. Including bare `http:` means the browser UI may send authenticated requests (with bearer tokens or cookies) to any plaintext HTTP endpoint, which is exploitable by a network-level attacker. The comment explains the motivation (user-configured MCP servers at arbitrary URLs), but it's worth considering a narrower alternative: keep `https:` / `wss:` for all remote endpoints and add `http://localhost http://127.0.0.1` (or the loopback CIDR if supported) for local-only MCP servers. This would still serve the Ollama / local-MCP use case without opening the full `http:` scheme to the whole internet.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: ui/src/components/MCPConfigModal/MCPConfigModal.tsx
Line: 404-413

Comment:
**`bearerToken` excluded from test-status reset watch**

The `useEffect` resets the test result only when `url` or `headers` change, but `bearerToken` also determines the outcome of `handleTestConnection`. If a user tests with a valid token and sees "success", then changes the token to an invalid value without touching the URL or headers, the stale success badge persists — misleading them into thinking the new token works.

The fix is to also watch the `bearerToken` field so any change to it clears the previous test result, the same way URL and headers changes do.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (2): Last reviewed commit: "Enable server when added" | Re-trigger Greptile}

[ScriptSmith]

@greptile-apps

[greptile-apps]

CSP http: scheme allows arbitrary insecure connections

The new connect-src 'self' https: http: wss: ws: is much broader than the previous explicit allowlist. Including bare http: means the browser UI may send authenticated requests (with bearer tokens or cookies) to any plaintext HTTP endpoint, which is exploitable by a network-level attacker. The comment explains the motivation (user-configured MCP servers at arbitrary URLs), but it's worth considering a narrower alternative: keep https: / wss: for all remote endpoints and add http://localhost http://127.0.0.1 (or the loopback CIDR if supported) for local-only MCP servers. This would still serve the Ollama / local-MCP use case without opening the full http: scheme to the whole internet.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/config/server.rs
Line: 477-479

Comment:
**CSP `http:` scheme allows arbitrary insecure connections**

The new `connect-src 'self' https: http: wss: ws:` is much broader than the previous explicit allowlist. Including bare `http:` means the browser UI may send authenticated requests (with bearer tokens or cookies) to any plaintext HTTP endpoint, which is exploitable by a network-level attacker. The comment explains the motivation (user-configured MCP servers at arbitrary URLs), but it's worth considering a narrower alternative: keep `https:` / `wss:` for all remote endpoints and add `http://localhost http://127.0.0.1` (or the loopback CIDR if supported) for local-only MCP servers. This would still serve the Ollama / local-MCP use case without opening the full `http:` scheme to the whole internet.

How can I resolve this? If you propose a fix, please make it concise.