fix: refresh CLI credentials before each invocation (#926)

- Refresh the credential file (or keyring on supported systems) via `cliManager.configure` inside `commands.ts:resolveCliEnv` so each CLI call reads a current token. Previously credentials were written only at connection time and could go stale across refreshes. mTLS still works (empty token allowed).
- Stop wrapping `AbortError` in `cliError` so cancellations no longer surface as stale CLI stderr to `withCancellableProgress`.
- Add `writeStdoutJs` / `writeStderrJs` test helpers that wrap `fs.writeSync`. Fixes intermittent flake where async pipe writes were lost on exit (see https://nodejs.org/api/process.html#a-note-on-process-io).

Author: EhabY

src/commands.ts

@@ -930,6 +930,12 @@ export class Commands {
 		const configDir = this.pathResolver.getGlobalConfigDir(safeHost);
 		const configs = vscode.workspace.getConfiguration();
 		const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir);
+		// Same threat model as the connection-time write in remote.ts: token
+		// goes to the file (or keyring on supported systems), never env, since
+		// child env is sibling-readable on most platforms.
+		await this.cliManager.configure(baseUrl, client.getSessionToken() ?? "", {
+			silent: true,
+		});
 		return { binary, configs, auth, featureSet };
 	}
 

src/core/cliCredentialManager.ts

@@ -56,26 +56,20 @@ export class CliCredentialManager {
 	 * setting is enabled and the CLI supports it; otherwise writes plaintext
 	 * files under --global-config.
 	 *
-	 * Keyring and files are mutually exclusive — never both.
-	 *
-	 * When `keyringOnly` is set, silently returns if the keyring is unavailable
-	 * instead of falling back to file storage.
+	 * Keyring and files are mutually exclusive, never both.
 	 */
 	public async storeToken(
 		url: string,
 		token: string,
 		configs: Pick<WorkspaceConfiguration, "get">,
-		options?: { signal?: AbortSignal; keyringOnly?: boolean },
+		options?: { signal?: AbortSignal },
 	): Promise<void> {
 		const binPath = await this.resolveKeyringBinary(
 			url,
 			configs,
 			"keyringAuth",
 		);
 		if (!binPath) {
-			if (options?.keyringOnly) {
-				return;
-			}
 			await this.writeCredentialFiles(url, token);
 			return;
 		}

src/core/cliExec.ts

@@ -2,7 +2,7 @@ import { type ExecFileException, execFile, spawn } from "node:child_process";
 import { promisify } from "node:util";
 import * as vscode from "vscode";
 
-import { toError } from "../error/errorUtils";
+import { isAbortError, toError } from "../error/errorUtils";
 import {
 	type CliAuth,
 	getGlobalFlags,
@@ -17,23 +17,6 @@ export interface CliEnv {
 	configs: Pick<vscode.WorkspaceConfiguration, "get">;
 }
 
-const execFileAsync = promisify(execFile);
-
-function isExecFileException(error: unknown): error is ExecFileException {
-	return error instanceof Error && "code" in error;
-}
-
-/** Prefer stderr over the default message which includes the full command line. */
-function cliError(error: unknown): Error {
-	if (isExecFileException(error)) {
-		const stderr = error.stderr?.trim();
-		if (stderr) {
-			return new Error(stderr, { cause: error });
-		}
-	}
-	return toError(error);
-}
-
 /**
  * Return the version from the binary.  Throw if unable to execute the binary or
  * find the version for any reason.
@@ -135,14 +118,56 @@ export function ping(env: CliEnv, workspaceName: string): vscode.Terminal {
 	});
 }
 
+/**
+ * SSH into a workspace and run a command via `terminal.sendText`.
+ */
+export async function openAppStatusTerminal(
+	env: CliEnv,
+	app: {
+		name?: string;
+		command?: string;
+		workspace_name: string;
+	},
+): Promise<void> {
+	const globalFlags = getGlobalShellFlags(env.configs, env.auth);
+	const terminal = vscode.window.createTerminal({ name: app.name });
+	terminal.sendText(
+		`${escapeCommandArg(env.binary)} ${globalFlags.join(" ")} ssh ${escapeCommandArg(app.workspace_name)}`,
+	);
+	await new Promise((resolve) => setTimeout(resolve, 5000));
+	terminal.sendText(app.command ?? "");
+	terminal.show(false);
+}
+
+const execFileAsync = promisify(execFile);
+
+/** Prefer stderr over the default message which includes the full command line. */
+function cliError(error: unknown): Error {
+	// Pass aborts through; wrapping erases the AbortError name and would surface stale CLI warnings as the failure.
+	if (isAbortError(error)) {
+		return error;
+	}
+	if (isExecFileException(error)) {
+		const stderr = error.stderr?.trim();
+		if (stderr) {
+			return new Error(stderr, { cause: error });
+		}
+	}
+	return toError(error);
+}
+
+function isExecFileException(error: unknown): error is ExecFileException {
+	return error instanceof Error && "code" in error;
+}
+
 /**
  * Spawn a CLI command in a PTY terminal with process lifecycle management.
  */
 function spawnCliInTerminal(options: {
 	name: string;
 	binary: string;
 	args: string[];
-	banner?: string[];
+	banner: string[];
 }): vscode.Terminal {
 	const writeEmitter = new vscode.EventEmitter<string>();
 	const closeEmitter = new vscode.EventEmitter<number | void>();
@@ -186,10 +211,8 @@ function spawnCliInTerminal(options: {
 			onDidWrite: writeEmitter.event,
 			onDidClose: closeEmitter.event,
 			open: () => {
-				if (options.banner) {
-					for (const line of options.banner) {
-						writeEmitter.fire(line + "\r\n");
-					}
+				for (const line of options.banner) {
+					writeEmitter.fire(line + "\r\n");
 				}
 			},
 			close: () => {
@@ -262,24 +285,3 @@ function spawnCliInTerminal(options: {
 	terminal.show(false);
 	return terminal;
 }
-
-/**
- * SSH into a workspace and run a command via `terminal.sendText`.
- */
-export async function openAppStatusTerminal(
-	env: CliEnv,
-	app: {
-		name?: string;
-		command?: string;
-		workspace_name: string;
-	},
-): Promise<void> {
-	const globalFlags = getGlobalShellFlags(env.configs, env.auth);
-	const terminal = vscode.window.createTerminal(app.name);
-	terminal.sendText(
-		`${escapeCommandArg(env.binary)} ${globalFlags.join(" ")} ssh ${escapeCommandArg(app.workspace_name)}`,
-	);
-	await new Promise((resolve) => setTimeout(resolve, 5000));
-	terminal.sendText(app.command ?? "");
-	terminal.show(false);
-}

src/core/cliManager.ts

@@ -817,6 +817,9 @@ 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.
+	 *
+	 * @param options.silent Suppress the progress notification only; failures
+	 *   still surface via {@link handleStoreError} (logged + error toast).
 	 */
 	public async configure(
 		url: string,

src/error/errorUtils.ts

@@ -4,7 +4,7 @@ import util from "node:util";
 import { toError as baseToError } from "@repo/shared";
 
 /** Check whether an unknown thrown value is an AbortError (signal cancellation). */
-export function isAbortError(error: unknown): boolean {
+export function isAbortError(error: unknown): error is Error {
 	return error instanceof Error && error.name === "AbortError";
 }
 

src/login/loginCoordinator.ts

@@ -153,17 +153,11 @@ export class LoginCoordinator implements vscode.Disposable {
 			});
 			await this.mementoManager.addToUrlHistory(url);
 
-			// Fire-and-forget: sync token to OS keyring for the CLI.
 			if (result.token) {
 				this.cliCredentialManager
-					.storeToken(url, result.token, vscode.workspace.getConfiguration(), {
-						keyringOnly: true,
-					})
+					.storeToken(url, result.token, vscode.workspace.getConfiguration())
 					.catch((error) => {
-						this.logger.warn(
-							"Failed to store token in keyring at login:",
-							error,
-						);
+						this.logger.warn("Failed to store token at login:", error);
 					});
 			}
 		}

test/unit/core/cliCredentialManager.test.ts

@@ -269,42 +269,6 @@ describe("CliCredentialManager", () => {
 				}),
 			).rejects.toThrow("The operation was aborted");
 		});
-
-		it.each([
-			{ scenario: "keyring disabled", keyringEnabled: false },
-			{ scenario: "CLI version too old", keyringEnabled: true },
-		])(
-			"never writes files when keyringOnly and $scenario",
-			async ({ keyringEnabled }) => {
-				vi.mocked(isKeyringEnabled).mockReturnValue(keyringEnabled);
-				if (keyringEnabled) {
-					vi.mocked(cliExec.version).mockResolvedValueOnce("2.28.0");
-				}
-				const { manager } = setup();
-
-				await manager.storeToken(TEST_URL, "token", configs, {
-					keyringOnly: true,
-				});
-
-				expect(execFile).not.toHaveBeenCalled();
-				expect(memfs.existsSync(URL_FILE)).toBe(false);
-				expect(memfs.existsSync(SESSION_FILE)).toBe(false);
-			},
-		);
-
-		it("uses keyring without writing files when keyringOnly and keyring available", async () => {
-			vi.mocked(isKeyringEnabled).mockReturnValue(true);
-			stubExecFile({ stdout: "" });
-			const { manager } = setup();
-
-			await manager.storeToken(TEST_URL, "my-token", configs, {
-				keyringOnly: true,
-			});
-
-			expect(execFile).toHaveBeenCalled();
-			expect(memfs.existsSync(URL_FILE)).toBe(false);
-			expect(memfs.existsSync(SESSION_FILE)).toBe(false);
-		});
 	});
 
 	describe("readToken", () => {

test/unit/core/cliExec.test.ts

@@ -4,7 +4,13 @@ import path from "path";
 import { beforeAll, describe, expect, it, vi } from "vitest";
 
 import { MockConfigurationProvider } from "../../mocks/testHelpers";
-import { isWindows, quoteCommand, writeExecutable } from "../../utils/platform";
+import {
+	isWindows,
+	quoteCommand,
+	writeExecutable,
+	writeStderrJs,
+	writeStdoutJs,
+} from "../../utils/platform";
 
 import type { CliEnv } from "@/core/cliExec";
 
@@ -36,7 +42,7 @@ describe("cliExec", () => {
 
 	/** JS code for a fake CLI that writes a fixed string to stdout. */
 	function echoBin(output: string): string {
-		return `process.stdout.write(${JSON.stringify(output)});`;
+		return writeStdoutJs(output);
 	}
 
 	/**
@@ -46,10 +52,11 @@ describe("cliExec", () => {
 	function oldCliBin(stderr: string, stdout: string): string {
 		return [
 			`if (process.argv.includes("--output")) {`,
-			`  process.stderr.write(${JSON.stringify(stderr)});`,
-			`  process.exitCode = 1;`,
+			`  ${writeStderrJs(stderr)}`,
+			`  process.exit(1);`,
 			`} else {`,
-			`  process.stdout.write(${JSON.stringify(stdout)});`,
+			`  ${writeStdoutJs(stdout)}`,
+			`  process.exit(0);`,
 			`}`,
 		].join("\n");
 	}
@@ -159,15 +166,27 @@ describe("cliExec", () => {
 
 		it("surfaces stderr instead of full command line on failure", async () => {
 			const code = [
-				`process.stderr.write("invalid argument for -t flag\\n");`,
-				`process.exitCode = 1;`,
+				writeStderrJs("invalid argument for -t flag\n"),
+				`process.exit(1);`,
 			].join("\n");
 			const bin = await writeExecutable(tmp, "speedtest-err", code);
 			const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin);
 			await expect(
 				cliExec.speedtest(env, "owner/workspace", "bad"),
 			).rejects.toThrow("invalid argument for -t flag");
 		});
+
+		it("preserves AbortError name when cancelled via signal", async () => {
+			// Hangs forever so the only way out is the abort signal.
+			const code = `setInterval(() => {}, 1000);`;
+			const bin = await writeExecutable(tmp, "speedtest-hang", code);
+			const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin);
+			const ac = new AbortController();
+			ac.abort();
+			await expect(
+				cliExec.speedtest(env, "owner/workspace", undefined, ac.signal),
+			).rejects.toMatchObject({ name: "AbortError" });
+		});
 	});
 
 	describe("supportBundle", () => {
@@ -204,8 +223,8 @@ describe("cliExec", () => {
 
 		it("surfaces stderr on failure", async () => {
 			const code = [
-				`process.stderr.write("workspace not found\\n");`,
-				`process.exitCode = 1;`,
+				writeStderrJs("workspace not found\n"),
+				`process.exit(1);`,
 			].join("\n");
 			const bin = await writeExecutable(tmp, "sb-err", code);
 			const { env } = setup({ mode: "global-config", configDir: "/tmp" }, bin);

test/unit/error/errorUtils.test.ts

@@ -1,6 +1,47 @@
 import { describe, it, expect } from "vitest";
 
-import { getErrorDetail, toError } from "@/error/errorUtils";
+import { getErrorDetail, isAbortError, toError } from "@/error/errorUtils";
+
+describe("isAbortError", () => {
+	it("returns true for an Error named AbortError", () => {
+		const err = new Error("aborted");
+		err.name = "AbortError";
+		expect(isAbortError(err)).toBe(true);
+	});
+
+	it("returns true for DOMException-style abort thrown by AbortController", () => {
+		const ac = new AbortController();
+		ac.abort();
+		// `signal.reason` is a DOMException with name "AbortError" in modern Node.
+		const reason = ac.signal.reason;
+		expect(isAbortError(reason)).toBe(true);
+	});
+
+	it("returns false for a plain Error", () => {
+		expect(isAbortError(new Error("nope"))).toBe(false);
+	});
+
+	it.each<[string, unknown]>([
+		["null", null],
+		["undefined", undefined],
+		["string", "AbortError"],
+		["object with name only", { name: "AbortError" }],
+	])("returns false for %s", (_name, input) => {
+		expect(isAbortError(input)).toBe(false);
+	});
+
+	it("narrows the type to Error", () => {
+		const err: unknown = Object.assign(new Error("aborted"), {
+			name: "AbortError",
+		});
+		if (isAbortError(err)) {
+			// Type-only assertion: this line must compile without a cast.
+			expect(err.message).toBe("aborted");
+		} else {
+			throw new Error("expected isAbortError to narrow");
+		}
+	});
+});
 
 describe("getErrorDetail", () => {
 	it("returns detail from API error", () => {

test/unit/login/loginCoordinator.test.ts

@@ -500,7 +500,7 @@ describe("LoginCoordinator", () => {
 			return { ...ctx, user, login };
 		}
 
-		it("calls storeToken with keyringOnly after successful login", async () => {
+		it("calls storeToken after successful login", async () => {
 			const { mockCredentialManager, login } = await loginWithStoredToken();
 
 			await login();
@@ -509,7 +509,6 @@ describe("LoginCoordinator", () => {
 				TEST_URL,
 				"stored-token",
 				expect.anything(),
-				{ keyringOnly: true },
 			);
 		});