refactor: move memento-backed state out of SecretsManager

src/announcements/manager.ts

@@ -16,7 +16,7 @@ import {
 import { AnnouncementsPreview } from "./preview";
 
 import type { CoderApi } from "../api/coderApi";
-import type { SecretsManager } from "../core/secretsManager";
+import type { MementoManager } from "../core/mementoManager";
 import type { SessionState } from "../deployment/sessionStore";
 import type { Logger } from "../logging/logger";
 
@@ -43,7 +43,7 @@ export class AnnouncementManager implements vscode.Disposable {
 	public constructor(
 		private readonly client: Pick<CoderApi, "getAppearance">,
 		private readonly sessionState: SessionState,
-		private readonly secretsManager: SecretsManager,
+		private readonly mementoManager: MementoManager,
 		private readonly logger: Logger,
 	) {
 		this.statusBarItem.command = "coder.viewAnnouncements";
@@ -149,7 +149,7 @@ export class AnnouncementManager implements vscode.Disposable {
 		this.banners = banners;
 
 		const surfacedKeys = new Set(
-			this.secretsManager.getSurfacedBanners(session.deployment.safeHostname),
+			this.mementoManager.getSurfacedBanners(session.deployment.safeHostname),
 		);
 		const newBanners = banners.filter(
 			(banner) => !surfacedKeys.has(banner.key),
@@ -168,36 +168,20 @@ export class AnnouncementManager implements vscode.Disposable {
 			return;
 		}
 		this.showPopup(newBanners);
-		await this.markSurfaced(banners, surfacedKeys);
+		await this.markSurfaced(banners);
 	}
 
 	/** Marks banners surfaced and clears the attention color; logs storage failures. */
-	private async markSurfaced(
-		banners: readonly Announcement[],
-		knownSurfacedKeys?: ReadonlySet<string>,
-	): Promise<void> {
+	private async markSurfaced(banners: readonly Announcement[]): Promise<void> {
 		const session = this.sessionState.current;
 		if (session.kind !== "signedIn") {
 			return;
 		}
 		try {
-			const surfacedKeys =
-				knownSurfacedKeys ??
-				new Set(
-					this.secretsManager.getSurfacedBanners(
-						session.deployment.safeHostname,
-					),
-				);
-			const merged = new Set([
-				...surfacedKeys,
-				...banners.map((banner) => banner.key),
-			]);
-			if (merged.size > surfacedKeys.size) {
-				await this.secretsManager.setSurfacedBanners(
-					session.deployment.safeHostname,
-					[...merged],
-				);
-			}
+			await this.mementoManager.addSurfacedBanners(
+				session.deployment.safeHostname,
+				banners.map((banner) => banner.key),
+			);
 			this.setAttentionIndicator(false);
 		} catch (error) {
 			this.logger.warn("Failed to mark Coder announcements as surfaced", error);

src/core/container.ts

@@ -50,7 +50,7 @@ export class ServiceContainer implements vscode.Disposable {
 		this.mementoManager = new MementoManager(context.globalState);
 		this.secretsManager = new SecretsManager(
 			context.secrets,
-			context.globalState,
+			this.mementoManager,
 			this.logger,
 		);
 

src/core/mementoManager.ts

@@ -1,10 +1,17 @@
+import { z } from "zod";
+
 import type { Memento } from "vscode";
 
-// Maximum number of recent URLs to store.
+/** Maximum number of recent URLs to store. */
 const MAX_URLS = 10;
 
-// Pending values expire after this duration to guard against stale
-// state from crashes or interrupted reloads.
+const DEPLOYMENT_ACCESS_PREFIX = "coder.access.";
+const SURFACED_BANNERS_PREFIX = "coder.surfacedBanners.";
+
+const SurfacedBannersSchema = z.array(z.string());
+
+/** Pending values expire after this duration to guard against stale
+ * state from crashes or interrupted reloads. */
 const PENDING_TTL_MS = 5 * 60 * 1000;
 
 /**
@@ -72,6 +79,68 @@ export class MementoManager {
 		return value ?? "none";
 	}
 
+	/** Record when a deployment was last accessed, for most-recently-used ordering. */
+	public async updateDeploymentAccess(safeHostname: string): Promise<void> {
+		await this.memento.update(
+			`${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`,
+			new Date().toISOString(),
+		);
+	}
+
+	public getDeploymentAccess(safeHostname: string): string | undefined {
+		return this.memento.get<string>(
+			`${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`,
+		);
+	}
+
+	/** The pre-multi-deployment URL key, read during legacy migration. */
+	public getLegacyUrl(): string | undefined {
+		return this.memento.get<string>("url");
+	}
+
+	public async clearLegacyUrl(): Promise<void> {
+		await this.memento.update("url", undefined);
+	}
+
+	public getSurfacedBanners(safeHostname: string): string[] {
+		const raw = this.memento.get<unknown>(
+			`${SURFACED_BANNERS_PREFIX}${safeHostname}`,
+		);
+		const result = SurfacedBannersSchema.safeParse(raw);
+		return result.success ? result.data : [];
+	}
+
+	/**
+	 * Merge banner keys into the surfaced set. The read-modify-write lives
+	 * here so a future atomic Memento update can be adopted in one place.
+	 */
+	public async addSurfacedBanners(
+		safeHostname: string,
+		bannerKeys: readonly string[],
+	): Promise<void> {
+		const existing = this.getSurfacedBanners(safeHostname);
+		const merged = new Set([...existing, ...bannerKeys]);
+		if (merged.size > existing.length) {
+			await this.memento.update(`${SURFACED_BANNERS_PREFIX}${safeHostname}`, [
+				...merged,
+			]);
+		}
+	}
+
+	/** Clear all per-deployment state (access timestamp, surfaced banners). */
+	public async clearDeploymentData(safeHostname: string): Promise<void> {
+		await Promise.all([
+			this.memento.update(
+				`${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`,
+				undefined,
+			),
+			this.memento.update(
+				`${SURFACED_BANNERS_PREFIX}${safeHostname}`,
+				undefined,
+			),
+		]);
+	}
+
 	private async setStamped<T>(key: string, value: T): Promise<void> {
 		await this.memento.update(key, { value, setAt: Date.now() });
 	}

src/core/secretsManager.ts

@@ -4,16 +4,16 @@ import { DeploymentSchema, type Deployment } from "../deployment/types";
 import { toSafeHost } from "../util/uri";
 
 import type { OAuth2ClientRegistrationResponse } from "coder/site/src/api/typesGenerated";
-import type { Memento, SecretStorage, Disposable } from "vscode";
+import type { SecretStorage, Disposable } from "vscode";
 
 import type { Logger } from "../logging/logger";
 
+import type { MementoManager } from "./mementoManager";
+
 // 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.";
-const SURFACED_BANNERS_PREFIX = "coder.surfacedBanners.";
 
 type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX;
 
@@ -30,8 +30,6 @@ export type CurrentDeploymentState = z.infer<
 	typeof CurrentDeploymentStateSchema
 >;
 
-const SurfacedBannersSchema = z.array(z.string());
-
 /**
  * OAuth token data stored alongside session auth.
  * When present, indicates the session is authenticated via OAuth.
@@ -56,7 +54,7 @@ export type SessionAuth = z.infer<typeof SessionAuthSchema>;
 export class SecretsManager {
 	constructor(
 		private readonly secrets: SecretStorage,
-		private readonly memento: Memento,
+		private readonly mementoManager: MementoManager,
 		private readonly logger: Logger,
 	) {}
 
@@ -215,11 +213,7 @@ export class SecretsManager {
 		safeHostname: string,
 		maxCount = DEFAULT_MAX_DEPLOYMENTS,
 	): Promise<void> {
-		// Update this deployment's access timestamp
-		await this.memento.update(
-			`${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`,
-			new Date().toISOString(),
-		);
+		await this.mementoManager.updateDeploymentAccess(safeHostname);
 
 		// Prune if needed - errors here shouldn't block deployment access
 		try {
@@ -232,44 +226,17 @@ export class SecretsManager {
 	}
 
 	/**
-	 * Clear all auth data for a deployment, including the access timestamp.
+	 * Clear all auth data for a deployment, including its memento-backed
+	 * state (access timestamp, surfaced banners), so both stores stay in sync.
 	 */
 	public async clearAllAuthData(safeHostname: string): Promise<void> {
 		await Promise.all([
 			this.clearSessionAuth(safeHostname),
 			this.clearOAuthClientRegistration(safeHostname),
-			this.clearSurfacedBanners(safeHostname),
-			this.memento.update(
-				`${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`,
-				undefined,
-			),
-		]);
-	}
-
-	public getSurfacedBanners(safeHostname: string): string[] {
-		const raw = this.memento.get<unknown>(
-			`${SURFACED_BANNERS_PREFIX}${safeHostname}`,
-		);
-		const result = SurfacedBannersSchema.safeParse(raw);
-		return result.success ? result.data : [];
-	}
-
-	public async setSurfacedBanners(
-		safeHostname: string,
-		bannerKeys: readonly string[],
-	): Promise<void> {
-		await this.memento.update(`${SURFACED_BANNERS_PREFIX}${safeHostname}`, [
-			...bannerKeys,
+			this.mementoManager.clearDeploymentData(safeHostname),
 		]);
 	}
 
-	private async clearSurfacedBanners(safeHostname: string): Promise<void> {
-		await this.memento.update(
-			`${SURFACED_BANNERS_PREFIX}${safeHostname}`,
-			undefined,
-		);
-	}
-
 	/**
 	 * Get all known hostnames, ordered by most recently accessed.
 	 * Derives the list from actual session secrets stored.
@@ -283,9 +250,7 @@ export class SecretsManager {
 		// Sort by access timestamp (most recent first)
 		const withTimestamps = sessionHostnames.map((hostname) => ({
 			hostname,
-			accessedAt:
-				this.memento.get<string>(`${DEPLOYMENT_ACCESS_PREFIX}${hostname}`) ??
-				"",
+			accessedAt: this.mementoManager.getDeploymentAccess(hostname) ?? "",
 		}));
 
 		withTimestamps.sort((a, b) => b.accessedAt.localeCompare(a.accessedAt));
@@ -297,15 +262,15 @@ export class SecretsManager {
 	 * Also sets the current deployment if none exists.
 	 */
 	public async migrateFromLegacyStorage(): Promise<string | undefined> {
-		const legacyUrl = this.memento.get<string>("url");
+		const legacyUrl = this.mementoManager.getLegacyUrl();
 		if (!legacyUrl) {
 			return undefined;
 		}
 
 		const oldToken = await this.secrets.get(LEGACY_SESSION_TOKEN_KEY);
 
 		await this.secrets.delete(LEGACY_SESSION_TOKEN_KEY);
-		await this.memento.update("url", undefined);
+		await this.mementoManager.clearLegacyUrl();
 
 		const safeHostname = toSafeHost(legacyUrl);
 		const existing = await this.getSessionAuth(safeHostname);

src/extension.ts

@@ -166,7 +166,7 @@ async function doActivate(
 	const announcementManager = new AnnouncementManager(
 		client,
 		deploymentManager.session,
-		secretsManager,
+		mementoManager,
 		output,
 	);
 	ctx.subscriptions.push(announcementManager);