Add web_search MCP tool with a provider adapter

index c2af613..e707792 100644 --- a/.agents/specs/mcp-server.md +++ b/.agents/specs/mcp-server.md @@ -1,6 +1,6 @@ --- name: mcp-server -description: "Specification for the siGit MCP server at /api/v1/mcp. A stateless Streamable-HTTP JSON-RPC endpoint that exposes siGit repositories to MCP clients (Claude Code and others) as six tools: list_repositories, get_file_contents, search_code, list_pull_requests, create_pull_request, add_issue_comment. Covers transport, auth, the protocol method set, the tool contracts, the pull request / issue / comment data model, authorization, and acceptance criteria." +description: "Specification for the siGit MCP server at /api/v1/mcp. A stateless Streamable-HTTP JSON-RPC endpoint that exposes siGit repositories to MCP clients (Claude Code and others) as seven tools: list_repositories, get_file_contents, search_code, list_pull_requests, create_pull_request, add_issue_comment, web_search. Covers transport, auth, the protocol method set, the tool contracts, the pull request / issue / comment data model, authorization, and acceptance criteria." status: implemented implements: - app/controllers/api/v1/mcp_controller.rb @@ -9,11 +9,14 @@ implements: - app/services/mcp/tool_error.rb - app/models/{pull_request,issue,comment,repository}.rb - app/services/{pull_request_service,comment_service}.rb + - app/services/web_search_service.rb - config/routes.rb verifies: - spec/requests/api/v1/mcp_spec.rb + - spec/requests/api/v1/mcp_web_search_spec.rb - spec/services/pull_request_service_spec.rb - spec/services/comment_service_spec.rb + - spec/services/web_search_service_spec.rb --- # siGit MCP server @@ -142,7 +145,7 @@ so adding a tool is a one-line change (append the class to `all`). Each tool declares `tool_name`, `description`, `input_schema` (JSON Schema surfaced to the model), and a `call`. -The six tools, in registry order: +The seven tools, in registry order: ### list_repositories @@ -206,6 +209,29 @@ The six tools, in registry order: - **Output:** `{ id, url }`. - **Errors (in band):** no issue or PR with that number; blank body. +### web_search + +- **Purpose:** search the public web and return result links with snippets, so + the agent can find pages to read (it can already fetch a known URL). Search is + a signed-in cloud feature of the official MCP server, not a client-side API + key. +- **Input (required):** `query`. Optional `count` (default 5, clamped 1..10 — + out-of-range values are clamped, never an error). +- **Behaviour:** delegates to `WebSearchService`, which selects a provider + adapter via `SEARCH_PROVIDER` (default `brave`) and returns a uniform result + shape regardless of provider. The Brave adapter calls the Brave Search API + (`GET https://api.search.brave.com/res/v1/web/search`, auth via the + `X-Subscription-Token` header set from `BRAVE_SEARCH_API_KEY`, params `q` and + `count`) and maps `web.results[]` `title`/`url`/`description` to the output. +- **Rate limit:** at most 60 searches per user per hour, a sliding window + counted in `Rails.cache` (solid_cache in production). Exceeding it is an + in-band `isError`, not a protocol error. +- **Output:** JSON array of `{ title, url, snippet }`. +- **Errors (in band):** missing query; unconfigured provider (the message names + the env var the operator must set — never a 500); unknown `SEARCH_PROVIDER`; + provider HTTP failure (the message carries the upstream status); rate limit + exceeded. + ## Data model The app had no pull request, issue, or comment concept before this feature; the @@ -258,7 +284,7 @@ authorization run for the MCP path as for any future web UI. 1. `claude mcp add --transport http sigit https://sigit.si/api/v1/mcp --header "Authorization: Bearer <token>"` connects. -2. `tools/list` returns all six tools. +2. `tools/list` returns all seven tools. 3. `list_repositories` and `get_file_contents` work end to end against a real repository. 4. An unauthenticated request returns a JSON-RPC 401 with a `WWW-Authenticate` @@ -268,8 +294,14 @@ authorization run for the MCP path as for any future web UI. ## Verification - Request specs (`spec/requests/api/v1/mcp_spec.rb`): initialize handshake, - `tools/list` shows all six, a successful `tools/call`, a notification answered + `tools/list` shows all seven, a successful `tools/call`, a notification answered with 202, a 401 with the challenge, and a tool returning `isError`. +- Web search request specs (`spec/requests/api/v1/mcp_web_search_spec.rb`): + happy path with the provider HTTP call stubbed, count clamping, missing query, + unconfigured and unknown provider reported in-band, provider 4xx/5xx surfaced + in-band, and the rate limit. Service specs + (`spec/services/web_search_service_spec.rb`): provider selection and the Brave + adapter's request shape, parsing, and error mapping. - Service specs (`spec/services/pull_request_service_spec.rb`, `comment_service_spec.rb`): authorization, branch validation, head-differs-from- base, shared numbering, comment resolution across issues and pull requests, @@ -294,6 +326,8 @@ authorization run for the MCP path as for any future web UI. - `app/services/mcp/tool_error.rb`: in-band tool failure type. - `app/services/pull_request_service.rb`, `app/services/comment_service.rb`: mutation services. +- `app/services/web_search_service.rb`: web search with the provider adapter + boundary (Brave first). - `app/models/pull_request.rb`, `issue.rb`, `comment.rb`, and the additions to `repository.rb` and `git_repository_service.rb`. - `config/routes.rb`: `post`/`get api/v1/mcp`.

