@sigitcodereview / sigit / commits / 9d2bb52

feat: add web_search tool backed by siGit Code Cloud's Brave Search API

sigit-si already runs a Brave-Search-backed web_search tool on its official MCP server (WebSearchService + Mcp::Tools::WebSearch, api/v1/mcp), and mcp.rs's existing generic MCP client already discovers and can forward to it as mcp__sigit__web_search whenever the user is signed in. That path alone isn't quite right for two reasons: permissions::classify treats every mcp__* tool as mutating, so a harmless search would trigger an "ask" permission prompt on every call; and the raw mcp__sigit__web_search name is a worse fit for prompting than a clean, purpose-described tool. So web_search is a thin native wrapper in tools.rs, not a second implementation: its schema mirrors the cloud tool's contract exactly (query + count) and execute_tool forwards the call verbatim to mcp::call_tool(mcp__sigit__web_search, ...) — no new HTTP/JSON-RPC/auth code. It's classified read-only in permissions.rs (zero-friction, like read_website), and offered only when web_search_available() confirms the MCP tool was actually discovered this session (i.e. the user is signed in) — same conditional-advertisement pattern as skill and task. The raw mcp__sigit__web_search entry is filtered out of the assembled tool list whenever the native wrapper is offered, so the model sees one option, not two names for the same tool. Investigated sigit-si's app/services/web_search_service.rb and app/services/mcp/tools.rb to confirm the tool's actual argument shape, that tools/list has no subscription-tier gating beyond sign-in (just a 60/hour per-user rate limit), and that MCP discovery already fails closed (contributes no tools) for an unauthenticated request — which is what web_search_available() relies on instead of adding a separate sign-in check. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

paydii committed Jul 10, 2026 at 23:59 UTC 9d2bb52e015d18cb2fe6602d75ce9c67ee6beba3

5 files changed +189 -7

.agents/AGENTS.md

+11 -1

@@ -110,7 +110,17 @@ feeds results back. Neither the loop nor ACP/TUI surfaces depend on a concrete b

110 spec list (`all_tools`) and the execute `match` (`execute_tool`). `run_command` also enforces

111 commit attribution: when a command creates a new commit that lacks the

112 `Co-Authored-By: siGit Code` trailer (`COMMIT_CO_AUTHOR_TRAILER`), it amends the trailer in —

113 - unless the commit already exists on a remote, which is never rewritten.

113 + unless the commit already exists on a remote, which is never rewritten. Also owns `web_search`:

114 + a thin native wrapper around the official MCP server's Brave-Search-backed `web_search` tool

115 + (`mcp__sigit__web_search`, implemented server-side in `sigit-si`'s `WebSearchService` +

116 + `Mcp::Tools::WebSearch`), offered only when that MCP tool was actually discovered (i.e. the

117 + user is signed in to siGit Code Cloud — see `web_search_available`). Wrapping it natively

118 + rather than leaving it as a raw `mcp__*` tool matters for two reasons: `permissions::classify`

119 + treats every `mcp__*` tool as mutating (an "ask" prompt on every call), while `web_search` is

120 + read-only like `read_website`; and the raw delegate name is filtered out of the assembled tool

121 + list (`is_web_search_delegate`) so the model sees one clean option, not two names for the same

122 + tool. Execution still forwards verbatim to `mcp::call_tool` — no second HTTP/JSON-RPC

123 + implementation.

124 - **`src/skills.rs`** — [Agent Skills](https://agentskills.io) support. Discovers skill

125 folders (each with a `SKILL.md`: YAML frontmatter `name` + `description`, then Markdown

126 instructions) from `.sigit/skills/` and `.claude/skills/` in the cwd, `$SIGIT_CONFIG_DIR/skills/`,

src/chat.rs

+15 -2

@@ -2403,8 +2403,21 @@ mod tui {

2403 specs.push(crate::tools::task_tool_spec());

2404 }

2405

2406 - // Tools discovered from configured MCP servers (incl. the official one).

2407 - specs.extend(crate::mcp::tool_specs());

2406 + // `web_search` is a native wrapper around the official MCP server's

2407 + // `web_search` tool, offered only when that MCP tool was actually

2408 + // discovered (i.e. the user is signed in to siGit Code Cloud).

2409 + let web_search_offered = crate::tools::web_search_available();

2410 + if web_search_offered {

2411 + specs.push(crate::tools::web_search_tool_spec());

2412 + }

2413 +

2414 + // Tools discovered from configured MCP servers (incl. the official

2415 + // one). Exclude the raw web_search delegate when the native wrapper

2416 + // above is already offered, so the model sees one clean option, not

2417 + // two names for the same tool.

2418 + specs.extend(crate::mcp::tool_specs().into_iter().filter(|spec| {

2419 + !(web_search_offered && crate::tools::is_web_search_delegate(&spec.name))

2420 + }));

2421

2422 specs

2423 }

