No recursive subagents #5355
No recursive subagents #5355
Conversation
zanesq
left a comment
zanesq
left a comment
There was a problem hiding this comment.
Verified working locally. Copilot had a few things that might be worth looking into:
Observation: list_tools always appends the dynamic task and subagent execute tools and reply_parts later filters them out for subagents. That means callers of list_tools who do not run through reply_parts could see those tools even for subagents.
Recommendation: filter out subagent-spawning tools inside list_tools when self.is_subagent is true (so the agent never sees them at the source). That centralizes behavior and avoids surprising callers.
Observation: is_subagent is a plain bool (pub(super) is_subagent) and mark_as_subagent(&mut self) mutates it. Current code sets the flag before Arc::new(agent), which is safe. But making it public(super) allows other code in the module to modify it after the Agent is shared.
Recommendation: make the flag private (or readonly after initialization) or only set at construction time (e.g., Agent::new_subagent()) to enforce immutability after creation.
DOsinga
left a comment
DOsinga
left a comment
There was a problem hiding this comment.
I don't know about this approach. it feels heavy handed given what we are trying to do here. the real solution would be to make creating sub agents a real extension and then just not enable that extension for subagents
| } | ||
|
|
||
| pub fn mark_as_subagent(&mut self) { | ||
| self.is_subagent = true; |
There was a problem hiding this comment.
this doesn't seem like a property that should be mutable
|
|
||
| let agent = agent_manager | ||
| .get_or_create_agent(session.id.clone()) | ||
| .get_or_create_agent(session.id.clone(), true) // true = is_subagent |
|
@tlongwell-block Are you able to give thoughts or take a rev on this per @DOsinga's comment?
|
I totally agree. This is hamfisted and not the right way. I mocked up moving subagents/subrecipes tools to a platform extension and it it just a big change. Want to get #5082 in before proposing something that significant. This PR was just an option as an emergency fix if we needed one for a new release. Since we seem to have figured out the endless subagent spawning root cause (no developer tools enabled, which caused goose to spawn endless subagents to try to do anything) we could just address that with a system prompt change or a tool description change. |
|
Closing this for now. Don't think this is a good idea |
No description provided.