index eb1c6d1..66ef870 100644 --- a/.env.example +++ b/.env.example @@ -43,6 +43,20 @@ ONDE_CLOUD_APP_SECRET=your-onde-app-secret-here # ONDE_CLOUD_BASE_URL=https://cloud.ondeinference.com/v1 +# ----------------------------------------------------------------------------- +# Web search (the MCP web_search tool) +# Search runs server-side so signed-in siGit Code users need no search API key +# of their own. Without a key the tool answers with an in-band error telling +# the operator to set this; nothing else breaks. +# ----------------------------------------------------------------------------- + +# Which provider adapter WebSearchService uses. Optional; defaults to "brave". +# SEARCH_PROVIDER=brave + +# Brave Search API subscription token (https://api-dashboard.search.brave.com/) +# BRAVE_SEARCH_API_KEY=your-brave-search-api-key + + # ----------------------------------------------------------------------------- # Stripe — siGit Code Pro/Team billing (the /billing page + Checkout/Portal). # Separate from Onde Cloud billing. Billing is a no-op unless STRIPE_SECRET_KEY

index 6e7f1eb..038e9fd 100644 --- a/app/services/mcp/server.rb +++ b/app/services/mcp/server.rb @@ -43,7 +43,8 @@ module Mcp "capabilities" => { "tools" => { "listChanged" => false } }, "serverInfo" => { "name" => "sigit", "title" => "siGit Code", "version" => "0.1.0" }, "instructions" => "Tools for siGit-hosted git repositories: list repos, " \ - "read files, search code, and open or comment on pull requests and issues." + "read files, search code, and open or comment on pull requests and issues. " \ + "Also web_search, for finding pages on the public web." } end

