feat(delegate): optional profile param so agent_profiles archetypes declare authoritative toolsets; MCP toolsets bypass parent intersection (non-MCP preserved) - #49699
Open
davidgut1982 wants to merge 1 commit into
Conversation
Contributor
|
Thanks for the detailed MCP-boundary analysis and focused tests. Problems
Suggested changes
Automated hermes-sweeper review. |
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.
feat(delegate): optional
profileparam so agent_profiles archetypes declare authoritative toolsets; MCP toolsets bypass parent intersection (non-MCP preserved)Closes/relates to: #32668 (also #32727)
Summary
Adds an optional
profileparameter todelegate_task(top-level and per-task intasks=[...]). A profile names an entry in the top-levelagent_profilesconfigmapping and supplies baseline
model,toolsets,max_iterations, andsystem_promptfor the spawned child. Explicit call parameters still overrideprofile values.
The motivating problem (#32668): an orchestrator that restricts its own context
via a
no_mcpplatform toolset (or simply never loaded a given domain MCP server)cannot hand a fat sub-agent the MCP toolsets that sub-agent needs. Today
_build_child_agentintersects the child's requested toolsets against theparent's loaded tools, which silently drops any MCP toolset the orchestrator
itself isn't carrying. The result: profile-driven workers come up missing exactly
the MCP servers their archetype declares.
This change lets a named profile act as the authoritative source of a child's
toolsets. When (and only when) a delegation resolves to a valid profile that
declares a non-empty
toolsetslist, MCP toolsets in that list bypass theparent-intersection check and are resolved directly from the global
mcp_serversconfig. Non-MCP toolsets continue to go through intersectionunchanged.
Why this is safe — the security boundary
The bypass is deliberately narrow. The privilege-escalation surface here is "a
model names a profile, then injects arbitrary toolsets the profile never
declared." Each of those vectors is closed:
Only MCP toolsets bypass. Non-MCP toolsets (
terminal,file,web, …)still go through
expanded_parentintersection in_build_child_agent. Aprofile cannot grant a child a non-MCP tool the parent lacks. (See
test_profile_non_mcp_toolsets_still_intersected.)Empty/toolset-less profiles do NOT activate the bypass.
profile_nameiscarried into
_build_child_agentonly when the resolved profile declares anon-empty
toolsetslist. A profile with no toolsets falls back to the normalintersection path with a warning — so a caller cannot name an empty profile and
then smuggle in
mcp-*toolsets via thetoolsetsargument.(See
test_empty_profile_toolsets_bypass_not_activated.)Batch-mode injection is blocked. When a top-level profile is
authoritative, its toolsets are used for every task in a
tasks=[...]batch; per-task model-supplied
toolsetsare ignored. This stops a model fromnaming a valid profile (to activate the bypass) while injecting a per-task evil
mcp-*toolset the profile never declared.(See
test_batch_injection_blocked_model_cannot_inject_evil_mcp_toolset.)Unknown profiles fail closed.
_resolve_profileraises on an unknown nameand
delegate_taskreturns atool_errorbefore any child is built — there isa single strict resolver, no lenient "warn and fall back to unrestricted
toolset" path. (See
test_unknown_profile_returns_error.)Profile toolsets are deep-copied (
_resolve_profiledeep-copies, anddelegate_tasktakes an additionallist()copy) so a child build can nevermutate the cached config
agent_profilesblock.(See
test_profile_toolsets_copy_prevents_config_corruption.)inherit_mcp_toolsetsparent-bleed is suppressed under a profile. When aprofile is authoritative the
_preserve_parent_mcp_toolsetsstep is skipped, soa profile-restricted child does not silently inherit the parent's MCP toolsets.
(See
test_parent_mcp_bleed_blocked_under_profile.)The empty-toolset privilege-escalation guard and the existing parent-intersection
boundary for ad-hoc (profile-less) delegation are fully preserved.
What changed
tools/delegate_tool.py(single production file):delegate_task: new optionalprofileparam (top-level) +profilekey in theper-task schema. Resolves the named profile, applies its
toolsets/model/max_iterations/system_promptas baselines (explicit args win), and computesresolved_profile_name/profile_resolved_toolsetsunder the non-empty-toolsetsgate. Per-task profiles are pre-resolved (so an unknown name surfaces as a clean
tool_errorbefore any child is built) and beat the top-level profile._build_child_agent: new optionalprofile_nameparam. When set, MCP toolsetsin the requested list bypass parent intersection (
_is_mcp_toolset_name(t) or t in expanded_parent); non-MCP toolsets still intersect._preserve_parent_mcp_toolsetsis skipped under a profile to prevent parent MCP bleed.
_load_profiles()(reads theagent_profilesconfig block),_load_agent_profiles()(deprecated alias → single strict source of truth), and_resolve_profile(name, profiles)(validates the name, deep-copies, resolvessystem_prompt_file/system_prompt→system_prompt_text, and coercesmax_iterationsto int).profilestring property added to both the top-level params and theper-task
tasks[]items, with a doc note that a profile'ssystem_promptreplaces (not appends to) the child's default prompt.
No other production files are touched. The change is additive —
profiledefaultsto
None, so existing profile-less delegation behaviour is byte-for-byte unchanged.Test summary
tests/tools/test_delegate_toolset_scope.pyextends the existingTestToolsetIntersectionbase cases (unchanged) with eight focused classes:TestProfileMcpToolsetBypassTestDelegateTaskProfileWiringprofile_nameforwarded to_build_child_agent; profile-less leaves itNone; unknown profile →tool_errorTestDelegateTaskSchemaProfileprofilepresent in schema, string-typed, not requiredTestProfileMcpBypassEndToEndno_mcpparent + profile retains MCP toolsets; without profile strips themTestBatchToolsetInjectionBlockedTestProfileToolsetsAliasingTestInheritMcpToolsetsProfileGuardResult on a clean v0.17.0 + this patch:
Compatibility
profile=Nonepreserves all current behaviour.(
_is_mcp_toolset_name,_preserve_parent_mcp_toolsets,_get_inherit_mcp_toolsets), so no new cross-module dependencies are introduced.import copy) for the profile deep-copy.