Skip to content

Conversation

@fioan89
Copy link
Collaborator

@fioan89 fioan89 commented Jan 9, 2026

I ended up with a workspace that has two build resources that have the same agent. We ended up creating two environments with the same id (workspace name + agent name) This caused issues in Toolbox.

I ended up with a workspace that has two build resources that have the
same agent. We ended up creating two environments with the same id (workspace name + agent name)
This caused issues in Toolbox.
Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am probably missing something but it looks like this is doing the same thing it already was, just slightly reorganized? The distinctBy looks like it should have already been preventing duplicate agent names.

Or if this is just to add tests, then looks good to me!

But, why not keep it as a set, does a set not actually guarantee uniqueness?

Edit: forgot to say, I see we removed the TODO, do we know now that we can just choose any agent arbitrarily or if it matters which one we pick? Even though the name is the same, other details about the duplicates might be different (status, ID, etc) so I am not sure we can just pick one at random.

@fioan89
Copy link
Collaborator Author

fioan89 commented Jan 10, 2026

Good points, I was planning to slowly refactor and improve this block for a couple of reasons:

  • in the last release I introduced a regression because one collection was a set (i.e lastEnvironments) while another one was a list (i.e resolvedEnv...) in the following comparison: if (lastEnvironments.size != resolvedEnvironments.size || lastEnvironments != resolvedEnvironments). It was my fault for not paying enough attention but I think the code is brittle, I had multiple issues in the pas around this part.
  • toSet() even though it resulted in unique pairs of workspace&agent, hided the fact there are workspaces with two resource and same agent name. The initial distinctBy was applied only between the agents of the same resources. Somehow I ended up with this borked workspace and I never knew it was possible. In fact Blink suggests that neither issues should be possible (two agents with same name under the same resource or two agents with same name in two different resources)
  • ahh you are right I forgot to add that comment back.
  • in the end the pairs need to end-up as a list in toolbox so I thought working only with lists makes the code more reasadble.

TLDR; this is an attempt to fix a regression while at the same time to improve the readability and make some things obvious. I can just convert everything to sets and the fix would be much clear.

@code-asher
Copy link
Member

code-asher commented Jan 12, 2026

Ahhhh I see the distinct by is applied to the merge of all the workspace's resources now, rather than applied to each individual set of resources. Makes sense!

@fioan89 fioan89 merged commit 22b6813 into main Jan 13, 2026
6 checks passed
@fioan89 fioan89 deleted the fix-support-for-two-resources-with-same-agent-name branch January 13, 2026 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants