fix: improve cliExec error handling and input validation

Author: EhabY

package.json

@@ -259,12 +259,6 @@
 				"when": "!coder.authenticated && coder.loaded"
 			}
 		],
-		"submenus": [
-			{
-				"id": "coder.diagnostics",
-				"label": "Diagnostics"
-			}
-		],
 		"commands": [
 			{
 				"command": "coder.login",
@@ -327,7 +321,7 @@
 			},
 			{
 				"command": "coder.speedTest",
-				"title": "Run Speed Test",
+				"title": "Speed Test Workspace",
 				"category": "Coder"
 			},
 			{
@@ -523,17 +517,14 @@
 					"group": "inline"
 				},
 				{
-					"submenu": "coder.diagnostics",
+					"command": "coder.pingWorkspace",
 					"when": "coder.authenticated && viewItem =~ /\\+running/",
 					"group": "navigation"
-				}
-			],
-			"coder.diagnostics": [
-				{
-					"command": "coder.pingWorkspace"
 				},
 				{
-					"command": "coder.speedTest"
+					"command": "coder.speedTest",
+					"when": "coder.authenticated && viewItem =~ /\\+running/",
+					"group": "navigation"
 				}
 			],
 			"statusBar/remoteIndicator": [

src/commands.ts

@@ -180,21 +180,31 @@ export class Commands {
 
 		const duration = await vscode.window.showInputBox({
 			title: "Speed Test Duration",
-			prompt: "Duration for the speed test (e.g., 5s, 10s, 1m)",
+			prompt: "Duration for the speed test",
 			value: "5s",
+			validateInput: (value) => {
+				const v = value.trim();
+				if (v && !cliExec.isGoDuration(v)) {
+					return "Invalid Go duration (e.g., 5s, 10s, 1m, 1m30s)";
+				}
+				return undefined;
+			},
 		});
 		if (duration === undefined) {
 			return;
 		}
+		const trimmedDuration = duration.trim();
 
 		const result = await withCancellableProgress(
 			async ({ signal }) => {
 				const env = await this.resolveCliEnv(client);
-				return cliExec.speedtest(env, workspaceId, duration.trim(), signal);
+				return cliExec.speedtest(env, workspaceId, trimmedDuration, signal);
 			},
 			{
 				location: vscode.ProgressLocation.Notification,
-				title: `Running ${duration.trim()} speed test...`,
+				title: trimmedDuration
+					? `Running speed test (${trimmedDuration})...`
+					: "Running speed test...",
 				cancellable: true,
 			},
 		);
@@ -752,13 +762,19 @@ export class Commands {
 		};
 	}
 
+	/** Resolve a CliEnv, preferring a locally cached binary over a network fetch. */
 	private async resolveCliEnv(client: CoderApi): Promise<cliExec.CliEnv> {
 		const baseUrl = client.getAxiosInstance().defaults.baseURL;
 		if (!baseUrl) {
 			throw new Error("You are not logged in");
 		}
 		const safeHost = toSafeHost(baseUrl);
-		const binary = await this.cliManager.fetchBinary(client);
+		let binary: string;
+		try {
+			binary = await this.cliManager.locateBinary(baseUrl);
+		} catch {
+			binary = await this.cliManager.fetchBinary(client);
+		}
 		const version = semver.parse(await cliExec.version(binary));
 		const featureSet = featureSetForVersion(version);
 		const configDir = this.pathResolver.getGlobalConfigDir(safeHost);

src/core/cliExec.ts

@@ -2,6 +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 {
 	type CliAuth,
 	getGlobalFlags,
@@ -22,6 +23,23 @@ const execFileAsync = promisify(execFile);
 // and we have to make our own.
 type ExecException = ExecFileException & { stdout?: string; stderr?: string };
 
+/** Prefer stderr over the default message which includes the full command line. */
+function cliError(error: unknown): Error {
+	const stderr = (error as ExecException)?.stderr?.trim();
+	if (stderr) {
+		return new Error(stderr, { cause: error });
+	}
+	return toError(error);
+}
+
+/** Go duration regex: one or more {number}{unit} segments (e.g. 5s, 1h30m). */
+const GO_DURATION_RE = /^(\d+(\.\d+)?(ns|us|µs|ms|s|m|h))+$/;
+
+/** Returns true if the string is a valid Go duration. */
+export function isGoDuration(value: string): boolean {
+	return GO_DURATION_RE.test(value);
+}
+
 /**
  * Return the version from the binary.  Throw if unable to execute the binary or
  * find the version for any reason.
@@ -49,7 +67,7 @@ export async function version(binPath: string): Promise<string> {
 				return v;
 			}
 		}
-		throw error;
+		throw cliError(error);
 	}
 
 	const json = JSON.parse(stdout) as { version?: string };
@@ -73,8 +91,12 @@ export async function speedtest(
 	if (duration) {
 		args.push("-t", duration);
 	}
-	const result = await execFileAsync(env.binary, args, { signal });
-	return result.stdout;
+	try {
+		const result = await execFileAsync(env.binary, args, { signal });
+		return result.stdout;
+	} catch (error) {
+		throw cliError(error);
+	}
 }
 
 /**
@@ -232,7 +254,7 @@ export async function openAppStatusTerminal(
 	const globalFlags = getGlobalShellFlags(env.configs, env.auth);
 	const terminal = vscode.window.createTerminal(app.name);
 	terminal.sendText(
-		`${escapeCommandArg(env.binary)} ${globalFlags.join(" ")} ssh ${app.workspace_name}`,
+		`${escapeCommandArg(env.binary)} ${globalFlags.join(" ")} ssh ${escapeCommandArg(app.workspace_name)}`,
 	);
 	await new Promise((resolve) => setTimeout(resolve, 5000));
 	terminal.sendText(app.command ?? "");

test/unit/core/cliExec.test.ts

@@ -207,5 +207,37 @@ describe("cliExec", () => {
 				"ENOENT",
 			);
 		});
+
+		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;`,
+			].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");
+		});
+	});
+
+	describe("isGoDuration", () => {
+		it.each([
+			"5s",
+			"10m",
+			"1h",
+			"1h30m",
+			"500ms",
+			"1.5s",
+			"2h45m10s",
+			"100ns",
+			"50us",
+			"50µs",
+		])("accepts %s", (v) => expect(cliExec.isGoDuration(v)).toBe(true));
+
+		it.each(["", "bjbmn", "5", "s", "5x", "1h 30m", "-5s", "5S"])(
+			"rejects %s",
+			(v) => expect(cliExec.isGoDuration(v)).toBe(false),
+		);
 	});
 });