- Notifications
You must be signed in to change notification settings - Fork914
fix(cli)!: enforce selection for multiple agents and select sub agent if available#18427
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 ourterms of service andprivacy 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
In the past we randomly selected workspace agent if there were multiple.Unless both are running on the same machine with the same configuration,this would be very confusing behavior for a user.With the introduction of sub agents (devcontainer agents), it wasdecided prioritize them if present. Similarly as before, selecting adevcontainer randomly would be confusing. We now error out if the agentname is not specified and there are multiple agents.Fixescoder/internal#696
0a47fd1
to647181e
Compare1c6193a
tof923edb
Compare
IMO if you have to ask yourself if this is a breaking change, it's best to err on the side of caution and label it as breaking (obligatory:https://xkcd.com/1172/). |
@mafredri my understanding of this change is as follows: Given: workspace foo, agent smith, no sub-agents
Given: workspace foo, agents smith, jones
Given: workspace foo, agents smith, sub-agents johnson
Given: workspace foo, agents smith, sub-agents johnson, thompson
Given: workspace foo, agents smith, johnes, sub-agents johnson, thompson
Am I correct? |
mafredri commentedJun 18, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@johnstcn Close, just two correction (as sub agents are prioritized same as in the Web UI). Given: workspace foo, agents smith, sub-agents johnson
Given: workspace foo, agents smith, sub-agents johnson, thompson
|
On the surface I find this mildly surprising, but given that it's predicated on the presence of a sub-agent and that it corresponds with the UI's prioritization, that seems fine to me. |
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.
Approving pending any blocking concerns from@deansheather
Uh oh!
There was an error while loading.Please reload this page.
In the past we randomly selected workspace agent if there were multiple.
Unless both are running on the same machine with the same configuration,
this would be very confusing behavior for a user.
With the introduction of sub agents (devcontainer agents), it was
decided prioritize them if present. Similarly as before, selecting a
devcontainer randomly would be confusing. We now error out if the agent
name is not specified and there are multiple agents.
Fixescoder/internal#696
Example output from running two devcontainers:
TBD: Do we classify this breaking or not? IMO it could slip by as not as random selection can't really be a feature, can it? And dev containers are new and you can just not use them.