refactor: consistent credential API, lightweight binary lookup, quieter logging

Make storeToken resolve its own binary internally via the injected
BinaryResolver, matching readToken and deleteToken. Remove the binPath
parameter from both storeToken and configure since callers no longer
need to supply it.

Add locateBinary to CliManager as a cheap stat-only lookup that the
container resolver tries before falling back to fetchBinary.

Downgrade implementation-detail log messages from info/warn to debug,
keeping info for significant events (server version, download start,
stored/deleted token).

Author: EhabY

src/core/cliCredentialManager.ts

@@ -1,6 +1,7 @@
 import { execFile } from "node:child_process";
 import { promisify } from "node:util";
 
+import { isKeyringEnabled } from "../cliConfig";
 import { getHeaderArgs } from "../headers";
 
 import type { WorkspaceConfiguration } from "vscode";
@@ -23,10 +24,8 @@ export function isKeyringSupported(): boolean {
 }
 
 /**
- * Delegates credential storage to the Coder CLI to keep the credentials in sync.
- *
- * For operations that don't have a binary path available (readToken, deleteToken),
- * uses the injected BinaryResolver to fetch/locate the CLI binary.
+ * Delegates credential storage to the Coder CLI. All operations resolve the
+ * binary via the injected BinaryResolver before invoking it.
  */
 export class CliCredentialManager {
 	constructor(
@@ -35,18 +34,22 @@ export class CliCredentialManager {
 	) {}
 
 	/**
-	 * Store a token by running:
-	 *   CODER_SESSION_TOKEN=<token> <bin> login --use-token-as-session <url>
-	 *
-	 * The token is passed via environment variable so it never appears in
-	 * process argument lists.
+	 * Store a token via `coder login --use-token-as-session`.
+	 * Token is passed via CODER_SESSION_TOKEN env var, never in args.
 	 */
 	public async storeToken(
-		binPath: string,
 		url: string,
 		token: string,
 		configs: Pick<WorkspaceConfiguration, "get">,
 	): Promise<void> {
+		let binPath: string;
+		try {
+			binPath = await this.resolveBinary(url);
+		} catch (error) {
+			this.logger.debug("Could not resolve CLI binary for token store:", error);
+			throw error;
+		}
+
 		const args = [
 			...getHeaderArgs(configs),
 			"login",
@@ -59,27 +62,28 @@ export class CliCredentialManager {
 			});
 			this.logger.info("Stored token via CLI for", url);
 		} catch (error) {
-			this.logger.error("Failed to store token via CLI:", error);
+			this.logger.debug("Failed to store token via CLI:", error);
 			throw error;
 		}
 	}
 
 	/**
-	 * Read a token by running:
-	 *   <bin> login token --url <url>
-	 *
-	 * Resolves the CLI binary automatically. Returns trimmed stdout,
-	 * or undefined if the binary can't be resolved or the CLI returns no token.
+	 * Read a token via `coder login token --url`. Returns trimmed stdout,
+	 * or undefined on any failure (resolver, CLI, empty output).
 	 */
 	public async readToken(
 		url: string,
 		configs: Pick<WorkspaceConfiguration, "get">,
 	): Promise<string | undefined> {
+		if (!isKeyringEnabled(configs)) {
+			return undefined;
+		}
+
 		let binPath: string;
 		try {
 			binPath = await this.resolveBinary(url);
 		} catch (error) {
-			this.logger.warn("Could not resolve CLI binary for token read:", error);
+			this.logger.debug("Could not resolve CLI binary for token read:", error);
 			return undefined;
 		}
 
@@ -89,27 +93,30 @@ export class CliCredentialManager {
 			const token = stdout.trim();
 			return token || undefined;
 		} catch (error) {
-			this.logger.warn("Failed to read token via CLI:", error);
+			this.logger.debug("Failed to read token via CLI:", error);
 			return undefined;
 		}
 	}
 
 	/**
-	 * Delete a token by running:
-	 *   <bin> logout --url <url>
-	 *
-	 * Resolves the CLI binary automatically. Best-effort: logs warnings
-	 * on failure but never throws.
+	 * Delete a token via `coder logout --url`. Best-effort: never throws.
 	 */
-	async deleteToken(
+	public async deleteToken(
 		url: string,
 		configs: Pick<WorkspaceConfiguration, "get">,
 	): Promise<void> {
+		if (!isKeyringEnabled(configs)) {
+			return;
+		}
+
 		let binPath: string;
 		try {
 			binPath = await this.resolveBinary(url);
 		} catch (error) {
-			this.logger.warn("Could not resolve CLI binary for token delete:", error);
+			this.logger.debug(
+				"Could not resolve CLI binary for token delete:",
+				error,
+			);
 			return;
 		}
 
@@ -118,7 +125,7 @@ export class CliCredentialManager {
 			await execFileAsync(binPath, args);
 			this.logger.info("Deleted token via CLI for", url);
 		} catch (error) {
-			this.logger.warn("Failed to delete token via CLI:", error);
+			this.logger.debug("Failed to delete token via CLI:", error);
 		}
 	}
 }

src/core/cliManager.ts

@@ -10,7 +10,7 @@ import * as semver from "semver";
 import * as vscode from "vscode";
 
 import { errToStr } from "../api/api-helper";
-import { isKeyringEnabled, shouldUseKeyring } from "../cliConfig";
+import { shouldUseKeyring } from "../cliConfig";
 import * as pgp from "../pgp";
 import { toSafeHost } from "../util";
 import { vscodeProposed } from "../vscodeProposed";
@@ -39,6 +39,23 @@ export class CliManager {
 		this.binaryLock = new BinaryLock(output);
 	}
 
+	/**
+	 * Return the path to a cached CLI binary for a deployment URL.
+	 * Stat check only — no network, no subprocess. Throws if absent.
+	 */
+	public async locateBinary(url: string): Promise<string> {
+		const safeHostname = toSafeHost(url);
+		const binPath = path.join(
+			this.pathResolver.getBinaryCachePath(safeHostname),
+			cliUtils.name(),
+		);
+		const stat = await cliUtils.stat(binPath);
+		if (!stat) {
+			throw new Error(`No CLI binary found at ${binPath}`);
+		}
+		return binPath;
+	}
+
 	/**
 	 * Download and return the path to a working binary for the deployment with
 	 * the provided hostname using the provided client.  If the hostname is empty,
@@ -60,7 +77,10 @@ export class CliManager {
 		// Settings can be undefined when set to their defaults (true in this case),
 		// so explicitly check against false.
 		const enableDownloads = cfg.get("enableDownloads") !== false;
-		this.output.info("Downloads are", enableDownloads ? "enabled" : "disabled");
+		this.output.debug(
+			"Downloads are",
+			enableDownloads ? "enabled" : "disabled",
+		);
 
 		// Get the build info to compare with the existing binary version, if any,
 		// and to log for debugging.
@@ -80,18 +100,18 @@ export class CliManager {
 			this.pathResolver.getBinaryCachePath(safeHostname),
 			cliUtils.name(),
 		);
-		this.output.info("Using binary path", binPath);
+		this.output.debug("Using binary path", binPath);
 		const stat = await cliUtils.stat(binPath);
 		if (stat === undefined) {
 			this.output.info("No existing binary found, starting download");
 		} else {
-			this.output.info("Existing binary size is", prettyBytes(stat.size));
+			this.output.debug("Existing binary size is", prettyBytes(stat.size));
 			try {
 				const version = await cliUtils.version(binPath);
-				this.output.info("Existing binary version is", version);
+				this.output.debug("Existing binary version is", version);
 				// If we have the right version we can avoid the request entirely.
 				if (version === buildInfo.version) {
-					this.output.info(
+					this.output.debug(
 						"Using existing binary since it matches the server version",
 					);
 					return binPath;
@@ -130,19 +150,19 @@ export class CliManager {
 				binPath,
 				progressLogPath,
 			);
-			this.output.info("Acquired download lock");
+			this.output.debug("Acquired download lock");
 
 			// If we waited for another process, re-check if binary is now ready
 			if (lockResult.waited) {
 				const latestBuildInfo = await restClient.getBuildInfo();
-				this.output.info("Got latest server version", latestBuildInfo.version);
+				this.output.debug("Got latest server version", latestBuildInfo.version);
 
 				const recheckAfterWait = await this.checkBinaryVersion(
 					binPath,
 					latestBuildInfo.version,
 				);
 				if (recheckAfterWait.matches) {
-					this.output.info(
+					this.output.debug(
 						"Using existing binary since it matches the latest server version",
 					);
 					return binPath;
@@ -174,7 +194,7 @@ export class CliManager {
 		} finally {
 			if (lockResult) {
 				await lockResult.release();
-				this.output.info("Released download lock");
+				this.output.debug("Released download lock");
 			}
 		}
 	}
@@ -721,12 +741,7 @@ export class CliManager {
 	 * Both URL and token are required. Empty tokens are allowed (e.g. mTLS
 	 * authentication) but the URL must be a non-empty string.
 	 */
-	public async configure(
-		url: string,
-		token: string,
-		featureSet: FeatureSet,
-		binPath: string,
-	) {
+	public async configure(url: string, token: string, featureSet: FeatureSet) {
 		if (!url) {
 			throw new Error("URL is required to configure the CLI");
 		}
@@ -735,12 +750,7 @@ export class CliManager {
 		const configs = vscode.workspace.getConfiguration();
 		if (shouldUseKeyring(configs, featureSet)) {
 			try {
-				await this.cliCredentialManager.storeToken(
-					binPath,
-					url,
-					token,
-					configs,
-				);
+				await this.cliCredentialManager.storeToken(url, token, configs);
 			} catch (error) {
 				this.output.error("Failed to store token via CLI keyring:", error);
 				vscode.window
@@ -775,9 +785,7 @@ export class CliManager {
 	public async clearCredentials(url: string): Promise<void> {
 		const safeHostname = toSafeHost(url);
 		const configs = vscode.workspace.getConfiguration();
-		if (isKeyringEnabled(configs)) {
-			await this.cliCredentialManager.deleteToken(url, configs);
-		}
+		await this.cliCredentialManager.deleteToken(url, configs);
 
 		const tokenPath = this.pathResolver.getSessionTokenPath(safeHostname);
 		const urlPath = this.pathResolver.getUrlPath(safeHostname);

src/core/container.ts

@@ -37,11 +37,17 @@ export class ServiceContainer implements vscode.Disposable {
 			context.globalState,
 			this.logger,
 		);
+		// Circular ref: cliCredentialManager ↔ cliManager. Safe because
+		// the resolver is only called after construction.
 		this.cliCredentialManager = new CliCredentialManager(
 			this.logger,
 			async (url) => {
-				const client = CoderApi.create(url, "", this.logger);
-				return this.cliManager.fetchBinary(client);
+				try {
+					return await this.cliManager.locateBinary(url);
+				} catch {
+					const client = CoderApi.create(url, "", this.logger);
+					return this.cliManager.fetchBinary(client);
+				}
 			},
 		);
 		this.cliManager = new CliManager(

src/login/loginCoordinator.ts

@@ -243,7 +243,10 @@ export class LoginCoordinator implements vscode.Disposable {
 		}
 
 		// Try keyring token (picks up tokens written by `coder login` in the terminal)
-		const keyringToken = await this.getCliKeyringToken(deployment);
+		const keyringToken = await this.cliCredentialManager.readToken(
+			deployment.url,
+			vscode.workspace.getConfiguration(),
+		);
 		if (
 			keyringToken &&
 			keyringToken !== providedToken &&
@@ -302,18 +305,6 @@ export class LoginCoordinator implements vscode.Disposable {
 		}
 	}
 
-	/**
-	 * Read a token from the CLI keyring.
-	 */
-	private async getCliKeyringToken(
-		deployment: Deployment,
-	): Promise<string | undefined> {
-		return this.cliCredentialManager.readToken(
-			deployment.url,
-			vscode.workspace.getConfiguration(),
-		);
-	}
-
 	/**
 	 * Shows auth error via dialog or logs it for autoLogin.
 	 */

src/remote/remote.ts

@@ -241,12 +241,7 @@ export class Remote {
 
 			// Write token to keyring or file (after CLI version is known)
 			if (baseUrlRaw && token !== undefined) {
-				await this.cliManager.configure(
-					baseUrlRaw,
-					token,
-					featureSet,
-					binaryPath,
-				);
+				await this.cliManager.configure(baseUrlRaw, token, featureSet);
 			}
 
 			// Listen for token changes for this deployment
@@ -261,7 +256,6 @@ export class Remote {
 									auth.url,
 									auth.token,
 									featureSet,
-									binaryPath,
 								);
 								this.logger.info(
 									"Updated CLI config with new token for remote deployment",