Repository navigation
fix(coaching): stop usetsearch blocking every native ToolSearch call - #368
Merged
Merged
Conversation
item(list) had no cap when limit was omitted, unlike search/groom, so a bare call on a large project could overflow the MCP response token limit (155 items / 52k chars observed). Default it to 50 (capped at 500) and return a pagination envelope (total/offset/limit/next_offset/prev_offset) so callers can page forward/backward without re-deriving offsets by hand.
…inding - cargo fmt wrap on the next_offset line (CI fmt check was failing) - prev_offset now clamps via offset.min(total) so an out-of-range offset navigates back into range instead of repeating an empty page, and both next_offset/prev_offset suppress when limit == 0
enforced_rule_reason_for_tool matched on tool_name alone, so the "usetsearch" coaching rule denied every ToolSearch call regardless of what it was searching for -- including WebFetch/EnterPlanMode/ mcp__claude-in-chrome__* and agentflare's own mcp__flare__* tools, contradicting the rule's own stated fallback. It now inspects the query and only blocks genuine leanctx/ctx_* lookups, the one namespace actually missing from the deferred-tool list. Also add a builtin-tool catalog so mcp__flare__tool(action="search") finds agentflare's own first-party tools (item, asset, handoff, ...) before falling back to the public MCP registry -- previously a query like "item" returned unrelated third-party servers because the gateway registry only indexes configured downstream servers.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughToolSearch enforcement now evaluates query input for gateway-shaped requests. MCP search now finds first-party tools locally before using downstream registry results. ChangesTool search routing
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPToolHandler
participant BuiltinCatalog
participant GatewayRegistry
Client->>MCPToolHandler: search(query, limit)
MCPToolHandler->>BuiltinCatalog: search_builtin_tools(query, limit)
BuiltinCatalog-->>MCPToolHandler: ranked first-party ToolHit results
MCPToolHandler->>GatewayRegistry: search remaining slots
GatewayRegistry-->>MCPToolHandler: registry ToolHit results
MCPToolHandler-->>Client: merged search results
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/mcp_server/builtin_tools.rs`:
- Around line 62-64: Update the `tool` description in the builtin tool metadata
and the matching `#[tool]` description in `src/mcp_server.rs` to state that
action="search" returns both first-party Agentflare tools and downstream MCP
server tools, while preserving the existing search and execute behavior.
- Around line 123-155: Update search_builtin_tools in
src/mcp_server/builtin_tools.rs:123-155 to accept MatchMode and require every
query term to match when mode is All, while preserving any-term matching for
other modes. Update the builtin search call in src/mcp_server.rs:1122-1124 to
pass the parsed mode through.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ac79e10-e6f1-4e8c-b30d-4b861a188d03
📒 Files selected for processing (5)
src/coaching/store.rssrc/components.rssrc/hook.rssrc/mcp_server.rssrc/mcp_server/builtin_tools.rs
Comment on lines
+62
to
+64
| "tool", | ||
| "Tool operations — search downstream MCP servers' tools by task description or execute one. Single consolidated tool with `action` field (search|execute).", | ||
| ), |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the tool description for first-party results.
This now advertises downstream-only search, but action="search" also returns built-in Agentflare tools. Update this entry and the matching #[tool] description in src/mcp_server.rs together.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/mcp_server/builtin_tools.rs` around lines 62 - 64, Update the `tool`
description in the builtin tool metadata and the matching `#[tool]` description
in `src/mcp_server.rs` to state that action="search" returns both first-party
Agentflare tools and downstream MCP server tools, while preserving the existing
search and execute behavior.
- Content filter now only applies to the Builtin-tier "usetsearch" rule; a user override (same id, tier Override) enforces on tool name alone again, per the coaching system's "override always wins" contract. - Normalize spaces/hyphens to underscores before the leanctx/ctx_* keyword match, so "ctx read" or "lean ctx" still trigger it, not just the literal underscored form. - Replace the unguaranteed -1.0 score sentinel on builtin tool hits with a documented 0.0 constant that's provably worse than any real bm25 score and better than the registry fallback sentinel. - Strengthen merge_builtin_hits test coverage to assert hit source per index, not just result length.
- search_builtin_tools now accepts and honors MatchMode: All requires every query term to match, Any requires at least one. Previously it always did Any-style matching regardless of the caller's requested mode, so mode="all" (the tool's own default) could still surface a builtin hit like mcp__flare__item on a query such as "item nonexistent" that shouldn't have matched anything. - Updated the "tool" action's description (both the builtin catalog entry and the matching #[tool] attribute) to state that action="search" now returns first-party agentflare tools alongside downstream MCP server tools, not just downstream ones.
getappz
added a commit
that referenced
this pull request
Jul 30, 2026
* fix(mcp): default-limit and paginate item(list) item(list) had no cap when limit was omitted, unlike search/groom, so a bare call on a large project could overflow the MCP response token limit (155 items / 52k chars observed). Default it to 50 (capped at 500) and return a pagination envelope (total/offset/limit/next_offset/prev_offset) so callers can page forward/backward without re-deriving offsets by hand. * fix(mcp): clamp item(list) pagination edges for CI fmt + CodeRabbit finding - cargo fmt wrap on the next_offset line (CI fmt check was failing) - prev_offset now clamps via offset.min(total) so an out-of-range offset navigates back into range instead of repeating an empty page, and both next_offset/prev_offset suppress when limit == 0 * fix(coaching): stop usetsearch blocking every native ToolSearch call enforced_rule_reason_for_tool matched on tool_name alone, so the "usetsearch" coaching rule denied every ToolSearch call regardless of what it was searching for -- including WebFetch/EnterPlanMode/ mcp__claude-in-chrome__* and agentflare's own mcp__flare__* tools, contradicting the rule's own stated fallback. It now inspects the query and only blocks genuine leanctx/ctx_* lookups, the one namespace actually missing from the deferred-tool list. Also add a builtin-tool catalog so mcp__flare__tool(action="search") finds agentflare's own first-party tools (item, asset, handoff, ...) before falling back to the public MCP registry -- previously a query like "item" returned unrelated third-party servers because the gateway registry only indexes configured downstream servers. * style: rustfmt fixes for the usetsearch/builtin-tools change * fix: address review findings on usetsearch/builtin-tools PR - Content filter now only applies to the Builtin-tier "usetsearch" rule; a user override (same id, tier Override) enforces on tool name alone again, per the coaching system's "override always wins" contract. - Normalize spaces/hyphens to underscores before the leanctx/ctx_* keyword match, so "ctx read" or "lean ctx" still trigger it, not just the literal underscored form. - Replace the unguaranteed -1.0 score sentinel on builtin tool hits with a documented 0.0 constant that's provably worse than any real bm25 score and better than the registry fallback sentinel. - Strengthen merge_builtin_hits test coverage to assert hit source per index, not just result length. * fix: address CodeRabbit findings on PR #368 - search_builtin_tools now accepts and honors MatchMode: All requires every query term to match, Any requires at least one. Previously it always did Any-style matching regardless of the caller's requested mode, so mode="all" (the tool's own default) could still surface a builtin hit like mcp__flare__item on a query such as "item nonexistent" that shouldn't have matched anything. - Updated the "tool" action's description (both the builtin catalog entry and the matching #[tool] attribute) to state that action="search" now returns first-party agentflare tools alongside downstream MCP server tools, not just downstream ones. * fix: update enforced_rule_reason_for_tool call site pulled in from master merge * agentflare-backend: add item::list_by_label query * supervisor: add confirmed-autonomous-agent check * supervisor: add discovery/dispatch tick * dashboard: wire supervisor discovery loop into daemon startup * fmt
This was referenced Oct 16, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
enforced_rule_reason_for_toolmatched on tool name alone, so the builtinusetsearchcoaching rule denied everyToolSearchcall (includingWebFetch/EnterPlanMode/mcp__claude-in-chrome__*and agentflare's ownmcp__flare__*tools), contradicting the rule's own stated fallback. It now inspects the query and only blocks a genuine leanctx/ctx_*lookup — the one namespace actually missing from the deferred-tool list.src/mcp_server/builtin_tools.rs) somcp__flare__tool(action="search")finds agentflare's own first-party tools (item,asset,handoff, ...) before falling back to the public MCP registry — previously a query like "item" returned unrelated third-party servers, since the gateway registry only indexes configured downstream servers (e.g. leanctx).Test plan
cargo check --workspace --all-featurescargo test --workspace --all-features coaching::— 49 passed, incl. newtoolsearch_only_enforces_for_gateway_shaped_queriescargo test --workspace --all-features builtin_tools::— 6 passedcargo clippy -p agentflare --all-features --all-targets -- -D warnings— clean except pre-existingunsafe_codefindings in untouched files (tracked separately)Summary by CodeRabbit
usetsearchto reflect the refined matching behavior.