Unify deployment tracking and support multiple deployments #677

EhabY · 2025-12-09T10:13:32Z

This change centralizes deployment state management and enables seamless multi-deployment support. The extension now properly tracks per-deployment credentials, syncs state across VS Code windows, and handles login/logout flows in a unified way.

Key changes:

Architecture:

Add DeploymentManager to centralize deployment state (url, label, token, user) and coordinate client updates, auth contexts, and workspace refreshes
Add LoginCoordinator to handle login prompts with cross-window detection, preventing duplicate login dialogs when multiple windows need auth
Move deployment types to src/deployment/ module with proper type guards

Storage & Auth:

SecretsManager now stores per-deployment credentials using label-based keys (coder.session.<label>) instead of flat sessionToken storage
Add LRU tracking for deployments with automatic cleanup of old credentials
Add migration from legacy flat storage format
Cross-window sync via secrets.onDidChange events
Debug command (coder.debug.listDeployments) for inspecting stored state

Commands & Remote:

Commands now use DeploymentManager instead of directly manipulating client
Remote connection uses LoginCoordinator for auth prompts during workspace connections
Remove forceLogout in favor of unified logout through DeploymentManager
CliManager.configure() now called on every remote connection, with secrets storage as the source of truth for credentials

WebSocket improvements:

CoderApi now implements Disposable to clean up WebSocket connections
Add setCredentials() method to update host+token atomically, avoiding unnecessary reconnection cycles
Add suspend() support to ReconnectingWebSocket for clean disconnects
Simplify WebSocket fallback logic with cleaner SSE fallback handling

Tests:

Update secretsManager tests for new per-deployment API
Add comprehensive reconnectingWebSocket tests for suspend/reconnect
Extend coderApi tests for credential handling

This change centralizes deployment state management and enables seamless multi-deployment support. The extension now properly tracks per-deployment credentials, syncs state across VS Code windows, and handles login/logout flows in a unified way. Key changes: Architecture: - Add DeploymentManager to centralize deployment state (url, label, token, user) and coordinate client updates, auth contexts, and workspace refreshes - Add LoginCoordinator to handle login prompts with cross-window detection, preventing duplicate login dialogs when multiple windows need auth - Move deployment types to src/deployment/ module with proper type guards Storage & Auth: - SecretsManager now stores per-deployment credentials using label-based keys (coder.session.<label>) instead of flat sessionToken storage - Add LRU tracking for deployments with automatic cleanup of old credentials - Add migration from legacy flat storage format - Cross-window sync via secrets.onDidChange events - Debug command (coder.debug.listDeployments) for inspecting stored state Commands & Remote: - Commands now use DeploymentManager instead of directly manipulating client - Remote connection uses LoginCoordinator for auth prompts during workspace connections - Remove forceLogout in favor of unified logout through DeploymentManager - CliManager.configure() now called on every remote connection, with secrets storage as the source of truth for credentials WebSocket improvements: - CoderApi now implements Disposable to clean up WebSocket connections - Add setCredentials() method to update host+token atomically, avoiding unnecessary reconnection cycles - Add suspend() support to ReconnectingWebSocket for clean disconnects - Simplify WebSocket fallback logic with cleaner SSE fallback handling Tests: - Update secretsManager tests for new per-deployment API - Add comprehensive reconnectingWebSocket tests for suspend/reconnect - Extend coderApi tests for credential handling

EhabY · 2025-12-10T13:43:16Z

src/api/coderApi.ts

+		const hostChanged = (currentHost || "") !== (host || "");
+		const tokenChanged = currentToken !== token;


Continuation of #633 (comment)

However, if we authenticate without a token (mTLS) then we might begin with the following: { host: XYZ, token: undefined }, but when we authenticate we change the token to "" and thus trigger a reconnect.

The input is the same since undefined might be treated the same as "" except that we might have started with invalid configuration (401s) and then changed the setting and prompted a login again that worked so we trigger this reconnect with the new info.

But why should this trigger a reconnect? Nothing changes from the Coder server's perspective, it has no distinction between a blank token and a missing token. If a missing token was a 401, so will a blank token, so why reconnect when we know it will fail? Or is that not the case?

Here's my thought process:

Let's assume that we authenticate without a token (mTLS) but we are pointing to the wrong files or there's an issue with the tlsCertFile/tlsKeyFile. Let's assume that we then trigger a login event that actually works, so we set the token to "" and thus trigger a reconnect. It's not necessarily about the empty vs. undefined but more about triggering something when we successfully login.

src/api/coderApi.ts

src/core/secretsManager.ts

EhabY · 2025-12-10T14:57:33Z

