Use primary agent for the remote authority when opening a workspace

[EhabY]

#121

This change updates the behavior when opening a workspace from the sidebar: the first agent is now opened automatically using the same remote authority URI as when opening the agent directly.

If a workspace has multiple agents, we always open the first one, skipping the agent selection QuickPick when the folder is opened. I implemented it this way since we're already opening the folderPath of the first agent by default.

If this behavior should only apply when there's a single agent, it's easy to adjust. But in that case, should we also avoid passing the folderPath when multiple agents are present?

[code-asher]

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, in openWorkspace we should make workspaceAgent non-undefined so we always have to pass one in, and we always get a consistent remote authority, no matter what calls the function.

[code-asher]

Also forgot to say, thank you for the contribution!

[EhabY]

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 that commands.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 call open.

[code-asher]

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 call maybeAskAgent() (instead of only when args.length === 0) and abort if the user declines to pick one.

[EhabY]

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

It is now done when calling open(...) 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 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)

[code-asher]

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 calls open 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.

[code-asher]

This would override any folder passed via query param, so I think this will have to be:

Suggested change

	folderPath = agent.expanded_directory;
	if (!folderPath) {
	folderPath = agent.expanded_directory;
	}

[code-asher]

I think maybe we want if (!workspaceAgent) in case someone has agent= in the URI (so a blank string).

[code-asher]

I just realized you were talking about the quick pick in remote.ts this whole time; I thought the first mention was the one in commands.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