Skip to content

Add MCP for SubAgents / BrowserMode for Mention Menu Item#2377

Closed
buko wants to merge 12 commits into
Hmbown:mainfrom
buko:main
Closed

Add MCP for SubAgents / BrowserMode for Mention Menu Item#2377
buko wants to merge 12 commits into
Hmbown:mainfrom
buko:main

Conversation

@buko
Copy link
Copy Markdown

@buko buko commented May 30, 2026

This PR adds several critical fixes for CodeWhale that were encountered during the first evaluation phase. It adds MCP support for subagents and adds an option to turn the menu mention into a simple file browser. It also adds several important fixes to support Xiaomi Mimo v2.5.

Testing

  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features
  • cargo test --workspace --all-features

Checklist

  • Updated docs or comments as needed
  • Added or updated tests where relevant
  • Verified TUI behavior manually if UI changes

Greptile Summary

This PR adds MCP tool support for subagents, introduces a configurable file-browser mode for the @-mention menu, and wires in Xiaomi (Mimo v2.5) as a new API provider alongside a config migration that notifies users and exits cleanly on first run.

  • MCP for subagents: SubAgentRuntime gains an optional mcp_pool field; the engine propagates the parent's pool to child runtimes so subagents can invoke MCP tools. The with_mcp_tools dead-code suppression is removed now that the builder is actively used.
  • Mention menu browser mode: Workspace is extended with walk_depth, mention_scan_limit, and mention_menu_behavior; MentionMenuBehavior::Browser limits the walk to one directory level, skips frecency re-ranking, and sorts results case-insensitively. The hard-coded MENTION_MENU_LIMIT = 6 constant is replaced by the per-app setting.
  • Xiaomi/Mimo provider: Full ApiProvider::Xiaomi integration with pricing for mimo-v2.5-pro and mimo-v2.5; unknown mimo-* models return None from pricing. Config migration now returns a bool so the caller can print a one-time notice and exit cleanly via return Ok(()).

Confidence Score: 4/5

Safe to merge for most users; one correctness gap in mention resolution affects users who explicitly raise mention_scan_limit above 4096.

The new mention_scan_limit setting is correctly threaded into the autocomplete popup and Workspace internals, but context_references_from_input and local_context_from_file_mentions both hard-code the limit to 4096 when constructing their resolution workspace. For users who increase the setting to surface files in a large monorepo, those files will appear in the completion menu but then silently fail to resolve when the message is sent. All other changes look self-consistent and correct.

crates/tui/src/tui/file_mention.rs hard-codes a 4096 scan limit in two resolution functions; the three test_*.rs scripts at the repo root should also be removed.

Important Files Changed

Filename Overview
crates/tui/src/tui/file_mention.rs Browser mode skips frecency re-ranking; walk_depth threaded through all public call sites. mention_scan_limit is hardcoded to 4096 in context_references_from_input and local_context_from_file_mentions, making the user-configured setting ineffective for actual mention resolution at message-send time.
crates/tui/src/working_set.rs Workspace gains walk_depth, mention_scan_limit, and mention_menu_behavior fields; browser mode computes a directory-scoped walk root from the partial input; sort changed to case-insensitive. Logic is correct and the depth/limit fields are properly threaded through all internal walk helpers.
crates/tui/src/tools/subagent/mod.rs Adds mcp_pool field and with_mcp_pool builder to SubAgentRuntime; MCP tools are registered in SubAgentToolRegistry when the pool is present. Field is correctly cloned in background_runtime and initialized to None in tests.
crates/tui/src/core/engine.rs MCP pool is now optionally passed to SubAgentRuntime in both the background dispatch path and the primary tool registry construction. Uses .ok() on ensure_mcp_pool to treat MCP errors as non-fatal, consistent with the MCP feature flag pattern.
crates/tui/src/config.rs Complete ApiProvider::Xiaomi integration: enum variant, parsing aliases, display name, default model (mimo-v2.5-pro), base URL, API key env var, provider config struct, and all match arm exhaustiveness.
crates/tui/src/main.rs Migration block now runs settings migration alongside config migration and exits cleanly via return Ok(()) when either migration fires, printing a user-facing notice. Terminal is not yet initialized at this point so the early return is safe.
crates/tui/src/settings.rs Adds mention_walk_depth (default 6), mention_scan_limit (default 4096), mention_menu_limit (default 128), and MentionMenuBehavior enum; adds migrate_settings_if_needed() with Windows AppData fallback. Defaults are consistent with previous hard-coded constants.
crates/tui/src/pricing.rs Adds exact USD and CNY pricing for mimo-v2.5-pro and mimo-v2.5; unknown mimo-* models correctly return None instead of falling through to DeepSeek pricing.
test_canonicalize.rs Ad-hoc debug script committed to the repo root; not part of the Cargo workspace.
test_git_root.rs Ad-hoc debug script committed to the repo root; not part of the Cargo workspace.
test_paths.rs Ad-hoc debug script with a hardcoded Windows path committed to the repo root; not part of the Cargo workspace.