index cc5d2a4..fa3ffa9 100644 --- a/app/services/mcp/tools.rb +++ b/app/services/mcp/tools.rb @@ -21,7 +21,8 @@ module Mcp SearchCode, ListPullRequests, CreatePullRequest, - AddIssueComment + AddIssueComment, + WebSearch ].freeze end @@ -241,5 +242,50 @@ module Mcp raise Mcp::ToolError, e.record.errors.full_messages.to_sentence end end + + class WebSearch < Base + # Per-user sliding window: at most RATE_LIMIT searches per RATE_WINDOW. + # Backed by Rails.cache (solid_cache in production) because the app has + # no rack-attack or Redis counter infrastructure to reuse. + RATE_LIMIT = 60 + RATE_WINDOW = 1.hour + + name! "web_search" + describe "Search the web and return matching pages as title, URL, and snippet. " \ + "Use this to find pages to read when you don't already know the URL." + input_schema( + "type" => "object", + "required" => %w[query], + "properties" => { + "query" => { "type" => "string", "description" => "The search query." }, + "count" => { "type" => "integer", "description" => "Number of results (default 5, max 10).", "default" => 5 } + } + ) + + def call(args) + query = require_arg(args, "query") + count = (args["count"] || 5).to_i.clamp(1, 10) + check_rate_limit! + + WebSearchService.search(query, count: count) + rescue WebSearchService::Error => e + raise Mcp::ToolError, e.message + end + + private + + def check_rate_limit! + key = "mcp:web_search:user:#{current_user.id}" + cutoff = Time.current.to_i - RATE_WINDOW.to_i + stamps = Array(Rails.cache.read(key)).select { |t| t > cutoff } + + if stamps.size >= RATE_LIMIT + raise Mcp::ToolError, + "Rate limit exceeded: at most #{RATE_LIMIT} web searches per hour per user. Try again later." + end + + Rails.cache.write(key, stamps + [ Time.current.to_i ], expires_in: RATE_WINDOW) + end + end end end

new file mode 100644 index 0000000..47c73a9 --- /dev/null +++ b/app/services/web_search_service.rb @@ -0,0 +1,114 @@ +# frozen_string_literal: true + +require "net/http" +require "json" + +# Web search for the MCP server, behind a provider adapter boundary. +# +# `WebSearchService.search` picks a provider adapter by ENV and returns a +# uniform result shape — an array of `{ "title", "url", "snippet" }` hashes — +# so the MCP tool (and any future caller) never sees provider-specific JSON. +# Adding a provider means adding an adapter class with a +# `search(query, count:)` method and registering it in PROVIDERS. +# +# Failures (unconfigured or unknown provider, upstream HTTP errors, network +# errors) raise WebSearchService::Error with an actionable message; callers +# convert that to their own error channel (the MCP tool re-raises it as an +# in-band Mcp::ToolError, never a 500). +# +# Env vars: +# SEARCH_PROVIDER — which adapter to use (default "brave") +# BRAVE_SEARCH_API_KEY — Brave Search API subscription token +class WebSearchService + # Raised when a search can't be performed (missing configuration, provider + # HTTP failure, unusable response). The message is safe to show the caller. + class Error < StandardError; end + + DEFAULT_COUNT = 5 + MAX_COUNT = 10 + + def self.search(query, count: DEFAULT_COUNT) + provider.search(query, count: count.to_i.clamp(1, MAX_COUNT)) + end + + def self.provider + name = ENV.fetch("SEARCH_PROVIDER", "brave") + klass = PROVIDERS[name] + unless klass + raise Error, "Unknown search provider #{name.inspect}. " \ + "Set SEARCH_PROVIDER to one of: #{PROVIDERS.keys.join(', ')}." + end + klass.new + end + + # ── providers ────────────────────────────────────────────────────────────── + + # Brave Search API (https://api-dashboard.search.brave.com/). + # GET /res/v1/web/search with `q` and `count`; auth is the + # X-Subscription-Token header; results live under web.results[]. + class Brave + ENDPOINT = "https://api.search.brave.com/res/v1/web/search" + + # Bound how long a search can pin a puma thread (same rationale as + # OndeCloudService: a slow upstream must never exhaust the web pool). + OPEN_TIMEOUT = 5 + READ_TIMEOUT = 15 + + def search(query, count:) + uri = URI(ENDPOINT) + uri.query = URI.encode_www_form(q: query, count: count) + + response = http_get(uri, api_key!) + unless response.is_a?(Net::HTTPSuccess) + Rails.logger.error("[web_search] Brave upstream #{response.code}: " \ + "#{response.body.to_s.byteslice(0, 500)}") + raise Error, "Search provider request failed (HTTP #{response.code})" + end + + parse(response.body) + end + + # Maps Brave's response to the service's uniform result shape. + def parse(body) + results = JSON.parse(body).dig("web", "results") || [] + results.map do |r| + { "title" => r["title"], "url" => r["url"], "snippet" => r["description"] } + end + rescue JSON::ParserError + raise Error, "Search provider returned an unreadable response" + end + + private + + def api_key! + ENV["BRAVE_SEARCH_API_KEY"].presence || raise( + Error, + "Web search is not configured. Set BRAVE_SEARCH_API_KEY " \ + "(Brave Search API subscription token) on the server, " \ + "or point SEARCH_PROVIDER at a configured provider." + ) + end + + # The one HTTP seam — specs stub this method directly (the repo carries no + # webmock/vcr) and hand back a real Net::HTTPResponse. + def http_get(uri, api_key) + Net::HTTP.start( + uri.hostname, uri.port, + use_ssl: uri.scheme == "https", + open_timeout: OPEN_TIMEOUT, + read_timeout: READ_TIMEOUT + ) do |http| + request = Net::HTTP::Get.new(uri) + request["Accept"] = "application/json" + request["X-Subscription-Token"] = api_key + http.request(request) + end + rescue StandardError => e + Rails.logger.error("[web_search] connection error: #{e.class}: #{e.message}") + raise Error, "Search provider is temporarily unavailable" + end + end + + # Adapter registry, keyed by SEARCH_PROVIDER. + PROVIDERS = { "brave" => Brave }.freeze +end

