Use per-deployment keys for race-free deployment tracking

src/core/secretsManager.ts

@@ -8,18 +8,16 @@ import type { Memento, SecretStorage, Disposable } from "vscode";
 
 import type { Logger } from "../logging/logger";
 
-// Each deployment has its own key to ensure atomic operations (multiple windows
-// writing to a shared key could drop data) and to receive proper VS Code events.
+// Per-deployment keys ensure atomic operations (multiple windows writing to a
+// shared key could drop data) and enable proper VS Code change events.
 const SESSION_KEY_PREFIX = "coder.session.";
 const OAUTH_CLIENT_PREFIX = "coder.oauth.client.";
+const DEPLOYMENT_ACCESS_PREFIX = "coder.access.";
 
 type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX;
 
 const OAUTH_CALLBACK_KEY = "coder.oauthCallback";
-
 const CURRENT_DEPLOYMENT_KEY = "coder.currentDeployment";
-
-const DEPLOYMENT_USAGE_KEY = "coder.deploymentUsage";
 const DEFAULT_MAX_DEPLOYMENTS = 10;
 
 const LEGACY_SESSION_TOKEN_KEY = "sessionToken";
@@ -53,14 +51,6 @@ const SessionAuthSchema = z.object({
 
 export type SessionAuth = z.infer<typeof SessionAuthSchema>;
 
-// Tracks when a deployment was last accessed for LRU pruning.
-const DeploymentUsageSchema = z.object({
-	safeHostname: z.string(),
-	lastAccessedAt: z.string(),
-});
-
-type DeploymentUsage = z.infer<typeof DeploymentUsageSchema>;
-
 const OAuthCallbackDataSchema = z.object({
 	state: z.string(),
 	code: z.string().nullable(),
@@ -224,61 +214,63 @@ export class SecretsManager {
 	}
 
 	/**
-	 * Record that a deployment was accessed, moving it to the front of the LRU list.
+	 * Record that a deployment was accessed by updating its timestamp.
 	 * Prunes deployments beyond maxCount, clearing their auth data.
 	 */
 	public async recordDeploymentAccess(
 		safeHostname: string,
 		maxCount = DEFAULT_MAX_DEPLOYMENTS,
 	): Promise<void> {
-		const usage = this.getDeploymentUsage();
-		const filtered = usage.filter((u) => u.safeHostname !== safeHostname);
-		const newEntry = DeploymentUsageSchema.parse({
-			safeHostname,
-			lastAccessedAt: new Date().toISOString(),
-		});
-		filtered.unshift(newEntry);
-
-		const toKeep = filtered.slice(0, maxCount);
-		const toRemove = filtered.slice(maxCount);
-
-		await Promise.all(
-			toRemove.map((u) => this.clearAllAuthData(u.safeHostname)),
+		// Update this deployment's access timestamp
+		await this.memento.update(
+			`${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`,
+			new Date().toISOString(),
 		);
-		await this.memento.update(DEPLOYMENT_USAGE_KEY, toKeep);
+
+		// Prune if needed - errors here shouldn't block deployment access
+		try {
+			const allHostnames = await this.getKnownSafeHostnames();
+			const toRemove = allHostnames.slice(maxCount);
+			await Promise.all(toRemove.map((h) => this.clearAllAuthData(h)));
+		} catch {
+			this.logger.warn("Failed to prune old deployments");
+		}
 	}
 
 	/**
-	 * Clear all auth data for a deployment and remove it from the usage list.
+	 * Clear all auth data for a deployment, including the access timestamp.
 	 */
 	public async clearAllAuthData(safeHostname: string): Promise<void> {
 		await Promise.all([
 			this.clearSessionAuth(safeHostname),
 			this.clearOAuthClientRegistration(safeHostname),
+			this.memento.update(
+				`${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`,
+				undefined,
+			),
 		]);
-		const usage = this.getDeploymentUsage().filter(
-			(u) => u.safeHostname !== safeHostname,
-		);
-		await this.memento.update(DEPLOYMENT_USAGE_KEY, usage);
 	}
 
 	/**
 	 * Get all known hostnames, ordered by most recently accessed.
+	 * Derives the list from actual session secrets stored.
 	 */
-	public getKnownSafeHostnames(): string[] {
-		return this.getDeploymentUsage().map((u) => u.safeHostname);
-	}
-
-	/**
-	 * Get the full deployment usage list with access timestamps.
-	 */
-	private getDeploymentUsage(): DeploymentUsage[] {
-		const data = this.memento.get<unknown>(DEPLOYMENT_USAGE_KEY);
-		if (!data) {
-			return [];
-		}
-		const result = z.array(DeploymentUsageSchema).safeParse(data);
-		return result.success ? result.data : [];
+	public async getKnownSafeHostnames(): Promise<string[]> {
+		const keys = await this.secrets.keys();
+		const sessionHostnames = keys
+			.filter((k) => k.startsWith(SESSION_KEY_PREFIX))
+			.map((k) => k.slice(SESSION_KEY_PREFIX.length));
+
+		// Sort by access timestamp (most recent first)
+		const withTimestamps = sessionHostnames.map((hostname) => ({
+			hostname,
+			accessedAt:
+				this.memento.get<string>(`${DEPLOYMENT_ACCESS_PREFIX}${hostname}`) ??
+				"",
+		}));
+
+		withTimestamps.sort((a, b) => b.accessedAt.localeCompare(a.accessedAt));
+		return withTimestamps.map((w) => w.hostname);
 	}
 
 	/**

src/extension.ts

@@ -11,7 +11,6 @@ import { AuthInterceptor } from "./api/authInterceptor";
 import { CoderApi } from "./api/coderApi";
 import { Commands } from "./commands";
 import { ServiceContainer } from "./core/container";
-import { type SecretsManager } from "./core/secretsManager";
 import { DeploymentManager } from "./deployment/deploymentManager";
 import { CertificateError } from "./error/certificateError";
 import { getErrorDetail, toError } from "./error/errorUtils";
@@ -243,7 +242,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
 			showTreeViewSearch(ALL_WORKSPACES_TREE_ID),
 		),
 		vscode.commands.registerCommand("coder.debug.listDeployments", () =>
-			listStoredDeployments(secretsManager),
+			listStoredDeployments(serviceContainer),
 		),
 	);
 
@@ -387,26 +386,36 @@ async function showTreeViewSearch(id: string): Promise<void> {
 }
 
 async function listStoredDeployments(
-	secretsManager: SecretsManager,
+	serviceContainer: ServiceContainer,
 ): Promise<void> {
-	const hostnames = secretsManager.getKnownSafeHostnames();
-	if (hostnames.length === 0) {
-		vscode.window.showInformationMessage("No deployments stored.");
-		return;
-	}
+	const secretsManager = serviceContainer.getSecretsManager();
+	const output = serviceContainer.getLogger();
 
-	const selected = await vscode.window.showQuickPick(
-		hostnames.map((hostname) => ({
-			label: hostname,
-			description: "Click to forget",
-		})),
-		{ placeHolder: "Select a deployment to forget" },
-	);
+	try {
+		const hostnames = await secretsManager.getKnownSafeHostnames();
+		if (hostnames.length === 0) {
+			vscode.window.showInformationMessage("No deployments stored.");
+			return;
+		}
+
+		const selected = await vscode.window.showQuickPick(
+			hostnames.map((hostname) => ({
+				label: hostname,
+				description: "Click to forget",
+			})),
+			{ placeHolder: "Select a deployment to forget" },
+		);
 
-	if (selected) {
-		await secretsManager.clearAllAuthData(selected.label);
-		vscode.window.showInformationMessage(
-			`Cleared auth data for ${selected.label}`,
+		if (selected) {
+			await secretsManager.clearAllAuthData(selected.label);
+			vscode.window.showInformationMessage(
+				`Cleared auth data for ${selected.label}`,
+			);
+		}
+	} catch (error: unknown) {
+		output.error("Failed to list stored deployments", error);
+		vscode.window.showErrorMessage(
+			"Failed to list stored deployments. Storage may be corrupted.",
 		);
 	}
 }

test/unit/core/secretsManager.test.ts

@@ -75,72 +75,103 @@ describe("SecretsManager", () => {
 		});
 
 		it("should track known safe hostnames", async () => {
-			expect(secretsManager.getKnownSafeHostnames()).toEqual([]);
+			expect(await secretsManager.getKnownSafeHostnames()).toEqual([]);
 
 			await secretsManager.setSessionAuth("example.com", {
 				url: "https://example.com",
 				token: "test-token",
 			});
-			expect(secretsManager.getKnownSafeHostnames()).toContain("example.com");
+			expect(await secretsManager.getKnownSafeHostnames()).toContain(
+				"example.com",
+			);
 
 			await secretsManager.setSessionAuth("other-com", {
 				url: "https://other.com",
 				token: "other-token",
 			});
-			expect(secretsManager.getKnownSafeHostnames()).toContain("example.com");
-			expect(secretsManager.getKnownSafeHostnames()).toContain("other-com");
+			expect(await secretsManager.getKnownSafeHostnames()).toContain(
+				"example.com",
+			);
+			expect(await secretsManager.getKnownSafeHostnames()).toContain(
+				"other-com",
+			);
 		});
 
 		it("should remove safe hostname on clearAllAuthData", async () => {
 			await secretsManager.setSessionAuth("example.com", {
 				url: "https://example.com",
 				token: "test-token",
 			});
-			expect(secretsManager.getKnownSafeHostnames()).toContain("example.com");
+			expect(await secretsManager.getKnownSafeHostnames()).toContain(
+				"example.com",
+			);
 
 			await secretsManager.clearAllAuthData("example.com");
-			expect(secretsManager.getKnownSafeHostnames()).not.toContain(
+			expect(await secretsManager.getKnownSafeHostnames()).not.toContain(
 				"example.com",
 			);
 		});
 
+		it("should throw when secrets storage is corrupted", async () => {
+			await secretsManager.setSessionAuth("example.com", {
+				url: "https://example.com",
+				token: "test-token",
+			});
+			secretStorage.corruptStorage();
+
+			await expect(secretsManager.getKnownSafeHostnames()).rejects.toThrow();
+		});
+
 		it("should order safe hostnames by most recently accessed", async () => {
+			vi.useFakeTimers();
+
 			await secretsManager.setSessionAuth("first.com", {
 				url: "https://first.com",
 				token: "token1",
 			});
+			vi.advanceTimersByTime(10);
 			await secretsManager.setSessionAuth("second.com", {
 				url: "https://second.com",
 				token: "token2",
 			});
+			vi.advanceTimersByTime(10);
 			await secretsManager.setSessionAuth("first.com", {
 				url: "https://first.com",
 				token: "token1-updated",
 			});
 
-			expect(secretsManager.getKnownSafeHostnames()).toEqual([
+			expect(await secretsManager.getKnownSafeHostnames()).toEqual([
 				"first.com",
 				"second.com",
 			]);
+
+			vi.useRealTimers();
 		});
 
 		it("should prune old deployments when exceeding maxCount", async () => {
+			// Use fake timers to ensure distinct timestamps for each host
+			vi.useFakeTimers();
+
 			for (let i = 1; i <= 5; i++) {
 				await secretsManager.setSessionAuth(`host${i}.com`, {
 					url: `https://host${i}.com`,
 					token: `token${i}`,
 				});
+				vi.advanceTimersByTime(10);
 			}
 
-			await secretsManager.recordDeploymentAccess("new.com", 3);
+			// With maxCount=3, only the 3 most recently accessed hosts should remain.
+			await secretsManager.recordDeploymentAccess("host5.com", 3);
 
-			expect(secretsManager.getKnownSafeHostnames()).toEqual([
-				"new.com",
+			expect(await secretsManager.getKnownSafeHostnames()).toEqual([
 				"host5.com",
 				"host4.com",
+				"host3.com",
 			]);
 			expect(await secretsManager.getSessionAuth("host1.com")).toBeUndefined();
 			expect(await secretsManager.getSessionAuth("host2.com")).toBeUndefined();
+
+			vi.useRealTimers();
 		});
 	});
 
@@ -382,11 +413,6 @@ describe("SecretsManager", () => {
 				);
 				expect(await secretsManager.getCurrentDeployment()).toBeNull();
 			});
-
-			it("returns empty array for invalid deployment usage data", async () => {
-				await memento.update("coder.deploymentUsage", [{ safeHostname: 123 }]);
-				expect(secretsManager.getKnownSafeHostnames()).toEqual([]);
-			});
 		});
 
 		describe("backwards compatibility", () => {