feat(mcp): support subagent-only tool scope - #66570
Conversation
b943333 to
539b7a3
Compare
e03bf3e to
4e8ea85
Compare
teknium1
left a comment
There was a problem hiding this comment.
Thanks for the scoped-MCP design and focused registry/cache coverage. The feature premise is real on current main, but there are blocking integration gaps.
Problems
scripts/ci/timings_report.py:939on PR head has an extra indentation underexcept TimingsUnavailable as e:, so the changed file is invalid Python.agent/agent_init.py:1218only applies scope during initial construction. With defaultenabled_toolsets=None,tools/delegate_tool.py:1110-1118derives a child’s toolsets from the parent’s visible names; hidden scoped MCP tools therefore do not contribute theirmcp-<server>toolset to the child.- Scope is also omitted from the rebuild paths:
tools/mcp_tool.py:5626-5630,model_tools.py:1106-1110, andagent/tool_executor.py:252-257. Refreshes and Tool Search will omit a child-scoped MCP tool. - The mixed-scope documentation at
website/docs/reference/mcp-config-reference.md:185-189conflicts with existingtools.includeregistration semantics intools/mcp_tool.py:5048-5058.
Suggested changes
- Propagate child scope through child inheritance, MCP refresh, and both Tool Search catalog/unwrap paths; add end-to-end coverage for all three.
- Correct the CI indentation and align the mixed-scope documentation with the implemented registration model.
Automated hermes-sweeper review.
4e8ea85 to
93f6256
Compare
Addressing the hermes-sweeper reviewThanks for the detailed sweep. Summary of what I've done in the latest push (now rebased clean on current Point 1 — Point 2 — child toolset inheritance (ADOPTED). Point 3 — refresh + Tool Search paths (ADOPTED). Point 4 — mixed-scope docs (ADOPTED). Added two end-to-end tests (child toolset derivation + Tool Search catalog scope). The feature does not exist on Re-requesting review. |
Deep upstream-already-fixed analysis — verdict: STILL_OPENI read the current What the problem actually requiresA way to register an MCP tool that is hidden from the MAIN agent but visible to Why upstream cannot do this today1. Children can only ever be a SUBSET of the parent's toolsets — never a superset.
2. The model cannot even request a custom child toolset.
3. MCP tools have no scope/visibility/role field.
4. No per-agent tool filter / plugin hook.
5. No config schema field.
6. Recent history shows no prior solution.
ConclusionThe PR's |
6b68b66 to
4b9c7a1
Compare
Thanks for the thorough iteration — routing all scope decisions through the single
Also: the PR description's "Clean branch provenance / Validation" section still reflects the original 8-file, 1-commit state — the branch is now 15 files / 3 commits after the review fixes. Worth refreshing so reviewers aren't checking against stale numbers. CI is fully green and the change is cleanly additive, so once the |
4b9c7a1 to
55f2d30
Compare
55f2d30 to
9c0e613
Compare
|
@teknium1 Thanks for the review — this concern has been addressed in the current head (rebased onto upstream/main). The fix is in
if _get_inherit_mcp_toolsets():
parent_toolsets.update(_registered_mcp_toolsets())This ensures that even when The docstring in Please verify the current head addresses your concern. Happy to iterate further if there's a gap I missed. |
|
All four items addressed:
|
9c0e613 to
28e6ed9
Compare
3ca7b6c to
78596de
Compare
78596de to
3c56c25
Compare
3c56c25 to
f3f5c16
Compare
f3f5c16 to
270e631
Compare
…l Search Address reviewer integration gaps (PR NousResearch#66570): - _build_child_agent: with default enabled_toolsets=None the child toolset set is derived from the parent's main-visible valid_tool_names, which excludes subagent_only-scoped MCP tools. Fold the registered mcp-* toolsets back in (gated on inherit_mcp_toolsets) so delegated children still receive them; MAIN never sees them. - Thread include_subagent_only through refresh_agent_mcp_tools (all callers key off agent.platform == 'subagent') and handle_function_call / Tool Search bridge catalog, so MCP refresh and tool_search unwrap expose scoped tools to children. - Correct mcp-config-reference docs: tools.include is a whitelist, so a scoped include leaves no main-visible remainder (the rest is unregistered). - Add end-to-end coverage for child toolset derivation and Tool Search catalog. The timings_report.py indentation claim was a stale-range false positive: the PR does not touch that file and it parses cleanly.
…agent_only kwarg The child-exposure fix added include_subagent_only to the handle_function_call call sites in agent_runtime_helpers.py and tool_executor.py. Update the exact call-assertion in test_invoke_tool_dispatches_to_handle_function_call so the signature match reflects the new kwarg (CI slice 1/8 breakage).
Address GottZ review:
1. tools.scope without include/exclude now applies to ALL tools of the
server instead of silently leaving every tool at 'main'. Previously
with no filter was a no-op — the
same failure class (inverted) that _normalize_mcp_scope warns against.
2. Doc wording: 'narrower scope wins' → 'tools.scope overrides for the
include/exclude-selected tools'; 'containment surface' → 'omitted
from MAIN's model schema' (accurate mechanism).
3. Added test_register_server_tools_scope_without_filter_applies_to_all
covering the no-filter path.
270e631 to
57e3de8
Compare
|
Closing as superseded by upstream's MCP tool progressive disclosure (deferred tool surfaces + tool_search catalog listing, landed in main via the 2026-07-26 batch). The subagent_only scope mechanism this PR introduced is no longer needed: large MCP tool surfaces are now held out of the main agent schema and discoverable through tool_search, which covers the original use case (keep heavy MCP tools off the parent schema while keeping them callable). Thanks for the review. |
Summary
Adds an opt-in MCP tool visibility scope for tools that should be available to delegated subagents but omitted from the main agent's model schema.
scope: subagent_onlytools.scope: subagent_onlywith existing include/exclude selection semanticstools/listrefreshesClean branch provenance
This PR is based directly on current upstream
main:d9ee342414042bba7bca43438f19d2fba9a54806b9433337aa722b03afdb84de2a4b7827e45594dcMotivation
Some MCP servers expose powerful or very large tool surfaces that are better used by focused delegated agents. Previously, Hermes had no way to keep those tools out of the main agent schema while retaining them for
delegate_taskchildren.Configuration
Per-selected-tool scope is also supported under
tools.scope.Compatibility
No
scopeorscope: mainpreserves existing behavior: tools remain visible to both the main agent and delegated children. Unknown values emit a warning and fall back tomain.Validation
git diff --check upstream/main..HEAD: clean