index 56628a5..b0b7957 100644 --- a/spec/requests/api/v1/mcp_spec.rb +++ b/spec/requests/api/v1/mcp_spec.rb @@ -61,13 +61,14 @@ RSpec.describe "Api::V1::Mcp", type: :request do end describe "tools/list" do - it "lists all six tools" do + it "lists all seven tools" do tools = rpc({ "jsonrpc" => "2.0", "id" => 2, "method" => "tools/list" })["result"]["tools"] names = tools.map { |t| t["name"] } expect(names).to contain_exactly( "list_repositories", "get_file_contents", "search_code", - "list_pull_requests", "create_pull_request", "add_issue_comment" + "list_pull_requests", "create_pull_request", "add_issue_comment", + "web_search" ) expect(tools).to all(include("description", "inputSchema")) end

new file mode 100644 index 0000000..965c09b --- /dev/null +++ b/spec/requests/api/v1/mcp_web_search_spec.rb @@ -0,0 +1,160 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe "Api::V1::Mcp web_search tool", type: :request do + let(:user) do + User.create!(smbcloud_id: 4243, email: "mcp-searcher@example.com", username: "mcpsearcher") + end + + let(:token) { "valid-access-token" } + let(:auth_headers) do + { "Authorization" => "Bearer #{token}", "Content-Type" => "application/json" } + end + + before do + allow_any_instance_of(Api::V1::McpController) + .to receive(:resolve_user) { |_controller, presented| presented == token ? user : nil } + + # The provider adapter reads its key from ENV; default each example to a + # configured provider and override per-context below. + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:[]).with("BRAVE_SEARCH_API_KEY").and_return("brave-key") + end + + def rpc(body) + post "/api/v1/mcp", params: body.to_json, headers: auth_headers + response.parsed_body + end + + def call_web_search(arguments) + rpc({ + "jsonrpc" => "2.0", "id" => 1, "method" => "tools/call", + "params" => { "name" => "web_search", "arguments" => arguments } + })["result"] + end + + # The repo has no webmock/vcr, so specs stub the adapter's single HTTP seam + # (WebSearchService::Brave#http_get) with a real Net::HTTPResponse. + def http_response(code, body) + klass = Net::HTTPResponse::CODE_TO_OBJ.fetch(code.to_s) + response = klass.new("1.1", code.to_s, nil) + response.instance_variable_set(:@read, true) + response.instance_variable_set(:@body, body) + response + end + + def brave_body(results) + { "web" => { "results" => results } }.to_json + end + + def stub_brave(code, body) + allow_any_instance_of(WebSearchService::Brave) + .to receive(:http_get).and_return(http_response(code, body)) + end + + describe "tools/list" do + it "advertises web_search with its schema" do + tools = rpc({ "jsonrpc" => "2.0", "id" => 1, "method" => "tools/list" })["result"]["tools"] + spec = tools.find { |t| t["name"] == "web_search" } + + expect(spec).to be_present + expect(spec["description"]).to include("Search the web") + expect(spec.dig("inputSchema", "required")).to eq([ "query" ]) + expect(spec.dig("inputSchema", "properties")).to include("query", "count") + end + end + + describe "tools/call web_search" do + it "returns a JSON list of {title, url, snippet}" do + stub_brave(200, brave_body([ + { "title" => "Ruby", "url" => "https://ruby-lang.org", "description" => "A language." } + ])) + + result = call_web_search({ "query" => "ruby" }) + + expect(response).to have_http_status(:ok) + expect(result["isError"]).to be(false) + parsed = JSON.parse(result["content"].first["text"]) + expect(parsed).to eq([ + { "title" => "Ruby", "url" => "https://ruby-lang.org", "snippet" => "A language." } + ]) + end + + it "clamps count to at most 10 instead of erroring" do + requested_uri = nil + allow_any_instance_of(WebSearchService::Brave).to receive(:http_get) do |_adapter, uri, _key| + requested_uri = uri + http_response(200, brave_body([])) + end + + result = call_web_search({ "query" => "ruby", "count" => 50 }) + + expect(result["isError"]).to be(false) + expect(URI.decode_www_form(requested_uri.query).to_h["count"]).to eq("10") + end + + it "reports a missing query in-band" do + result = call_web_search({}) + + expect(response).to have_http_status(:ok) + expect(result["isError"]).to be(true) + expect(result["content"].first["text"]).to include("Missing required argument: query") + end + + it "reports an unconfigured provider in-band, naming the env var (not a 500)" do + allow(ENV).to receive(:[]).with("BRAVE_SEARCH_API_KEY").and_return(nil) + + result = call_web_search({ "query" => "ruby" }) + + expect(response).to have_http_status(:ok) + expect(result["isError"]).to be(true) + expect(result["content"].first["text"]).to include("BRAVE_SEARCH_API_KEY") + end + + it "reports an unknown SEARCH_PROVIDER in-band (not a 500)" do + allow(ENV).to receive(:fetch).with("SEARCH_PROVIDER", "brave").and_return("altavista") + + result = call_web_search({ "query" => "ruby" }) + + expect(response).to have_http_status(:ok) + expect(result["isError"]).to be(true) + expect(result["content"].first["text"]).to include("SEARCH_PROVIDER") + end + + it "surfaces a provider 4xx in-band with the status" do + stub_brave(429, "slow down") + + result = call_web_search({ "query" => "ruby" }) + + expect(result["isError"]).to be(true) + expect(result["content"].first["text"]).to include("HTTP 429") + end + + it "surfaces a provider 5xx in-band with the status" do + stub_brave(500, "boom") + + result = call_web_search({ "query" => "ruby" }) + + expect(result["isError"]).to be(true) + expect(result["content"].first["text"]).to include("HTTP 500") + end + + it "rejects a user over the hourly rate limit in-band" do + # The test env cache is :null_store; give the limiter a real store. + allow(Rails).to receive(:cache).and_return(ActiveSupport::Cache::MemoryStore.new) + Rails.cache.write( + "mcp:web_search:user:#{user.id}", + Array.new(Mcp::Tools::WebSearch::RATE_LIMIT) { Time.current.to_i } + ) + expect_any_instance_of(WebSearchService::Brave).not_to receive(:http_get) + + result = call_web_search({ "query" => "ruby" }) + + expect(response).to have_http_status(:ok) + expect(result["isError"]).to be(true) + expect(result["content"].first["text"]).to match(/rate limit/i) + end + end +end