Sequence Diagram

sequenceDiagram
    participant Engine
    participant McpPool
    participant SubAgentRuntime
    participant SubAgentToolRegistry
    participant McpTool

    Engine->>McpPool: ensure_mcp_pool() (if Feature::Mcp enabled)
    McpPool-->>Engine: "Arc<Mutex<McpPool>>"

    Engine->>SubAgentRuntime: new(...).with_mcp_pool(pool)
    Engine->>SubAgentRuntime: background_runtime()

    SubAgentRuntime->>SubAgentToolRegistry: build registry
    SubAgentToolRegistry->>SubAgentToolRegistry: with_full_agent_surface(...)
    alt mcp_pool is Some
        SubAgentToolRegistry->>SubAgentToolRegistry: with_mcp_tools(Arc::clone(pool))
    end
    SubAgentToolRegistry-->>SubAgentRuntime: registered tools incl. MCP

    SubAgentRuntime->>McpTool: execute MCP tool call
    McpTool-->>SubAgentRuntime: result
Loading

Fix All in Codex Fix All in Claude Code Fix All in Cursor

Reviews (2): Last reviewed commit: "Fix Codewhale PR 2377 issues: Make scan ..." | Re-trigger Greptile

Test and others added 11 commits May 31, 2026 01:50
…ings

This commit addresses several critical configuration path issues:
1. Environment Variable Fallback: codewhale_home() and legacy_deepseek_home() now explicitly check the $HOME environment variable before falling back to dirs::home_dir(). This fixes a bug on Windows where dirs::home_dir() could resolve to a different path than $HOME, causing split configuration states.
2. Robust Settings Migration: Added migrate_settings_if_needed() to safely migrate settings.toml from legacy DeepSeek folders (and Windows AppData) to the new CodeWhale configuration directory.
3. Loud Migration Exit: If any configuration or settings files are migrated on startup, the application now loudly notifies the user with colorful terminal output, displaying the exact paths to the new and legacy files, and safely exits to ensure the user verifies the new configuration before continuing.
fix: resolve configuration mismatch and implement loud migration warnings
This commit replaces the hardcoded MENTION_MENU_LIMIT (previously hardcoded to 6) with a configurable mention_menu_limit setting in settings.toml. The default limit is now increased to 128. This allows users to view and scroll through a much larger list of candidate files when using the @-mention popup in the composer widget, significantly improving the file search experience in large workspaces.
feat: make mention menu limit configurable (fixes Hmbown#2360)
This commit introduces significant enhancements to the @-mention file completion menu:
1. Mention Menu Behavior (mention_menu_behavior): Adds a new setting to toggle between uzzy (default) and �rowser mode. In �rowser mode, the menu acts as a strict directory-level file browser, making it much easier to navigate through specific folders without fuzzy matching cluttering the results.
2. Configurable Walk Depth (mention_walk_depth): Replaces the hardcoded COMPLETIONS_WALK_DEPTH of 6 with a configurable setting. Users with deeply-nested workspaces can increase this to ensure deep files are discovered, or set it to 0 for unlimited depth (with caution).
3. Local Reference Path Scanning: Increases the LOCAL_REFERENCE_SCAN_LIMIT from 4,096 to 100,000, ensuring massive repositories can fully populate the completion menu when requested.
# Conflicts:
#	crates/tui/src/settings.rs
#	crates/tui/src/tui/app.rs
feat: add mention menu browser mode and configure walk depth
This commit allows subagents to inherit the main agent's MCP tool pool. Previously, subagents did not have the MCP tool pool attached to their runtime, meaning they were unable to call MCP tools even when the user had them enabled. This passes the mcp_pool down when initializing the SubAgentRuntime and wires it up to the SubAgentToolRegistry.
feat: allow subagents to inherit MCP tool access
Adds balance cost calculation for the Xiaomi provider. It allows mimo models to pass through the pricing checks and wires Xiaomi into the /balance command scaffold.
feat: Implement Xiaomi Balance Cost Tracking
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces configuration and settings migration from legacy paths to the new codewhale directory, wires up MCP tools to subagents, adds Xiaomi to the balance command, and implements configurable @-mention file completion behaviors (including a new directory-level browser mode and configurable walk depth/limits). Feedback focuses on critical performance and correctness issues in the workspace file completion logic: specifically, the risk of UI freezes due to an excessively high LOCAL_REFERENCE_SCAN_LIMIT combined with an unlimited default walk depth, incorrect path splitting on mixed-separator paths (Windows), and inefficient string allocations during case-insensitive sorting.

Comment thread crates/tui/src/working_set.rs Outdated
}

