From 0e6d1a1e9a249c785b6ddf458e896bfc22a41d1f Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 5 May 2026 15:07:04 +0300 Subject: [PATCH 1/2] feat(telemetry): central command dispatcher (#902) Introduce CommandManager so every coder.* registration funnels through one place and inherits command.invoked instrumentation (durationMs + result) via TelemetryService.trace, no per-command wrapping. CoderCommandId is a string-literal union derived from a static array; a unit test pins it to package.json's contributes.commands so drift breaks CI. Wire CommandManager into ServiceContainer so its dispose runs before TelemetryService dispose, letting in-flight events flush. Migrate all 25 vscode.commands.registerCommand("coder.*", ...) sites in extension.ts to commandManager.register(...). Returns are not pushed onto ctx.subscriptions because the manager owns its registrations and is itself disposed via the service container chain. Add a no-restricted-syntax ESLint rule blocking direct registerCommand of coder.* ids, with commandManager.ts carved out, mirroring the existing setContext guard. Declare coder.openDevContainer in package.json contributes.commands (commandPalette when:false to preserve current behavior). It was registered in code but never declared, which the new manifest-parity test would have flagged. --- eslint.config.mjs | 10 +- package.json | 9 ++ src/core/commandManager.ts | 98 ++++++++++++++ src/core/container.ts | 8 ++ src/extension.ts | 178 ++++++++++++-------------- test/unit/core/commandManager.test.ts | 171 +++++++++++++++++++++++++ 6 files changed, 379 insertions(+), 95 deletions(-) create mode 100644 src/core/commandManager.ts create mode 100644 test/unit/core/commandManager.test.ts diff --git a/eslint.config.mjs b/eslint.config.mjs index 2c3289c6..51d2b2e0 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -113,6 +113,12 @@ export default defineConfig( message: "Do not use executeCommand('setContext', ...) directly. Use the ContextManager class instead.", }, + { + selector: + "CallExpression[callee.property.name='registerCommand'][arguments.0.value=/^coder\\./][arguments.length>=2]", + message: + "Do not use registerCommand('coder.*', ...) directly. Use the CommandManager class instead.", + }, ], }, }, @@ -146,9 +152,9 @@ export default defineConfig( }, }, - // Disable no-restricted-syntax for contextManager + // Disable no-restricted-syntax for contextManager and commandManager { - files: ["src/core/contextManager.ts"], + files: ["src/core/contextManager.ts", "src/core/commandManager.ts"], rules: { "no-restricted-syntax": "off", }, diff --git a/package.json b/package.json index e389916f..fea710b3 100644 --- a/package.json +++ b/package.json @@ -367,6 +367,11 @@ "category": "Coder", "icon": "$(play)" }, + { + "command": "coder.openDevContainer", + "title": "Open Dev Container", + "category": "Coder" + }, { "command": "coder.createWorkspace", "title": "Create Workspace", @@ -543,6 +548,10 @@ "command": "coder.openFromSidebar", "when": "false" }, + { + "command": "coder.openDevContainer", + "when": "false" + }, { "command": "coder.searchMyWorkspaces", "when": "false" diff --git a/src/core/commandManager.ts b/src/core/commandManager.ts new file mode 100644 index 00000000..a9e72e8b --- /dev/null +++ b/src/core/commandManager.ts @@ -0,0 +1,98 @@ +import * as vscode from "vscode"; + +import { type TelemetryService } from "../telemetry/service"; + +/** + * Every `coder.*` command id contributed by this extension. Kept in sync with + * `contributes.commands` in package.json by a unit test. + */ +export const CODER_COMMAND_IDS = [ + "coder.login", + "coder.logout", + "coder.switchDeployment", + "coder.open", + "coder.openDevContainer", + "coder.openFromSidebar", + "coder.openAppStatus", + "coder.workspace.update", + "coder.createWorkspace", + "coder.navigateToWorkspace", + "coder.navigateToWorkspaceSettings", + "coder.refreshWorkspaces", + "coder.viewLogs", + "coder.searchMyWorkspaces", + "coder.searchAllWorkspaces", + "coder.manageCredentials", + "coder.applyRecommendedSettings", + "coder.pingWorkspace", + "coder.pingWorkspace:views", + "coder.speedTest", + "coder.speedTest:views", + "coder.supportBundle", + "coder.supportBundle:views", + "coder.tasks.refresh", + "coder.chat.refresh", +] as const; + +export type CoderCommandId = (typeof CODER_COMMAND_IDS)[number]; + +const VALID_IDS: ReadonlySet = new Set(CODER_COMMAND_IDS); + +const COMMAND_INVOKED_EVENT = "command.invoked"; + +// `never[]` parameter type lets any concrete handler shape satisfy the +// constraint, sidestepping function-parameter contravariance. +type CommandHandler = (...args: never[]) => unknown; + +/** + * Single registration point for `coder.*` commands. Wraps every handler in + * `TelemetryService.trace("command.invoked", ...)` so duration plus + * success/error are captured uniformly. + */ +export class CommandManager implements vscode.Disposable { + private readonly registrations = new Set(); + + public constructor(private readonly telemetry: TelemetryService) {} + + public register( + id: CoderCommandId, + handler: CommandHandler, + ): vscode.Disposable { + if (!VALID_IDS.has(id)) { + throw new Error(`Unknown coder command id: ${id}`); + } + + const invoke = handler as (...args: unknown[]) => unknown; + const properties = { commandId: id }; + const wrapped = (...args: unknown[]): Thenable => + this.telemetry.trace( + COMMAND_INVOKED_EVENT, + () => Promise.resolve(invoke(...args)), + properties, + ); + + let live: vscode.Disposable | null = vscode.commands.registerCommand( + id, + wrapped, + ); + this.registrations.add(live); + + return { + dispose: () => { + if (!live) { + return; + } + this.registrations.delete(live); + live.dispose(); + live = null; + }, + }; + } + + public dispose(): void { + for (const inner of this.registrations) { + inner.dispose(); + } + this.registrations.clear(); + } +} diff --git a/src/core/container.ts b/src/core/container.ts index 1b61741e..2b5877dc 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -10,6 +10,7 @@ import { DuplicateWorkspaceIpc } from "../workspace/duplicateWorkspaceIpc"; import { CliCredentialManager } from "./cliCredentialManager"; import { CliManager } from "./cliManager"; +import { CommandManager } from "./commandManager"; import { ContextManager } from "./contextManager"; import { MementoManager } from "./mementoManager"; import { PathResolver } from "./pathResolver"; @@ -34,6 +35,7 @@ export class ServiceContainer implements vscode.Disposable { private readonly oauthCallback: OAuthCallback; private readonly speedtestPanelFactory: SpeedtestPanelFactory; private readonly telemetryService: TelemetryService; + private readonly commandManager: CommandManager; constructor(context: vscode.ExtensionContext) { this.logger = vscode.window.createOutputChannel("Coder", { log: true }); @@ -103,6 +105,7 @@ export class ServiceContainer implements vscode.Disposable { [localJsonlSink], this.logger, ); + this.commandManager = new CommandManager(this.telemetryService); } getPathResolver(): PathResolver { @@ -153,8 +156,13 @@ export class ServiceContainer implements vscode.Disposable { return this.telemetryService; } + getCommandManager(): CommandManager { + return this.commandManager; + } + /** Dispose logger last so telemetry teardown warnings still reach it. */ async dispose(): Promise { + this.commandManager.dispose(); this.contextManager.dispose(); this.loginCoordinator.dispose(); try { diff --git a/src/extension.ts b/src/extension.ts index c8aad5c1..c8714586 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -66,6 +66,7 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { const mementoManager = serviceContainer.getMementoManager(); const secretsManager = serviceContainer.getSecretsManager(); const contextManager = serviceContainer.getContextManager(); + const commandManager = serviceContainer.getCommandManager(); // Migrate auth storage from old flat format to new label-based format await migrateAuthStorage(serviceContainer); @@ -214,14 +215,14 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { tasksPanelProvider, { webviewOptions: { retainContextWhenHidden: true } }, ), - vscode.commands.registerCommand("coder.tasks.refresh", () => - tasksPanelProvider.refresh(), - ), // Refresh tasks panel when deployment changes (login/logout/switch) secretsManager.onDidChangeCurrentDeployment(() => tasksPanelProvider.refresh(), ), ); + commandManager.register("coder.tasks.refresh", () => + tasksPanelProvider.refresh(), + ); // Register Chat embed panel with dependencies const chatPanelProvider = new ChatPanelProvider( @@ -236,9 +237,9 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { chatPanelProvider, { webviewOptions: { retainContextWhenHidden: true } }, ), - vscode.commands.registerCommand("coder.chat.refresh", () => - chatPanelProvider.refresh(), - ), + ); + commandManager.register("coder.chat.refresh", () => + chatPanelProvider.refresh(), ); ctx.subscriptions.push( @@ -248,93 +249,84 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { commands, chatPanelProvider, }), - vscode.commands.registerCommand( - "coder.login", - commands.login.bind(commands), - ), - vscode.commands.registerCommand( - "coder.logout", - commands.logout.bind(commands), - ), - vscode.commands.registerCommand( - "coder.switchDeployment", - commands.switchDeployment.bind(commands), - ), - vscode.commands.registerCommand("coder.open", commands.open.bind(commands)), - vscode.commands.registerCommand( - "coder.openDevContainer", - commands.openDevContainer.bind(commands), - ), - vscode.commands.registerCommand( - "coder.openFromSidebar", - commands.openFromSidebar.bind(commands), - ), - vscode.commands.registerCommand( - "coder.openAppStatus", - commands.openAppStatus.bind(commands), - ), - vscode.commands.registerCommand( - "coder.workspace.update", - commands.updateWorkspace.bind(commands), - ), - vscode.commands.registerCommand( - "coder.createWorkspace", - commands.createWorkspace.bind(commands), - ), - vscode.commands.registerCommand( - "coder.navigateToWorkspace", - commands.navigateToWorkspace.bind(commands), - ), - vscode.commands.registerCommand( - "coder.navigateToWorkspaceSettings", - commands.navigateToWorkspaceSettings.bind(commands), - ), - vscode.commands.registerCommand("coder.refreshWorkspaces", () => { - void myWorkspacesProvider.fetchAndRefresh(); - void allWorkspacesProvider.fetchAndRefresh(); - }), - vscode.commands.registerCommand( - "coder.viewLogs", - commands.viewLogs.bind(commands), - ), - vscode.commands.registerCommand("coder.searchMyWorkspaces", async () => - showTreeViewSearch(MY_WORKSPACES_TREE_ID), - ), - vscode.commands.registerCommand("coder.searchAllWorkspaces", async () => - showTreeViewSearch(ALL_WORKSPACES_TREE_ID), - ), - vscode.commands.registerCommand( - "coder.manageCredentials", - commands.manageCredentials.bind(commands), - ), - vscode.commands.registerCommand( - "coder.applyRecommendedSettings", - commands.applyRecommendedSettings.bind(commands), - ), - vscode.commands.registerCommand( - "coder.pingWorkspace", - commands.pingWorkspace.bind(commands), - ), - vscode.commands.registerCommand( - "coder.pingWorkspace:views", - commands.pingWorkspace.bind(commands), - ), - vscode.commands.registerCommand( - "coder.speedTest", - commands.speedTest.bind(commands), - ), - vscode.commands.registerCommand( - "coder.speedTest:views", - commands.speedTest.bind(commands), - ), - vscode.commands.registerCommand( - "coder.supportBundle", - commands.supportBundle.bind(commands), - ), - vscode.commands.registerCommand( - "coder.supportBundle:views", - commands.supportBundle.bind(commands), - ), + ); + + // Coder commands. The CommandManager owns disposal via ServiceContainer, + // so registrations do not need to be pushed onto ctx.subscriptions. + commandManager.register("coder.login", commands.login.bind(commands)); + commandManager.register("coder.logout", commands.logout.bind(commands)); + commandManager.register( + "coder.switchDeployment", + commands.switchDeployment.bind(commands), + ); + commandManager.register("coder.open", commands.open.bind(commands)); + commandManager.register( + "coder.openDevContainer", + commands.openDevContainer.bind(commands), + ); + commandManager.register( + "coder.openFromSidebar", + commands.openFromSidebar.bind(commands), + ); + commandManager.register( + "coder.openAppStatus", + commands.openAppStatus.bind(commands), + ); + commandManager.register( + "coder.workspace.update", + commands.updateWorkspace.bind(commands), + ); + commandManager.register( + "coder.createWorkspace", + commands.createWorkspace.bind(commands), + ); + commandManager.register( + "coder.navigateToWorkspace", + commands.navigateToWorkspace.bind(commands), + ); + commandManager.register( + "coder.navigateToWorkspaceSettings", + commands.navigateToWorkspaceSettings.bind(commands), + ); + commandManager.register("coder.refreshWorkspaces", () => { + void myWorkspacesProvider.fetchAndRefresh(); + void allWorkspacesProvider.fetchAndRefresh(); + }); + commandManager.register("coder.viewLogs", commands.viewLogs.bind(commands)); + commandManager.register("coder.searchMyWorkspaces", async () => + showTreeViewSearch(MY_WORKSPACES_TREE_ID), + ); + commandManager.register("coder.searchAllWorkspaces", async () => + showTreeViewSearch(ALL_WORKSPACES_TREE_ID), + ); + commandManager.register( + "coder.manageCredentials", + commands.manageCredentials.bind(commands), + ); + commandManager.register( + "coder.applyRecommendedSettings", + commands.applyRecommendedSettings.bind(commands), + ); + commandManager.register( + "coder.pingWorkspace", + commands.pingWorkspace.bind(commands), + ); + commandManager.register( + "coder.pingWorkspace:views", + commands.pingWorkspace.bind(commands), + ); + commandManager.register("coder.speedTest", commands.speedTest.bind(commands)); + commandManager.register( + "coder.speedTest:views", + commands.speedTest.bind(commands), + ); + commandManager.register( + "coder.supportBundle", + commands.supportBundle.bind(commands), + ); + commandManager.register( + "coder.supportBundle:views", + commands.supportBundle.bind(commands), ); const remote = new Remote(serviceContainer, commands, ctx); diff --git a/test/unit/core/commandManager.test.ts b/test/unit/core/commandManager.test.ts new file mode 100644 index 00000000..280604ab --- /dev/null +++ b/test/unit/core/commandManager.test.ts @@ -0,0 +1,171 @@ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { beforeEach, describe, expect, it, vi } from "vitest"; +import * as vscode from "vscode"; + +import { + CODER_COMMAND_IDS, + CommandManager, + type CoderCommandId, +} from "@/core/commandManager"; +import { TelemetryService } from "@/telemetry/service"; + +import { TestSink } from "../../mocks/telemetry"; +import { + createMockLogger, + MockConfigurationProvider, +} from "../../mocks/testHelpers"; + +import type * as vscodeTypes from "vscode"; + +function fakeContext(): vscodeTypes.ExtensionContext { + return { + extension: { packageJSON: { version: "1.2.3-test" } }, + } as unknown as vscodeTypes.ExtensionContext; +} + +interface Harness { + manager: CommandManager; + sink: TestSink; +} + +function makeHarness(): Harness { + const config = new MockConfigurationProvider(); + config.set("coder.telemetry.level", "local"); + const sink = new TestSink(); + const telemetry = new TelemetryService( + fakeContext(), + [sink], + createMockLogger(), + ); + return { manager: new CommandManager(telemetry), sink }; +} + +function getRegisteredCallback( + id: CoderCommandId, +): (...args: unknown[]) => Thenable { + const match = vi + .mocked(vscode.commands.registerCommand) + .mock.calls.find(([cmdId]) => cmdId === id); + if (!match) { + throw new Error(`No registration captured for ${id}`); + } + return match[1] as (...args: unknown[]) => Thenable; +} + +describe("CommandManager", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + describe("register", () => { + it("invokes the handler with forwarded args and resolves with its return value", async () => { + const { manager } = makeHarness(); + const handler = vi.fn((a: number, b: number) => a + b); + + manager.register("coder.refreshWorkspaces", handler); + const result = await getRegisteredCallback("coder.refreshWorkspaces")( + 2, + 3, + ); + + expect(handler).toHaveBeenCalledWith(2, 3); + expect(result).toBe(5); + }); + + it("throws synchronously for an unknown id", () => { + const { manager } = makeHarness(); + + expect(() => + manager.register( + "coder.unknownThing" as CoderCommandId, + () => undefined, + ), + ).toThrow(/Unknown coder command id/); + }); + }); + + describe("telemetry", () => { + it("emits one command.invoked event with success and a numeric durationMs on success", async () => { + const { manager, sink } = makeHarness(); + manager.register("coder.refreshWorkspaces", () => "ok"); + + await getRegisteredCallback("coder.refreshWorkspaces")(); + + expect(sink.events).toHaveLength(1); + expect(sink.events[0]).toMatchObject({ + eventName: "command.invoked", + properties: { + commandId: "coder.refreshWorkspaces", + result: "success", + }, + }); + expect(typeof sink.events[0].measurements.durationMs).toBe("number"); + }); + + it("emits command.invoked with error and rethrows when the handler throws", async () => { + const { manager, sink } = makeHarness(); + const boom = new TypeError("boom"); + manager.register("coder.login", () => { + throw boom; + }); + + await expect(getRegisteredCallback("coder.login")()).rejects.toBe(boom); + + expect(sink.events).toHaveLength(1); + expect(sink.events[0]).toMatchObject({ + eventName: "command.invoked", + properties: { commandId: "coder.login", result: "error" }, + error: { message: "boom", type: "TypeError" }, + }); + }); + }); + + describe("disposal", () => { + it("disposing the returned Disposable unregisters the command exactly once", () => { + const innerDispose = vi.fn(); + vi.mocked(vscode.commands.registerCommand).mockReturnValueOnce({ + dispose: innerDispose, + }); + + const { manager } = makeHarness(); + const disposable = manager.register("coder.login", () => undefined); + + disposable.dispose(); + disposable.dispose(); + + expect(innerDispose).toHaveBeenCalledTimes(1); + }); + + it("manager.dispose() unregisters every command it registered", () => { + const inner1 = vi.fn(); + const inner2 = vi.fn(); + vi.mocked(vscode.commands.registerCommand) + .mockReturnValueOnce({ dispose: inner1 }) + .mockReturnValueOnce({ dispose: inner2 }); + + const { manager } = makeHarness(); + manager.register("coder.login", () => undefined); + manager.register("coder.logout", () => undefined); + + manager.dispose(); + + expect(inner1).toHaveBeenCalledTimes(1); + expect(inner2).toHaveBeenCalledTimes(1); + }); + }); + + it("CODER_COMMAND_IDS matches the coder.* ids declared in package.json", () => { + const pkgPath = join(__dirname, "../../../package.json"); + const pkg = JSON.parse(readFileSync(pkgPath, "utf8")) as { + contributes: { commands: Array<{ command: string }> }; + }; + const declared = pkg.contributes.commands + .map((c) => c.command) + .filter((id) => id.startsWith("coder.")) + .sort(); + const inUnion = [...CODER_COMMAND_IDS].sort(); + + expect(inUnion).toEqual(declared); + }); +}); From 2f905d8fe0b1d718dafd81ee2e2d280105fc8e30 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Wed, 6 May 2026 17:07:09 +0300 Subject: [PATCH 2/2] handle review comments --- src/core/commandManager.ts | 3 +-- src/core/container.ts | 3 ++- src/extension.ts | 14 +++++------ src/telemetry/event.ts | 16 ++++++------ src/telemetry/service.ts | 4 +-- test/unit/core/commandManager.test.ts | 10 +------- test/unit/telemetry/event.test.ts | 35 ++++++++++++++------------- test/unit/telemetry/service.test.ts | 12 +++------ 8 files changed, 42 insertions(+), 55 deletions(-) diff --git a/src/core/commandManager.ts b/src/core/commandManager.ts index a9e72e8b..241183c1 100644 --- a/src/core/commandManager.ts +++ b/src/core/commandManager.ts @@ -40,8 +40,7 @@ const VALID_IDS: ReadonlySet = new Set(CODER_COMMAND_IDS); const COMMAND_INVOKED_EVENT = "command.invoked"; -// `never[]` parameter type lets any concrete handler shape satisfy the -// constraint, sidestepping function-parameter contravariance. +// `never[]` accepts any concrete handler shape (function-parameter contravariance). type CommandHandler = (...args: never[]) => unknown; /** diff --git a/src/core/container.ts b/src/core/container.ts index 2b5877dc..c3f3bb0a 100644 --- a/src/core/container.ts +++ b/src/core/container.ts @@ -3,6 +3,7 @@ import * as vscode from "vscode"; import { CoderApi } from "../api/coderApi"; import { LoginCoordinator } from "../login/loginCoordinator"; import { OAuthCallback } from "../oauth/oauthCallback"; +import { extractExtensionVersion } from "../telemetry/event"; import { TelemetryService } from "../telemetry/service"; import { LocalJsonlSink } from "../telemetry/sinks/localJsonlSink"; import { SpeedtestPanelFactory } from "../webviews/speedtest/speedtestPanelFactory"; @@ -101,7 +102,7 @@ export class ServiceContainer implements vscode.Disposable { this.logger, ); this.telemetryService = new TelemetryService( - context, + extractExtensionVersion(context.extension.packageJSON), [localJsonlSink], this.logger, ); diff --git a/src/extension.ts b/src/extension.ts index c8714586..0ba96aff 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -208,6 +208,9 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { client, output, ); + commandManager.register("coder.tasks.refresh", () => + tasksPanelProvider.refresh(), + ); ctx.subscriptions.push( tasksPanelProvider, vscode.window.registerWebviewViewProvider( @@ -220,9 +223,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { tasksPanelProvider.refresh(), ), ); - commandManager.register("coder.tasks.refresh", () => - tasksPanelProvider.refresh(), - ); // Register Chat embed panel with dependencies const chatPanelProvider = new ChatPanelProvider( @@ -230,6 +230,9 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { client, output, ); + commandManager.register("coder.chat.refresh", () => + chatPanelProvider.refresh(), + ); ctx.subscriptions.push( chatPanelProvider, vscode.window.registerWebviewViewProvider( @@ -238,9 +241,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { { webviewOptions: { retainContextWhenHidden: true } }, ), ); - commandManager.register("coder.chat.refresh", () => - chatPanelProvider.refresh(), - ); ctx.subscriptions.push( registerUriHandler({ @@ -251,8 +251,6 @@ export async function activate(ctx: vscode.ExtensionContext): Promise { }), ); - // Coder commands. The CommandManager owns disposal via ServiceContainer, - // so registrations do not need to be pushed onto ctx.subscriptions. commandManager.register("coder.login", commands.login.bind(commands)); commandManager.register("coder.logout", commands.logout.bind(commands)); commandManager.register( diff --git a/src/telemetry/event.ts b/src/telemetry/event.ts index 068b6c01..d9608763 100644 --- a/src/telemetry/event.ts +++ b/src/telemetry/event.ts @@ -68,13 +68,8 @@ export interface TelemetrySink { dispose(): Promise; } -/** Build session attributes. `extensionVersion` falls back to `"unknown"`. */ -export function buildSession(ctx: vscode.ExtensionContext): SessionContext { - // "unknown" only for malformed package.json or test fixtures missing `version`. - const packageJson = ctx.extension.packageJSON as { version?: unknown }; - const extensionVersion = - typeof packageJson.version === "string" ? packageJson.version : "unknown"; - +/** Build session attributes from the extension version and ambient host data. */ +export function buildSession(extensionVersion: string): SessionContext { return { extensionVersion, machineId: vscode.env.machineId, @@ -87,6 +82,13 @@ export function buildSession(ctx: vscode.ExtensionContext): SessionContext { }; } +/** Read `version` from a package.json-like object, falling back to `"unknown"`. */ +export function extractExtensionVersion(packageJSON: unknown): string { + const version = (packageJSON as { version?: unknown } | null | undefined) + ?.version; + return typeof version === "string" ? version : "unknown"; +} + /** Normalize a thrown value into the event's `error` block. */ export function buildErrorBlock( value: unknown, diff --git a/src/telemetry/service.ts b/src/telemetry/service.ts index 00efd011..f6d06785 100644 --- a/src/telemetry/service.ts +++ b/src/telemetry/service.ts @@ -52,11 +52,11 @@ export class TelemetryService implements vscode.Disposable { readonly #configWatcher: vscode.Disposable; public constructor( - ctx: vscode.ExtensionContext, + extensionVersion: string, private readonly sinks: readonly TelemetrySink[], private readonly logger: Logger, ) { - this.#session = buildSession(ctx); + this.#session = buildSession(extensionVersion); this.#level = readLevel(); this.#configWatcher = watchConfigurationChanges( [{ setting: TELEMETRY_LEVEL_SETTING, getValue: readLevel }], diff --git a/test/unit/core/commandManager.test.ts b/test/unit/core/commandManager.test.ts index 280604ab..1a05803c 100644 --- a/test/unit/core/commandManager.test.ts +++ b/test/unit/core/commandManager.test.ts @@ -16,14 +16,6 @@ import { MockConfigurationProvider, } from "../../mocks/testHelpers"; -import type * as vscodeTypes from "vscode"; - -function fakeContext(): vscodeTypes.ExtensionContext { - return { - extension: { packageJSON: { version: "1.2.3-test" } }, - } as unknown as vscodeTypes.ExtensionContext; -} - interface Harness { manager: CommandManager; sink: TestSink; @@ -34,7 +26,7 @@ function makeHarness(): Harness { config.set("coder.telemetry.level", "local"); const sink = new TestSink(); const telemetry = new TelemetryService( - fakeContext(), + "1.2.3-test", [sink], createMockLogger(), ); diff --git a/test/unit/telemetry/event.test.ts b/test/unit/telemetry/event.test.ts index 1ad5d455..12057ae3 100644 --- a/test/unit/telemetry/event.test.ts +++ b/test/unit/telemetry/event.test.ts @@ -1,18 +1,14 @@ import { describe, expect, it } from "vitest"; -import { buildErrorBlock, buildSession } from "@/telemetry/event"; - -import type * as vscode from "vscode"; - -function fakeContext(version: unknown = "1.2.3-test"): vscode.ExtensionContext { - return { - extension: { packageJSON: { version } }, - } as unknown as vscode.ExtensionContext; -} +import { + buildErrorBlock, + buildSession, + extractExtensionVersion, +} from "@/telemetry/event"; describe("buildSession", () => { - it("populates session-stable fields from the extension context, vscode env, and host", () => { - const session = buildSession(fakeContext()); + it("populates session-stable fields from the version, vscode env, and host", () => { + const session = buildSession("1.2.3-test"); expect(session).toMatchObject({ extensionVersion: "1.2.3-test", @@ -27,13 +23,18 @@ describe("buildSession", () => { ); expect(session.osVersion).toBeTruthy(); }); +}); + +describe("extractExtensionVersion", () => { + it("returns the version when packageJSON.version is a string", () => { + expect(extractExtensionVersion({ version: "4.5.6" })).toBe("4.5.6"); + }); - it("uses the 'unknown' sentinel when packageJSON.version is missing or non-string", () => { - const noVersion = { - extension: { packageJSON: {} }, - } as unknown as vscode.ExtensionContext; - expect(buildSession(noVersion).extensionVersion).toBe("unknown"); - expect(buildSession(fakeContext(123)).extensionVersion).toBe("unknown"); + it("falls back to 'unknown' when packageJSON is missing, malformed, or non-string", () => { + expect(extractExtensionVersion({})).toBe("unknown"); + expect(extractExtensionVersion({ version: 123 })).toBe("unknown"); + expect(extractExtensionVersion(null)).toBe("unknown"); + expect(extractExtensionVersion(undefined)).toBe("unknown"); }); }); diff --git a/test/unit/telemetry/service.test.ts b/test/unit/telemetry/service.test.ts index 8bcafae2..dfcdd0fa 100644 --- a/test/unit/telemetry/service.test.ts +++ b/test/unit/telemetry/service.test.ts @@ -8,15 +8,9 @@ import { MockConfigurationProvider, } from "../../mocks/testHelpers"; -import type * as vscode from "vscode"; - import type { TelemetrySink } from "@/telemetry/event"; -function fakeContext(): vscode.ExtensionContext { - return { - extension: { packageJSON: { version: "1.2.3-test" } }, - } as unknown as vscode.ExtensionContext; -} +const TEST_VERSION = "1.2.3-test"; interface Harness { service: TelemetryService; @@ -29,7 +23,7 @@ function makeHarness(level: "off" | "local" = "local"): Harness { config.set("coder.telemetry.level", level); const sink = new TestSink(); const service = new TelemetryService( - fakeContext(), + TEST_VERSION, [sink], createMockLogger(), ); @@ -38,7 +32,7 @@ function makeHarness(level: "off" | "local" = "local"): Harness { function makeService(sinks: TelemetrySink[]): TelemetryService { new MockConfigurationProvider().set("coder.telemetry.level", "local"); - return new TelemetryService(fakeContext(), sinks, createMockLogger()); + return new TelemetryService(TEST_VERSION, sinks, createMockLogger()); } describe("TelemetryService", () => {