src/main.rs

+18 -2

@@ -255,7 +255,7 @@ fn tool_kind_for(tool_name: &str) -> ToolKind {

255 "delete_file" => ToolKind::Delete,

256 "run_command" | "kill_command" => ToolKind::Execute,

257 "read_file" | "list_directory" | "command_output" => ToolKind::Read,

258 - "search_files" | "glob" => ToolKind::Search,

258 + "search_files" | "glob" | "web_search" => ToolKind::Search,

259 "read_website" => ToolKind::Fetch,

260 "write_todos" => ToolKind::Think,

261 _ => ToolKind::Other,

@@ -314,8 +314,24 @@ fn agent_tools_as_specs() -> Vec<ToolSpec> {

314 specs.push(tools::task_tool_spec());

315 }

316

317 + // `web_search` is a native wrapper around the official MCP server's

318 + // `web_search` tool (see the module doc in tools.rs), offered only when

319 + // that MCP tool was actually discovered (i.e. the user is signed in to

320 + // siGit Code Cloud).

321 + let web_search_offered = tools::web_search_available();

322 + if web_search_offered {

323 + specs.push(tools::web_search_tool_spec());

324 + }

325 +

326 // Tools discovered from configured MCP servers (incl. the official one).

318 - specs.extend(mcp::tool_specs());

327 + // Exclude the raw web_search delegate when the native wrapper above is

328 + // already offered, so the model sees one clean option, not two names for

329 + // the same tool.

330 + specs.extend(

331 + mcp::tool_specs()

332 + .into_iter()

333 + .filter(|spec| !(web_search_offered && tools::is_web_search_delegate(&spec.name))),

334 + );

335

336 specs

337 }

src/permissions.rs

+6 -2

@@ -87,11 +87,14 @@ pub enum Decision {

87 /// conservative default for anything whose side effects we can't see.

88 /// `task` is read-only because the subagent it launches is restricted to the

89 /// read-only toolset (see `SUBAGENT_TOOL_NAMES` in `tools.rs`), so delegated

90 -/// research stays available in plan mode.

90 +/// research stays available in plan mode. `web_search` is read-only for the

91 +/// same reason `read_website` is — it only ever returns data, even though it

92 +/// forwards to an `mcp__*` tool under the hood (see `tools.rs`'s `web_search`

93 +/// module doc for why it isn't classified by that `mcp__` prefix).

94 pub fn classify(tool_name: &str) -> ToolRisk {

95 match tool_name {

96 "read_file" | "list_directory" | "search_files" | "glob" | "read_website"

94 - | "write_todos" | "skill" | "task" | "command_output" => ToolRisk::ReadOnly,

98 _ => ToolRisk::Mutating,

99 }

100 }

@@ -418,6 +421,7 @@ mod tests {

421 "skill",

422 "task",

423 "command_output",

424 + "web_search",

425 ] {

426 assert_eq!(classify(tool), ToolRisk::ReadOnly, "{tool}");

427 assert_eq!(decision_for("t-ro", tool, "{}"), Decision::Allow, "{tool}");

src/tools.rs

+139

@@ -453,6 +453,7 @@ pub async fn execute_tool(name: &str, arguments: &str) -> String {

453 "kill_command" => exec_kill_command(arguments),

454 "skill" => crate::skills::activate_skill(arguments),

455 TASK_TOOL_NAME => exec_task(arguments).await,

456 + WEB_SEARCH_TOOL_NAME => exec_web_search(arguments).await,

457 // Tools discovered from MCP servers are namespaced `mcp__<server>__<tool>`

458 // and forwarded to the owning server.

459 _ if crate::mcp::is_mcp_tool(name) => crate::mcp::call_tool(name, arguments).await,

@@ -460,6 +461,101 @@ pub async fn execute_tool(name: &str, arguments: &str) -> String {

461 }

462 }

463

464 +// ── web_search ───────────────────────────────────────────────────────────────

465 +//

466 +// siGit Code Cloud runs a Brave-Search-backed `web_search` tool on its official

467 +// MCP server (`mcp.rs`'s baked-in "sigit" server, discovered as

468 +// `mcp__sigit__web_search`) — the API key and billing live server-side, so

469 +// there is no on-device/local equivalent. This is a thin native wrapper

470 +// around that MCP tool rather than a second implementation of it, for two

471 +// reasons: (1) `mcp.rs` already owns the auth, transport, and error handling,

472 +// so re-doing that here would just be a second copy of the same HTTP/JSON-RPC

473 +// code; (2) every `mcp__*` tool is classified `Mutating` by

474 +// `permissions::classify` (an unknown external tool's side effects can't be

475 +// assumed safe), which would put an "ask" prompt in front of a harmless

476 +// read-only search on every call. Presenting it under the clean `web_search`

477 +// name instead lets `permissions::classify` treat it as read-only, matching

478 +// `read_website`'s zero-friction behavior, while `execute_tool_impl` still

479 +// delegates the actual call to the exact same `mcp::call_tool` codepath as

480 +// any other MCP tool.

481 +//

482 +// Advertised only when the official server has actually discovered a

483 +// `web_search` tool (see `web_search_available`) — the same conditional

484 +// pattern as `skill` and `task` — so a signed-out or MCP-disabled session

485 +// never sees a capability it can't use.

486 +

487 +pub const WEB_SEARCH_TOOL_NAME: &str = "web_search";

488 +

489 +/// The discovered MCP tool this delegates to: siGit Code Cloud's official

490 +/// server is always registered under the name "sigit" (see `mcp.rs`), so its

491 +/// tools are namespaced `mcp__sigit__<tool>`.

492 +const WEB_SEARCH_MCP_DELEGATE: &str = "mcp__sigit__web_search";

493 +

494 +/// Whether `web_search` should be offered this turn: the official MCP server

495 +/// has to have actually discovered a `web_search` tool, which in turn only

496 +/// happens when the user is signed in to siGit Code Cloud (an unauthenticated

497 +/// request to the official server fails during `mcp::init`'s discovery and

498 +/// contributes no tools — see `mcp.rs`). No separate sign-in check is needed

499 +/// here: this is exactly the same conditional-advertisement pattern as

500 +/// `subagent_available`, just checking a tool name instead of a factory.

501 +pub fn web_search_available() -> bool {

502 + crate::mcp::tool_specs()

503 + .iter()

504 + .any(|spec| spec.name == WEB_SEARCH_MCP_DELEGATE)

505 +}

506 +

507 +/// Whether `name` is the raw MCP tool [`web_search_tool_spec`] delegates to.

508 +/// Callers assembling the full tool list should exclude this name from

509 +/// whatever they append from `mcp::tool_specs()`, so the model sees the one

510 +/// clean `web_search` option rather than that *and* the raw `mcp__*` name for

511 +/// the same underlying tool.

512 +pub fn is_web_search_delegate(name: &str) -> bool {

513 + name == WEB_SEARCH_MCP_DELEGATE

514 +}

515 +

516 +/// Spec for the `web_search` tool. Lives in the `*_as_specs`/`build_tool_specs`

517 +/// layer (like `skill` and `task`), not in [`all_tools`], because it is only

518 +/// offered when [`web_search_available`] is true. The schema mirrors

519 +/// siGit Code Cloud's actual `web_search` MCP tool exactly (`query` + `count`)

520 +/// so arguments pass through to [`exec_web_search`] unchanged.

521 +pub fn web_search_tool_spec() -> ToolSpec {

522 + ToolSpec {

523 + name: WEB_SEARCH_TOOL_NAME.to_string(),

524 + description: "Search the public web and return matching pages as title, URL, and \

525 + snippet. Use this to find pages when you don't already know the URL — for \

526 + current events, library/API documentation, error messages, or anything else \

527 + outside this repository. Once you have a promising URL, use read_website to \

528 + fetch its full content. Requires a signed-in siGit Code Cloud account (the \

529 + search runs server-side); rate-limited per account."

530 + .to_string(),

531 + parameters_schema: json!({

532 + "type": "object",

533 + "properties": {

534 + "query": {

535 + "type": "string",

536 + "description": "The search query."

537 + },

538 + "count": {

539 + "type": "integer",

540 + "description": "Number of results to return (default 5, max 10)."

541 + }

542 + },

543 + "required": ["query"],

544 + "additionalProperties": false

545 + })

546 + .to_string(),

547 + }

548 +}

549 +

550 +/// The `web_search` tool entry point used by [`execute_tool`]: forward

551 +/// verbatim to the discovered MCP tool. `mcp::call_tool` already handles a

552 +/// missing/uninitialized server gracefully (an in-band error string, never a

553 +/// panic), so there's nothing extra to do here — see the module doc above for

554 +/// why this delegates instead of reimplementing the call.

555 +async fn exec_web_search(arguments: &str) -> String {

556 + crate::mcp::call_tool(WEB_SEARCH_MCP_DELEGATE, arguments).await

557 +}

558 +

559 // ── task (subagent) ──────────────────────────────────────────────────────────

560 //

561 // The `task` tool delegates a self-contained research question to a *nested*

@@ -3161,6 +3257,49 @@ mod tests {

3257 );

3258 }

3259

3260 + // ── web_search tests ─────────────────────────────────────────────────

3261 +

3262 + #[test]

3263 + fn test_web_search_tool_spec_matches_the_cloud_mcp_tools_contract() {

3264 + let spec = web_search_tool_spec();

3265 + assert_eq!(spec.name, "web_search");

3266 + let schema: Value = serde_json::from_str(&spec.parameters_schema).unwrap();

3267 + assert_eq!(schema["required"], serde_json::json!(["query"]));

3268 + assert!(schema["properties"]["query"].is_object());

3269 + assert!(schema["properties"]["count"].is_object());

3270 + }

3271 +

3272 + #[test]

3273 + fn test_is_web_search_delegate() {

3274 + assert!(is_web_search_delegate("mcp__sigit__web_search"));

3275 + assert!(!is_web_search_delegate("web_search"));

3276 + assert!(!is_web_search_delegate("mcp__other__web_search"));

3277 + }

3278 +

3279 + #[test]

3280 + fn test_web_search_unavailable_without_mcp_init() {

3281 + // `mcp::init()` is never called anywhere in this test binary (it does

3282 + // real network I/O and is a one-shot `OnceLock`, same constraint as

3283 + // `SUBAGENT_FACTORY`), so the official server's tools — including

3284 + // web_search — are never discovered here. This proves the

3285 + // conditional-advertisement gate fails closed, matching a

3286 + // signed-out/MCP-disabled session, rather than defaulting to "always

3287 + // offered".

3288 + assert!(!web_search_available());

3289 + }

3290 +

3291 + #[tokio::test]

3292 + async fn test_web_search_delegates_to_mcp_and_degrades_gracefully_when_uninitialized() {

3293 + // With MCP uninitialized (see the test above), `mcp::call_tool`

3294 + // returns its own in-band error string rather than panicking — proves

3295 + // execute_tool's WEB_SEARCH_TOOL_NAME arm actually reaches

3296 + // mcp::call_tool(WEB_SEARCH_MCP_DELEGATE, ...) instead of, say, silently

3297 + // no-op'ing or hitting the `Unknown tool` fallback.

3298 + let args = serde_json::json!({ "query": "siGit Code" }).to_string();

3299 + let result = execute_tool(WEB_SEARCH_TOOL_NAME, &args).await;

3300 + assert_eq!(result, "Error: MCP is not initialized.");

3301 + }

3302 +

3303 #[test]

3304 fn test_command_output_missing_task_id() {

3305 let result = exec_command_output("{}");