const LOCAL_REFERENCE_SCAN_LIMIT: usize = 4096;
const LOCAL_REFERENCE_SCAN_LIMIT: usize = 100000;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

Increasing LOCAL_REFERENCE_SCAN_LIMIT to 100,000 combined with default_mention_walk_depth being 0 (unlimited) and git_ignore(false) means that on every single keystroke when typing a path-like mention (e.g., @src/), the TUI will synchronously walk up to 100,000 files on the main UI thread. This will completely freeze the user interface on medium-to-large repositories. Please keep this limit much lower (e.g., 4096 or 10000) and ensure the default walk depth is bounded.

Suggested change
const LOCAL_REFERENCE_SCAN_LIMIT: usize = 100000;
const LOCAL_REFERENCE_SCAN_LIMIT: usize = 4096;

Comment on lines +307 to +309
fn default_mention_walk_depth() -> usize {
0
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The doc comment for mention_walk_depth states that the default is 6, and 0 means unlimited depth (use with caution). However, default_mention_walk_depth() is implemented to return 0. This means by default, users will have unlimited walk depth, which can cause severe performance issues in large repositories. Please change the default to 6 to match the documentation and prevent accidental performance degradation.

fn default_mention_walk_depth() -> usize {
    6
}

Comment on lines +221 to +239
let (walk_depth, cwd_walk_root, root_walk_root) = if self.mention_menu_behavior == crate::settings::MentionMenuBehavior::Browser {
let dir_prefix = if let Some((dir, _)) = partial.rsplit_once('/') {
Some(dir)
} else if let Some((dir, _)) = partial.rsplit_once('\\') {
Some(dir)
} else {
None
};

let cwd_walk_root = if let Some(cwd) = self.cwd.as_deref() {
if let Some(p) = dir_prefix { Some(cwd.join(p)) } else { Some(cwd.to_path_buf()) }
} else {
None
};
let root_walk_root = if let Some(p) = dir_prefix { self.root.join(p) } else { self.root.clone() };
(1, cwd_walk_root, root_walk_root)
} else {
(self.walk_depth, self.cwd.clone(), self.root.clone())
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The code splits partial using rsplit_once('/') first, and only if that fails, it tries rsplit_once('\\'). In mixed-separator paths (common on Windows, e.g., foo/bar\baz), this will split at the / instead of the rightmost separator \. This results in an incorrect dir_prefix (foo instead of foo/bar), which breaks directory-level completion walks because the walk depth is set to 1 and it won't descend into bar. Using rfind to find the rightmost separator of either / or \ is much more robust.

        let (walk_depth, cwd_walk_root, root_walk_root) = if self.mention_menu_behavior == crate::settings::MentionMenuBehavior::Browser { 
            let dir_prefix = partial
                .rfind(|c| c == '/' || c == '\\')
                .map(|idx| &partial[..idx]);
            
            let cwd_walk_root = if let Some(cwd) = self.cwd.as_deref() {
                if let Some(p) = dir_prefix { Some(cwd.join(p)) } else { Some(cwd.to_path_buf()) }
            } else {
                None
            };
            let root_walk_root = if let Some(p) = dir_prefix { self.root.join(p) } else { self.root.clone() };
            (1, cwd_walk_root, root_walk_root)
        } else {
            (self.walk_depth, self.cwd.clone(), self.root.clone())
        };

Comment on lines +287 to +293
let filter_str = if let Some((_, f)) = partial.rsplit_once('/') {
f
} else if let Some((_, f)) = partial.rsplit_once('\\') {
f
} else {
partial
};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Similar to the directory prefix splitting issue, using rsplit_once('/') first will fail to correctly identify the rightmost filename filter in mixed-separator paths (e.g., foo/bar\.baz). This causes filter_str to be evaluated as bar\.baz instead of .baz, which prevents dotfile filtering from working correctly when the user explicitly types a dotfile. Using rfind avoids this issue.

            let filter_str = partial
                .rfind(|c| c == '/' || c == '\\')
                .map(|idx| &partial[idx + 1..])
                .unwrap_or(partial);

Comment on lines +303 to +304
prefix_hits.sort_by(|a, b| a.to_ascii_lowercase().cmp(&b.to_ascii_lowercase()));
substring_hits.sort_by(|a, b| a.to_ascii_lowercase().cmp(&b.to_ascii_lowercase()));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Calling to_ascii_lowercase() on every comparison in sort_by allocates a new String each time. Since sorting is an O(N log N) operation, this results in a large number of temporary allocations. You can perform an allocation-free case-insensitive comparison by comparing character/byte iterators directly.

        prefix_hits.sort_by(|a, b| {
            let a_iter = a.as_bytes().iter().map(|&b| b.to_ascii_lowercase());
            let b_iter = b.as_bytes().iter().map(|&b| b.to_ascii_lowercase());
            a_iter.cmp(b_iter)
        });
        substring_hits.sort_by(|a, b| {
            let a_iter = a.as_bytes().iter().map(|&b| b.to_ascii_lowercase());
            let b_iter = b.as_bytes().iter().map(|&b| b.to_ascii_lowercase());
            a_iter.cmp(b_iter)
        });

Comment thread crates/tui/src/settings.rs
Comment thread crates/tui/src/working_set.rs Outdated
Comment thread crates/tui/src/main.rs Outdated
Comment thread crates/tui/src/pricing.rs Outdated
@buko
Copy link
Copy Markdown
Author

buko commented May 30, 2026

I've pushed the fixes for the issues raised in the review. Here is a summary of the updates:

  1. Fix Unbounded Filesystem Walk & Default Depth:

    • Reverted default_mention_walk_depth to correctly return 6 (as documented) instead of 0.
    • Both the walk_depth and the LOCAL_REFERENCE_SCAN_LIMIT (now mention_scan_limit with a safe default of 4096) have been made fully user-configurable, protecting users from massive synchronous walks blocking the TUI.
  2. Graceful Config Migration Exit:

    • Removed the std::process::exit(0) call. The migration logic now propagates a Result<bool> upward, ensuring that destructors run and the terminal is safely restored from raw/alt-screen mode before exiting.
  3. Accurate Xiaomi Mimo Pricing:

    • Restructured the starts_with("mimo") guard so that mimo-v2.5-pro maps to correct explicit pricing.
    • We included a direct source reference to https://ai.xiaomi.com/pricing in the source code so maintainers can independently verify these numbers.
    • Non-pro mimo variants are now prevented from inadvertently inheriting DeepSeek Flash pricing.
  4. Exhaustive Provider Match Arms:

    • Performed a comprehensive sweep to handle ApiProvider::Xiaomi everywhere in the codebase (client.rs, config.rs, ui.rs, etc.). All exhaustive compiler requirements are met.

All tests have been updated and are passing successfully locally!

@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented May 31, 2026

great points here - thank you so much!

@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented May 31, 2026

Thanks @buko — there is useful material in here, especially around sub-agent MCP inheritance.

I’m not merging it as one PR because it bundles several separately risky ideas: MCP pool inheritance, browser mention mode, Xiaomi defaults, and settings changes. I harvested the MCP inheritance slice now because it is narrow and directly improves child-agent behavior; the browser-mode and provider pieces should land as smaller follow-ups so each one can get a clean review.

@buko
Copy link
Copy Markdown
Author

buko commented May 31, 2026

I understand. This PR got pretty big. There are two big issues for us:

  1. Allowing SubAgents to use MCP tools. This allows the main orchestrator agent to delegate much more work to the subagents and is very important to several key workflows.
  2. A new mode for the mention menu that acts more like a file browser. This is very important when working with large, deep (10-20 folder deep!) Workspaces.

Additionally we ended up being forced to make certain elements configurable (walk depth, scan limit) because we are Java programmers and our Workspace directories are large and deep (haha). Having a "walk depth" hardcoded to 6 causes a lot of problems because large chunks of the workspace never show up and the frencency stuff takes over.

The Xiaomi changes are also small and straightforwards and very useful.

I can break this up into smaller PRs but the code is nothing complicated. Feel free to close this PR and make your own. Either way I'd just love to see these features merged.

Thanks!

@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented May 31, 2026

Thanks @buko — there is useful work in here, especially the subagent MCP inheritance path. I cannot merge the full PR as-is because a current-main merge still has a few concrete correctness/build blockers:

  • formatting and whitespace checks fail
  • a focused TUI test build hits a stale Workspace::with_cwd(...) call after the signature changed
  • the configured mention scan limit is not carried through the send-time resolution path yet
  • the Xiaomi/provider changes overlap with the now-merged xiaomi-mimo provider surface
  • the root-level debug scripts should not land

The best next shape is probably a small MCP-only PR first, then a separate mention-browser PR with send-time resolution tests. The provider/config bits can be a separate follow-up only if there is still a gap after #2246. Really appreciate the push here — this is close to a harvestable path, just too many unrelated edges in one branch to merge directly.

Hmbown added a commit that referenced this pull request May 31, 2026
Harvested from #2377 with thanks to @buko.

Threads the parent MCP tool pool into child SubAgentRuntime construction and registers MCP-backed tools for child agents when MCP is enabled, while leaving the broader mention-browser/provider/config work for focused follow-ups.

Validation:
- cargo fmt --all -- --check
- CARGO_TARGET_DIR=/Volumes/VIXinSSD/codewhale-pr2377-target cargo test -p codewhale-tui tools::subagent
- CARGO_TARGET_DIR=/Volumes/VIXinSSD/codewhale-target/harvest-2377-recheck cargo test -p codewhale-tui tools::subagent --all-features
@github-actions
Copy link
Copy Markdown

Thanks @buko — your contribution landed in 58e45d384e3b on main:

feat(subagents): inherit MCP tools in child runtimes (#2422)

Closing this PR now that the code is on main. Credit lives in the commit message and (where applicable) the CHANGELOG.md entry for the next release. Apologies for not closing this at the time of the merge — the auto-close workflow is new in v0.8.31.

If you want to land more work and would prefer your future PRs merge cleanly without a harvest step, the CONTRIBUTING.md doc has a short note on what makes a contribution mergeable as-is.

@Hmbown
Copy link
Copy Markdown
Owner

Hmbown commented May 31, 2026

Thanks again @buko. The focused subagent MCP inheritance slice landed through #2422 as 58e45d384e3b5052c3cbebc978a399dad64ea40e, preserving your contribution while keeping the conflicted provider/config and mention-browser work out of this merge.

I’m closing this PR as superseded for that landed slice. Browser-style mentions still look useful as a smaller follow-up once split from the Xiaomi/provider/config changes, and the root debug scripts should stay out of the final patch.

@github-actions github-actions Bot closed this May 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants