refactor: address review feedback on global-config

- rename FeatureSet.keyringTokenRead to tokenRead (token subcommand works for both stores)
- rename resolveUiUrl to resolveCoderDashboardUrl
- clarify openInBrowser docstring (connectionUrl is arbitrary)
- split buildGlobalFlags on auth mode to drop the nested ternary

Author: EhabY

src/core/cliCredentialManager.ts

@@ -257,11 +257,11 @@ export class CliCredentialManager {
 			return { kind: "none" };
 		}
 		if (isKeyringEnabled(configs) && cli.featureSet.keyringAuth) {
-			return cli.featureSet.keyringTokenRead
+			return cli.featureSet.tokenRead
 				? { kind: "keyring", binPath: cli.binPath }
 				: { kind: "none" };
 		}
-		if (cli.featureSet.keyringTokenRead) {
+		if (cli.featureSet.tokenRead) {
 			return cliFileTransport(cli);
 		}
 		return { kind: "none" };
@@ -338,7 +338,7 @@ function cliFileTransport(cli: {
 	return {
 		kind: "cli-file",
 		binPath: cli.binPath,
-		allowOverride: cli.featureSet.keyringTokenRead,
+		allowOverride: cli.featureSet.tokenRead,
 	};
 }
 

src/featureSet.ts

@@ -7,7 +7,7 @@ export interface FeatureSet {
 	buildReason: boolean;
 	cliUpdate: boolean;
 	keyringAuth: boolean;
-	keyringTokenRead: boolean;
+	tokenRead: boolean;
 	supportBundle: boolean;
 }
 
@@ -45,8 +45,8 @@ export function featureSetForVersion(
 		cliUpdate: versionAtLeast(version, "2.24.0"),
 		// Keyring-backed token storage via `coder login`
 		keyringAuth: versionAtLeast(version, "2.29.0"),
-		// `coder login token` for reading tokens from the keyring
-		keyringTokenRead: versionAtLeast(version, "2.31.0"),
+		// `coder login token` for reading tokens (keyring or file)
+		tokenRead: versionAtLeast(version, "2.31.0"),
 		// `coder support bundle` (officially released/unhidden in 2.10.0)
 		supportBundle: versionAtLeast(version, "2.10.0"),
 	};

src/oauth/authorizer.ts

@@ -1,7 +1,7 @@
 import * as vscode from "vscode";
 
 import { CoderApi } from "../api/coderApi";
-import { resolveUiUrl } from "../util/uri";
+import { resolveCoderDashboardUrl } from "../util/uri";
 
 import {
 	AUTH_GRANT_TYPE,
@@ -382,7 +382,7 @@ function toBrowserAuthorizationUrl(
 ): URL {
 	const endpoint = new URL(authorizationEndpoint);
 	const connectionBase = new URL(connectionUrl);
-	const browserBase = new URL(resolveUiUrl(connectionUrl));
+	const browserBase = new URL(resolveCoderDashboardUrl(connectionUrl));
 	const connectionPrefix = connectionBase.pathname.replace(/\/$/, "");
 	const browserPrefix = browserBase.pathname.replace(/\/$/, "");
 	const underConnection =

src/settings/cli.ts

@@ -52,26 +52,25 @@ function buildGlobalFlags(
 	escHeader: (s: string) => string,
 ): string[] {
 	const userFlags = getExpandedUserGlobalFlags(configs);
+	const headers = getHeaderArgs(configs, escHeader);
 
-	// Honor a user `--global-config` only when allowOverride (file mode, 2.31+);
-	// otherwise strip it and emit our own so it matches where we read/write.
+	// Escape after stripping so expansion whitespace stays in one shell token.
+	const cleanUserFlags = (stripGlobalConfig: boolean) =>
+		stripManagedFlags(userFlags, stripGlobalConfig).map(escAuth);
+
+	// Keyring mode: --url auth; drop user --global-config (would force file storage).
+	if (auth.mode === "url") {
+		return [...cleanUserFlags(true), "--url", escAuth(auth.url), ...headers];
+	}
+
+	// File mode: keep the user's --global-config on 2.31+, else emit our own.
 	const honorOverride =
-		auth.mode === "global-config" &&
 		auth.allowOverride &&
 		userFlags.some((flag) => isFlag(flag, "--global-config"));
-
-	// Escape each user flag so expansion-introduced whitespace stays inside
-	// one shell token. `escAuth` is `identity` on the array path.
-	const filtered = stripManagedFlags(userFlags, !honorOverride).map(escAuth);
-
-	const authFlags =
-		auth.mode === "url"
-			? ["--url", escAuth(auth.url)]
-			: honorOverride
-				? []
-				: ["--global-config", escAuth(auth.configDir)];
-
-	return [...filtered, ...authFlags, ...getHeaderArgs(configs, escHeader)];
+	const authFlags = honorOverride
+		? []
+		: ["--global-config", escAuth(auth.configDir)];
+	return [...cleanUserFlags(!honorOverride), ...authFlags, ...headers];
 }
 
 function stripManagedFlags(
@@ -129,7 +128,7 @@ export function resolveCliAuth(
 	return {
 		mode: "global-config",
 		configDir,
-		allowOverride: featureSet.keyringTokenRead,
+		allowOverride: featureSet.tokenRead,
 	};
 }
 

src/util/uri.ts

@@ -25,7 +25,7 @@ export function normalizeUrl(value: string): string {
  * `coder.alternativeWebUrl` setting when configured, otherwise returns
  * the connection URL unchanged.
  */
-export function resolveUiUrl(connectionUrl: string): string {
+export function resolveCoderDashboardUrl(connectionUrl: string): string {
 	const alt = normalizeUrl(
 		vscode.workspace
 			.getConfiguration("coder")
@@ -35,13 +35,13 @@ export function resolveUiUrl(connectionUrl: string): string {
 }
 
 /**
- * Open a path on the Coder deployment in the user's browser, applying
- * `coder.alternativeWebUrl` when configured.
+ * Open a path in the user's browser, resolved against `coder.alternativeWebUrl`
+ * when set, otherwise against `connectionUrl`.
  */
 export function openInBrowser(
 	connectionUrl: string,
 	path: string,
 ): Thenable<boolean> {
-	const base = vscode.Uri.parse(resolveUiUrl(connectionUrl));
+	const base = vscode.Uri.parse(resolveCoderDashboardUrl(connectionUrl));
 	return vscode.env.openExternal(vscode.Uri.joinPath(base, path));
 }

test/unit/api/workspace.test.ts

@@ -28,7 +28,7 @@ const featureSet: FeatureSet = {
 	buildReason: true,
 	cliUpdate: true,
 	keyringAuth: true,
-	keyringTokenRead: true,
+	tokenRead: true,
 	supportBundle: true,
 };
 

test/unit/core/cliCredentialManager.test.ts

@@ -491,7 +491,7 @@ describe("CliCredentialManager", () => {
 
 		it("returns undefined when CLI version too old for token read", async () => {
 			vi.mocked(isKeyringEnabled).mockReturnValue(true);
-			// 2.30 supports keyringAuth but not keyringTokenRead (requires 2.31+)
+			// 2.30 supports keyringAuth but not tokenRead (requires 2.31+)
 			vi.mocked(cliExec.version).mockResolvedValueOnce("2.30.0");
 			stubExecFile({ stdout: "my-token" });
 			const { manager } = setup();

test/unit/featureSet.test.ts

@@ -48,9 +48,9 @@ describe("check version support", () => {
 			["v2.29.0", "v2.29.1", "v2.30.0", "v3.0.0"],
 		);
 	});
-	it("keyring token read", () => {
+	it("token read", () => {
 		expectFlag(
-			"keyringTokenRead",
+			"tokenRead",
 			["v2.30.0", "v2.29.0", "v2.28.0", "v1.0.0"],
 			["v2.31.0", "v2.31.1", "v2.32.0", "v3.0.0"],
 		);

test/unit/util/uri.test.ts

@@ -5,7 +5,7 @@ import {
 	normalizeUrl,
 	openInBrowser,
 	removeTrailingSlashes,
-	resolveUiUrl,
+	resolveCoderDashboardUrl,
 	toSafeHost,
 } from "@/util/uri";
 
@@ -51,15 +51,15 @@ describe("normalizeUrl", () => {
 	});
 });
 
-describe("resolveUiUrl", () => {
+describe("resolveCoderDashboardUrl", () => {
 	let configurationProvider: MockConfigurationProvider;
 
 	beforeEach(() => {
 		configurationProvider = new MockConfigurationProvider();
 	});
 
 	it("returns the connection URL when no alternative is configured", () => {
-		expect(resolveUiUrl("https://coder.example.com:7004")).toBe(
+		expect(resolveCoderDashboardUrl("https://coder.example.com:7004")).toBe(
 			"https://coder.example.com:7004",
 		);
 	});
@@ -71,7 +71,7 @@ describe("resolveUiUrl", () => {
 		"returns the connection URL when the alternative is $name",
 		({ value }) => {
 			configurationProvider.set("coder.alternativeWebUrl", value);
-			expect(resolveUiUrl("https://coder.example.com:7004")).toBe(
+			expect(resolveCoderDashboardUrl("https://coder.example.com:7004")).toBe(
 				"https://coder.example.com:7004",
 			);
 		},
@@ -90,7 +90,7 @@ describe("resolveUiUrl", () => {
 		{ name: "trims whitespace", value: "  https://coder.example.com  " },
 	])("$name", ({ value }) => {
 		configurationProvider.set("coder.alternativeWebUrl", value);
-		expect(resolveUiUrl("https://coder.example.com:7004")).toBe(
+		expect(resolveCoderDashboardUrl("https://coder.example.com:7004")).toBe(
 			"https://coder.example.com",
 		);
 	});