Skip to content

Commit 7671617

Browse files
feat: #750
1 parent d167396 commit 7671617

File tree

8 files changed

+184
-71
lines changed

8 files changed

+184
-71
lines changed

eslint.config.mjs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ export default defineConfig(
1919
"**/vite.config*.ts",
2020
"**/createWebviewConfig.ts",
2121
".vscode-test/**",
22+
"test/fixtures/scripts/**",
2223
]),
2324

2425
// Base ESLint recommended rules (for JS/TS/TSX files only)

src/cliConfig.ts

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -20,17 +20,37 @@ export function getGlobalFlagsRaw(
2020
}
2121

2222
/**
23-
* Returns global configuration flags for Coder CLI commands.
24-
* Includes either `--global-config` or `--url` depending on the auth mode.
23+
* Returns global configuration flags for Coder CLI commands with auth values
24+
* escaped for shell use (e.g., `terminal.sendText`, `spawn({ shell: true })`).
2525
*/
2626
export function getGlobalFlags(
2727
configs: Pick<WorkspaceConfiguration, "get">,
2828
auth: CliAuth,
2929
): string[] {
30+
return buildGlobalFlags(configs, auth, true);
31+
}
32+
33+
/**
34+
* Returns global configuration flags for Coder CLI commands with raw auth
35+
* values suitable for `execFile` (no shell escaping).
36+
*/
37+
export function getGlobalFlagsForExec(
38+
configs: Pick<WorkspaceConfiguration, "get">,
39+
auth: CliAuth,
40+
): string[] {
41+
return buildGlobalFlags(configs, auth, false);
42+
}
43+
44+
function buildGlobalFlags(
45+
configs: Pick<WorkspaceConfiguration, "get">,
46+
auth: CliAuth,
47+
escapeAuth: boolean,
48+
): string[] {
49+
const esc = escapeAuth ? escapeCommandArg : (s: string) => s;
3050
const authFlags =
3151
auth.mode === "url"
32-
? ["--url", escapeCommandArg(auth.url)]
33-
: ["--global-config", escapeCommandArg(auth.configDir)];
52+
? ["--url", esc(auth.url)]
53+
: ["--global-config", esc(auth.configDir)];
3454

3555
const raw = getGlobalFlagsRaw(configs);
3656
const filtered = stripManagedFlags(raw);

src/commands.ts

Lines changed: 71 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,18 @@ import {
33
type WorkspaceAgent,
44
} from "coder/site/src/api/typesGenerated";
55
import * as fs from "node:fs/promises";
6+
import * as os from "node:os";
67
import * as path from "node:path";
78
import * as semver from "semver";
89
import * as vscode from "vscode";
910

1011
import { createWorkspaceIdentifier, extractAgents } from "./api/api-helper";
1112
import { type CoderApi } from "./api/coderApi";
12-
import { getGlobalFlags, resolveCliAuth } from "./cliConfig";
13+
import {
14+
getGlobalFlags,
15+
getGlobalFlagsForExec,
16+
resolveCliAuth,
17+
} from "./cliConfig";
1318
import { type CliManager } from "./core/cliManager";
1419
import * as cliUtils from "./core/cliUtils";
1520
import { type ServiceContainer } from "./core/container";
@@ -22,7 +27,7 @@ import { toError } from "./error/errorUtils";
2227
import { featureSetForVersion } from "./featureSet";
2328
import { type Logger } from "./logging/logger";
2429
import { type LoginCoordinator } from "./login/loginCoordinator";
25-
import { withProgress } from "./progress";
30+
import { withCancellableProgress, withProgress } from "./progress";
2631
import { maybeAskAgent, maybeAskUrl } from "./promptUtils";
2732
import {
2833
RECOMMENDED_SSH_SETTINGS,
@@ -147,21 +152,32 @@ export class Commands {
147152
}
148153

149154
/**
150-
* Run a speed test against the currently connected workspace and display the
151-
* results in a new editor document.
155+
* Run a speed test against the currently connected workspace and save the
156+
* results to a file chosen by the user.
152157
*/
153158
public async speedTest(): Promise<void> {
154-
if (!this.workspace) {
159+
const workspace = this.workspace;
160+
if (!workspace) {
155161
vscode.window.showInformationMessage("No workspace connected.");
156162
return;
157163
}
158164

159-
await withProgress(
160-
{
161-
location: vscode.ProgressLocation.Notification,
162-
title: "Running speed test...",
165+
const duration = await vscode.window.showInputBox({
166+
title: "Speed Test Duration",
167+
prompt: "Duration for the speed test (e.g., 5s, 10s, 1m)",
168+
value: "5s",
169+
validateInput: (v) => {
170+
return /^\d+[sm]$/.test(v.trim())
171+
? null
172+
: "Enter a duration like 5s, 10s, or 1m";
163173
},
164-
async () => {
174+
});
175+
if (duration === undefined) {
176+
return;
177+
}
178+
179+
const result = await withCancellableProgress(
180+
async ({ signal }) => {
165181
const baseUrl = this.requireExtensionBaseUrl();
166182
const safeHost = toSafeHost(baseUrl);
167183
const binary = await this.cliManager.fetchBinary(this.extensionClient);
@@ -170,27 +186,54 @@ export class Commands {
170186
const configDir = this.pathResolver.getGlobalConfigDir(safeHost);
171187
const configs = vscode.workspace.getConfiguration();
172188
const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir);
173-
const workspaceName = createWorkspaceIdentifier(this.workspace!);
189+
const globalFlags = getGlobalFlagsForExec(configs, auth);
190+
const workspaceName = createWorkspaceIdentifier(workspace);
174191

175-
try {
176-
const stdout = await cliUtils.speedtest(
177-
binary,
178-
auth,
179-
workspaceName,
180-
);
181-
const doc = await vscode.workspace.openTextDocument({
182-
content: stdout,
183-
language: "json",
184-
});
185-
await vscode.window.showTextDocument(doc);
186-
} catch (error) {
187-
this.logger.error("Speed test failed", error);
188-
vscode.window.showErrorMessage(
189-
`Speed test failed: ${error instanceof Error ? error.message : String(error)}`,
190-
);
191-
}
192+
return cliUtils.speedtest(binary, globalFlags, workspaceName, {
193+
signal,
194+
duration: duration.trim(),
195+
});
196+
},
197+
{
198+
location: vscode.ProgressLocation.Notification,
199+
title: `Running speed test (${duration.trim()})...`,
200+
cancellable: true,
192201
},
193202
);
203+
204+
if (!result.ok) {
205+
if (!result.cancelled) {
206+
this.logger.error("Speed test failed", result.error);
207+
vscode.window.showErrorMessage(
208+
`Speed test failed: ${result.error instanceof Error ? result.error.message : String(result.error)}`,
209+
);
210+
}
211+
return;
212+
}
213+
214+
const defaultName = `speedtest-${workspace.name}-${new Date().toISOString().slice(0, 10)}.json`;
215+
const defaultUri = vscode.Uri.joinPath(
216+
vscode.workspace.workspaceFolders?.[0]?.uri ??
217+
vscode.Uri.file(os.homedir()),
218+
defaultName,
219+
);
220+
const uri = await vscode.window.showSaveDialog({
221+
defaultUri,
222+
filters: { JSON: ["json"] },
223+
});
224+
if (!uri) {
225+
return;
226+
}
227+
228+
await vscode.workspace.fs.writeFile(uri, Buffer.from(result.value));
229+
const action = await vscode.window.showInformationMessage(
230+
"Speed test results saved.",
231+
"Open File",
232+
);
233+
if (action === "Open File") {
234+
const doc = await vscode.workspace.openTextDocument(uri);
235+
await vscode.window.showTextDocument(doc);
236+
}
194237
}
195238

196239
/**

src/core/cliUtils.ts

Lines changed: 11 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import os from "node:os";
66
import path from "node:path";
77
import { promisify } from "node:util";
88

9-
import type { CliAuth } from "../cliConfig";
10-
119
/**
1210
* Custom error thrown when a binary file is locked (typically on Windows).
1311
*/
@@ -75,33 +73,25 @@ export async function version(binPath: string): Promise<string> {
7573
}
7674

7775
/**
78-
* Run a speed test against the specified workspace and return the JSON output.
79-
* Throw if unable to execute the binary or parse the output.
76+
* Run a speed test against the specified workspace and return the raw output.
77+
* Throw if unable to execute the binary.
8078
*/
8179
export async function speedtest(
8280
binPath: string,
83-
auth: CliAuth,
81+
globalFlags: string[],
8482
workspaceName: string,
83+
options?: { signal?: AbortSignal; duration?: string },
8584
): Promise<string> {
86-
const result = await promisify(execFile)(binPath, [
87-
...authArgs(auth),
88-
"speedtest",
89-
workspaceName,
90-
"--output",
91-
"json",
92-
]);
85+
const args = [...globalFlags, "speedtest", workspaceName, "--output", "json"];
86+
if (options?.duration) {
87+
args.push("-t", options.duration);
88+
}
89+
const result = await promisify(execFile)(binPath, args, {
90+
signal: options?.signal,
91+
});
9392
return result.stdout;
9493
}
9594

96-
/**
97-
* Build CLI auth flags for execFile (no shell escaping).
98-
*/
99-
function authArgs(auth: CliAuth): string[] {
100-
return auth.mode === "url"
101-
? ["--url", auth.url]
102-
: ["--global-config", auth.configDir];
103-
}
104-
10595
export interface RemovalResult {
10696
fileName: string;
10797
error: unknown;

test/fixtures/scripts/echo-args.bash

Lines changed: 0 additions & 6 deletions
This file was deleted.

test/fixtures/scripts/echo-args.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
/* eslint-env node */
2+
// Prints each argument on its own line, so tests can verify exact args.
3+
process.argv.slice(2).forEach((arg) => console.log(arg));

test/unit/cliConfig.test.ts

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { it, expect, describe, vi } from "vitest";
55
import {
66
type CliAuth,
77
getGlobalFlags,
8+
getGlobalFlagsForExec,
89
getGlobalFlagsRaw,
910
getSshFlags,
1011
isKeyringEnabled,
@@ -172,6 +173,43 @@ describe("cliConfig", () => {
172173
);
173174
});
174175

176+
describe("getGlobalFlagsForExec", () => {
177+
const urlAuth: CliAuth = { mode: "url", url: "https://dev.coder.com" };
178+
179+
it("should not escape auth flags", () => {
180+
const config = new MockConfigurationProvider();
181+
expect(getGlobalFlagsForExec(config, globalConfigAuth)).toStrictEqual([
182+
"--global-config",
183+
"/config/dir",
184+
]);
185+
expect(getGlobalFlagsForExec(config, urlAuth)).toStrictEqual([
186+
"--url",
187+
"https://dev.coder.com",
188+
]);
189+
});
190+
191+
it("should still escape header-command flags", () => {
192+
const config = new MockConfigurationProvider();
193+
config.set("coder.headerCommand", "echo test");
194+
expect(getGlobalFlagsForExec(config, globalConfigAuth)).toStrictEqual([
195+
"--global-config",
196+
"/config/dir",
197+
"--header-command",
198+
quoteCommand("echo test"),
199+
]);
200+
});
201+
202+
it("should include user global flags", () => {
203+
const config = new MockConfigurationProvider();
204+
config.set("coder.globalFlags", ["--verbose"]);
205+
expect(getGlobalFlagsForExec(config, globalConfigAuth)).toStrictEqual([
206+
"--verbose",
207+
"--global-config",
208+
"/config/dir",
209+
]);
210+
});
211+
});
212+
175213
describe("getGlobalFlagsRaw", () => {
176214
it("returns empty array when no global flags configured", () => {
177215
const config = new MockConfigurationProvider();

test/unit/core/cliUtils.test.ts

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,21 +143,25 @@ describe("CliUtils", () => {
143143
});
144144

145145
describe("speedtest", () => {
146-
const echoArgsBin = path.join(tmp, "echo-args");
146+
const echoArgsBin = isWindows()
147+
? path.join(tmp, "echo-args.cmd")
148+
: path.join(tmp, "echo-args");
147149

148150
beforeAll(async () => {
149-
const tmpl = await fs.readFile(
150-
getFixturePath("scripts", "echo-args.bash"),
151-
"utf8",
152-
);
153-
await fs.writeFile(echoArgsBin, tmpl);
154-
await fs.chmod(echoArgsBin, "755");
151+
const scriptPath = getFixturePath("scripts", "echo-args.js");
152+
if (isWindows()) {
153+
await fs.writeFile(echoArgsBin, `@node "${scriptPath}" %*\r\n`);
154+
} else {
155+
const content = await fs.readFile(scriptPath, "utf8");
156+
await fs.writeFile(echoArgsBin, `#!/usr/bin/env node\n${content}`);
157+
await fs.chmod(echoArgsBin, "755");
158+
}
155159
});
156160

157-
it("passes global-config auth flags", async () => {
161+
it("passes global flags", async () => {
158162
const result = await cliUtils.speedtest(
159163
echoArgsBin,
160-
{ mode: "global-config", configDir: "/tmp/test-config" },
164+
["--global-config", "/tmp/test-config"],
161165
"owner/workspace",
162166
);
163167
const args = result.trim().split("\n");
@@ -171,11 +175,29 @@ describe("CliUtils", () => {
171175
]);
172176
});
173177

174-
it("passes url auth flags", async () => {
178+
it("passes url flags", async () => {
179+
const result = await cliUtils.speedtest(
180+
echoArgsBin,
181+
["--url", "http://localhost:3000"],
182+
"owner/workspace",
183+
);
184+
const args = result.trim().split("\n");
185+
expect(args).toEqual([
186+
"--url",
187+
"http://localhost:3000",
188+
"speedtest",
189+
"owner/workspace",
190+
"--output",
191+
"json",
192+
]);
193+
});
194+
195+
it("passes duration flag", async () => {
175196
const result = await cliUtils.speedtest(
176197
echoArgsBin,
177-
{ mode: "url", url: "http://localhost:3000" },
198+
["--url", "http://localhost:3000"],
178199
"owner/workspace",
200+
{ duration: "10s" },
179201
);
180202
const args = result.trim().split("\n");
181203
expect(args).toEqual([
@@ -185,14 +207,16 @@ describe("CliUtils", () => {
185207
"owner/workspace",
186208
"--output",
187209
"json",
210+
"-t",
211+
"10s",
188212
]);
189213
});
190214

191215
it("throws when binary does not exist", async () => {
192216
await expect(
193217
cliUtils.speedtest(
194218
"/nonexistent/binary",
195-
{ mode: "global-config", configDir: "/tmp" },
219+
["--global-config", "/tmp"],
196220
"owner/workspace",
197221
),
198222
).rejects.toThrow("ENOENT");

0 commit comments

Comments
 (0)