-
Notifications
You must be signed in to change notification settings - Fork 203
feat(a2a): agents-as-tools #424
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add an integ test for this feature.
inputSchema={ | ||
"type": "object", | ||
"properties": { | ||
"query": {"type": "string", "description": "The query or task to send to the sub-agent"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to support multi-modal inputs here? If so, query could be an object type as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for sure, i think that would be a great addition here. do you think this is something that can be implemented in a followup PR or would you recommend including it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also makes me wonder if the tool spec should effectively be the JSON representation of Union[str | list[ContentBlocks]]
🤔
Co-authored-by: Nick Clegg <nac542@gmail.com>
agent: The Agent instance to wrap | ||
""" | ||
super().__init__() | ||
self._agent = agent |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that that this ends up being a stateful tool. Unlike most other tools, once invoked this tool retains history (the agent messages), which means future invocations will keep the prior context. Similarly, agents as tools aren't really safe to share between agents because of the shared history and because of potential concurrency issues.
To that end, I wonder if additional options are needed for things like having a per-tool-invocation history or copying the agent as part of the tool so that it could be more easily shared but that also has it's own downsides.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Related to session persistence, I'm also curious if this should be persisting it's tool state (the tool-agent's state) in the parent agent's state - otherwise we run into the problem where the parent agent cannot be persisted/hydrated correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for sure, great points! i see you've been advocating for these since the start: #84 (comment). 😄
- i think supporting more configuration options is definitely worth investigating. do you view that as a blocker for this initial implementation or something that can be done in a followup task? given some of the intricacies of the current concurrency model and freshness of session persistence, i worry about not having enough bandwidth to implement the changes in time.
- just to make sure i understand correctly: the main limitation with the implementation is that the tool agent's state will not be persisted, thus it cannot be hydrated correctly. however, the parent agent will store interactions with the tool agent as tool calls/results in the messages array - which can be persisted/hydrated. so your concern is that this fragmentation in state (the parent agent having a history of interactions with the tool agent, and the tool agent not having any history) could hinder performance? it definitely can. my initial thought on this is that since the tool agent is itself an agent, it should be responsible for maintaining it's own state and can be persisted/hydrating. i think strand's notion of persistence is tied to an agent which is where i think the thoughts around adding the tool agent's state to the parent agent may be coming from. however, i think for these use multi agent use cases, where the role of each agent is more complex and stateful, the thing that ties all the components in the system together is more akin to a "sessionId" or "systemId" where each component of that "system" is responsible for it's own state, and they are tied together by an identifier which effectively functions as a foreign key.
but all in all, i'm more than happy to address any blocking items you have in this PR. otherwise, i will create followup tasks for these topics.
Description
Support "agents-as-tools".
Related Issues
Documentation PR
Type of Change
New feature
Testing
How have you tested the change? Verify that the changes do not break functionality or introduce warnings in consuming repositories: agents-docs, agents-tools, agents-cli
hatch run prepare
Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.