I think it is reasonable to open the first agent when you click on the workspace. I could see an argument for making it pop open the agent select dialog, but if a user wants a different agent they could just click the right one directly on the sidebar instead. But, idk if users feel differently. We could always change it if we get feedback, this is already an improvement IMO.

That said, I think we should make an additional fix, inopenWorkspace we should makeworkspaceAgent non-undefined so we always have to pass one in, and we always get a consistent remote authority, no matter what calls the function.

Also forgot to say, thank you for the contribution!

Happy to contribute to this project :)!

I could see an argument for making it pop open the agent select dialog

This is the current behavior if a workspace is opened with no agent in the remote authority, and there are multiple agents in the workspace.

Should I then get rid of this QuickPick? Or offer it when trying to open the workspace and there are multiple agents?

in openWorkspace we should make workspaceAgent non-undefined so we always have to pass one in

The issue there is thatcommands.ts#open can sometimes be called without an agent, I can instead offer a QuickPick when there are multiple agents so we always pass an agent as you indicated or else never even callopen.

This is the current behavior if a workspace is opened with no agent in the remote authority, and there are multiple agents in the workspace.

True, but it happens after we open with that remote authority, which can cause the issue in#121. Ideally we do it before, so we are guaranteed one stable remote authority per agent.

Should I then get rid of this QuickPick

I think we should keep it in case someone uses the open command from the command palette. When going that route, I think it makes sense to show the agent picker still.

The issue there is that commands.ts#open can sometimes be called without an agent, I can instead offer a QuickPick when there are multiple agents

Exactly my thinking, we refactor this function just a bit so we always callmaybeAskAgent() (instead of only whenargs.length === 0) and abort if the user declines to pick one.

Ideally we do it before, so we are guaranteed one stable remote authority per agent.

It is now done when callingopen(...) so it happens before we open the workspace folder.

I think we should keep it in case someone uses the open command from the command palette. When going that route, I think it makes sense to show the agent picker still.

Actually for line 410 inremote.ts#setup:const gotAgent = await this.commands.maybeAskAgent(workspace, parts.agent); seems useless now because we never open without an agent (from the tree view or the command palette). I guess it's useful to keep if users manually input their remote authority (or use one from an older version)

Actually for line 410 in remote.ts#setup: const gotAgent = await this.commands.maybeAskAgent(workspace, parts.agent); seems useless now because we never open without an agent (from the tree view or the command palette). I guess it's useful to keep if users manually input their remote authority (or use one from an older version)

I was thinking the same thing, it would be nice to remove it but I guess we need it for existing connections from the recents menu which might lack the agent. I think we do not need it for the manual input case though, because that would trigger the URI handler which callsopen which now ensures it gets an agent.

Possibly we could rewrite these entries to add the agent, then we could delete that code, but not sure how hard that would be.

I just realized you were talking about the quick pick inremote.ts this whole time; I thought the first mention was the one incommands.ts which is why I was talking about the command palette, my bad 🤦 (well, they both use the same picker/code but you know what I mean)

So:

• remote.ts picker: now only used when opening a recent connection that has no agent in the host name, possibly we can remove this one day
• commands.ts picker: used when handling a URI that lacks an agent or when launching from the command palette, will stick around

Indeed, I was talking about the one in theremote.ts but it seems like it'll have to stay (for now)