src/deployment/deploymentManager.ts

+					// If we don't have a user yet, try to fetch one
+					if (!this.currentDeployment?.user) {
+						await this.tryFetchAndUpgradeUser();
+					}


I think we should always try to fetch the user here to make sure the new authentication info is valid. If they are not valid it would be like we are "logged out" so looks more consistent. What do you think?

Whenever we set session auth, I assume we do so after having already validated the user right? If so it seems like it would result in an extra request for no benefit.

Also the function makes me a bit nervous because it relies on a property this.currentDeployment and then has an async function, and then sets that property again but it could have been changed in the meantime.

IMO we should set the deployment only after auth is validated so we set it all at once without any opportunity for race conditions, but is there a use case for having the current deployment set and not any auth details? To me they always go together.

but is there a use case for having the current deployment set and not any auth details? To me they always go together.

We could be logged in to a deployment then the token is revoked or expired and thus no "auth" details. This is why I lean towards adding that extra request since there's no guarantee that the info we just read is always valid. Like we can have a "current" deployment that is not signed in but another window signs in and thus we should react to that and fetch the user to update it.

I do get your concern about the deployment changing in the meantime. I'll capture safeHostname and check it before trying to modify the currentDeployment

code-asher

I ran out of time to test it but will do so tomorrow!

src/core/cliManager.ts

code-asher · 2025-12-10T22:56:54Z

src/core/contextManager.ts

+	}
+
+	private setInternalContexts(extensionContext: vscode.ExtensionContext): void {
+		vscode.commands.executeCommand(


Just curious why this uses vscode.commands.executeCommand instead of this.set. Seems like the main result is that this.context would not get updated, is that OK?

this.set(CoderContext, Boolean) would mean that we have to expose this flag to clients of this class. I want to keep this flag hidden and only be set once so that's why it's not of type CoderContext and it cannot be changed.

code-asher · 2025-12-10T23:05:59Z

src/core/secretsManager.ts

+		if (!safeHostname) {
+			return undefined;
+		}


I would guess this is for convenience but just from the perspective of the function contracts it feels unexpected to me because:

setSessionAuth("", auth) // no error, so i assume my auth is set getSessionAuth("") // no error, but also no auth that i thought i set earlier onDidChangeSessionAuth("") // this will still fire, but with no auth

Maybe it would be better for the check to be external like:

if (deployment?.safeHostname) { getSessionAuth(deployment.safeHostname) }

Or, we make these throw errors if we want to formalize that the hostname should be non-empty strings.

This actually made me realize that if we open a deployment with no label we might not be able to "login" ever. So instead I've added the following lookup:

private getSessionKey(safeHostname: string): string { return `${SESSION_KEY_PREFIX}${safeHostname || "<legacy>"}`; }

The only place that can provide a blank hostname is remote.ts when connecting to a workspace that uses this old format.

src/deployment/deploymentManager.ts

code-asher · 2025-12-10T23:17:54Z

src/deployment/deploymentManager.ts

+					// If we don't have a user yet, try to fetch one
+					if (!this.currentDeployment?.user) {
+						await this.tryFetchAndUpgradeUser();
+					}


Whenever we set session auth, I assume we do so after having already validated the user right? If so it seems like it would result in an extra request for no benefit.

Also the function makes me a bit nervous because it relies on a property this.currentDeployment and then has an async function, and then sets that property again but it could have been changed in the meantime.

IMO we should set the deployment only after auth is validated so we set it all at once without any opportunity for race conditions, but is there a use case for having the current deployment set and not any auth details? To me they always go together.

src/commands.ts

code-asher · 2025-12-11T00:59:59Z

src/commands.ts

-		// Clear from memory.
-		await this.mementoManager.setUrl(undefined);
-		await this.secretsManager.setSessionToken(undefined);
+		await this.deploymentManager.logout();


I ran into some confusion between how deploymentManager has auth-related stuff like logout but then there is also a loginCoordinator and that does some auth-related stuff and it feels like they are cross-contaminated.

I think to me conceptually the current deployment and its auth are one singular object that would not make sense to set separately, and conceptually having them split feels like too much, and it could be unclear on what the responsibilities are for each.

Like right now it seems possible to be in this half-state where you have a deployment set but are not logged in, but when we would want that?

But, this is just me musing out loud, possibly I just need more time with it, so...well if anything stands out to you from this then that is great, if not then feel free to ignore lol

I think I see the confusion now, let me explain my perspective on these objects lol:

LoginCoordinator is solely responsible for logging in into a deployment and persisting the auth if it was successful. There is no logic about logout and technically we can move the persist part out of it if it's confusing (I kept it there to keep code DRY but I can see why this might be confusing).

DeploymentManager is solely responsible for managing the extensionClient and it's authentication state. It does not attempt logins or logout (logout will be renamed to clear maybe). It's solely a data structure that contains this info and reacts to changes but does not do actual login/logout. (It manages CurrentDeployment only). Maybe this should renamed into CurrentDeploymentManager?

code-asher · 2025-12-11T01:07:03Z

src/extension.ts

-				client.setHost(details.url);
-				client.setSessionToken(details.token);
+
+				// Will automatically fetch the user and upgrade the deployment


What do you mean by upgrade? I saw it referenced in deploymentManager.ts as well but was not too sure. Should it be update? Or is it like it gets "upgraded" from a not-logged-in deployment to a logged-in deployment?

Also the function says "without auth" but we pass it a token which feels weird.

"upgraded" from a not-logged-in deployment to a logged-in deployment

Yes it means that.

Also the function says "without auth" but we pass it a token which feels weird.

True... maybe something like setDeploymentAndValidate?

src/extension.ts

code-asher · 2025-12-11T01:20:26Z

src/extension.ts

-					allWorkspacesProvider.fetchAndRefresh();
 				} else {
-					output.warn("No error, but got unexpected response", user);
+					output.info("Deployment set but not authenticated");


Ah yeah this is the use case I am curious about. Initially I felt like we would not want to set the deployment until we have valid credentials so we can set it all at once together and avoid having to synchronize, but thinking about it, this could allow you to try logging in, fail or cancel, close the window, come back later, and be able to try logging in again without having to enter the URL a second time?

But, if I try to log into a new deployment, then change my mind and cancel, I could lose being logged into that previous deployment, which might feel bad. We could handle the above case by just having the URL in the history I suppose.

try logging in again without having to enter the URL a second time?

The idea here is that you could login, not open VS Code in a while then open it again and now suddenly you are not authenticated (but still you maintain the last deployment that you were connected to). So you can have stuff like the URL prepopulated when you click login.

I could lose being logged into that previous deployment, which might feel bad

I mean currently you cannot do this WITHOUT clicking logout anyway (unless the current deployment is not authenticated because the token is gone) but then setCurrentDeployment is only set at the very end anyway...

EhabY · 2025-12-11T11:20:35Z

@code-asher I've add tests in the last commit BTW, if you wish I can make it a separate PR 🙏

EhabY mentioned this pull request Dec 9, 2025

Implement OAuth client in VS Code #633

Open

EhabY force-pushed the multi-deployment-squashed branch from ead6f43 to bc9bce5 Compare December 9, 2025 20:49

EhabY force-pushed the multi-deployment-squashed branch from bc9bce5 to 642fd52 Compare December 10, 2025 13:34

EhabY commented Dec 10, 2025

View reviewed changes

EhabY force-pushed the multi-deployment-squashed branch from 642fd52 to 5fd7241 Compare December 10, 2025 14:38

EhabY added 2 commits December 10, 2025 17:39

Handle CoderApi review comments

3a9e159

Handle secretsManager comments part 1

e06aedb

EhabY force-pushed the multi-deployment-squashed branch from 5fd7241 to 5e83d93 Compare December 10, 2025 14:39

Handle secretsManager comments part 2

190d8ed

EhabY force-pushed the multi-deployment-squashed branch from 5e83d93 to 46afe7f Compare December 10, 2025 14:50

EhabY commented Dec 10, 2025

View reviewed changes

Deployment management refactoring

acf26aa

EhabY force-pushed the multi-deployment-squashed branch from 46afe7f to acf26aa Compare December 10, 2025 14:59

code-asher reviewed Dec 11, 2025

View reviewed changes

Review comments

cb586a5

Fix serialization issue and workspace provider refresh

c975fc0

EhabY force-pushed the multi-deployment-squashed branch from 1751bde to 542c240 Compare December 11, 2025 13:15

Add tests for deployment management

1abfb6e

EhabY force-pushed the multi-deployment-squashed branch from 542c240 to 1abfb6e Compare December 11, 2025 13:16

		const hostChanged = (currentHost \|\| "") !== (host \|\| "");
		const tokenChanged = currentToken !== token;

Unify deployment tracking and support multiple deployments #677

Are you sure you want to change the base?

Unify deployment tracking and support multiple deployments #677

Uh oh!

Conversation

EhabY commented Dec 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

code-asher left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

EhabY commented Dec 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

EhabY commented Dec 9, 2025 •

edited

Loading

code-asher left a comment •

edited

Loading