new file mode 100644 index 0000000..b392839 --- /dev/null +++ b/spec/services/web_search_service_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +require "rails_helper" + +RSpec.describe WebSearchService do + # Builds a real Net::HTTPResponse (the class the adapter checks with + # Net::HTTPSuccess) carrying a canned body. The repo has no webmock/vcr, so + # specs stub the adapter's http_get seam with one of these. + def http_response(code, body) + klass = Net::HTTPResponse::CODE_TO_OBJ.fetch(code.to_s) + response = klass.new("1.1", code.to_s, nil) + response.instance_variable_set(:@read, true) + response.instance_variable_set(:@body, body) + response + end + + def brave_body(results) + { "web" => { "results" => results } }.to_json + end + + before do + allow(ENV).to receive(:[]).and_call_original + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:[]).with("BRAVE_SEARCH_API_KEY").and_return("brave-key") + end + + describe ".provider" do + it "defaults to the Brave adapter" do + expect(described_class.provider).to be_a(WebSearchService::Brave) + end + + it "raises a configuration error for an unknown SEARCH_PROVIDER" do + allow(ENV).to receive(:fetch).with("SEARCH_PROVIDER", "brave").and_return("bing") + + expect { described_class.provider } + .to raise_error(WebSearchService::Error, /unknown search provider "bing".*SEARCH_PROVIDER.*brave/im) + end + end + + describe ".search" do + it "clamps count to at most 10 before hitting the provider" do + adapter = instance_double(WebSearchService::Brave) + allow(described_class).to receive(:provider).and_return(adapter) + expect(adapter).to receive(:search).with("rust", count: 10).and_return([]) + + described_class.search("rust", count: 50) + end + end + + describe WebSearchService::Brave do + subject(:adapter) { described_class.new } + + it "requests the Brave endpoint with q, count, and the subscription token" do + captured_uri = nil + captured_key = nil + allow(adapter).to receive(:http_get) do |uri, api_key| + captured_uri = uri + captured_key = api_key + http_response(200, brave_body([])) + end + + adapter.search("ruby net/http", count: 3) + + expect(captured_uri.to_s).to start_with("https://api.search.brave.com/res/v1/web/search?") + expect(URI.decode_www_form(captured_uri.query).to_h) + .to eq("q" => "ruby net/http", "count" => "3") + expect(captured_key).to eq("brave-key") + end + + it "parses web.results[] into {title, url, snippet}" do + body = brave_body([ + { "title" => "Ruby", "url" => "https://ruby-lang.org", "description" => "A language.", + "extra" => "ignored" }, + { "title" => "Rails", "url" => "https://rubyonrails.org", "description" => "A framework." } + ]) + allow(adapter).to receive(:http_get).and_return(http_response(200, body)) + + expect(adapter.search("ruby", count: 5)).to eq([ + { "title" => "Ruby", "url" => "https://ruby-lang.org", "snippet" => "A language." }, + { "title" => "Rails", "url" => "https://rubyonrails.org", "snippet" => "A framework." } + ]) + end + + it "returns an empty list when the response has no web.results" do + allow(adapter).to receive(:http_get).and_return(http_response(200, "{}")) + + expect(adapter.search("nothing", count: 5)).to eq([]) + end + + it "raises when BRAVE_SEARCH_API_KEY is not set, naming the env var" do + allow(ENV).to receive(:[]).with("BRAVE_SEARCH_API_KEY").and_return(nil) + + expect { adapter.search("ruby", count: 5) } + .to raise_error(WebSearchService::Error, /BRAVE_SEARCH_API_KEY/) + end + + it "surfaces an upstream HTTP failure with its status" do + allow(adapter).to receive(:http_get).and_return(http_response(429, "slow down")) + + expect { adapter.search("ruby", count: 5) } + .to raise_error(WebSearchService::Error, /HTTP 429/) + end + + it "raises a readable error for an unparseable body" do + allow(adapter).to receive(:http_get).and_return(http_response(200, "<html>nope</html>")) + + expect { adapter.search("ruby", count: 5) } + .to raise_error(WebSearchService::Error, /unreadable response/) + end + end +end