From eee5e15c5dc6a9fc8334efa4a68843ef7f1180c7 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 30 Jun 2026 10:27:44 +0000 Subject: [PATCH 01/10] feat: show announcement banners --- package.json | 10 + src/announcements/announcementBanners.ts | 324 ++++++++++++++++++ src/core/commandManager.ts | 1 + src/core/secretsManager.ts | 33 ++ src/extension.ts | 13 + .../announcements/announcementBanners.test.ts | 290 ++++++++++++++++ test/unit/core/secretsManager.test.ts | 28 ++ 7 files changed, 699 insertions(+) create mode 100644 src/announcements/announcementBanners.ts create mode 100644 test/unit/announcements/announcementBanners.test.ts diff --git a/package.json b/package.json index 87757a4f5c..d8c4eb4ef1 100644 --- a/package.json +++ b/package.json @@ -460,6 +460,12 @@ "title": "Coder: Export Telemetry", "icon": "$(save)" }, + { + "command": "coder.viewAnnouncements", + "title": "View Announcements", + "category": "Coder", + "icon": "$(megaphone)" + }, { "command": "coder.openAppStatus", "title": "Open App Status", @@ -588,6 +594,10 @@ "command": "coder.exportTelemetry", "when": "true" }, + { + "command": "coder.viewAnnouncements", + "when": "coder.authenticated" + }, { "command": "coder.openAppStatus", "when": "false" diff --git a/src/announcements/announcementBanners.ts b/src/announcements/announcementBanners.ts new file mode 100644 index 0000000000..42dbf15f6d --- /dev/null +++ b/src/announcements/announcementBanners.ts @@ -0,0 +1,324 @@ +import { createHash } from "node:crypto"; +import * as vscode from "vscode"; + +import { type CoderApi } from "../api/coderApi"; +import { type SecretsManager } from "../core/secretsManager"; +import { + type SessionData, + type SessionState, +} from "../deployment/sessionStore"; +import { type Logger } from "../logging/logger"; +import { areNotificationsDisabled } from "../settings/notifications"; + +import type { + AppearanceConfig, + BannerConfig, +} from "coder/site/src/api/typesGenerated"; + +const REFRESH_INTERVAL_MS = 30 * 60 * 1000; +const VIEW_ACTION = "View"; +const POPUP_MESSAGE_MAX_LENGTH = 120; + +export type AnnouncementBannerSource = "announcement" | "service"; + +export interface ActiveAnnouncementBanner { + readonly source: AnnouncementBannerSource; + readonly message: string; + readonly backgroundColor?: string; + readonly key: string; +} + +interface RefreshOptions { + readonly notify: boolean; + readonly showErrors?: boolean; +} + +interface BannerFingerprintInput { + readonly source: AnnouncementBannerSource; + readonly message: string; + readonly backgroundColor?: string; +} + +export class AnnouncementBannerManager implements vscode.Disposable { + private readonly statusBarItem: vscode.StatusBarItem; + private readonly sessionChangeDisposable: vscode.Disposable; + private activeBanners: readonly ActiveAnnouncementBanner[] = []; + private refreshTimeout: NodeJS.Timeout | undefined; + private disposed = false; + + public constructor( + private readonly client: CoderApi, + private readonly sessionState: SessionState, + private readonly secretsManager: SecretsManager, + private readonly logger: Logger, + ) { + this.statusBarItem = vscode.window.createStatusBarItem( + vscode.StatusBarAlignment.Left, + 998, + ); + this.statusBarItem.name = "Coder Announcements"; + this.statusBarItem.command = "coder.viewAnnouncements"; + this.sessionChangeDisposable = this.sessionState.onDidChange(() => { + this.handleSessionChange(); + }); + this.handleSessionChange(); + } + + public dispose(): void { + this.disposed = true; + this.cancelRefresh(); + this.sessionChangeDisposable.dispose(); + this.statusBarItem.dispose(); + } + + public refresh( + options: RefreshOptions = { notify: true }, + ): Promise { + if (this.disposed) { + return Promise.resolve(undefined); + } + + this.cancelRefresh(); + return this.runRefresh(options) + .catch((error: unknown) => { + this.logger.warn("Failed to refresh Coder announcements", error); + if (options.showErrors) { + void vscode.window.showErrorMessage( + `Failed to refresh Coder announcements: ${formatError(error)}`, + ); + } + return undefined; + }) + .finally(() => { + this.scheduleRefresh(); + }); + } + + public async showAnnouncements(): Promise { + const banners = + (await this.refresh({ notify: false, showErrors: true })) ?? + this.activeBanners; + + if (banners.length === 0) { + void vscode.window.showInformationMessage( + "No active Coder announcements.", + ); + return; + } + + const selected = await vscode.window.showQuickPick( + banners.map((banner, index) => ({ + label: `${sourceIcon(banner.source)} ${sourceLabel(banner.source)} ${index + 1}`, + detail: banner.message, + description: banner.backgroundColor, + banner, + })), + { + title: "Coder Announcements", + placeHolder: "Select an announcement to view the full message", + }, + ); + + if (selected) { + void vscode.window.showInformationMessage(selected.banner.message); + } + } + + private handleSessionChange(): void { + this.cancelRefresh(); + if (this.sessionState.current.kind !== "signedIn") { + this.setActiveBanners([]); + return; + } + + this.setActiveBanners([]); + void this.refresh({ notify: true }); + } + + private async runRefresh( + options: RefreshOptions, + ): Promise { + const session = this.sessionState.current; + if (session.kind !== "signedIn") { + this.setActiveBanners([]); + return []; + } + + const appearance = await this.client.getAppearance(); + if (this.sessionChangedSince(session)) { + return undefined; + } + + const banners = normalizeAnnouncementBanners(appearance); + this.setActiveBanners(banners); + + const seen = new Set( + this.secretsManager.getSeenBanners(session.deployment.safeHostname), + ); + const newBanners = banners.filter((banner) => !seen.has(banner.key)); + + const cfg = vscode.workspace.getConfiguration(); + if ( + options.notify && + newBanners.length > 0 && + !areNotificationsDisabled(cfg) + ) { + this.showPopup(newBanners); + } + + await this.secretsManager.setSeenBanners( + session.deployment.safeHostname, + banners.map((banner) => banner.key), + ); + + return banners; + } + + private sessionChangedSince(session: SessionData): boolean { + return this.disposed || this.sessionState.current !== session; + } + + private setActiveBanners(banners: readonly ActiveAnnouncementBanner[]): void { + this.activeBanners = banners; + if (banners.length === 0) { + this.statusBarItem.hide(); + return; + } + + this.statusBarItem.text = formatStatusBarText(banners.length); + this.statusBarItem.tooltip = formatStatusBarTooltip(banners); + this.statusBarItem.show(); + } + + private showPopup(banners: readonly ActiveAnnouncementBanner[]): void { + const message = formatPopupMessage(banners); + void Promise.resolve( + vscode.window.showInformationMessage(message, VIEW_ACTION), + ) + .then((action) => { + if (action === VIEW_ACTION) { + void this.showAnnouncements(); + } + }) + .catch((error: unknown) => { + this.logger.warn("Failed to show Coder announcement popup", error); + }); + } + + private scheduleRefresh(): void { + if ( + this.disposed || + this.refreshTimeout || + this.sessionState.current.kind !== "signedIn" + ) { + return; + } + + this.refreshTimeout = setTimeout(() => { + this.refreshTimeout = undefined; + void this.refresh({ notify: true }); + }, REFRESH_INTERVAL_MS); + } + + private cancelRefresh(): void { + if (this.refreshTimeout) { + clearTimeout(this.refreshTimeout); + this.refreshTimeout = undefined; + } + } +} + +export function normalizeAnnouncementBanners( + appearance: AppearanceConfig, +): readonly ActiveAnnouncementBanner[] { + const banners: ActiveAnnouncementBanner[] = []; + const serviceBanner = toActiveBanner("service", appearance.service_banner); + if (serviceBanner) { + banners.push(serviceBanner); + } + + for (const banner of appearance.announcement_banners) { + const activeBanner = toActiveBanner("announcement", banner); + if (activeBanner) { + banners.push(activeBanner); + } + } + + return banners; +} + +function toActiveBanner( + source: AnnouncementBannerSource, + banner: BannerConfig, +): ActiveAnnouncementBanner | undefined { + const message = banner.message?.trim(); + if (!banner.enabled || !message) { + return undefined; + } + const backgroundColor = banner.background_color?.trim(); + return { + source, + message, + backgroundColor: backgroundColor || undefined, + key: getBannerKey({ source, message, backgroundColor }), + }; +} + +export function getBannerKey(input: BannerFingerprintInput): string { + return createHash("sha256") + .update( + JSON.stringify({ + source: input.source, + message: input.message, + backgroundColor: input.backgroundColor ?? "", + }), + ) + .digest("hex") + .slice(0, 16); +} + +function formatStatusBarText(count: number): string { + return count === 1 ? "$(megaphone) Coder" : `$(megaphone) Coder ${count}`; +} + +function formatStatusBarTooltip( + banners: readonly ActiveAnnouncementBanner[], +): string { + const heading = + banners.length === 1 + ? "Coder deployment announcement" + : "Coder deployment announcements"; + return [ + heading, + "", + ...banners.map((banner, index) => `${index + 1}. ${banner.message}`), + ].join("\n"); +} + +function formatPopupMessage( + banners: readonly ActiveAnnouncementBanner[], +): string { + if (banners.length === 1) { + return `Coder announcement: ${truncateMessage(banners[0].message)}`; + } + return `Coder has ${banners.length} new deployment announcements.`; +} + +function truncateMessage(message: string): string { + if (message.length <= POPUP_MESSAGE_MAX_LENGTH) { + return message; + } + return `${message.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1)}…`; +} + +function sourceIcon(source: AnnouncementBannerSource): string { + return source === "service" ? "$(info)" : "$(megaphone)"; +} + +function sourceLabel(source: AnnouncementBannerSource): string { + return source === "service" ? "Service banner" : "Announcement"; +} + +function formatError(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/core/commandManager.ts b/src/core/commandManager.ts index 723a4fe21b..c9f721c3cd 100644 --- a/src/core/commandManager.ts +++ b/src/core/commandManager.ts @@ -21,6 +21,7 @@ export const CODER_COMMAND_IDS = [ "coder.refreshWorkspaces", "coder.viewLogs", "coder.exportTelemetry", + "coder.viewAnnouncements", "coder.searchMyWorkspaces", "coder.searchSharedWorkspaces", "coder.searchAllWorkspaces", diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index 5255c122c8..a8004abd12 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -17,6 +17,7 @@ const DEPLOYMENT_ACCESS_PREFIX = "coder.access."; type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX; const CURRENT_DEPLOYMENT_KEY = "coder.currentDeployment"; +const SEEN_BANNERS_KEY = "seenBanners"; const DEFAULT_MAX_DEPLOYMENTS = 10; const LEGACY_SESSION_TOKEN_KEY = "sessionToken"; @@ -29,6 +30,9 @@ export type CurrentDeploymentState = z.infer< typeof CurrentDeploymentStateSchema >; +const SeenBannersSchema = z.record(z.string(), z.array(z.string())); +type SeenBanners = z.infer; + /** * OAuth token data stored alongside session auth. * When present, indicates the session is authenticated via OAuth. @@ -235,6 +239,7 @@ export class SecretsManager { await Promise.all([ this.clearSessionAuth(safeHostname), this.clearOAuthClientRegistration(safeHostname), + this.clearSeenBanners(safeHostname), this.memento.update( `${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`, undefined, @@ -242,6 +247,34 @@ export class SecretsManager { ]); } + public getSeenBanners(safeHostname: string): string[] { + return this.getSeenBannersState()[safeHostname] ?? []; + } + + public async setSeenBanners( + safeHostname: string, + bannerKeys: readonly string[], + ): Promise { + const seenBanners = this.getSeenBannersState(); + seenBanners[safeHostname] = [...bannerKeys]; + await this.memento.update(SEEN_BANNERS_KEY, seenBanners); + } + + private async clearSeenBanners(safeHostname: string): Promise { + const seenBanners = this.getSeenBannersState(); + delete seenBanners[safeHostname]; + await this.memento.update( + SEEN_BANNERS_KEY, + Object.keys(seenBanners).length > 0 ? seenBanners : undefined, + ); + } + + private getSeenBannersState(): SeenBanners { + const raw = this.memento.get(SEEN_BANNERS_KEY); + const result = SeenBannersSchema.safeParse(raw); + return result.success ? result.data : {}; + } + /** * Get all known hostnames, ordered by most recently accessed. * Derives the list from actual session secrets stored. diff --git a/src/extension.ts b/src/extension.ts index e097daa876..8417a71e77 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -6,6 +6,7 @@ import { createRequire } from "node:module"; import * as path from "node:path"; import * as vscode from "vscode"; +import { AnnouncementBannerManager } from "./announcements/announcementBanners"; import { errToStr } from "./api/api-helper"; import { AuthInterceptor } from "./api/authInterceptor"; import { CoderApi } from "./api/coderApi"; @@ -162,6 +163,14 @@ async function doActivate( ); ctx.subscriptions.push(deploymentManager); + const announcementBannerManager = new AnnouncementBannerManager( + client, + deploymentManager.session, + secretsManager, + output, + ); + ctx.subscriptions.push(announcementBannerManager); + const myWorkspacesProvider = new WorkspaceProvider( WorkspaceQuery.Mine, client, @@ -328,6 +337,10 @@ async function doActivate( "coder.exportTelemetry", commands.exportTelemetry.bind(commands), ); + commandManager.register( + "coder.viewAnnouncements", + announcementBannerManager.showAnnouncements.bind(announcementBannerManager), + ); commandManager.register("coder.searchMyWorkspaces", async () => showTreeViewSearch(MY_WORKSPACES_TREE_ID), ); diff --git a/test/unit/announcements/announcementBanners.test.ts b/test/unit/announcements/announcementBanners.test.ts new file mode 100644 index 0000000000..1d79086f50 --- /dev/null +++ b/test/unit/announcements/announcementBanners.test.ts @@ -0,0 +1,290 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; + +import { + AnnouncementBannerManager, + getBannerKey, + normalizeAnnouncementBanners, +} from "@/announcements/announcementBanners"; +import { SecretsManager } from "@/core/secretsManager"; +import { SessionStore } from "@/deployment/sessionStore"; + +import { + createMockLogger, + createMockUser, + flushPromises, + InMemoryMemento, + InMemorySecretStorage, + MockConfigurationProvider, + MockStatusBarItem, +} from "../../mocks/testHelpers"; + +import type { + AppearanceConfig, + BannerConfig, +} from "coder/site/src/api/typesGenerated"; + +import type { CoderApi } from "@/api/coderApi"; + +const DEPLOYMENT = { + url: "https://coder.example.com", + safeHostname: "coder.example.com", +}; + +function banner(overrides: Partial = {}): BannerConfig { + return { + enabled: true, + message: "Maintenance tonight", + background_color: "#004852", + ...overrides, + }; +} + +function appearance( + overrides: Partial = {}, +): AppearanceConfig { + return { + application_name: "Coder", + logo_url: "", + docs_url: "", + service_banner: { enabled: false }, + announcement_banners: [], + ...overrides, + }; +} + +class MockAppearanceClient { + readonly getAppearance = vi.fn<() => Promise>(); +} + +function setup() { + const config = new MockConfigurationProvider(); + const client = new MockAppearanceClient(); + client.getAppearance.mockResolvedValue(appearance()); + const session = new SessionStore(); + const secretsManager = new SecretsManager( + new InMemorySecretStorage(), + new InMemoryMemento(), + createMockLogger(), + ); + const statusBar = new MockStatusBarItem(); + const logger = createMockLogger(); + const manager = new AnnouncementBannerManager( + client as unknown as CoderApi, + session, + secretsManager, + logger, + ); + return { + config, + client, + logger, + manager, + secretsManager, + session, + statusBar, + }; +} + +async function signIn(session: SessionStore): Promise { + session.signIn(DEPLOYMENT, createMockUser()); + await flushPromises(); +} + +describe("normalizeAnnouncementBanners", () => { + it("returns active service and announcement banners", () => { + const banners = normalizeAnnouncementBanners( + appearance({ + service_banner: banner({ message: "Service banner" }), + announcement_banners: [ + banner({ message: "Announcement" }), + banner({ enabled: false, message: "Disabled" }), + banner({ message: " " }), + ], + }), + ); + + expect(banners.map((b) => [b.source, b.message])).toEqual([ + ["service", "Service banner"], + ["announcement", "Announcement"], + ]); + }); + + it("keys ignore order but change when content changes", () => { + const key = getBannerKey({ + source: "announcement", + message: "Maintenance tonight", + backgroundColor: "#004852", + }); + + expect( + getBannerKey({ + source: "announcement", + message: "Maintenance tonight", + backgroundColor: "#004852", + }), + ).toBe(key); + expect( + getBannerKey({ + source: "announcement", + message: "Maintenance tomorrow", + backgroundColor: "#004852", + }), + ).not.toBe(key); + }); +}); + +describe("AnnouncementBannerManager", () => { + beforeEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); + }); + + it("shows all active banners in the status bar", async () => { + const { client, session, statusBar } = setup(); + client.getAppearance.mockResolvedValueOnce( + appearance({ + announcement_banners: [ + banner({ message: "First" }), + banner({ message: "Second" }), + ], + }), + ); + + await signIn(session); + + expect(statusBar.text).toBe("$(megaphone) Coder 2"); + expect(statusBar.tooltip).toContain("1. First"); + expect(statusBar.tooltip).toContain("2. Second"); + expect(statusBar.show).toHaveBeenCalled(); + }); + + it("notifies only newly seen banners", async () => { + const { client, manager, secretsManager, session } = setup(); + client.getAppearance.mockResolvedValueOnce( + appearance({ + announcement_banners: [ + banner({ message: "First" }), + banner({ message: "Second" }), + ], + }), + ); + await signIn(session); + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( + "Coder has 2 new deployment announcements.", + "View", + ); + vi.mocked(vscode.window.showInformationMessage).mockClear(); + + client.getAppearance.mockResolvedValueOnce( + appearance({ + announcement_banners: [ + banner({ message: "First" }), + banner({ message: "Second" }), + banner({ message: "Third" }), + ], + }), + ); + await manager.refresh({ notify: true }); + + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( + "Coder announcement: Third", + "View", + ); + expect(secretsManager.getSeenBanners(DEPLOYMENT.safeHostname)).toHaveLength( + 3, + ); + }); + + it("does not notify for banners already seen on the same deployment", async () => { + const { client, manager, session } = setup(); + client.getAppearance.mockResolvedValue( + appearance({ announcement_banners: [banner()] }), + ); + await signIn(session); + vi.mocked(vscode.window.showInformationMessage).mockClear(); + + await manager.refresh({ notify: true }); + + expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); + }); + + it("suppresses popups when notifications are disabled but keeps status bar", async () => { + const { client, config, session, statusBar } = setup(); + config.set("coder.disableNotifications", true); + client.getAppearance.mockResolvedValueOnce( + appearance({ announcement_banners: [banner()] }), + ); + + await signIn(session); + + expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); + expect(statusBar.show).toHaveBeenCalled(); + expect(statusBar.text).toBe("$(megaphone) Coder"); + }); + + it("shows newly seen banners on a different deployment", async () => { + const { client, session } = setup(); + client.getAppearance.mockResolvedValue( + appearance({ announcement_banners: [banner()] }), + ); + + session.signIn(DEPLOYMENT, createMockUser()); + await Promise.resolve(); + vi.mocked(vscode.window.showInformationMessage).mockClear(); + + session.signIn( + { url: "https://other.example.com", safeHostname: "other.example.com" }, + createMockUser(), + ); + await flushPromises(); + + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( + "Coder announcement: Maintenance tonight", + "View", + ); + }); + + it("refreshes before showing announcements from the command", async () => { + const { client, manager, session } = setup(); + client.getAppearance.mockResolvedValue( + appearance({ + announcement_banners: [banner({ message: "Full details" })], + }), + ); + vi.mocked(vscode.window.showQuickPick).mockResolvedValueOnce({ + label: "$(megaphone) Announcement 1", + detail: "Full details", + banner: { + source: "announcement", + message: "Full details", + backgroundColor: "#004852", + key: "key", + }, + } as never); + await signIn(session); + vi.mocked(vscode.window.showInformationMessage).mockClear(); + + await manager.showAnnouncements(); + + expect(vscode.window.showQuickPick).toHaveBeenCalledWith( + [expect.objectContaining({ detail: "Full details" })], + expect.objectContaining({ title: "Coder Announcements" }), + ); + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( + "Full details", + ); + }); + + it("clears status bar when signed out", async () => { + const { client, session, statusBar } = setup(); + client.getAppearance.mockResolvedValueOnce( + appearance({ announcement_banners: [banner()] }), + ); + await signIn(session); + + session.signOut(null); + + expect(statusBar.hide).toHaveBeenCalled(); + }); +}); diff --git a/test/unit/core/secretsManager.test.ts b/test/unit/core/secretsManager.test.ts index 9997b89519..3cee2dac0e 100644 --- a/test/unit/core/secretsManager.test.ts +++ b/test/unit/core/secretsManager.test.ts @@ -170,6 +170,34 @@ describe("SecretsManager", () => { vi.useRealTimers(); }); + describe("seen banners", () => { + it("stores seen banner keys by safe hostname", async () => { + await secretsManager.setSeenBanners("example.com", ["one", "two"]); + await secretsManager.setSeenBanners("other.com", ["three"]); + + expect(secretsManager.getSeenBanners("example.com")).toEqual([ + "one", + "two", + ]); + expect(secretsManager.getSeenBanners("other.com")).toEqual(["three"]); + }); + + it("clears seen banner keys with auth data", async () => { + await secretsManager.setSeenBanners("example.com", ["one"]); + await secretsManager.setSeenBanners("other.com", ["two"]); + + await secretsManager.clearAllAuthData("example.com"); + + expect(secretsManager.getSeenBanners("example.com")).toEqual([]); + expect(secretsManager.getSeenBanners("other.com")).toEqual(["two"]); + }); + + it("ignores corrupted seen banner storage", async () => { + await memento.update("seenBanners", { "example.com": "bad" }); + + expect(secretsManager.getSeenBanners("example.com")).toEqual([]); + }); + }); }); describe("current deployment", () => { From 94c73559af4da46abcd360a3bae54a98aea7645f Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 30 Jun 2026 10:45:52 +0000 Subject: [PATCH 02/10] refactor: simplify announcement banner modules --- src/announcements/announcementBanners.ts | 324 ------------------ src/announcements/banners.ts | 94 +++++ src/announcements/manager.ts | 205 +++++++++++ src/extension.ts | 8 +- test/unit/announcements/banners.test.ts | 158 +++++++++ ...ncementBanners.test.ts => manager.test.ts} | 232 ++++++------- 6 files changed, 572 insertions(+), 449 deletions(-) delete mode 100644 src/announcements/announcementBanners.ts create mode 100644 src/announcements/banners.ts create mode 100644 src/announcements/manager.ts create mode 100644 test/unit/announcements/banners.test.ts rename test/unit/announcements/{announcementBanners.test.ts => manager.test.ts} (55%) diff --git a/src/announcements/announcementBanners.ts b/src/announcements/announcementBanners.ts deleted file mode 100644 index 42dbf15f6d..0000000000 --- a/src/announcements/announcementBanners.ts +++ /dev/null @@ -1,324 +0,0 @@ -import { createHash } from "node:crypto"; -import * as vscode from "vscode"; - -import { type CoderApi } from "../api/coderApi"; -import { type SecretsManager } from "../core/secretsManager"; -import { - type SessionData, - type SessionState, -} from "../deployment/sessionStore"; -import { type Logger } from "../logging/logger"; -import { areNotificationsDisabled } from "../settings/notifications"; - -import type { - AppearanceConfig, - BannerConfig, -} from "coder/site/src/api/typesGenerated"; - -const REFRESH_INTERVAL_MS = 30 * 60 * 1000; -const VIEW_ACTION = "View"; -const POPUP_MESSAGE_MAX_LENGTH = 120; - -export type AnnouncementBannerSource = "announcement" | "service"; - -export interface ActiveAnnouncementBanner { - readonly source: AnnouncementBannerSource; - readonly message: string; - readonly backgroundColor?: string; - readonly key: string; -} - -interface RefreshOptions { - readonly notify: boolean; - readonly showErrors?: boolean; -} - -interface BannerFingerprintInput { - readonly source: AnnouncementBannerSource; - readonly message: string; - readonly backgroundColor?: string; -} - -export class AnnouncementBannerManager implements vscode.Disposable { - private readonly statusBarItem: vscode.StatusBarItem; - private readonly sessionChangeDisposable: vscode.Disposable; - private activeBanners: readonly ActiveAnnouncementBanner[] = []; - private refreshTimeout: NodeJS.Timeout | undefined; - private disposed = false; - - public constructor( - private readonly client: CoderApi, - private readonly sessionState: SessionState, - private readonly secretsManager: SecretsManager, - private readonly logger: Logger, - ) { - this.statusBarItem = vscode.window.createStatusBarItem( - vscode.StatusBarAlignment.Left, - 998, - ); - this.statusBarItem.name = "Coder Announcements"; - this.statusBarItem.command = "coder.viewAnnouncements"; - this.sessionChangeDisposable = this.sessionState.onDidChange(() => { - this.handleSessionChange(); - }); - this.handleSessionChange(); - } - - public dispose(): void { - this.disposed = true; - this.cancelRefresh(); - this.sessionChangeDisposable.dispose(); - this.statusBarItem.dispose(); - } - - public refresh( - options: RefreshOptions = { notify: true }, - ): Promise { - if (this.disposed) { - return Promise.resolve(undefined); - } - - this.cancelRefresh(); - return this.runRefresh(options) - .catch((error: unknown) => { - this.logger.warn("Failed to refresh Coder announcements", error); - if (options.showErrors) { - void vscode.window.showErrorMessage( - `Failed to refresh Coder announcements: ${formatError(error)}`, - ); - } - return undefined; - }) - .finally(() => { - this.scheduleRefresh(); - }); - } - - public async showAnnouncements(): Promise { - const banners = - (await this.refresh({ notify: false, showErrors: true })) ?? - this.activeBanners; - - if (banners.length === 0) { - void vscode.window.showInformationMessage( - "No active Coder announcements.", - ); - return; - } - - const selected = await vscode.window.showQuickPick( - banners.map((banner, index) => ({ - label: `${sourceIcon(banner.source)} ${sourceLabel(banner.source)} ${index + 1}`, - detail: banner.message, - description: banner.backgroundColor, - banner, - })), - { - title: "Coder Announcements", - placeHolder: "Select an announcement to view the full message", - }, - ); - - if (selected) { - void vscode.window.showInformationMessage(selected.banner.message); - } - } - - private handleSessionChange(): void { - this.cancelRefresh(); - if (this.sessionState.current.kind !== "signedIn") { - this.setActiveBanners([]); - return; - } - - this.setActiveBanners([]); - void this.refresh({ notify: true }); - } - - private async runRefresh( - options: RefreshOptions, - ): Promise { - const session = this.sessionState.current; - if (session.kind !== "signedIn") { - this.setActiveBanners([]); - return []; - } - - const appearance = await this.client.getAppearance(); - if (this.sessionChangedSince(session)) { - return undefined; - } - - const banners = normalizeAnnouncementBanners(appearance); - this.setActiveBanners(banners); - - const seen = new Set( - this.secretsManager.getSeenBanners(session.deployment.safeHostname), - ); - const newBanners = banners.filter((banner) => !seen.has(banner.key)); - - const cfg = vscode.workspace.getConfiguration(); - if ( - options.notify && - newBanners.length > 0 && - !areNotificationsDisabled(cfg) - ) { - this.showPopup(newBanners); - } - - await this.secretsManager.setSeenBanners( - session.deployment.safeHostname, - banners.map((banner) => banner.key), - ); - - return banners; - } - - private sessionChangedSince(session: SessionData): boolean { - return this.disposed || this.sessionState.current !== session; - } - - private setActiveBanners(banners: readonly ActiveAnnouncementBanner[]): void { - this.activeBanners = banners; - if (banners.length === 0) { - this.statusBarItem.hide(); - return; - } - - this.statusBarItem.text = formatStatusBarText(banners.length); - this.statusBarItem.tooltip = formatStatusBarTooltip(banners); - this.statusBarItem.show(); - } - - private showPopup(banners: readonly ActiveAnnouncementBanner[]): void { - const message = formatPopupMessage(banners); - void Promise.resolve( - vscode.window.showInformationMessage(message, VIEW_ACTION), - ) - .then((action) => { - if (action === VIEW_ACTION) { - void this.showAnnouncements(); - } - }) - .catch((error: unknown) => { - this.logger.warn("Failed to show Coder announcement popup", error); - }); - } - - private scheduleRefresh(): void { - if ( - this.disposed || - this.refreshTimeout || - this.sessionState.current.kind !== "signedIn" - ) { - return; - } - - this.refreshTimeout = setTimeout(() => { - this.refreshTimeout = undefined; - void this.refresh({ notify: true }); - }, REFRESH_INTERVAL_MS); - } - - private cancelRefresh(): void { - if (this.refreshTimeout) { - clearTimeout(this.refreshTimeout); - this.refreshTimeout = undefined; - } - } -} - -export function normalizeAnnouncementBanners( - appearance: AppearanceConfig, -): readonly ActiveAnnouncementBanner[] { - const banners: ActiveAnnouncementBanner[] = []; - const serviceBanner = toActiveBanner("service", appearance.service_banner); - if (serviceBanner) { - banners.push(serviceBanner); - } - - for (const banner of appearance.announcement_banners) { - const activeBanner = toActiveBanner("announcement", banner); - if (activeBanner) { - banners.push(activeBanner); - } - } - - return banners; -} - -function toActiveBanner( - source: AnnouncementBannerSource, - banner: BannerConfig, -): ActiveAnnouncementBanner | undefined { - const message = banner.message?.trim(); - if (!banner.enabled || !message) { - return undefined; - } - const backgroundColor = banner.background_color?.trim(); - return { - source, - message, - backgroundColor: backgroundColor || undefined, - key: getBannerKey({ source, message, backgroundColor }), - }; -} - -export function getBannerKey(input: BannerFingerprintInput): string { - return createHash("sha256") - .update( - JSON.stringify({ - source: input.source, - message: input.message, - backgroundColor: input.backgroundColor ?? "", - }), - ) - .digest("hex") - .slice(0, 16); -} - -function formatStatusBarText(count: number): string { - return count === 1 ? "$(megaphone) Coder" : `$(megaphone) Coder ${count}`; -} - -function formatStatusBarTooltip( - banners: readonly ActiveAnnouncementBanner[], -): string { - const heading = - banners.length === 1 - ? "Coder deployment announcement" - : "Coder deployment announcements"; - return [ - heading, - "", - ...banners.map((banner, index) => `${index + 1}. ${banner.message}`), - ].join("\n"); -} - -function formatPopupMessage( - banners: readonly ActiveAnnouncementBanner[], -): string { - if (banners.length === 1) { - return `Coder announcement: ${truncateMessage(banners[0].message)}`; - } - return `Coder has ${banners.length} new deployment announcements.`; -} - -function truncateMessage(message: string): string { - if (message.length <= POPUP_MESSAGE_MAX_LENGTH) { - return message; - } - return `${message.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1)}…`; -} - -function sourceIcon(source: AnnouncementBannerSource): string { - return source === "service" ? "$(info)" : "$(megaphone)"; -} - -function sourceLabel(source: AnnouncementBannerSource): string { - return source === "service" ? "Service banner" : "Announcement"; -} - -function formatError(error: unknown): string { - return error instanceof Error ? error.message : String(error); -} diff --git a/src/announcements/banners.ts b/src/announcements/banners.ts new file mode 100644 index 0000000000..a46b1bb570 --- /dev/null +++ b/src/announcements/banners.ts @@ -0,0 +1,94 @@ +import { createHash } from "node:crypto"; + +import type { + AppearanceConfig, + BannerConfig, +} from "coder/site/src/api/typesGenerated"; + +const POPUP_MESSAGE_MAX_LENGTH = 120; + +export type AnnouncementSource = "announcement" | "service"; + +export interface Announcement { + readonly source: AnnouncementSource; + readonly message: string; + readonly backgroundColor?: string; + readonly key: string; +} + +export function normalizeBanners( + appearance: AppearanceConfig, +): readonly Announcement[] { + return [ + toAnnouncement("service", appearance.service_banner), + ...appearance.announcement_banners.map((banner) => + toAnnouncement("announcement", banner), + ), + ].filter((banner): banner is Announcement => banner !== undefined); +} + +export function bannerKey( + banner: Pick, +): string { + return createHash("sha256") + .update( + JSON.stringify({ + source: banner.source, + message: banner.message, + backgroundColor: banner.backgroundColor ?? "", + }), + ) + .digest("hex") + .slice(0, 16); +} + +export function statusText(count: number): string { + return count === 1 ? "$(megaphone) Coder" : `$(megaphone) Coder ${count}`; +} + +export function statusTooltip(banners: readonly Announcement[]): string { + return [ + banners.length === 1 + ? "Coder deployment announcement" + : "Coder deployment announcements", + "", + ...banners.map((banner, index) => `${index + 1}. ${banner.message}`), + ].join("\n"); +} + +export function popupMessage(banners: readonly Announcement[]): string { + return banners.length === 1 + ? `Coder announcement: ${truncate(banners[0].message)}` + : `Coder has ${banners.length} new deployment announcements.`; +} + +export function sourceLabel(source: AnnouncementSource): string { + return source === "service" ? "Service banner" : "Announcement"; +} + +export function sourceIcon(source: AnnouncementSource): string { + return source === "service" ? "$(info)" : "$(megaphone)"; +} + +function toAnnouncement( + source: AnnouncementSource, + banner: BannerConfig, +): Announcement | undefined { + const message = banner.message?.trim(); + const backgroundColor = banner.background_color?.trim() || undefined; + if (!banner.enabled || !message) { + return undefined; + } + return { + source, + message, + backgroundColor, + key: bannerKey({ source, message, backgroundColor }), + }; +} + +function truncate(message: string): string { + return message.length <= POPUP_MESSAGE_MAX_LENGTH + ? message + : `${message.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1)}…`; +} diff --git a/src/announcements/manager.ts b/src/announcements/manager.ts new file mode 100644 index 0000000000..bfd9ac0a9f --- /dev/null +++ b/src/announcements/manager.ts @@ -0,0 +1,205 @@ +import * as vscode from "vscode"; + +import { type CoderApi } from "../api/coderApi"; +import { type SecretsManager } from "../core/secretsManager"; +import { + type SessionData, + type SessionState, +} from "../deployment/sessionStore"; +import { type Logger } from "../logging/logger"; +import { areNotificationsDisabled } from "../settings/notifications"; + +import { + type Announcement, + normalizeBanners, + popupMessage, + sourceIcon, + sourceLabel, + statusText, + statusTooltip, +} from "./banners"; + +const REFRESH_INTERVAL_MS = 30 * 60 * 1000; +const VIEW_ACTION = "View"; + +interface RefreshOptions { + readonly notify?: boolean; + readonly showErrors?: boolean; +} + +export class AnnouncementManager implements vscode.Disposable { + private readonly statusBarItem: vscode.StatusBarItem; + private readonly sessionChangeDisposable: vscode.Disposable; + private banners: readonly Announcement[] = []; + private refreshTimeout: NodeJS.Timeout | undefined; + private disposed = false; + + public constructor( + private readonly client: Pick, + private readonly sessionState: SessionState, + private readonly secretsManager: SecretsManager, + private readonly logger: Logger, + ) { + this.statusBarItem = vscode.window.createStatusBarItem( + vscode.StatusBarAlignment.Left, + 998, + ); + this.statusBarItem.name = "Coder Announcements"; + this.statusBarItem.command = "coder.viewAnnouncements"; + this.sessionChangeDisposable = this.sessionState.onDidChange(() => { + this.onSessionChange(); + }); + this.onSessionChange(); + } + + public dispose(): void { + this.disposed = true; + this.cancelRefresh(); + this.sessionChangeDisposable.dispose(); + this.statusBarItem.dispose(); + } + + public async refresh( + options: RefreshOptions = {}, + ): Promise { + if (this.disposed) { + return undefined; + } + this.cancelRefresh(); + try { + return await this.fetch(options); + } catch (error) { + this.logger.warn("Failed to refresh Coder announcements", error); + if (options.showErrors) { + void vscode.window.showErrorMessage( + `Failed to refresh Coder announcements: ${errorMessage(error)}`, + ); + } + return undefined; + } finally { + this.scheduleRefresh(); + } + } + + public async showAnnouncements(): Promise { + const banners = + (await this.refresh({ notify: false, showErrors: true })) ?? this.banners; + if (banners.length === 0) { + void vscode.window.showInformationMessage( + "No active Coder announcements.", + ); + return; + } + + const selected = await vscode.window.showQuickPick( + banners.map((banner, index) => ({ + label: `${sourceIcon(banner.source)} ${sourceLabel(banner.source)} ${index + 1}`, + detail: banner.message, + description: banner.backgroundColor, + banner, + })), + { + title: "Coder Announcements", + placeHolder: "Select an announcement to view the full message", + }, + ); + if (selected) { + void vscode.window.showInformationMessage(selected.banner.message); + } + } + + private onSessionChange(): void { + this.cancelRefresh(); + this.setBanners([]); + if (this.sessionState.current.kind === "signedIn") { + void this.refresh({ notify: true }); + } + } + + private async fetch( + options: RefreshOptions, + ): Promise { + const session = this.sessionState.current; + if (session.kind !== "signedIn") { + this.setBanners([]); + return []; + } + + const banners = normalizeBanners(await this.client.getAppearance()); + if (this.sessionChangedSince(session)) { + return undefined; + } + this.setBanners(banners); + + const seen = new Set( + this.secretsManager.getSeenBanners(session.deployment.safeHostname), + ); + const unseen = banners.filter((banner) => !seen.has(banner.key)); + if (options.notify && unseen.length > 0 && notificationsEnabled()) { + this.showPopup(unseen); + } + await this.secretsManager.setSeenBanners( + session.deployment.safeHostname, + banners.map((banner) => banner.key), + ); + return banners; + } + + private setBanners(banners: readonly Announcement[]): void { + this.banners = banners; + if (banners.length === 0) { + this.statusBarItem.hide(); + return; + } + this.statusBarItem.text = statusText(banners.length); + this.statusBarItem.tooltip = statusTooltip(banners); + this.statusBarItem.show(); + } + + private showPopup(banners: readonly Announcement[]): void { + void Promise.resolve( + vscode.window.showInformationMessage(popupMessage(banners), VIEW_ACTION), + ) + .then((action) => { + if (action === VIEW_ACTION) { + void this.showAnnouncements(); + } + }) + .catch((error: unknown) => { + this.logger.warn("Failed to show Coder announcement popup", error); + }); + } + + private scheduleRefresh(): void { + if ( + this.disposed || + this.refreshTimeout || + this.sessionState.current.kind !== "signedIn" + ) { + return; + } + this.refreshTimeout = setTimeout(() => { + this.refreshTimeout = undefined; + void this.refresh({ notify: true }); + }, REFRESH_INTERVAL_MS); + } + + private cancelRefresh(): void { + if (this.refreshTimeout) { + clearTimeout(this.refreshTimeout); + this.refreshTimeout = undefined; + } + } + + private sessionChangedSince(session: SessionData): boolean { + return this.disposed || this.sessionState.current !== session; + } +} + +function notificationsEnabled(): boolean { + return !areNotificationsDisabled(vscode.workspace.getConfiguration()); +} + +function errorMessage(error: unknown): string { + return error instanceof Error ? error.message : String(error); +} diff --git a/src/extension.ts b/src/extension.ts index 8417a71e77..541331562a 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -6,7 +6,7 @@ import { createRequire } from "node:module"; import * as path from "node:path"; import * as vscode from "vscode"; -import { AnnouncementBannerManager } from "./announcements/announcementBanners"; +import { AnnouncementManager } from "./announcements/manager"; import { errToStr } from "./api/api-helper"; import { AuthInterceptor } from "./api/authInterceptor"; import { CoderApi } from "./api/coderApi"; @@ -163,13 +163,13 @@ async function doActivate( ); ctx.subscriptions.push(deploymentManager); - const announcementBannerManager = new AnnouncementBannerManager( + const announcementManager = new AnnouncementManager( client, deploymentManager.session, secretsManager, output, ); - ctx.subscriptions.push(announcementBannerManager); + ctx.subscriptions.push(announcementManager); const myWorkspacesProvider = new WorkspaceProvider( WorkspaceQuery.Mine, @@ -339,7 +339,7 @@ async function doActivate( ); commandManager.register( "coder.viewAnnouncements", - announcementBannerManager.showAnnouncements.bind(announcementBannerManager), + announcementManager.showAnnouncements.bind(announcementManager), ); commandManager.register("coder.searchMyWorkspaces", async () => showTreeViewSearch(MY_WORKSPACES_TREE_ID), diff --git a/test/unit/announcements/banners.test.ts b/test/unit/announcements/banners.test.ts new file mode 100644 index 0000000000..9edca9420e --- /dev/null +++ b/test/unit/announcements/banners.test.ts @@ -0,0 +1,158 @@ +import { describe, expect, it } from "vitest"; + +import { + bannerKey, + normalizeBanners, + popupMessage, + sourceIcon, + sourceLabel, + statusText, + statusTooltip, +} from "@/announcements/banners"; + +import type { + AppearanceConfig, + BannerConfig, +} from "coder/site/src/api/typesGenerated"; + +function banner(overrides: Partial = {}): BannerConfig { + return { + enabled: true, + message: "Maintenance tonight", + background_color: "#004852", + ...overrides, + }; +} + +function appearance( + overrides: Partial = {}, +): AppearanceConfig { + return { + application_name: "Coder", + logo_url: "", + docs_url: "", + service_banner: { enabled: false }, + announcement_banners: [], + ...overrides, + }; +} + +function announcements(...messages: string[]) { + return normalizeBanners( + appearance({ + announcement_banners: messages.map((message) => banner({ message })), + }), + ); +} + +describe("normalizeBanners", () => { + it("returns active service and announcement banners", () => { + const banners = normalizeBanners( + appearance({ + service_banner: banner({ + message: " Service banner ", + background_color: " #123456 ", + }), + announcement_banners: [ + banner({ message: " Announcement " }), + banner({ enabled: false, message: "Disabled" }), + banner({ message: " " }), + ], + }), + ); + + expect(banners).toMatchObject([ + { + source: "service", + message: "Service banner", + backgroundColor: "#123456", + }, + { + source: "announcement", + message: "Announcement", + backgroundColor: "#004852", + }, + ]); + expect(banners).toHaveLength(2); + expect(banners[0].key).toBe( + bannerKey({ + source: "service", + message: "Service banner", + backgroundColor: "#123456", + }), + ); + }); + + it("keeps keys stable when banners reorder", () => { + const original = announcements("First", "Second"); + const reordered = announcements("Second", "First"); + const keyFor = (message: string, banners = original) => + banners.find((banner) => banner.message === message)?.key; + + expect(keyFor("First", reordered)).toBe(keyFor("First")); + expect(keyFor("Second", reordered)).toBe(keyFor("Second")); + }); + + it("changes keys when fingerprint fields change", () => { + const key = bannerKey({ + source: "announcement", + message: "Maintenance tonight", + backgroundColor: "#004852", + }); + + expect( + bannerKey({ + source: "service", + message: "Maintenance tonight", + backgroundColor: "#004852", + }), + ).not.toBe(key); + expect( + bannerKey({ + source: "announcement", + message: "Maintenance tomorrow", + backgroundColor: "#004852", + }), + ).not.toBe(key); + expect( + bannerKey({ + source: "announcement", + message: "Maintenance tonight", + backgroundColor: "#111111", + }), + ).not.toBe(key); + }); +}); + +describe("banner copy", () => { + it("formats status bar text and tooltip", () => { + const banners = announcements("First", "Second"); + + expect(statusText(1)).toBe("$(megaphone) Coder"); + expect(statusText(2)).toBe("$(megaphone) Coder 2"); + expect(statusTooltip(banners)).toBe( + "Coder deployment announcements\n\n1. First\n2. Second", + ); + }); + + it("formats popup messages", () => { + const longMessage = "a".repeat(121); + + expect(popupMessage(announcements("Maintenance tonight"))).toBe( + "Coder announcement: Maintenance tonight", + ); + expect(popupMessage(announcements("First", "Second"))).toBe( + "Coder has 2 new deployment announcements.", + ); + expect(popupMessage(announcements(longMessage))).toBe( + `Coder announcement: ${"a".repeat(119)}…`, + ); + }); + + it("formats QuickPick source labels", () => { + expect(sourceIcon("service")).toBe("$(info)"); + expect(sourceIcon("announcement")).toBe("$(megaphone)"); + expect(sourceLabel("service")).toBe("Service banner"); + expect(sourceLabel("announcement")).toBe("Announcement"); + }); +}); diff --git a/test/unit/announcements/announcementBanners.test.ts b/test/unit/announcements/manager.test.ts similarity index 55% rename from test/unit/announcements/announcementBanners.test.ts rename to test/unit/announcements/manager.test.ts index 1d79086f50..dbdbb853f8 100644 --- a/test/unit/announcements/announcementBanners.test.ts +++ b/test/unit/announcements/manager.test.ts @@ -1,11 +1,7 @@ -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import * as vscode from "vscode"; -import { - AnnouncementBannerManager, - getBannerKey, - normalizeAnnouncementBanners, -} from "@/announcements/announcementBanners"; +import { AnnouncementManager } from "@/announcements/manager"; import { SecretsManager } from "@/core/secretsManager"; import { SessionStore } from "@/deployment/sessionStore"; @@ -24,13 +20,17 @@ import type { BannerConfig, } from "coder/site/src/api/typesGenerated"; -import type { CoderApi } from "@/api/coderApi"; - const DEPLOYMENT = { url: "https://coder.example.com", safeHostname: "coder.example.com", }; +class MockAppearanceClient { + readonly getAppearance = vi.fn<() => Promise>(); +} + +const managers: AnnouncementManager[] = []; + function banner(overrides: Partial = {}): BannerConfig { return { enabled: true, @@ -40,23 +40,16 @@ function banner(overrides: Partial = {}): BannerConfig { }; } -function appearance( - overrides: Partial = {}, -): AppearanceConfig { +function appearance(messages: readonly string[] = []): AppearanceConfig { return { application_name: "Coder", logo_url: "", docs_url: "", service_banner: { enabled: false }, - announcement_banners: [], - ...overrides, + announcement_banners: messages.map((message) => banner({ message })), }; } -class MockAppearanceClient { - readonly getAppearance = vi.fn<() => Promise>(); -} - function setup() { const config = new MockConfigurationProvider(); const client = new MockAppearanceClient(); @@ -69,15 +62,16 @@ function setup() { ); const statusBar = new MockStatusBarItem(); const logger = createMockLogger(); - const manager = new AnnouncementBannerManager( - client as unknown as CoderApi, + const manager = new AnnouncementManager( + client, session, secretsManager, logger, ); + managers.push(manager); return { - config, client, + config, logger, manager, secretsManager, @@ -91,65 +85,35 @@ async function signIn(session: SessionStore): Promise { await flushPromises(); } -describe("normalizeAnnouncementBanners", () => { - it("returns active service and announcement banners", () => { - const banners = normalizeAnnouncementBanners( - appearance({ - service_banner: banner({ message: "Service banner" }), - announcement_banners: [ - banner({ message: "Announcement" }), - banner({ enabled: false, message: "Disabled" }), - banner({ message: " " }), - ], - }), - ); - - expect(banners.map((b) => [b.source, b.message])).toEqual([ - ["service", "Service banner"], - ["announcement", "Announcement"], - ]); - }); - - it("keys ignore order but change when content changes", () => { - const key = getBannerKey({ - source: "announcement", - message: "Maintenance tonight", - backgroundColor: "#004852", - }); +function nextAppearance( + client: MockAppearanceClient, + messages: readonly string[], +): void { + client.getAppearance.mockResolvedValueOnce(appearance(messages)); +} - expect( - getBannerKey({ - source: "announcement", - message: "Maintenance tonight", - backgroundColor: "#004852", - }), - ).toBe(key); - expect( - getBannerKey({ - source: "announcement", - message: "Maintenance tomorrow", - backgroundColor: "#004852", - }), - ).not.toBe(key); - }); -}); +function expectInfo(message: string, ...items: string[]): void { + expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( + message, + ...items, + ); +} -describe("AnnouncementBannerManager", () => { +describe("AnnouncementManager", () => { beforeEach(() => { vi.useRealTimers(); vi.clearAllMocks(); }); + afterEach(() => { + while (managers.length > 0) { + managers.pop()?.dispose(); + } + }); + it("shows all active banners in the status bar", async () => { const { client, session, statusBar } = setup(); - client.getAppearance.mockResolvedValueOnce( - appearance({ - announcement_banners: [ - banner({ message: "First" }), - banner({ message: "Second" }), - ], - }), - ); + nextAppearance(client, ["First", "Second"]); await signIn(session); @@ -161,36 +125,15 @@ describe("AnnouncementBannerManager", () => { it("notifies only newly seen banners", async () => { const { client, manager, secretsManager, session } = setup(); - client.getAppearance.mockResolvedValueOnce( - appearance({ - announcement_banners: [ - banner({ message: "First" }), - banner({ message: "Second" }), - ], - }), - ); + nextAppearance(client, ["First", "Second"]); await signIn(session); - expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( - "Coder has 2 new deployment announcements.", - "View", - ); + expectInfo("Coder has 2 new deployment announcements.", "View"); vi.mocked(vscode.window.showInformationMessage).mockClear(); - client.getAppearance.mockResolvedValueOnce( - appearance({ - announcement_banners: [ - banner({ message: "First" }), - banner({ message: "Second" }), - banner({ message: "Third" }), - ], - }), - ); + nextAppearance(client, ["First", "Second", "Third"]); await manager.refresh({ notify: true }); - expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( - "Coder announcement: Third", - "View", - ); + expectInfo("Coder announcement: Third", "View"); expect(secretsManager.getSeenBanners(DEPLOYMENT.safeHostname)).toHaveLength( 3, ); @@ -198,9 +141,7 @@ describe("AnnouncementBannerManager", () => { it("does not notify for banners already seen on the same deployment", async () => { const { client, manager, session } = setup(); - client.getAppearance.mockResolvedValue( - appearance({ announcement_banners: [banner()] }), - ); + client.getAppearance.mockResolvedValue(appearance(["Maintenance tonight"])); await signIn(session); vi.mocked(vscode.window.showInformationMessage).mockClear(); @@ -212,9 +153,7 @@ describe("AnnouncementBannerManager", () => { it("suppresses popups when notifications are disabled but keeps status bar", async () => { const { client, config, session, statusBar } = setup(); config.set("coder.disableNotifications", true); - client.getAppearance.mockResolvedValueOnce( - appearance({ announcement_banners: [banner()] }), - ); + nextAppearance(client, ["Maintenance tonight"]); await signIn(session); @@ -225,12 +164,9 @@ describe("AnnouncementBannerManager", () => { it("shows newly seen banners on a different deployment", async () => { const { client, session } = setup(); - client.getAppearance.mockResolvedValue( - appearance({ announcement_banners: [banner()] }), - ); - + client.getAppearance.mockResolvedValue(appearance(["Maintenance tonight"])); session.signIn(DEPLOYMENT, createMockUser()); - await Promise.resolve(); + await flushPromises(); vi.mocked(vscode.window.showInformationMessage).mockClear(); session.signIn( @@ -239,19 +175,12 @@ describe("AnnouncementBannerManager", () => { ); await flushPromises(); - expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( - "Coder announcement: Maintenance tonight", - "View", - ); + expectInfo("Coder announcement: Maintenance tonight", "View"); }); it("refreshes before showing announcements from the command", async () => { const { client, manager, session } = setup(); - client.getAppearance.mockResolvedValue( - appearance({ - announcement_banners: [banner({ message: "Full details" })], - }), - ); + client.getAppearance.mockResolvedValue(appearance(["Full details"])); vi.mocked(vscode.window.showQuickPick).mockResolvedValueOnce({ label: "$(megaphone) Announcement 1", detail: "Full details", @@ -271,20 +200,81 @@ describe("AnnouncementBannerManager", () => { [expect.objectContaining({ detail: "Full details" })], expect.objectContaining({ title: "Coder Announcements" }), ); - expect(vscode.window.showInformationMessage).toHaveBeenCalledWith( - "Full details", + expectInfo("Full details"); + }); + + it("shows an empty message when there are no active announcements", async () => { + const { manager, session } = setup(); + await signIn(session); + vi.mocked(vscode.window.showInformationMessage).mockClear(); + + await manager.showAnnouncements(); + + expectInfo("No active Coder announcements."); + }); + + it("shows refresh errors from the command", async () => { + const { client, logger, manager, session } = setup(); + await signIn(session); + vi.mocked(vscode.window.showInformationMessage).mockClear(); + client.getAppearance.mockRejectedValueOnce(new Error("boom")); + + await manager.showAnnouncements(); + + expect(logger.warn).toHaveBeenCalledWith( + "Failed to refresh Coder announcements", + expect.any(Error), + ); + expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( + "Failed to refresh Coder announcements: boom", ); + expectInfo("No active Coder announcements."); }); - it("clears status bar when signed out", async () => { + it("opens the announcements command from the popup action", async () => { + const { client, manager, session } = setup(); + nextAppearance(client, ["Maintenance tonight"]); + const showAnnouncements = vi + .spyOn(manager, "showAnnouncements") + .mockResolvedValue(); + vi.mocked(vscode.window.showInformationMessage).mockResolvedValueOnce( + "View" as never, + ); + + await signIn(session); + await flushPromises(); + + expect(showAnnouncements).toHaveBeenCalledOnce(); + }); + + it("ignores stale refreshes after the session changes", async () => { const { client, session, statusBar } = setup(); - client.getAppearance.mockResolvedValueOnce( - appearance({ announcement_banners: [banner()] }), + const stale = Promise.withResolvers(); + client.getAppearance + .mockReturnValueOnce(stale.promise) + .mockResolvedValueOnce(appearance(["Current"])); + + session.signIn(DEPLOYMENT, createMockUser()); + await Promise.resolve(); + session.signIn( + { url: "https://other.example.com", safeHostname: "other.example.com" }, + createMockUser(), ); + stale.resolve(appearance(["Stale"])); + await flushPromises(); + + expect(statusBar.tooltip).toContain("Current"); + expect(statusBar.tooltip).not.toContain("Stale"); + }); + + it("clears status bar when signed out", async () => { + const { client, session, statusBar } = setup(); + nextAppearance(client, ["Maintenance tonight"]); await signIn(session); + statusBar.hide.mockClear(); session.signOut(null); - expect(statusBar.hide).toHaveBeenCalled(); + expect(statusBar.hide).toHaveBeenCalledOnce(); }); }); From bcccbb2c3d9b9e0d0509c07754016e3dbe9d7a61 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 30 Jun 2026 10:59:51 +0000 Subject: [PATCH 03/10] refactor: simplify announcement banner code --- src/announcements/banners.ts | 8 ---- src/announcements/manager.ts | 51 ++++++++++--------------- test/unit/announcements/banners.test.ts | 44 +++++++-------------- test/unit/announcements/manager.test.ts | 49 +++++++++++------------- 4 files changed, 58 insertions(+), 94 deletions(-) diff --git a/src/announcements/banners.ts b/src/announcements/banners.ts index a46b1bb570..745f89f1e4 100644 --- a/src/announcements/banners.ts +++ b/src/announcements/banners.ts @@ -62,14 +62,6 @@ export function popupMessage(banners: readonly Announcement[]): string { : `Coder has ${banners.length} new deployment announcements.`; } -export function sourceLabel(source: AnnouncementSource): string { - return source === "service" ? "Service banner" : "Announcement"; -} - -export function sourceIcon(source: AnnouncementSource): string { - return source === "service" ? "$(info)" : "$(megaphone)"; -} - function toAnnouncement( source: AnnouncementSource, banner: BannerConfig, diff --git a/src/announcements/manager.ts b/src/announcements/manager.ts index bfd9ac0a9f..5c81997a08 100644 --- a/src/announcements/manager.ts +++ b/src/announcements/manager.ts @@ -2,10 +2,7 @@ import * as vscode from "vscode"; import { type CoderApi } from "../api/coderApi"; import { type SecretsManager } from "../core/secretsManager"; -import { - type SessionData, - type SessionState, -} from "../deployment/sessionStore"; +import { type SessionState } from "../deployment/sessionStore"; import { type Logger } from "../logging/logger"; import { areNotificationsDisabled } from "../settings/notifications"; @@ -13,8 +10,6 @@ import { type Announcement, normalizeBanners, popupMessage, - sourceIcon, - sourceLabel, statusText, statusTooltip, } from "./banners"; @@ -93,7 +88,7 @@ export class AnnouncementManager implements vscode.Disposable { const selected = await vscode.window.showQuickPick( banners.map((banner, index) => ({ - label: `${sourceIcon(banner.source)} ${sourceLabel(banner.source)} ${index + 1}`, + label: `${banner.source === "service" ? "$(info) Service banner" : "$(megaphone) Announcement"} ${index + 1}`, detail: banner.message, description: banner.backgroundColor, banner, @@ -126,7 +121,7 @@ export class AnnouncementManager implements vscode.Disposable { } const banners = normalizeBanners(await this.client.getAppearance()); - if (this.sessionChangedSince(session)) { + if (this.disposed || this.sessionState.current !== session) { return undefined; } this.setBanners(banners); @@ -135,8 +130,12 @@ export class AnnouncementManager implements vscode.Disposable { this.secretsManager.getSeenBanners(session.deployment.safeHostname), ); const unseen = banners.filter((banner) => !seen.has(banner.key)); - if (options.notify && unseen.length > 0 && notificationsEnabled()) { - this.showPopup(unseen); + if ( + options.notify && + unseen.length > 0 && + !areNotificationsDisabled(vscode.workspace.getConfiguration()) + ) { + void this.showPopup(unseen); } await this.secretsManager.setSeenBanners( session.deployment.safeHostname, @@ -156,18 +155,18 @@ export class AnnouncementManager implements vscode.Disposable { this.statusBarItem.show(); } - private showPopup(banners: readonly Announcement[]): void { - void Promise.resolve( - vscode.window.showInformationMessage(popupMessage(banners), VIEW_ACTION), - ) - .then((action) => { - if (action === VIEW_ACTION) { - void this.showAnnouncements(); - } - }) - .catch((error: unknown) => { - this.logger.warn("Failed to show Coder announcement popup", error); - }); + private async showPopup(banners: readonly Announcement[]): Promise { + try { + const action = await vscode.window.showInformationMessage( + popupMessage(banners), + VIEW_ACTION, + ); + if (action === VIEW_ACTION) { + void this.showAnnouncements(); + } + } catch (error) { + this.logger.warn("Failed to show Coder announcement popup", error); + } } private scheduleRefresh(): void { @@ -190,14 +189,6 @@ export class AnnouncementManager implements vscode.Disposable { this.refreshTimeout = undefined; } } - - private sessionChangedSince(session: SessionData): boolean { - return this.disposed || this.sessionState.current !== session; - } -} - -function notificationsEnabled(): boolean { - return !areNotificationsDisabled(vscode.workspace.getConfiguration()); } function errorMessage(error: unknown): string { diff --git a/test/unit/announcements/banners.test.ts b/test/unit/announcements/banners.test.ts index 9edca9420e..67848e9ac9 100644 --- a/test/unit/announcements/banners.test.ts +++ b/test/unit/announcements/banners.test.ts @@ -4,8 +4,6 @@ import { bannerKey, normalizeBanners, popupMessage, - sourceIcon, - sourceLabel, statusText, statusTooltip, } from "@/announcements/banners"; @@ -100,27 +98,20 @@ describe("normalizeBanners", () => { backgroundColor: "#004852", }); - expect( - bannerKey({ - source: "service", - message: "Maintenance tonight", - backgroundColor: "#004852", - }), - ).not.toBe(key); - expect( - bannerKey({ - source: "announcement", - message: "Maintenance tomorrow", - backgroundColor: "#004852", - }), - ).not.toBe(key); - expect( - bannerKey({ - source: "announcement", - message: "Maintenance tonight", - backgroundColor: "#111111", - }), - ).not.toBe(key); + for (const changed of [ + { source: "service" as const }, + { message: "Maintenance tomorrow" }, + { backgroundColor: "#111111" }, + ]) { + expect( + bannerKey({ + source: "announcement", + message: "Maintenance tonight", + backgroundColor: "#004852", + ...changed, + }), + ).not.toBe(key); + } }); }); @@ -148,11 +139,4 @@ describe("banner copy", () => { `Coder announcement: ${"a".repeat(119)}…`, ); }); - - it("formats QuickPick source labels", () => { - expect(sourceIcon("service")).toBe("$(info)"); - expect(sourceIcon("announcement")).toBe("$(megaphone)"); - expect(sourceLabel("service")).toBe("Service banner"); - expect(sourceLabel("announcement")).toBe("Announcement"); - }); }); diff --git a/test/unit/announcements/manager.test.ts b/test/unit/announcements/manager.test.ts index dbdbb853f8..5e568001d5 100644 --- a/test/unit/announcements/manager.test.ts +++ b/test/unit/announcements/manager.test.ts @@ -15,30 +15,20 @@ import { MockStatusBarItem, } from "../../mocks/testHelpers"; -import type { - AppearanceConfig, - BannerConfig, -} from "coder/site/src/api/typesGenerated"; +import type { AppearanceConfig } from "coder/site/src/api/typesGenerated"; const DEPLOYMENT = { url: "https://coder.example.com", safeHostname: "coder.example.com", }; -class MockAppearanceClient { - readonly getAppearance = vi.fn<() => Promise>(); +function createClient() { + return { getAppearance: vi.fn<() => Promise>() }; } -const managers: AnnouncementManager[] = []; +type MockAppearanceClient = ReturnType; -function banner(overrides: Partial = {}): BannerConfig { - return { - enabled: true, - message: "Maintenance tonight", - background_color: "#004852", - ...overrides, - }; -} +let manager: AnnouncementManager | undefined; function appearance(messages: readonly string[] = []): AppearanceConfig { return { @@ -46,13 +36,17 @@ function appearance(messages: readonly string[] = []): AppearanceConfig { logo_url: "", docs_url: "", service_banner: { enabled: false }, - announcement_banners: messages.map((message) => banner({ message })), + announcement_banners: messages.map((message) => ({ + enabled: true, + message, + background_color: "#004852", + })), }; } function setup() { const config = new MockConfigurationProvider(); - const client = new MockAppearanceClient(); + const client = createClient(); client.getAppearance.mockResolvedValue(appearance()); const session = new SessionStore(); const secretsManager = new SecretsManager( @@ -62,18 +56,18 @@ function setup() { ); const statusBar = new MockStatusBarItem(); const logger = createMockLogger(); - const manager = new AnnouncementManager( + const announcementManager = new AnnouncementManager( client, session, secretsManager, logger, ); - managers.push(manager); + manager = announcementManager; return { client, config, logger, - manager, + manager: announcementManager, secretsManager, session, statusBar, @@ -106,9 +100,8 @@ describe("AnnouncementManager", () => { }); afterEach(() => { - while (managers.length > 0) { - managers.pop()?.dispose(); - } + manager?.dispose(); + manager = undefined; }); it("shows all active banners in the status bar", async () => { @@ -165,8 +158,7 @@ describe("AnnouncementManager", () => { it("shows newly seen banners on a different deployment", async () => { const { client, session } = setup(); client.getAppearance.mockResolvedValue(appearance(["Maintenance tonight"])); - session.signIn(DEPLOYMENT, createMockUser()); - await flushPromises(); + await signIn(session); vi.mocked(vscode.window.showInformationMessage).mockClear(); session.signIn( @@ -197,7 +189,12 @@ describe("AnnouncementManager", () => { await manager.showAnnouncements(); expect(vscode.window.showQuickPick).toHaveBeenCalledWith( - [expect.objectContaining({ detail: "Full details" })], + [ + expect.objectContaining({ + label: "$(megaphone) Announcement 1", + detail: "Full details", + }), + ], expect.objectContaining({ title: "Coder Announcements" }), ); expectInfo("Full details"); From 70c567fe42beea8c002aa7c5aeeaab9d8b4b5fd5 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Sun, 5 Jul 2026 15:42:23 +0000 Subject: [PATCH 04/10] refactor: address announcement banner review findings - Store seen banners per deployment (coder.seenBanners.) to match the per-deployment key convention and avoid cross-window races - Skip the seen-banner write when keys are unchanged - Reuse errToStr so API error messages surface in refresh failures - Guard against appearance responses missing banner fields - Drop the raw hex color from the announcements QuickPick - Cover the periodic refresh timer with a fake-timers test --- src/announcements/banners.ts | 11 +++++----- src/announcements/manager.ts | 19 ++++++++-------- src/core/secretsManager.ts | 29 ++++++++++--------------- test/unit/announcements/banners.test.ts | 7 ++++++ test/unit/announcements/manager.test.ts | 19 ++++++++++++++++ test/unit/core/secretsManager.test.ts | 2 +- 6 files changed, 54 insertions(+), 33 deletions(-) diff --git a/src/announcements/banners.ts b/src/announcements/banners.ts index 745f89f1e4..33aea55240 100644 --- a/src/announcements/banners.ts +++ b/src/announcements/banners.ts @@ -21,7 +21,8 @@ export function normalizeBanners( ): readonly Announcement[] { return [ toAnnouncement("service", appearance.service_banner), - ...appearance.announcement_banners.map((banner) => + // Nullish guards tolerate older deployments that omit banner fields. + ...(appearance.announcement_banners ?? []).map((banner) => toAnnouncement("announcement", banner), ), ].filter((banner): banner is Announcement => banner !== undefined); @@ -64,13 +65,13 @@ export function popupMessage(banners: readonly Announcement[]): string { function toAnnouncement( source: AnnouncementSource, - banner: BannerConfig, + banner: BannerConfig | undefined, ): Announcement | undefined { - const message = banner.message?.trim(); - const backgroundColor = banner.background_color?.trim() || undefined; - if (!banner.enabled || !message) { + const message = banner?.message?.trim(); + if (!banner?.enabled || !message) { return undefined; } + const backgroundColor = banner.background_color?.trim() || undefined; return { source, message, diff --git a/src/announcements/manager.ts b/src/announcements/manager.ts index 5c81997a08..227948dd19 100644 --- a/src/announcements/manager.ts +++ b/src/announcements/manager.ts @@ -1,5 +1,6 @@ import * as vscode from "vscode"; +import { errToStr } from "../api/api-helper"; import { type CoderApi } from "../api/coderApi"; import { type SecretsManager } from "../core/secretsManager"; import { type SessionState } from "../deployment/sessionStore"; @@ -67,7 +68,7 @@ export class AnnouncementManager implements vscode.Disposable { this.logger.warn("Failed to refresh Coder announcements", error); if (options.showErrors) { void vscode.window.showErrorMessage( - `Failed to refresh Coder announcements: ${errorMessage(error)}`, + `Failed to refresh Coder announcements: ${errToStr(error)}`, ); } return undefined; @@ -90,7 +91,6 @@ export class AnnouncementManager implements vscode.Disposable { banners.map((banner, index) => ({ label: `${banner.source === "service" ? "$(info) Service banner" : "$(megaphone) Announcement"} ${index + 1}`, detail: banner.message, - description: banner.backgroundColor, banner, })), { @@ -129,6 +129,7 @@ export class AnnouncementManager implements vscode.Disposable { const seen = new Set( this.secretsManager.getSeenBanners(session.deployment.safeHostname), ); + const keys = banners.map((banner) => banner.key); const unseen = banners.filter((banner) => !seen.has(banner.key)); if ( options.notify && @@ -137,10 +138,12 @@ export class AnnouncementManager implements vscode.Disposable { ) { void this.showPopup(unseen); } - await this.secretsManager.setSeenBanners( - session.deployment.safeHostname, - banners.map((banner) => banner.key), - ); + if (unseen.length > 0 || seen.size !== new Set(keys).size) { + await this.secretsManager.setSeenBanners( + session.deployment.safeHostname, + keys, + ); + } return banners; } @@ -190,7 +193,3 @@ export class AnnouncementManager implements vscode.Disposable { } } } - -function errorMessage(error: unknown): string { - return error instanceof Error ? error.message : String(error); -} diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index a8004abd12..78b7eb83ea 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -13,11 +13,11 @@ import type { Logger } from "../logging/logger"; const SESSION_KEY_PREFIX = "coder.session."; const OAUTH_CLIENT_PREFIX = "coder.oauth.client."; const DEPLOYMENT_ACCESS_PREFIX = "coder.access."; +const SEEN_BANNERS_PREFIX = "coder.seenBanners."; type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX; const CURRENT_DEPLOYMENT_KEY = "coder.currentDeployment"; -const SEEN_BANNERS_KEY = "seenBanners"; const DEFAULT_MAX_DEPLOYMENTS = 10; const LEGACY_SESSION_TOKEN_KEY = "sessionToken"; @@ -30,8 +30,7 @@ export type CurrentDeploymentState = z.infer< typeof CurrentDeploymentStateSchema >; -const SeenBannersSchema = z.record(z.string(), z.array(z.string())); -type SeenBanners = z.infer; +const SeenBannersSchema = z.array(z.string()); /** * OAuth token data stored alongside session auth. @@ -248,33 +247,29 @@ export class SecretsManager { } public getSeenBanners(safeHostname: string): string[] { - return this.getSeenBannersState()[safeHostname] ?? []; + const raw = this.memento.get( + `${SEEN_BANNERS_PREFIX}${safeHostname}`, + ); + const result = SeenBannersSchema.safeParse(raw); + return result.success ? result.data : []; } public async setSeenBanners( safeHostname: string, bannerKeys: readonly string[], ): Promise { - const seenBanners = this.getSeenBannersState(); - seenBanners[safeHostname] = [...bannerKeys]; - await this.memento.update(SEEN_BANNERS_KEY, seenBanners); + await this.memento.update(`${SEEN_BANNERS_PREFIX}${safeHostname}`, [ + ...bannerKeys, + ]); } private async clearSeenBanners(safeHostname: string): Promise { - const seenBanners = this.getSeenBannersState(); - delete seenBanners[safeHostname]; await this.memento.update( - SEEN_BANNERS_KEY, - Object.keys(seenBanners).length > 0 ? seenBanners : undefined, + `${SEEN_BANNERS_PREFIX}${safeHostname}`, + undefined, ); } - private getSeenBannersState(): SeenBanners { - const raw = this.memento.get(SEEN_BANNERS_KEY); - const result = SeenBannersSchema.safeParse(raw); - return result.success ? result.data : {}; - } - /** * Get all known hostnames, ordered by most recently accessed. * Derives the list from actual session secrets stored. diff --git a/test/unit/announcements/banners.test.ts b/test/unit/announcements/banners.test.ts index 67848e9ac9..a4f0621995 100644 --- a/test/unit/announcements/banners.test.ts +++ b/test/unit/announcements/banners.test.ts @@ -81,6 +81,13 @@ describe("normalizeBanners", () => { ); }); + it("tolerates responses missing banner fields", () => { + expect(normalizeBanners({} as AppearanceConfig)).toEqual([]); + expect( + normalizeBanners({ service_banner: banner() } as AppearanceConfig), + ).toMatchObject([{ source: "service", message: "Maintenance tonight" }]); + }); + it("keeps keys stable when banners reorder", () => { const original = announcements("First", "Second"); const reordered = announcements("Second", "First"); diff --git a/test/unit/announcements/manager.test.ts b/test/unit/announcements/manager.test.ts index 5e568001d5..ff11ab0d26 100644 --- a/test/unit/announcements/manager.test.ts +++ b/test/unit/announcements/manager.test.ts @@ -264,6 +264,25 @@ describe("AnnouncementManager", () => { expect(statusBar.tooltip).not.toContain("Stale"); }); + it("polls for new banners while signed in and stops after sign-out", async () => { + vi.useFakeTimers(); + const { client, session, statusBar } = setup(); + await signIn(session); + nextAppearance(client, ["Scheduled maintenance"]); + + await vi.advanceTimersByTimeAsync(30 * 60 * 1000); + await flushPromises(); + + expect(statusBar.text).toBe("$(megaphone) Coder"); + expectInfo("Coder announcement: Scheduled maintenance", "View"); + + session.signOut(null); + client.getAppearance.mockClear(); + await vi.advanceTimersByTimeAsync(30 * 60 * 1000); + + expect(client.getAppearance).not.toHaveBeenCalled(); + }); + it("clears status bar when signed out", async () => { const { client, session, statusBar } = setup(); nextAppearance(client, ["Maintenance tonight"]); diff --git a/test/unit/core/secretsManager.test.ts b/test/unit/core/secretsManager.test.ts index 3cee2dac0e..dda92c1dcb 100644 --- a/test/unit/core/secretsManager.test.ts +++ b/test/unit/core/secretsManager.test.ts @@ -193,7 +193,7 @@ describe("SecretsManager", () => { }); it("ignores corrupted seen banner storage", async () => { - await memento.update("seenBanners", { "example.com": "bad" }); + await memento.update("coder.seenBanners.example.com", { bad: true }); expect(secretsManager.getSeenBanners("example.com")).toEqual([]); }); From fada5052700e2b7cad1c1a95815ecd92517fb2cc Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Sun, 5 Jul 2026 16:47:00 +0000 Subject: [PATCH 05/10] test: deduplicate announcement manager test fixtures --- test/unit/announcements/manager.test.ts | 58 ++++++++++++------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/test/unit/announcements/manager.test.ts b/test/unit/announcements/manager.test.ts index ff11ab0d26..8cfd6d8711 100644 --- a/test/unit/announcements/manager.test.ts +++ b/test/unit/announcements/manager.test.ts @@ -22,6 +22,13 @@ const DEPLOYMENT = { safeHostname: "coder.example.com", }; +const OTHER_DEPLOYMENT = { + url: "https://other.example.com", + safeHostname: "other.example.com", +}; + +const REFRESH_INTERVAL_MS = 30 * 60 * 1000; + function createClient() { return { getAppearance: vi.fn<() => Promise>() }; } @@ -74,8 +81,11 @@ function setup() { }; } -async function signIn(session: SessionStore): Promise { - session.signIn(DEPLOYMENT, createMockUser()); +async function signIn( + session: SessionStore, + deployment = DEPLOYMENT, +): Promise { + session.signIn(deployment, createMockUser()); await flushPromises(); } @@ -93,6 +103,10 @@ function expectInfo(message: string, ...items: string[]): void { ); } +function clearInfoMessages(): void { + vi.mocked(vscode.window.showInformationMessage).mockClear(); +} + describe("AnnouncementManager", () => { beforeEach(() => { vi.useRealTimers(); @@ -121,7 +135,7 @@ describe("AnnouncementManager", () => { nextAppearance(client, ["First", "Second"]); await signIn(session); expectInfo("Coder has 2 new deployment announcements.", "View"); - vi.mocked(vscode.window.showInformationMessage).mockClear(); + clearInfoMessages(); nextAppearance(client, ["First", "Second", "Third"]); await manager.refresh({ notify: true }); @@ -136,7 +150,7 @@ describe("AnnouncementManager", () => { const { client, manager, session } = setup(); client.getAppearance.mockResolvedValue(appearance(["Maintenance tonight"])); await signIn(session); - vi.mocked(vscode.window.showInformationMessage).mockClear(); + clearInfoMessages(); await manager.refresh({ notify: true }); @@ -159,13 +173,9 @@ describe("AnnouncementManager", () => { const { client, session } = setup(); client.getAppearance.mockResolvedValue(appearance(["Maintenance tonight"])); await signIn(session); - vi.mocked(vscode.window.showInformationMessage).mockClear(); + clearInfoMessages(); - session.signIn( - { url: "https://other.example.com", safeHostname: "other.example.com" }, - createMockUser(), - ); - await flushPromises(); + await signIn(session, OTHER_DEPLOYMENT); expectInfo("Coder announcement: Maintenance tonight", "View"); }); @@ -173,18 +183,11 @@ describe("AnnouncementManager", () => { it("refreshes before showing announcements from the command", async () => { const { client, manager, session } = setup(); client.getAppearance.mockResolvedValue(appearance(["Full details"])); - vi.mocked(vscode.window.showQuickPick).mockResolvedValueOnce({ - label: "$(megaphone) Announcement 1", - detail: "Full details", - banner: { - source: "announcement", - message: "Full details", - backgroundColor: "#004852", - key: "key", - }, - } as never); + vi.mocked(vscode.window.showQuickPick).mockImplementationOnce( + async (items) => (await items)[0], + ); await signIn(session); - vi.mocked(vscode.window.showInformationMessage).mockClear(); + clearInfoMessages(); await manager.showAnnouncements(); @@ -203,7 +206,7 @@ describe("AnnouncementManager", () => { it("shows an empty message when there are no active announcements", async () => { const { manager, session } = setup(); await signIn(session); - vi.mocked(vscode.window.showInformationMessage).mockClear(); + clearInfoMessages(); await manager.showAnnouncements(); @@ -213,7 +216,7 @@ describe("AnnouncementManager", () => { it("shows refresh errors from the command", async () => { const { client, logger, manager, session } = setup(); await signIn(session); - vi.mocked(vscode.window.showInformationMessage).mockClear(); + clearInfoMessages(); client.getAppearance.mockRejectedValueOnce(new Error("boom")); await manager.showAnnouncements(); @@ -253,10 +256,7 @@ describe("AnnouncementManager", () => { session.signIn(DEPLOYMENT, createMockUser()); await Promise.resolve(); - session.signIn( - { url: "https://other.example.com", safeHostname: "other.example.com" }, - createMockUser(), - ); + session.signIn(OTHER_DEPLOYMENT, createMockUser()); stale.resolve(appearance(["Stale"])); await flushPromises(); @@ -270,7 +270,7 @@ describe("AnnouncementManager", () => { await signIn(session); nextAppearance(client, ["Scheduled maintenance"]); - await vi.advanceTimersByTimeAsync(30 * 60 * 1000); + await vi.advanceTimersByTimeAsync(REFRESH_INTERVAL_MS); await flushPromises(); expect(statusBar.text).toBe("$(megaphone) Coder"); @@ -278,7 +278,7 @@ describe("AnnouncementManager", () => { session.signOut(null); client.getAppearance.mockClear(); - await vi.advanceTimersByTimeAsync(30 * 60 * 1000); + await vi.advanceTimersByTimeAsync(REFRESH_INTERVAL_MS); expect(client.getAppearance).not.toHaveBeenCalled(); }); From abf54b54a6a5fa6922a2ebb33dbbccc88aaa49a0 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Mon, 6 Jul 2026 19:27:11 +0300 Subject: [PATCH 06/10] refactor: address announcement banner review notes Centralize status bar item names and priorities in util/statusBar while preserving each item's pre-registry id and placement. Read the deprecated service_banner only when announcement_banners is absent so modern deployments that mirror the first announcement into it show no duplicates, and derive banner keys from the message alone. Union seen keys instead of overwriting, mark banners seen only when the user could see them, and guard overlapping fetches with a generation counter so stale responses never apply. Open the picker from the popup without refetching, show the empty-state message only after a successful refresh, avoid awaiting non-modal notifications, truncate popup text by code points, and align tests with the setup() factory pattern. --- src/announcements/banners.ts | 59 +++------ src/announcements/manager.ts | 165 +++++++++++++----------- src/remote/remote.ts | 6 +- src/remote/sshProcess.ts | 6 +- src/util/statusBar.ts | 44 +++++++ src/workspace/workspaceMonitor.ts | 7 +- test/unit/announcements/banners.test.ts | 92 +++++-------- test/unit/announcements/manager.test.ts | 123 ++++++++++-------- 8 files changed, 263 insertions(+), 239 deletions(-) create mode 100644 src/util/statusBar.ts diff --git a/src/announcements/banners.ts b/src/announcements/banners.ts index 33aea55240..01c8f975c7 100644 --- a/src/announcements/banners.ts +++ b/src/announcements/banners.ts @@ -7,51 +7,31 @@ import type { const POPUP_MESSAGE_MAX_LENGTH = 120; -export type AnnouncementSource = "announcement" | "service"; - export interface Announcement { - readonly source: AnnouncementSource; readonly message: string; - readonly backgroundColor?: string; + /** Stable fingerprint used to track which banners were already seen. */ readonly key: string; } export function normalizeBanners( appearance: AppearanceConfig, ): readonly Announcement[] { - return [ - toAnnouncement("service", appearance.service_banner), - // Nullish guards tolerate older deployments that omit banner fields. - ...(appearance.announcement_banners ?? []).map((banner) => - toAnnouncement("announcement", banner), - ), - ].filter((banner): banner is Announcement => banner !== undefined); -} - -export function bannerKey( - banner: Pick, -): string { - return createHash("sha256") - .update( - JSON.stringify({ - source: banner.source, - message: banner.message, - backgroundColor: banner.backgroundColor ?? "", - }), - ) - .digest("hex") - .slice(0, 16); + // Modern servers mirror announcements[0] into the deprecated service_banner. + const banners = appearance.announcement_banners ?? [ + appearance.service_banner, + ]; + return banners + .map((banner) => toAnnouncement(banner)) + .filter((banner): banner is Announcement => banner !== undefined); } export function statusText(count: number): string { - return count === 1 ? "$(megaphone) Coder" : `$(megaphone) Coder ${count}`; + return `$(megaphone) Coder${count === 1 ? "" : ` ${count}`}`; } export function statusTooltip(banners: readonly Announcement[]): string { return [ - banners.length === 1 - ? "Coder deployment announcement" - : "Coder deployment announcements", + `Coder deployment announcement${banners.length === 1 ? "" : "s"}`, "", ...banners.map((banner, index) => `${index + 1}. ${banner.message}`), ].join("\n"); @@ -64,24 +44,23 @@ export function popupMessage(banners: readonly Announcement[]): string { } function toAnnouncement( - source: AnnouncementSource, banner: BannerConfig | undefined, ): Announcement | undefined { const message = banner?.message?.trim(); if (!banner?.enabled || !message) { return undefined; } - const backgroundColor = banner.background_color?.trim() || undefined; - return { - source, - message, - backgroundColor, - key: bannerKey({ source, message, backgroundColor }), - }; + return { message, key: bannerKey(message) }; +} + +function bannerKey(message: string): string { + return createHash("sha256").update(message).digest("hex").slice(0, 16); } function truncate(message: string): string { - return message.length <= POPUP_MESSAGE_MAX_LENGTH + // Slice code points so emoji are never cut in half. + const chars = [...message]; + return chars.length <= POPUP_MESSAGE_MAX_LENGTH ? message - : `${message.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1)}…`; + : `${chars.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1).join("")}…`; } diff --git a/src/announcements/manager.ts b/src/announcements/manager.ts index 227948dd19..eb649165b6 100644 --- a/src/announcements/manager.ts +++ b/src/announcements/manager.ts @@ -1,11 +1,8 @@ import * as vscode from "vscode"; import { errToStr } from "../api/api-helper"; -import { type CoderApi } from "../api/coderApi"; -import { type SecretsManager } from "../core/secretsManager"; -import { type SessionState } from "../deployment/sessionStore"; -import { type Logger } from "../logging/logger"; import { areNotificationsDisabled } from "../settings/notifications"; +import { createStatusBarItem } from "../util/statusBar"; import { type Announcement, @@ -15,7 +12,12 @@ import { statusTooltip, } from "./banners"; -const REFRESH_INTERVAL_MS = 30 * 60 * 1000; +import type { CoderApi } from "../api/coderApi"; +import type { SecretsManager } from "../core/secretsManager"; +import type { SessionState } from "../deployment/sessionStore"; +import type { Logger } from "../logging/logger"; + +export const REFRESH_INTERVAL_MS = 30 * 60 * 1000; const VIEW_ACTION = "View"; interface RefreshOptions { @@ -24,9 +26,10 @@ interface RefreshOptions { } export class AnnouncementManager implements vscode.Disposable { - private readonly statusBarItem: vscode.StatusBarItem; + private readonly statusBarItem = createStatusBarItem("announcements"); private readonly sessionChangeDisposable: vscode.Disposable; - private banners: readonly Announcement[] = []; + #banners: readonly Announcement[] = []; + private fetchGeneration = 0; private refreshTimeout: NodeJS.Timeout | undefined; private disposed = false; @@ -36,11 +39,6 @@ export class AnnouncementManager implements vscode.Disposable { private readonly secretsManager: SecretsManager, private readonly logger: Logger, ) { - this.statusBarItem = vscode.window.createStatusBarItem( - vscode.StatusBarAlignment.Left, - 998, - ); - this.statusBarItem.name = "Coder Announcements"; this.statusBarItem.command = "coder.viewAnnouncements"; this.sessionChangeDisposable = this.sessionState.onDidChange(() => { this.onSessionChange(); @@ -48,22 +46,15 @@ export class AnnouncementManager implements vscode.Disposable { this.onSessionChange(); } - public dispose(): void { - this.disposed = true; - this.cancelRefresh(); - this.sessionChangeDisposable.dispose(); - this.statusBarItem.dispose(); - } - - public async refresh( - options: RefreshOptions = {}, - ): Promise { + /** Refreshes the banners; resolves false when the refresh failed. */ + public async refresh(options: RefreshOptions = {}): Promise { if (this.disposed) { - return undefined; + return false; } this.cancelRefresh(); try { - return await this.fetch(options); + await this.fetch(options); + return true; } catch (error) { this.logger.warn("Failed to refresh Coder announcements", error); if (options.showErrors) { @@ -71,84 +62,89 @@ export class AnnouncementManager implements vscode.Disposable { `Failed to refresh Coder announcements: ${errToStr(error)}`, ); } - return undefined; + return false; } finally { this.scheduleRefresh(); } } public async showAnnouncements(): Promise { - const banners = - (await this.refresh({ notify: false, showErrors: true })) ?? this.banners; - if (banners.length === 0) { + const refreshed = await this.refresh({ notify: false, showErrors: true }); + if (this.banners.length > 0) { + await this.pickAnnouncement(); + } else if (refreshed) { + // On failure the error popup is the whole story. void vscode.window.showInformationMessage( "No active Coder announcements.", ); - return; } + } - const selected = await vscode.window.showQuickPick( - banners.map((banner, index) => ({ - label: `${banner.source === "service" ? "$(info) Service banner" : "$(megaphone) Announcement"} ${index + 1}`, - detail: banner.message, - banner, - })), - { - title: "Coder Announcements", - placeHolder: "Select an announcement to view the full message", - }, - ); - if (selected) { - void vscode.window.showInformationMessage(selected.banner.message); - } + public dispose(): void { + this.disposed = true; + this.cancelRefresh(); + this.sessionChangeDisposable.dispose(); + this.banners = []; + this.statusBarItem.dispose(); } private onSessionChange(): void { this.cancelRefresh(); - this.setBanners([]); + this.banners = []; if (this.sessionState.current.kind === "signedIn") { void this.refresh({ notify: true }); } } - private async fetch( - options: RefreshOptions, - ): Promise { + private async fetch(options: RefreshOptions): Promise { const session = this.sessionState.current; if (session.kind !== "signedIn") { - this.setBanners([]); - return []; + this.banners = []; + return; } - const banners = normalizeBanners(await this.client.getAppearance()); - if (this.disposed || this.sessionState.current !== session) { - return undefined; + const generation = ++this.fetchGeneration; + const appearance = await this.client.getAppearance(); + // A newer fetch or a session change supersedes this response. + if ( + this.disposed || + generation !== this.fetchGeneration || + this.sessionState.current !== session + ) { + return; } - this.setBanners(banners); + const banners = normalizeBanners(appearance); + this.banners = banners; const seen = new Set( this.secretsManager.getSeenBanners(session.deployment.safeHostname), ); - const keys = banners.map((banner) => banner.key); const unseen = banners.filter((banner) => !seen.has(banner.key)); - if ( - options.notify && - unseen.length > 0 && - !areNotificationsDisabled(vscode.workspace.getConfiguration()) - ) { - void this.showPopup(unseen); + if (unseen.length === 0) { + return; } - if (unseen.length > 0 || seen.size !== new Set(keys).size) { + const notifiable = !areNotificationsDisabled( + vscode.workspace.getConfiguration(), + ); + if (options.notify && notifiable) { + this.showPopup(unseen); + } + // Mark seen only what the user could see; suppressed banners notify later. + if (!options.notify || notifiable) { await this.secretsManager.setSeenBanners( session.deployment.safeHostname, - keys, + [...seen, ...unseen.map((banner) => banner.key)], ); } - return banners; } - private setBanners(banners: readonly Announcement[]): void { - this.banners = banners; + /** The setter syncs the status bar so the two cannot drift apart. */ + private get banners(): readonly Announcement[] { + return this.#banners; + } + + private set banners(banners: readonly Announcement[]) { + this.#banners = banners; if (banners.length === 0) { this.statusBarItem.hide(); return; @@ -158,17 +154,34 @@ export class AnnouncementManager implements vscode.Disposable { this.statusBarItem.show(); } - private async showPopup(banners: readonly Announcement[]): Promise { - try { - const action = await vscode.window.showInformationMessage( - popupMessage(banners), - VIEW_ACTION, - ); - if (action === VIEW_ACTION) { - void this.showAnnouncements(); - } - } catch (error) { - this.logger.warn("Failed to show Coder announcement popup", error); + /** Non-modal messages may never settle, so chain instead of awaiting. */ + private showPopup(banners: readonly Announcement[]): void { + Promise.resolve( + vscode.window.showInformationMessage(popupMessage(banners), VIEW_ACTION), + ) + .then((action) => + // Banners are seconds old; skip the refetch. + action === VIEW_ACTION ? this.pickAnnouncement() : undefined, + ) + .catch((error) => { + this.logger.warn("Failed to show Coder announcement popup", error); + }); + } + + private async pickAnnouncement(): Promise { + const selected = await vscode.window.showQuickPick( + this.banners.map((banner, index) => ({ + label: `$(megaphone) Announcement ${index + 1}`, + detail: banner.message, + banner, + })), + { + title: "Coder Announcements", + placeHolder: "Select an announcement to view the full message", + }, + ); + if (selected) { + void vscode.window.showInformationMessage(selected.banner.message); } } diff --git a/src/remote/remote.ts b/src/remote/remote.ts index 14f7b46097..b08bc8d9ff 100644 --- a/src/remote/remote.ts +++ b/src/remote/remote.ts @@ -43,6 +43,7 @@ import { type AuthorityParts, parseRemoteAuthority, } from "../util/authority"; +import { createStatusBarItem } from "../util/statusBar"; import { vscodeProposed } from "../vscodeProposed"; import { WorkspaceMonitor } from "../workspace/workspaceMonitor"; @@ -1080,10 +1081,7 @@ export class Remote { agent: WorkspaceAgent, client: CoderApi, ): Promise { - const statusBarItem = vscode.window.createStatusBarItem( - "agentMetadata", - vscode.StatusBarAlignment.Left, - ); + const statusBarItem = createStatusBarItem("agentMetadata"); const agentWatcher = await createAgentMetadataWatcher(agent.id, client); diff --git a/src/remote/sshProcess.ts b/src/remote/sshProcess.ts index 58d0fee497..3b48189961 100644 --- a/src/remote/sshProcess.ts +++ b/src/remote/sshProcess.ts @@ -6,6 +6,7 @@ import * as vscode from "vscode"; import { SshTelemetry, type ProcessLossCause } from "../instrumentation/ssh"; import { findPort } from "../util"; import { cleanupFiles } from "../util/fileCleanup"; +import { createStatusBarItem } from "../util/statusBar"; import { NetworkStatusReporter } from "./networkStatus"; import { @@ -140,10 +141,7 @@ export class SshProcessMonitor implements vscode.Disposable { networkPollInterval: options.networkPollInterval ?? 3000, }; this.telemetry = new SshTelemetry(options.telemetry); - this.statusBarItem = vscode.window.createStatusBarItem( - vscode.StatusBarAlignment.Left, - 1000, - ); + this.statusBarItem = createStatusBarItem("networkStatus"); this.reporter = new NetworkStatusReporter(this.statusBarItem); } diff --git a/src/util/statusBar.ts b/src/util/statusBar.ts new file mode 100644 index 0000000000..c39d99339d --- /dev/null +++ b/src/util/statusBar.ts @@ -0,0 +1,44 @@ +import * as vscode from "vscode"; + +interface StatusBarItemSpec { + /** Omitted for pre-registry items; a new id orphans hide preferences. */ + readonly id?: string; + readonly name: string; + readonly priority: number; +} + +/** All Coder status bar items; higher priority renders further left. */ +const STATUS_BAR_ITEMS = { + networkStatus: { name: "Coder Network Status", priority: 1000 }, + workspaceUpdate: { name: "Coder Workspace Update", priority: 999 }, + announcements: { + id: "announcements", + name: "Coder Announcements", + priority: 998, + }, + // Priority 0 preserves its pre-registry placement. + agentMetadata: { + id: "agentMetadata", + name: "Coder Agent Metadata", + priority: 0, + }, +} satisfies Record; + +/** Creates a registered status bar item with its id, name, and priority. */ +export function createStatusBarItem( + key: keyof typeof STATUS_BAR_ITEMS, +): vscode.StatusBarItem { + const { id, name, priority }: StatusBarItemSpec = STATUS_BAR_ITEMS[key]; + const item = id + ? vscode.window.createStatusBarItem( + id, + vscode.StatusBarAlignment.Left, + priority, + ) + : vscode.window.createStatusBarItem( + vscode.StatusBarAlignment.Left, + priority, + ); + item.name = name; + return item; +} diff --git a/src/workspace/workspaceMonitor.ts b/src/workspace/workspaceMonitor.ts index 150fac1e9b..ff6d23da46 100644 --- a/src/workspace/workspaceMonitor.ts +++ b/src/workspace/workspaceMonitor.ts @@ -11,6 +11,7 @@ import { areNotificationsDisabled, areUpdateNotificationsDisabled, } from "../settings/notifications"; +import { createStatusBarItem } from "../util/statusBar"; import { vscodeProposed } from "../vscodeProposed"; import type { CoderApi } from "../api/coderApi"; @@ -64,11 +65,7 @@ export class WorkspaceMonitor implements vscode.Disposable { ); this.latestWorkspace = workspace; - const statusBarItem = vscode.window.createStatusBarItem( - vscode.StatusBarAlignment.Left, - 999, - ); - statusBarItem.name = "Coder Workspace Update"; + const statusBarItem = createStatusBarItem("workspaceUpdate"); statusBarItem.text = "$(fold-up) Update Workspace"; statusBarItem.command = "coder.workspace.update"; diff --git a/test/unit/announcements/banners.test.ts b/test/unit/announcements/banners.test.ts index a4f0621995..54d2ca7d77 100644 --- a/test/unit/announcements/banners.test.ts +++ b/test/unit/announcements/banners.test.ts @@ -1,7 +1,6 @@ import { describe, expect, it } from "vitest"; import { - bannerKey, normalizeBanners, popupMessage, statusText, @@ -44,81 +43,52 @@ function announcements(...messages: string[]) { } describe("normalizeBanners", () => { - it("returns active service and announcement banners", () => { + it("returns active announcement banners trimmed", () => { const banners = normalizeBanners( appearance({ - service_banner: banner({ - message: " Service banner ", - background_color: " #123456 ", - }), announcement_banners: [ - banner({ message: " Announcement " }), + banner({ message: " First " }), banner({ enabled: false, message: "Disabled" }), banner({ message: " " }), + banner({ message: "Second" }), ], }), ); - expect(banners).toMatchObject([ - { - source: "service", - message: "Service banner", - backgroundColor: "#123456", - }, - { - source: "announcement", - message: "Announcement", - backgroundColor: "#004852", - }, + expect(banners.map((banner) => banner.message)).toEqual([ + "First", + "Second", ]); - expect(banners).toHaveLength(2); - expect(banners[0].key).toBe( - bannerKey({ - source: "service", - message: "Service banner", - backgroundColor: "#123456", + }); + + it("ignores the service banner that modern deployments mirror from the first announcement", () => { + const banners = normalizeBanners( + appearance({ + service_banner: banner(), + announcement_banners: [banner(), banner({ message: "Second" })], }), ); + + expect(banners.map((banner) => banner.message)).toEqual([ + "Maintenance tonight", + "Second", + ]); }); - it("tolerates responses missing banner fields", () => { + it("falls back to the service banner on older deployments", () => { expect(normalizeBanners({} as AppearanceConfig)).toEqual([]); expect( normalizeBanners({ service_banner: banner() } as AppearanceConfig), - ).toMatchObject([{ source: "service", message: "Maintenance tonight" }]); + ).toMatchObject([{ message: "Maintenance tonight" }]); }); - it("keeps keys stable when banners reorder", () => { - const original = announcements("First", "Second"); - const reordered = announcements("Second", "First"); - const keyFor = (message: string, banners = original) => - banners.find((banner) => banner.message === message)?.key; + it("keys follow the message and survive reordering", () => { + const [first, second] = announcements("First", "Second"); + const [reorderedSecond, reorderedFirst] = announcements("Second", "First"); - expect(keyFor("First", reordered)).toBe(keyFor("First")); - expect(keyFor("Second", reordered)).toBe(keyFor("Second")); - }); - - it("changes keys when fingerprint fields change", () => { - const key = bannerKey({ - source: "announcement", - message: "Maintenance tonight", - backgroundColor: "#004852", - }); - - for (const changed of [ - { source: "service" as const }, - { message: "Maintenance tomorrow" }, - { backgroundColor: "#111111" }, - ]) { - expect( - bannerKey({ - source: "announcement", - message: "Maintenance tonight", - backgroundColor: "#004852", - ...changed, - }), - ).not.toBe(key); - } + expect(reorderedFirst.key).toBe(first.key); + expect(reorderedSecond.key).toBe(second.key); + expect(first.key).not.toBe(second.key); }); }); @@ -134,16 +104,20 @@ describe("banner copy", () => { }); it("formats popup messages", () => { - const longMessage = "a".repeat(121); - expect(popupMessage(announcements("Maintenance tonight"))).toBe( "Coder announcement: Maintenance tonight", ); expect(popupMessage(announcements("First", "Second"))).toBe( "Coder has 2 new deployment announcements.", ); - expect(popupMessage(announcements(longMessage))).toBe( + }); + + it("truncates long popup messages without splitting emoji", () => { + expect(popupMessage(announcements("a".repeat(121)))).toBe( `Coder announcement: ${"a".repeat(119)}…`, ); + expect( + popupMessage(announcements(`${"a".repeat(118)}🚀${"b".repeat(10)}`)), + ).toBe(`Coder announcement: ${"a".repeat(118)}🚀…`); }); }); diff --git a/test/unit/announcements/manager.test.ts b/test/unit/announcements/manager.test.ts index 8cfd6d8711..54f79cc4dc 100644 --- a/test/unit/announcements/manager.test.ts +++ b/test/unit/announcements/manager.test.ts @@ -1,7 +1,10 @@ -import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { beforeEach, describe, expect, it, onTestFinished, vi } from "vitest"; import * as vscode from "vscode"; -import { AnnouncementManager } from "@/announcements/manager"; +import { + AnnouncementManager, + REFRESH_INTERVAL_MS, +} from "@/announcements/manager"; import { SecretsManager } from "@/core/secretsManager"; import { SessionStore } from "@/deployment/sessionStore"; @@ -17,26 +20,10 @@ import { import type { AppearanceConfig } from "coder/site/src/api/typesGenerated"; -const DEPLOYMENT = { - url: "https://coder.example.com", - safeHostname: "coder.example.com", -}; - -const OTHER_DEPLOYMENT = { - url: "https://other.example.com", - safeHostname: "other.example.com", -}; - -const REFRESH_INTERVAL_MS = 30 * 60 * 1000; - -function createClient() { - return { getAppearance: vi.fn<() => Promise>() }; +function deployment(host = "coder.example.com") { + return { url: `https://${host}`, safeHostname: host }; } -type MockAppearanceClient = ReturnType; - -let manager: AnnouncementManager | undefined; - function appearance(messages: readonly string[] = []): AppearanceConfig { return { application_name: "Coder", @@ -53,7 +40,7 @@ function appearance(messages: readonly string[] = []): AppearanceConfig { function setup() { const config = new MockConfigurationProvider(); - const client = createClient(); + const client = { getAppearance: vi.fn<() => Promise>() }; client.getAppearance.mockResolvedValue(appearance()); const session = new SessionStore(); const secretsManager = new SecretsManager( @@ -63,30 +50,29 @@ function setup() { ); const statusBar = new MockStatusBarItem(); const logger = createMockLogger(); - const announcementManager = new AnnouncementManager( + const manager = new AnnouncementManager( client, session, secretsManager, logger, ); - manager = announcementManager; + onTestFinished(() => manager.dispose()); return { client, config, logger, - manager: announcementManager, + manager, secretsManager, session, statusBar, }; } -async function signIn( - session: SessionStore, - deployment = DEPLOYMENT, -): Promise { - session.signIn(deployment, createMockUser()); - await flushPromises(); +type MockAppearanceClient = ReturnType["client"]; + +function signIn(session: SessionStore, host?: string): Promise { + session.signIn(deployment(host), createMockUser()); + return flushPromises(); } function nextAppearance( @@ -113,11 +99,6 @@ describe("AnnouncementManager", () => { vi.clearAllMocks(); }); - afterEach(() => { - manager?.dispose(); - manager = undefined; - }); - it("shows all active banners in the status bar", async () => { const { client, session, statusBar } = setup(); nextAppearance(client, ["First", "Second"]); @@ -141,9 +122,9 @@ describe("AnnouncementManager", () => { await manager.refresh({ notify: true }); expectInfo("Coder announcement: Third", "View"); - expect(secretsManager.getSeenBanners(DEPLOYMENT.safeHostname)).toHaveLength( - 3, - ); + expect( + secretsManager.getSeenBanners(deployment().safeHostname), + ).toHaveLength(3); }); it("does not notify for banners already seen on the same deployment", async () => { @@ -157,16 +138,39 @@ describe("AnnouncementManager", () => { expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); }); - it("suppresses popups when notifications are disabled but keeps status bar", async () => { - const { client, config, session, statusBar } = setup(); + it("suppresses popups when notifications are disabled without marking banners seen", async () => { + const { client, config, manager, secretsManager, session, statusBar } = + setup(); config.set("coder.disableNotifications", true); - nextAppearance(client, ["Maintenance tonight"]); + client.getAppearance.mockResolvedValue(appearance(["Maintenance tonight"])); await signIn(session); expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); expect(statusBar.show).toHaveBeenCalled(); expect(statusBar.text).toBe("$(megaphone) Coder"); + expect(secretsManager.getSeenBanners(deployment().safeHostname)).toEqual( + [], + ); + + config.set("coder.disableNotifications", false); + await manager.refresh({ notify: true }); + + expectInfo("Coder announcement: Maintenance tonight", "View"); + }); + + it("does not re-notify banners that temporarily disappear", async () => { + const { client, manager, session } = setup(); + nextAppearance(client, ["Maintenance tonight"]); + await signIn(session); + clearInfoMessages(); + + nextAppearance(client, []); + await manager.refresh({ notify: true }); + nextAppearance(client, ["Maintenance tonight"]); + await manager.refresh({ notify: true }); + + expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); }); it("shows newly seen banners on a different deployment", async () => { @@ -175,7 +179,7 @@ describe("AnnouncementManager", () => { await signIn(session); clearInfoMessages(); - await signIn(session, OTHER_DEPLOYMENT); + await signIn(session, "other.example.com"); expectInfo("Coder announcement: Maintenance tonight", "View"); }); @@ -228,15 +232,12 @@ describe("AnnouncementManager", () => { expect(vscode.window.showErrorMessage).toHaveBeenCalledWith( "Failed to refresh Coder announcements: boom", ); - expectInfo("No active Coder announcements."); + expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); }); - it("opens the announcements command from the popup action", async () => { - const { client, manager, session } = setup(); + it("opens the picker from the popup action without refetching", async () => { + const { client, session } = setup(); nextAppearance(client, ["Maintenance tonight"]); - const showAnnouncements = vi - .spyOn(manager, "showAnnouncements") - .mockResolvedValue(); vi.mocked(vscode.window.showInformationMessage).mockResolvedValueOnce( "View" as never, ); @@ -244,7 +245,27 @@ describe("AnnouncementManager", () => { await signIn(session); await flushPromises(); - expect(showAnnouncements).toHaveBeenCalledOnce(); + expect(vscode.window.showQuickPick).toHaveBeenCalledWith( + [expect.objectContaining({ detail: "Maintenance tonight" })], + expect.objectContaining({ title: "Coder Announcements" }), + ); + expect(client.getAppearance).toHaveBeenCalledTimes(1); + }); + + it("ignores stale responses from an overlapping refresh", async () => { + const { client, manager, session, statusBar } = setup(); + await signIn(session); + const stale = Promise.withResolvers(); + client.getAppearance.mockReturnValueOnce(stale.promise); + + const slowRefresh = manager.refresh(); + nextAppearance(client, ["Current"]); + await manager.refresh(); + stale.resolve(appearance(["Stale"])); + await slowRefresh; + + expect(statusBar.tooltip).toContain("Current"); + expect(statusBar.tooltip).not.toContain("Stale"); }); it("ignores stale refreshes after the session changes", async () => { @@ -254,9 +275,9 @@ describe("AnnouncementManager", () => { .mockReturnValueOnce(stale.promise) .mockResolvedValueOnce(appearance(["Current"])); - session.signIn(DEPLOYMENT, createMockUser()); + session.signIn(deployment(), createMockUser()); await Promise.resolve(); - session.signIn(OTHER_DEPLOYMENT, createMockUser()); + session.signIn(deployment("other.example.com"), createMockUser()); stale.resolve(appearance(["Stale"])); await flushPromises(); From 88bab76aad7e74e9a7d894116d69ac926fbcaf8a Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 8 Jul 2026 16:07:05 +0300 Subject: [PATCH 07/10] refactor: address remaining announcement banner review comments Fixes truncation to break on word boundaries, dedupes banners with identical content, clarifies the empty vs. absent announcement_banners case, guards against showing an empty picker after banners are cleared, and renames "seen" banners to "surfaced" throughout to match what the tracking actually represents. --- src/announcements/banners.ts | 24 +++++++++++++------ src/announcements/manager.ts | 11 ++++++--- src/core/secretsManager.ts | 20 ++++++++-------- test/unit/announcements/banners.test.ts | 28 ++++++++++++++++++++++ test/unit/announcements/manager.test.ts | 24 +++++++++++++++---- test/unit/core/secretsManager.test.ts | 32 ++++++++++++++----------- 6 files changed, 101 insertions(+), 38 deletions(-) diff --git a/src/announcements/banners.ts b/src/announcements/banners.ts index 01c8f975c7..985d7aa3ab 100644 --- a/src/announcements/banners.ts +++ b/src/announcements/banners.ts @@ -5,24 +5,29 @@ import type { BannerConfig, } from "coder/site/src/api/typesGenerated"; +/** Popups are read at a glance; keep the single-banner message short. */ const POPUP_MESSAGE_MAX_LENGTH = 120; export interface Announcement { readonly message: string; - /** Stable fingerprint used to track which banners were already seen. */ + /** Fingerprint used to track which banners have been surfaced. */ readonly key: string; } +/** + * Falls back to the deprecated service_banner only when announcement_banners + * is absent, not merely empty. Duplicate messages collapse into one. + */ export function normalizeBanners( appearance: AppearanceConfig, ): readonly Announcement[] { - // Modern servers mirror announcements[0] into the deprecated service_banner. const banners = appearance.announcement_banners ?? [ appearance.service_banner, ]; - return banners + const announcements = banners .map((banner) => toAnnouncement(banner)) .filter((banner): banner is Announcement => banner !== undefined); + return [...new Map(announcements.map((a) => [a.key, a])).values()]; } export function statusText(count: number): string { @@ -57,10 +62,15 @@ function bannerKey(message: string): string { return createHash("sha256").update(message).digest("hex").slice(0, 16); } +/** Truncates to a word boundary when possible; splits on code points so emoji survive. */ function truncate(message: string): string { - // Slice code points so emoji are never cut in half. const chars = [...message]; - return chars.length <= POPUP_MESSAGE_MAX_LENGTH - ? message - : `${chars.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1).join("")}…`; + if (chars.length <= POPUP_MESSAGE_MAX_LENGTH) { + return message; + } + const cut = chars.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1); + const lastSpace = cut.lastIndexOf(" "); + const trimmed = + lastSpace > POPUP_MESSAGE_MAX_LENGTH / 2 ? cut.slice(0, lastSpace) : cut; + return `${trimmed.join("")}…`; } diff --git a/src/announcements/manager.ts b/src/announcements/manager.ts index eb649165b6..e823c63ca5 100644 --- a/src/announcements/manager.ts +++ b/src/announcements/manager.ts @@ -17,6 +17,7 @@ import type { SecretsManager } from "../core/secretsManager"; import type { SessionState } from "../deployment/sessionStore"; import type { Logger } from "../logging/logger"; +/** Background poll interval; sign-in and manual refresh happen immediately either way. */ export const REFRESH_INTERVAL_MS = 30 * 60 * 1000; const VIEW_ACTION = "View"; @@ -117,7 +118,7 @@ export class AnnouncementManager implements vscode.Disposable { this.banners = banners; const seen = new Set( - this.secretsManager.getSeenBanners(session.deployment.safeHostname), + this.secretsManager.getSurfacedBanners(session.deployment.safeHostname), ); const unseen = banners.filter((banner) => !seen.has(banner.key)); if (unseen.length === 0) { @@ -129,9 +130,9 @@ export class AnnouncementManager implements vscode.Disposable { if (options.notify && notifiable) { this.showPopup(unseen); } - // Mark seen only what the user could see; suppressed banners notify later. + // Marks banners as surfaced, not read; suppressed ones notify later. if (!options.notify || notifiable) { - await this.secretsManager.setSeenBanners( + await this.secretsManager.setSurfacedBanners( session.deployment.safeHostname, [...seen, ...unseen.map((banner) => banner.key)], ); @@ -168,7 +169,11 @@ export class AnnouncementManager implements vscode.Disposable { }); } + /** Skips the picker if banners were cleared (e.g. sign-out) after the popup appeared. */ private async pickAnnouncement(): Promise { + if (this.banners.length === 0) { + return; + } const selected = await vscode.window.showQuickPick( this.banners.map((banner, index) => ({ label: `$(megaphone) Announcement ${index + 1}`, diff --git a/src/core/secretsManager.ts b/src/core/secretsManager.ts index 78b7eb83ea..b5cbc7419a 100644 --- a/src/core/secretsManager.ts +++ b/src/core/secretsManager.ts @@ -13,7 +13,7 @@ import type { Logger } from "../logging/logger"; const SESSION_KEY_PREFIX = "coder.session."; const OAUTH_CLIENT_PREFIX = "coder.oauth.client."; const DEPLOYMENT_ACCESS_PREFIX = "coder.access."; -const SEEN_BANNERS_PREFIX = "coder.seenBanners."; +const SURFACED_BANNERS_PREFIX = "coder.surfacedBanners."; type SecretKeyPrefix = typeof SESSION_KEY_PREFIX | typeof OAUTH_CLIENT_PREFIX; @@ -30,7 +30,7 @@ export type CurrentDeploymentState = z.infer< typeof CurrentDeploymentStateSchema >; -const SeenBannersSchema = z.array(z.string()); +const SurfacedBannersSchema = z.array(z.string()); /** * OAuth token data stored alongside session auth. @@ -238,7 +238,7 @@ export class SecretsManager { await Promise.all([ this.clearSessionAuth(safeHostname), this.clearOAuthClientRegistration(safeHostname), - this.clearSeenBanners(safeHostname), + this.clearSurfacedBanners(safeHostname), this.memento.update( `${DEPLOYMENT_ACCESS_PREFIX}${safeHostname}`, undefined, @@ -246,26 +246,26 @@ export class SecretsManager { ]); } - public getSeenBanners(safeHostname: string): string[] { + public getSurfacedBanners(safeHostname: string): string[] { const raw = this.memento.get( - `${SEEN_BANNERS_PREFIX}${safeHostname}`, + `${SURFACED_BANNERS_PREFIX}${safeHostname}`, ); - const result = SeenBannersSchema.safeParse(raw); + const result = SurfacedBannersSchema.safeParse(raw); return result.success ? result.data : []; } - public async setSeenBanners( + public async setSurfacedBanners( safeHostname: string, bannerKeys: readonly string[], ): Promise { - await this.memento.update(`${SEEN_BANNERS_PREFIX}${safeHostname}`, [ + await this.memento.update(`${SURFACED_BANNERS_PREFIX}${safeHostname}`, [ ...bannerKeys, ]); } - private async clearSeenBanners(safeHostname: string): Promise { + private async clearSurfacedBanners(safeHostname: string): Promise { await this.memento.update( - `${SEEN_BANNERS_PREFIX}${safeHostname}`, + `${SURFACED_BANNERS_PREFIX}${safeHostname}`, undefined, ); } diff --git a/test/unit/announcements/banners.test.ts b/test/unit/announcements/banners.test.ts index 54d2ca7d77..fb9a985096 100644 --- a/test/unit/announcements/banners.test.ts +++ b/test/unit/announcements/banners.test.ts @@ -90,6 +90,23 @@ describe("normalizeBanners", () => { expect(reorderedSecond.key).toBe(second.key); expect(first.key).not.toBe(second.key); }); + + it("collapses banners with identical content", () => { + const banners = announcements("Same message", "Same message"); + + expect(banners).toHaveLength(1); + }); + + it("shows nothing when a modern deployment reports no announcements", () => { + const banners = normalizeBanners( + appearance({ + service_banner: banner(), + announcement_banners: [], + }), + ); + + expect(banners).toEqual([]); + }); }); describe("banner copy", () => { @@ -113,6 +130,9 @@ describe("banner copy", () => { }); it("truncates long popup messages without splitting emoji", () => { + expect(popupMessage(announcements("a".repeat(120)))).toBe( + `Coder announcement: ${"a".repeat(120)}`, + ); expect(popupMessage(announcements("a".repeat(121)))).toBe( `Coder announcement: ${"a".repeat(119)}…`, ); @@ -120,4 +140,12 @@ describe("banner copy", () => { popupMessage(announcements(`${"a".repeat(118)}🚀${"b".repeat(10)}`)), ).toBe(`Coder announcement: ${"a".repeat(118)}🚀…`); }); + + it("truncates on a word boundary instead of mid-word", () => { + const message = `${"word ".repeat(24)}tail`; + + expect(popupMessage(announcements(message))).toBe( + `Coder announcement: ${"word ".repeat(23).trimEnd()}…`, + ); + }); }); diff --git a/test/unit/announcements/manager.test.ts b/test/unit/announcements/manager.test.ts index 54f79cc4dc..8aa069e691 100644 --- a/test/unit/announcements/manager.test.ts +++ b/test/unit/announcements/manager.test.ts @@ -123,7 +123,7 @@ describe("AnnouncementManager", () => { expectInfo("Coder announcement: Third", "View"); expect( - secretsManager.getSeenBanners(deployment().safeHostname), + secretsManager.getSurfacedBanners(deployment().safeHostname), ).toHaveLength(3); }); @@ -149,9 +149,9 @@ describe("AnnouncementManager", () => { expect(vscode.window.showInformationMessage).not.toHaveBeenCalled(); expect(statusBar.show).toHaveBeenCalled(); expect(statusBar.text).toBe("$(megaphone) Coder"); - expect(secretsManager.getSeenBanners(deployment().safeHostname)).toEqual( - [], - ); + expect( + secretsManager.getSurfacedBanners(deployment().safeHostname), + ).toEqual([]); config.set("coder.disableNotifications", false); await manager.refresh({ notify: true }); @@ -252,6 +252,22 @@ describe("AnnouncementManager", () => { expect(client.getAppearance).toHaveBeenCalledTimes(1); }); + it("does not show an empty picker when banners are cleared before the popup action resolves", async () => { + const { client, session } = setup(); + nextAppearance(client, ["Maintenance tonight"]); + const popupAction = Promise.withResolvers(); + vi.mocked(vscode.window.showInformationMessage).mockReturnValueOnce( + popupAction.promise as never, + ); + + await signIn(session); + session.signOut(null); + popupAction.resolve("View"); + await flushPromises(); + + expect(vscode.window.showQuickPick).not.toHaveBeenCalled(); + }); + it("ignores stale responses from an overlapping refresh", async () => { const { client, manager, session, statusBar } = setup(); await signIn(session); diff --git a/test/unit/core/secretsManager.test.ts b/test/unit/core/secretsManager.test.ts index dda92c1dcb..e8acc7fe37 100644 --- a/test/unit/core/secretsManager.test.ts +++ b/test/unit/core/secretsManager.test.ts @@ -170,32 +170,36 @@ describe("SecretsManager", () => { vi.useRealTimers(); }); - describe("seen banners", () => { - it("stores seen banner keys by safe hostname", async () => { - await secretsManager.setSeenBanners("example.com", ["one", "two"]); - await secretsManager.setSeenBanners("other.com", ["three"]); + describe("surfaced banners", () => { + it("stores surfaced banner keys by safe hostname", async () => { + await secretsManager.setSurfacedBanners("example.com", ["one", "two"]); + await secretsManager.setSurfacedBanners("other.com", ["three"]); - expect(secretsManager.getSeenBanners("example.com")).toEqual([ + expect(secretsManager.getSurfacedBanners("example.com")).toEqual([ "one", "two", ]); - expect(secretsManager.getSeenBanners("other.com")).toEqual(["three"]); + expect(secretsManager.getSurfacedBanners("other.com")).toEqual([ + "three", + ]); }); - it("clears seen banner keys with auth data", async () => { - await secretsManager.setSeenBanners("example.com", ["one"]); - await secretsManager.setSeenBanners("other.com", ["two"]); + it("clears surfaced banner keys with auth data", async () => { + await secretsManager.setSurfacedBanners("example.com", ["one"]); + await secretsManager.setSurfacedBanners("other.com", ["two"]); await secretsManager.clearAllAuthData("example.com"); - expect(secretsManager.getSeenBanners("example.com")).toEqual([]); - expect(secretsManager.getSeenBanners("other.com")).toEqual(["two"]); + expect(secretsManager.getSurfacedBanners("example.com")).toEqual([]); + expect(secretsManager.getSurfacedBanners("other.com")).toEqual(["two"]); }); - it("ignores corrupted seen banner storage", async () => { - await memento.update("coder.seenBanners.example.com", { bad: true }); + it("ignores corrupted surfaced banner storage", async () => { + await memento.update("coder.surfacedBanners.example.com", { + bad: true, + }); - expect(secretsManager.getSeenBanners("example.com")).toEqual([]); + expect(secretsManager.getSurfacedBanners("example.com")).toEqual([]); }); }); }); From 133e4c592b78a63bd588ae4825764eb36a0b2e5a Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 8 Jul 2026 18:43:30 +0300 Subject: [PATCH 08/10] refactor: rework announcement banner UX (markdown preview, progress, colors) Replaces the QuickPick + per-item toast flow with a real Markdown Preview tab (via a TextDocumentContentProvider on an extensionless virtual URI), so the full list renders with working markdown instead of raw syntax. Each banner is boxed in its own div using the deployment's configured background_color when set (matching coder/coder's own dashboard, contrast heuristic included), falling back to the VS Code blockquote theme otherwise; links inside custom-colored boxes inherit the box's text color instead of the theme's link-blue, which can clash. Other changes bundled into this pass: - Status bar hover renders as MarkdownString, capped at 5 banners with a "+N more" pointer to the full preview, lines hard-broken instead of a numbered/bulleted list (indent, implied order). - Popups always show a generic count, never raw (possibly markdown-laden) banner text, since they can't render it and "View" already does. - A warning-colored status bar background flags unsurfaced banners, clearing as soon as the list is opened rather than requiring a fetch. - showAnnouncements() debounces concurrent calls and wraps the load in a window progress indicator. - "seen" terminology renamed to "surfaced" throughout to match what's actually being tracked (shown, not necessarily read). - Fixed a race where a session change mid-refresh could open a blank preview, and centralized error handling so preview/storage failures show a friendly message instead of an unhandled rejection. --- src/announcements/banners.ts | 88 +++++++++---- src/announcements/manager.ts | 151 ++++++++++++++++------ src/announcements/preview.ts | 28 +++++ test/mocks/vscode.runtime.ts | 1 + test/unit/announcements/banners.test.ts | 101 +++++++++++---- test/unit/announcements/manager.test.ts | 160 +++++++++++++++++++----- 6 files changed, 420 insertions(+), 109 deletions(-) create mode 100644 src/announcements/preview.ts diff --git a/src/announcements/banners.ts b/src/announcements/banners.ts index 985d7aa3ab..066f350985 100644 --- a/src/announcements/banners.ts +++ b/src/announcements/banners.ts @@ -5,13 +5,12 @@ import type { BannerConfig, } from "coder/site/src/api/typesGenerated"; -/** Popups are read at a glance; keep the single-banner message short. */ -const POPUP_MESSAGE_MAX_LENGTH = 120; - export interface Announcement { readonly message: string; /** Fingerprint used to track which banners have been surfaced. */ readonly key: string; + /** Admin-configured hex color (e.g. "#d32f2f"), if set and valid. */ + readonly backgroundColor?: string; } /** @@ -30,24 +29,80 @@ export function normalizeBanners( return [...new Map(announcements.map((a) => [a.key, a])).values()]; } +/** Status bar label: a megaphone plus the count when there's more than one. */ export function statusText(count: number): string { return `$(megaphone) Coder${count === 1 ? "" : ` ${count}`}`; } -export function statusTooltip(banners: readonly Announcement[]): string { +/** Hover is a quick glance; cap the list before pointing at the full preview. */ +const HOVER_BANNER_LIMIT = 5; + +/** Hard-breaks lines in one paragraph instead of a list: no indent, no implied order. */ +export function hoverMarkdown(banners: readonly Announcement[]): string { + const shown = banners.slice(0, HOVER_BANNER_LIMIT); + const remaining = banners.length - shown.length; + const list = shown.map((banner) => banner.message).join(" \n"); + return remaining > 0 + ? `${list}\n\n+${remaining} more (click to view all)` + : list; +} + +/** Full markdown for the preview tab: each banner boxed in its own styled div. */ +export function previewMarkdown(banners: readonly Announcement[]): string { + const boxes = banners.map((banner) => announcementBox(banner)); + return [BOX_STYLE_RESET, ...boxes].join("\n\n"); +} + +/** + *

only gets bottom margin, skewing the box's padding. Custom-colored + * boxes also need links to inherit the text color instead of theme-blue. + */ +const BOX_STYLE_RESET = + ""; + +/** Reuses the blockquote theme colors so the box still reads as native VS Code. */ +const BOX_STYLE = + "padding: 10px 14px; margin-bottom: 8px; border-radius: 4px; " + + "background: var(--vscode-textBlockQuote-background); " + + "border-left: 4px solid var(--vscode-textBlockQuote-border);"; + +/** A blank line around the content keeps it as real markdown, not opaque HTML. */ +function announcementBox(banner: Announcement): string { + // Admin colors stay fixed across themes, like coder/coder's dashboard. + const className = banner.backgroundColor + ? "coder-announcement coder-announcement-custom-color" + : "coder-announcement"; + const colorOverride = banner.backgroundColor + ? `background: ${banner.backgroundColor}; border-left-color: ${banner.backgroundColor}; color: ${readableForegroundColor(banner.backgroundColor)};` + : ""; return [ - `Coder deployment announcement${banners.length === 1 ? "" : "s"}`, + `

`, + "", + banner.message, "", - ...banners.map((banner, index) => `${index + 1}. ${banner.message}`), + "
", ].join("\n"); } +/** Mirrors coder/coder's dashboard heuristic so banner colors read the same way. */ +function readableForegroundColor(hexColor: string): string { + const r = parseInt(hexColor.slice(1, 3), 16); + const g = parseInt(hexColor.slice(3, 5), 16); + const b = parseInt(hexColor.slice(5, 7), 16); + const yiq = (r * 299 + g * 587 + b * 114) / 1000; + return yiq >= 128 ? "#000" : "#fff"; +} + +/** Plain text, no markdown, so it's always a generic count, never banner content. */ export function popupMessage(banners: readonly Announcement[]): string { return banners.length === 1 - ? `Coder announcement: ${truncate(banners[0].message)}` + ? "Coder has a new deployment announcement." : `Coder has ${banners.length} new deployment announcements.`; } +const HEX_COLOR = /^#[0-9a-f]{6}$/i; + function toAnnouncement( banner: BannerConfig | undefined, ): Announcement | undefined { @@ -55,22 +110,13 @@ function toAnnouncement( if (!banner?.enabled || !message) { return undefined; } - return { message, key: bannerKey(message) }; + const backgroundColor = + banner.background_color && HEX_COLOR.test(banner.background_color) + ? banner.background_color + : undefined; + return { message, key: bannerKey(message), backgroundColor }; } function bannerKey(message: string): string { return createHash("sha256").update(message).digest("hex").slice(0, 16); } - -/** Truncates to a word boundary when possible; splits on code points so emoji survive. */ -function truncate(message: string): string { - const chars = [...message]; - if (chars.length <= POPUP_MESSAGE_MAX_LENGTH) { - return message; - } - const cut = chars.slice(0, POPUP_MESSAGE_MAX_LENGTH - 1); - const lastSpace = cut.lastIndexOf(" "); - const trimmed = - lastSpace > POPUP_MESSAGE_MAX_LENGTH / 2 ? cut.slice(0, lastSpace) : cut; - return `${trimmed.join("")}…`; -} diff --git a/src/announcements/manager.ts b/src/announcements/manager.ts index e823c63ca5..32f194582c 100644 --- a/src/announcements/manager.ts +++ b/src/announcements/manager.ts @@ -1,16 +1,19 @@ import * as vscode from "vscode"; import { errToStr } from "../api/api-helper"; +import { withProgress } from "../progress"; import { areNotificationsDisabled } from "../settings/notifications"; import { createStatusBarItem } from "../util/statusBar"; import { type Announcement, + hoverMarkdown, normalizeBanners, popupMessage, + previewMarkdown, statusText, - statusTooltip, } from "./banners"; +import { AnnouncementsPreview } from "./preview"; import type { CoderApi } from "../api/coderApi"; import type { SecretsManager } from "../core/secretsManager"; @@ -26,13 +29,16 @@ interface RefreshOptions { readonly showErrors?: boolean; } +/** Fetches, tracks, and surfaces Coder deployment announcement banners. */ export class AnnouncementManager implements vscode.Disposable { private readonly statusBarItem = createStatusBarItem("announcements"); private readonly sessionChangeDisposable: vscode.Disposable; + private readonly preview = new AnnouncementsPreview(); #banners: readonly Announcement[] = []; private fetchGeneration = 0; private refreshTimeout: NodeJS.Timeout | undefined; private disposed = false; + private loadingAnnouncements: Promise | undefined; public constructor( private readonly client: Pick, @@ -69,34 +75,59 @@ export class AnnouncementManager implements vscode.Disposable { } } - public async showAnnouncements(): Promise { - const refreshed = await this.refresh({ notify: false, showErrors: true }); - if (this.banners.length > 0) { - await this.pickAnnouncement(); - } else if (refreshed) { - // On failure the error popup is the whole story. - void vscode.window.showInformationMessage( - "No active Coder announcements.", - ); - } + /** Concurrent calls share one in-flight load, shown via a window progress indicator. */ + public showAnnouncements(): Promise { + this.loadingAnnouncements ??= this.loadAnnouncements().finally(() => { + this.loadingAnnouncements = undefined; + }); + return this.loadingAnnouncements; + } + + private async loadAnnouncements(): Promise { + await withProgress( + { + location: vscode.ProgressLocation.Window, + title: "Loading Coder announcements…", + }, + async () => { + const refreshed = await this.refresh({ + notify: false, + showErrors: true, + }); + if (this.banners.length > 0) { + await this.showAnnouncementsPreview(); + } else if (refreshed) { + // On failure the error popup is the whole story. + void vscode.window.showInformationMessage( + "No active Coder announcements.", + ); + } + }, + ); } + /** Releases listeners/timers and hides the status bar item. */ public dispose(): void { this.disposed = true; this.cancelRefresh(); this.sessionChangeDisposable.dispose(); + this.preview.dispose(); this.banners = []; this.statusBarItem.dispose(); } + /** Re-fetches on sign-in; clears the display on sign-out. */ private onSessionChange(): void { this.cancelRefresh(); - this.banners = []; - if (this.sessionState.current.kind === "signedIn") { - void this.refresh({ notify: true }); + if (this.sessionState.current.kind !== "signedIn") { + this.banners = []; + return; } + // Leaves current banners visible while refresh() below confirms them. + void this.refresh({ notify: true }); } + /** Fetches the latest banners and reconciles popup/surfaced/attention state. */ private async fetch(options: RefreshOptions): Promise { const session = this.sessionState.current; if (session.kind !== "signedIn") { @@ -117,26 +148,74 @@ export class AnnouncementManager implements vscode.Disposable { const banners = normalizeBanners(appearance); this.banners = banners; - const seen = new Set( + const surfacedKeys = new Set( this.secretsManager.getSurfacedBanners(session.deployment.safeHostname), ); - const unseen = banners.filter((banner) => !seen.has(banner.key)); - if (unseen.length === 0) { + const newBanners = banners.filter( + (banner) => !surfacedKeys.has(banner.key), + ); + if (newBanners.length === 0) { + this.setAttentionIndicator(false); return; } const notifiable = !areNotificationsDisabled( vscode.workspace.getConfiguration(), ); if (options.notify && notifiable) { - this.showPopup(unseen); + this.showPopup(newBanners); } // Marks banners as surfaced, not read; suppressed ones notify later. if (!options.notify || notifiable) { + await this.tryMarkSurfaced(banners, surfacedKeys); + } else { + this.setAttentionIndicator(true); + } + } + + /** Like markSurfaced, but logs storage failures instead of throwing. */ + private async tryMarkSurfaced( + banners: readonly Announcement[], + knownSurfacedKeys?: ReadonlySet, + ): Promise { + try { + await this.markSurfaced(banners, knownSurfacedKeys); + } catch (error) { + this.logger.warn("Failed to mark Coder announcements as surfaced", error); + } + } + + /** Marks banners surfaced and clears the attention color; skips redundant writes. */ + private async markSurfaced( + banners: readonly Announcement[], + knownSurfacedKeys?: ReadonlySet, + ): Promise { + const session = this.sessionState.current; + if (session.kind !== "signedIn") { + return; + } + 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, - [...seen, ...unseen.map((banner) => banner.key)], + [...merged], ); } + this.setAttentionIndicator(false); + } + + /** Toggles a warning background so unsurfaced banners stand out in the status bar. */ + private setAttentionIndicator(hasUnsurfaced: boolean): void { + this.statusBarItem.backgroundColor = hasUnsurfaced + ? new vscode.ThemeColor("statusBarItem.warningBackground") + : undefined; } /** The setter syncs the status bar so the two cannot drift apart. */ @@ -151,7 +230,9 @@ export class AnnouncementManager implements vscode.Disposable { return; } this.statusBarItem.text = statusText(banners.length); - this.statusBarItem.tooltip = statusTooltip(banners); + this.statusBarItem.tooltip = new vscode.MarkdownString( + hoverMarkdown(banners), + ); this.statusBarItem.show(); } @@ -162,31 +243,29 @@ export class AnnouncementManager implements vscode.Disposable { ) .then((action) => // Banners are seconds old; skip the refetch. - action === VIEW_ACTION ? this.pickAnnouncement() : undefined, + action === VIEW_ACTION ? this.showAnnouncementsPreview() : undefined, ) .catch((error) => { this.logger.warn("Failed to show Coder announcement popup", error); }); } - /** Skips the picker if banners were cleared (e.g. sign-out) after the popup appeared. */ - private async pickAnnouncement(): Promise { + /** Skips the preview if banners were cleared (e.g. sign-out) before or during marking. */ + private async showAnnouncementsPreview(): Promise { if (this.banners.length === 0) { return; } - const selected = await vscode.window.showQuickPick( - this.banners.map((banner, index) => ({ - label: `$(megaphone) Announcement ${index + 1}`, - detail: banner.message, - banner, - })), - { - title: "Coder Announcements", - placeHolder: "Select an announcement to view the full message", - }, - ); - if (selected) { - void vscode.window.showInformationMessage(selected.banner.message); + await this.tryMarkSurfaced(this.banners); + if (this.banners.length === 0) { + return; + } + try { + await this.preview.show(previewMarkdown(this.banners)); + } catch (error) { + this.logger.warn("Failed to show Coder announcements preview", error); + void vscode.window.showErrorMessage( + `Failed to show Coder announcements: ${errToStr(error)}`, + ); } } diff --git a/src/announcements/preview.ts b/src/announcements/preview.ts new file mode 100644 index 0000000000..f0e89d9230 --- /dev/null +++ b/src/announcements/preview.ts @@ -0,0 +1,28 @@ +import * as vscode from "vscode"; + +const SCHEME = "coder-announcements"; +// No .md extension: the preview renders content as markdown regardless. +const URI = vscode.Uri.parse(`${SCHEME}:Coder Announcements`); + +/** Shows deployment announcements in VS Code's built-in Markdown Preview. */ +export class AnnouncementsPreview implements vscode.Disposable { + private markdown = ""; + private readonly changeEmitter = new vscode.EventEmitter(); + private readonly providerDisposable = + vscode.workspace.registerTextDocumentContentProvider(SCHEME, { + onDidChange: this.changeEmitter.event, + provideTextDocumentContent: () => this.markdown, + }); + + /** Opens (or refreshes, if already open) the preview tab with new content. */ + public async show(markdown: string): Promise { + this.markdown = markdown; + this.changeEmitter.fire(URI); + await vscode.commands.executeCommand("markdown.showPreview", URI); + } + + public dispose(): void { + this.changeEmitter.dispose(); + this.providerDisposable.dispose(); + } +} diff --git a/test/mocks/vscode.runtime.ts b/test/mocks/vscode.runtime.ts index 03dc548d25..816fe4489c 100644 --- a/test/mocks/vscode.runtime.ts +++ b/test/mocks/vscode.runtime.ts @@ -187,6 +187,7 @@ export const workspace = { readDirectory: vi.fn(), }, openTextDocument: vi.fn(), + registerTextDocumentContentProvider: vi.fn(() => ({ dispose: vi.fn() })), onDidChangeConfiguration: onDidChangeConfiguration.event, onDidChangeWorkspaceFolders: onDidChangeWorkspaceFolders.event, diff --git a/test/unit/announcements/banners.test.ts b/test/unit/announcements/banners.test.ts index fb9a985096..a09e885cef 100644 --- a/test/unit/announcements/banners.test.ts +++ b/test/unit/announcements/banners.test.ts @@ -1,10 +1,11 @@ import { describe, expect, it } from "vitest"; import { + hoverMarkdown, normalizeBanners, popupMessage, + previewMarkdown, statusText, - statusTooltip, } from "@/announcements/banners"; import type { @@ -110,42 +111,100 @@ describe("normalizeBanners", () => { }); describe("banner copy", () => { - it("formats status bar text and tooltip", () => { + it("formats status bar text and the hover markdown", () => { const banners = announcements("First", "Second"); expect(statusText(1)).toBe("$(megaphone) Coder"); expect(statusText(2)).toBe("$(megaphone) Coder 2"); - expect(statusTooltip(banners)).toBe( - "Coder deployment announcements\n\n1. First\n2. Second", + expect(hoverMarkdown(banners)).toBe("First \nSecond"); + }); + + it("caps the hover list and points at the full preview", () => { + const banners = announcements("A", "B", "C", "D", "E", "F", "G"); + + expect(hoverMarkdown(banners)).toBe( + "A \nB \nC \nD \nE\n\n+2 more (click to view all)", ); }); - it("formats popup messages", () => { - expect(popupMessage(announcements("Maintenance tonight"))).toBe( - "Coder announcement: Maintenance tonight", + it("boxes each announcement in its own styled div, blank-line separated so markdown still renders inside", () => { + const banners = announcements("First"); + + expect(previewMarkdown(banners)).toMatch( + /^