fix(agent-core-v2): cap default subagent delegation at one level - #3012
fix(agent-core-v2): cap default subagent delegation at one level#3012sailist wants to merge 1 commit into
Conversation
🦋 Changeset detectedLatest commit: 6c7b94c The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1d4b39aa51
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }, | ||
| ): readonly string[] | undefined { | ||
| return caller.profileName === undefined ? catalog.getDefault().subagents : caller.subagents; | ||
| const declared = caller.subagents ?? catalog.getDefault().subagents; |
There was a problem hiding this comment.
Inherit from the built-in default for overridden agent profiles
When a user or project agent.md sets override: true but omits subagents, catalog.getDefault() resolves to that same overridden profile, whose allowlist is also undefined. This therefore returns undefined, which both Agent and AgentSwarm interpret as unrestricted; because agent files without a tools field also receive the Agent tool, the overridden default can recursively spawn itself without declaring the documented explicit * opt-out. Resolve the fallback from the built-in default rather than the merged default profile.
Useful? React with 👍 / 👎.
| const rawSubagents = parseStringList(frontmatter['subagents'], 'subagents', options.path); | ||
| const subagents = | ||
| rawSubagents?.length === 1 && rawSubagents[0] === '*' ? undefined : rawSubagents; | ||
| const subagents = rawSubagents; |
There was a problem hiding this comment.
Correct the parser header's wildcard contract
Preserving ['*'] here makes a lone wildcard explicitly unrestricted, while an omitted subagents field now inherits the default allowlist. The module header still says the wildcard and omission are equivalent and unrestricted, so it documents the opposite of the new contract; update the header to describe the distinction.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L36-L40
Useful? React with 👍 / 👎.
| // No subagents allowlist: enforced when present, so `undefined` keeps | ||
| // user-defined profiles delegatable, tower-worker included. | ||
| expect(agent.subagents).toBeUndefined(); | ||
| expect(agent.subagents).toEqual(['coder', 'explore', 'plan']); |
There was a problem hiding this comment.
Split the delegation assertion into its own test
This adds the delegation-allowlist contract to an it named only for wiring TowerInit, leaving two independent behaviors in one test and making a failure report misleading. The file header and adjacent comment also still assert that the profile has no allowlist. Move this assertion into a separately named test and update the scenario text so the test documents the behavior it proves.
Useful? React with 👍 / 👎.
1d4b39a to
e0144ef
Compare
commit: |
e0144ef to
56aedf3
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 56aedf3cc5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| name: 'agent', | ||
| description: 'Default agent', | ||
| tools: AGENT_TOOLS, | ||
| subagents: ['coder', 'explore', 'plan'], |
There was a problem hiding this comment.
Preserve delegation to discovered custom profiles
When the built-in default profile is active, this hard-coded allowlist filters the Agent tool description and is enforced again at launch, so a discovered custom profile such as reviewer is neither advertised nor callable. Declaring subagents on reviewer only controls what that agent may spawn and cannot opt it into its parent's list, which regresses the documented custom-agent delegation flow; derive the restriction so it blocks recursive agent launches without excluding custom catalog profiles.
Useful? React with 👍 / 👎.
56aedf3 to
40435d8
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40435d8643
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| readonly subagents?: readonly string[]; | ||
| }): readonly string[] | undefined { | ||
| if (this.callerAgentId !== 'main') return undefined; | ||
| return rootDelegationExtras(this.catalog, own, this.catalogProfiles()); |
There was a problem hiding this comment.
Use the live catalog when enforcing root delegation
When a custom agent file is added after the first tool description is built, catalogProfiles() has already frozen the initial list; launch() reaches this helper and therefore omits the newly discovered profile from the effective allowlist, rejecting it even though catalog.get() and AgentSwarm see the refreshed catalog. Unlike the earlier general custom-profile finding, the fresh evidence here is the supported fs-watch reload path: keep the frozen list for description stability, but derive execution-time extras from the current catalog.list().
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
| caller.profileName !== undefined && | ||
| caller.profileName !== DEFAULT_AGENT_PROFILE_NAME && | ||
| caller.subagents !== undefined && | ||
| isDiscoveredAgentProfileSource(catalog.inspect(caller.profileName)?.sourceId) |
There was a problem hiding this comment.
Preserve explicit allowlists after a profile is withdrawn
When a session's main agent was bound to a discovered custom profile with an explicit subagents list and that profile file is later removed, catalog.inspect(caller.profileName) returns undefined. This check then stops recognizing the persisted caller as a discovered profile and falls through to union every remaining custom profile, allowing both Agent and AgentSwarm to launch types excluded by the binding's persisted allowlist; honoring the explicit snapshot must not depend on the profile still existing in the live catalog.
AGENTS.md reference: packages/agent-core-v2/AGENTS.md:L7-L7
Useful? React with 👍 / 👎.
40435d8 to
595dd7f
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 595dd7fa79
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| extras?: readonly string[], | ||
| ): readonly string[] | undefined { | ||
| return caller.profileName === undefined ? catalog.getDefault().subagents : caller.subagents; | ||
| const declared = caller.subagents ?? catalog.getDefault().subagents; |
There was a problem hiding this comment.
Prevent inherited allowlists from enabling self-recursion
When a custom agent overrides an allowlisted built-in such as coder with override: true but omits both tools and subagents, it receives the Agent tool and this fallback inherits ['coder', 'explore', 'plan']; the resulting coder can therefore spawn another coder indefinitely without the explicit delegation opt-in described by this change. Unlike the earlier default-profile case, the fresh evidence is an override of an allowlisted subagent profile whose own name appears in the inherited default list; exclude the caller itself from an inherited allowlist or otherwise give overridden built-in subagents a non-recursive fallback.
Useful? React with 👍 / 👎.
595dd7f to
036aecb
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 036aecb381
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (allowlist === undefined || own.subagents !== undefined) return allowlist; | ||
| return withoutDelegatingTargets(this.catalog, allowlist); |
There was a problem hiding this comment.
Enforce the advertised one-hop cap for implicit profiles
When a discovered custom profile omits both tools and subagents, it receives the Agent tool, inherits coder/explore/plan, and this filter retains terminal targets such as explore; consequently the default main agent can launch that custom agent, which can then launch explore, contradicting the documented main → subagent cap and the requirement for an explicit allowlist to enable deeper chains. Unlike the earlier self-recursion report, the fresh evidence is this retained terminal-target path; inherited delegation should be disabled for subagents unless they explicitly declare subagents.
Useful? React with 👍 / 👎.
036aecb to
d33d67d
Compare
- give the builtin agent profile an explicit subagents allowlist (coder, explore, plan), restoring v1 semantics - inherit the default profile's allowlist when a caller profile declares none, instead of leaving delegation unrestricted - pass a lone "*" subagents field through as an explicit unrestricted marker
d33d67d to
6c7b94c
Compare
Related Issue
No linked issue — the problem is explained below.
Problem
The main agent could spawn a subagent of type
agent, and that child inherited the same unrestricted delegation ability, soAgenttool calls could chain recursively without bound (main → agent → agent → …). This regressed in the v2 engine: v1 bounded the default profile's delegatable set to coder/explore/plan and let profiles without their ownsubagentsdeclaration inherit that set, while v2 dropped the field from the built-in profile and left the fallback unrestricted.What changed
agentprofile now declares an explicit subagent allowlist (coder,explore,plan) — none of which hold theAgenttool — so default delegation is structurally capped at two levels (main → sub).subagentsof its own now inherits the default profile's allowlist instead of delegating unrestricted; a lone*passes through as an explicit unrestricted opt-out.AgentSwarmresolves through the same helper, so both spawn paths stay consistent.TowerSpawnbinds thetower-workerprofile directly without consulting the allowlist, andtower-workerkeeps its explicitsubagents: ['explore', 'plan']. As a side effect,tower-workerno longer appears in the defaultAgenttool's type list (it was never meant to be spawned directly).agents.md, en + zh) updated for the newsubagentssemantics and the two-level default; existing inline snapshots and pinned token counts in compaction tests were regenerated for the shorterAgenttool description.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update.