Review comments 2

Author: EhabY

package.json

@@ -316,7 +316,6 @@
 		"eventsource": "^3.0.6",
 		"find-process": "https://github.com/coder/find-process#fix/sequoia-compat",
 		"jsonc-parser": "^3.3.1",
-		"memfs": "^4.17.1",
 		"node-forge": "^1.3.1",
 		"openpgp": "^6.2.0",
 		"pretty-bytes": "^7.0.0",
@@ -351,6 +350,7 @@
 		"eslint-plugin-prettier": "^5.4.1",
 		"glob": "^10.4.2",
 		"jsonc-eslint-parser": "^2.4.0",
+		"memfs": "^4.46.0",
 		"nyc": "^17.1.0",
 		"prettier": "^3.5.3",
 		"ts-loader": "^9.5.1",

src/__mocks__/testHelpers.ts

@@ -213,3 +213,62 @@ export class MockUserInteraction {
 		);
 	}
 }
+
+// Simple in-memory implementation of Memento
+export class InMemoryMemento implements vscode.Memento {
+	private storage = new Map<string, unknown>();
+
+	get<T>(key: string): T | undefined;
+	get<T>(key: string, defaultValue: T): T;
+	get<T>(key: string, defaultValue?: T): T | undefined {
+		return this.storage.has(key) ? (this.storage.get(key) as T) : defaultValue;
+	}
+
+	async update(key: string, value: unknown): Promise<void> {
+		if (value === undefined) {
+			this.storage.delete(key);
+		} else {
+			this.storage.set(key, value);
+		}
+		return Promise.resolve();
+	}
+
+	keys(): readonly string[] {
+		return Array.from(this.storage.keys());
+	}
+}
+
+// Simple in-memory implementation of SecretStorage
+export class InMemorySecretStorage implements vscode.SecretStorage {
+	private secrets = new Map<string, string>();
+	private isCorrupted = false;
+
+	onDidChange: vscode.Event<vscode.SecretStorageChangeEvent> = () => ({
+		dispose: () => {},
+	});
+
+	async get(key: string): Promise<string | undefined> {
+		if (this.isCorrupted) {
+			return Promise.reject(new Error("Storage corrupted"));
+		}
+		return this.secrets.get(key);
+	}
+
+	async store(key: string, value: string): Promise<void> {
+		if (this.isCorrupted) {
+			return Promise.reject(new Error("Storage corrupted"));
+		}
+		this.secrets.set(key, value);
+	}
+
+	async delete(key: string): Promise<void> {
+		if (this.isCorrupted) {
+			return Promise.reject(new Error("Storage corrupted"));
+		}
+		this.secrets.delete(key);
+	}
+
+	corruptStorage(): void {
+		this.isCorrupted = true;
+	}
+}

src/cliManager.test.ts src/cliUtils.test.tssrc/cliManager.test.ts renamed to src/cliUtils.test.ts

@@ -2,9 +2,9 @@ import fs from "fs/promises";
 import os from "os";
 import path from "path";
 import { beforeAll, describe, expect, it } from "vitest";
-import * as cli from "./cliManager";
+import * as cli from "./cliUtils";
 
-describe("cliManager", () => {
+describe("cliUtils", () => {
 	const tmp = path.join(os.tmpdir(), "vscode-coder-tests");
 
 	beforeAll(async () => {
@@ -25,14 +25,6 @@ describe("cliManager", () => {
 		expect((await cli.stat(binPath))?.size).toBe(4);
 	});
 
-	it("rm", async () => {
-		const binPath = path.join(tmp, "rm");
-		await cli.rm(binPath);
-
-		await fs.writeFile(binPath, "test");
-		await cli.rm(binPath);
-	});
-
 	// TODO: CI only runs on Linux but we should run it on Windows too.
 	it("version", async () => {
 		const binPath = path.join(tmp, "version");

src/cliManager.ts src/cliUtils.tssrc/cliManager.ts renamed to src/cliUtils.ts

@@ -21,20 +21,6 @@ export async function stat(binPath: string): Promise<Stats | undefined> {
 	}
 }
 
-/**
- * Remove the path.  Throw if unable to remove.
- */
-export async function rm(binPath: string): Promise<void> {
-	try {
-		await fs.rm(binPath, { force: true });
-	} catch (error) {
-		// Just in case; we should never get an ENOENT because of force: true.
-		if ((error as NodeJS.ErrnoException)?.code !== "ENOENT") {
-			throw error;
-		}
-	}
-}
-
 // util.promisify types are dynamic so there is no concrete type we can import
 // and we have to make our own.
 type ExecException = ExecFileException & { stdout?: string; stderr?: string };

src/commands.ts

@@ -9,8 +9,7 @@ import * as vscode from "vscode";
 import { createWorkspaceIdentifier, extractAgents } from "./api/api-helper";
 import { CoderApi } from "./api/coderApi";
 import { needToken } from "./api/utils";
-import { BinaryManager } from "./core/binaryManager";
-import { CliConfigManager } from "./core/cliConfig";
+import { CliManager } from "./core/cliManager";
 import { MementoManager } from "./core/mementoManager";
 import { PathResolver } from "./core/pathResolver";
 import { SecretsManager } from "./core/secretsManager";
@@ -25,7 +24,6 @@ import {
 } from "./workspacesProvider";
 
 export class Commands {
-	private readonly cliConfigManager: CliConfigManager;
 	// These will only be populated when actively connected to a workspace and are
 	// used in commands.  Because commands can be executed by the user, it is not
 	// possible to pass in arguments, so we have to store the current workspace
@@ -44,10 +42,8 @@ export class Commands {
 		private readonly pathResolver: PathResolver,
 		private readonly mementoManager: MementoManager,
 		private readonly secretsManager: SecretsManager,
-		private readonly binaryManager: BinaryManager,
-	) {
-		this.cliConfigManager = new CliConfigManager(pathResolver);
-	}
+		private readonly cliManager: CliManager,
+	) {}
 
 	/**
 	 * Find the requested agent if specified, otherwise return the agent if there
@@ -209,7 +205,7 @@ export class Commands {
 		await this.secretsManager.setSessionToken(res.token);
 
 		// Store on disk to be used by the cli.
-		await this.cliConfigManager.configure(label, url, res.token);
+		await this.cliManager.configure(label, url, res.token);
 
 		// These contexts control various menu items and the sidebar.
 		await vscode.commands.executeCommand(
@@ -515,7 +511,7 @@ export class Commands {
 					if (!url) {
 						throw new Error("No coder url found for sidebar");
 					}
-					const binary = await this.binaryManager.fetchBinary(
+					const binary = await this.cliManager.fetchBinary(
 						this.restClient,
 						toSafeHost(url),
 					);