Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions eslint.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ export default defineConfig(
"**/vite.config*.ts",
"**/createWebviewConfig.ts",
".vscode-test/**",
"test/fixtures/scripts/**",
]),

// Base ESLint recommended rules (for JS/TS/TSX files only)
Expand Down
9 changes: 9 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,11 @@
"category": "Coder",
"icon": "$(refresh)"
},
{
"command": "coder.speedTest",
"title": "Run Speed Test",
"category": "Coder"
},
{
"command": "coder.viewLogs",
"title": "Coder: View Logs",
Expand Down Expand Up @@ -371,6 +376,10 @@
"command": "coder.createWorkspace",
"when": "coder.authenticated"
},
{
"command": "coder.speedTest",
"when": "coder.workspace.connected"
},
{
"command": "coder.navigateToWorkspace",
"when": "coder.workspace.connected"
Expand Down
28 changes: 24 additions & 4 deletions src/cliConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,37 @@ export function getGlobalFlagsRaw(
}

/**
* Returns global configuration flags for Coder CLI commands.
* Includes either `--global-config` or `--url` depending on the auth mode.
* Returns global configuration flags for Coder CLI commands with auth values
* escaped for shell use (e.g., `terminal.sendText`, `spawn({ shell: true })`).
*/
export function getGlobalFlags(
configs: Pick<WorkspaceConfiguration, "get">,
auth: CliAuth,
): string[] {
return buildGlobalFlags(configs, auth, true);
}

/**
* Returns global configuration flags for Coder CLI commands with raw auth
* values suitable for `execFile` (no shell escaping).
*/
export function getGlobalFlagsForExec(
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about we rename those to be clearer to something like this:

  • getGlobalFlags → raw args array (unescaped), the default, used by execFile
  • getGlobalShellFlags → shell-escaped args, used by spawn({ shell: true }), terminal.sendText, and SSH ProxyCommand

configs: Pick<WorkspaceConfiguration, "get">,
auth: CliAuth,
): string[] {
return buildGlobalFlags(configs, auth, false);
}

function buildGlobalFlags(
configs: Pick<WorkspaceConfiguration, "get">,
auth: CliAuth,
escapeAuth: boolean,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer we pass the escape method itself here instead of a boolean

): string[] {
const esc = escapeAuth ? escapeCommandArg : (s: string) => s;
const authFlags =
auth.mode === "url"
? ["--url", escapeCommandArg(auth.url)]
: ["--global-config", escapeCommandArg(auth.configDir)];
? ["--url", esc(auth.url)]
: ["--global-config", esc(auth.configDir)];

const raw = getGlobalFlagsRaw(configs);
const filtered = stripManagedFlags(raw);
Expand Down
94 changes: 92 additions & 2 deletions src/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,18 @@ import {
type WorkspaceAgent,
} from "coder/site/src/api/typesGenerated";
import * as fs from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import * as semver from "semver";
import * as vscode from "vscode";

import { createWorkspaceIdentifier, extractAgents } from "./api/api-helper";
import { type CoderApi } from "./api/coderApi";
import { getGlobalFlags, resolveCliAuth } from "./cliConfig";
import {
getGlobalFlags,
getGlobalFlagsForExec,
resolveCliAuth,
} from "./cliConfig";
import { type CliManager } from "./core/cliManager";
import * as cliUtils from "./core/cliUtils";
import { type ServiceContainer } from "./core/container";
Expand All @@ -22,7 +27,7 @@ import { toError } from "./error/errorUtils";
import { featureSetForVersion } from "./featureSet";
import { type Logger } from "./logging/logger";
import { type LoginCoordinator } from "./login/loginCoordinator";
import { withProgress } from "./progress";
import { withCancellableProgress, withProgress } from "./progress";
import { maybeAskAgent, maybeAskUrl } from "./promptUtils";
import {
RECOMMENDED_SSH_SETTINGS,
Expand Down Expand Up @@ -146,6 +151,91 @@ export class Commands {
this.logger.debug("Login complete to deployment:", url);
}

/**
* Run a speed test against the currently connected workspace and save the
* results to a file chosen by the user.
*/
public async speedTest(): Promise<void> {
const workspace = this.workspace;
if (!workspace) {
vscode.window.showInformationMessage("No workspace connected.");
return;
}

const duration = await vscode.window.showInputBox({
title: "Speed Test Duration",
prompt: "Duration for the speed test (e.g., 5s, 10s, 1m)",
value: "5s",
validateInput: (v) => {
return /^\d+[sm]$/.test(v.trim())
? null
: "Enter a duration like 5s, 10s, or 1m";
Comment on lines +170 to +172
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this only take seconds and minutes 🤔 ?

},
});
if (duration === undefined) {
return;
}

const result = await withCancellableProgress(
async ({ signal }) => {
const baseUrl = this.requireExtensionBaseUrl();
const safeHost = toSafeHost(baseUrl);
const binary = await this.cliManager.fetchBinary(this.extensionClient);
const version = semver.parse(await cliUtils.version(binary));
const featureSet = featureSetForVersion(version);
const configDir = this.pathResolver.getGlobalConfigDir(safeHost);
const configs = vscode.workspace.getConfiguration();
const auth = resolveCliAuth(configs, featureSet, baseUrl, configDir);
const globalFlags = getGlobalFlagsForExec(configs, auth);
const workspaceName = createWorkspaceIdentifier(workspace);

return cliUtils.speedtest(binary, globalFlags, workspaceName, {
signal,
duration: duration.trim(),
});
},
{
location: vscode.ProgressLocation.Notification,
title: `Running speed test (${duration.trim()})...`,
cancellable: true,
},
);

if (!result.ok) {
if (!result.cancelled) {
this.logger.error("Speed test failed", result.error);
vscode.window.showErrorMessage(
`Speed test failed: ${result.error instanceof Error ? result.error.message : String(result.error)}`,
);
}
return;
}

const defaultName = `speedtest-${workspace.name}-${new Date().toISOString().slice(0, 10)}.json`;
const defaultUri = vscode.Uri.joinPath(
vscode.workspace.workspaceFolders?.[0]?.uri ??
vscode.Uri.file(os.homedir()),
defaultName,
);
const uri = await vscode.window.showSaveDialog({
defaultUri,
filters: { JSON: ["json"] },
});
if (!uri) {
return;
}

await vscode.workspace.fs.writeFile(uri, Buffer.from(result.value));
const action = await vscode.window.showInformationMessage(
"Speed test results saved.",
"Open File",
);
if (action === "Open File") {
Comment on lines +229 to +233
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be removed but we should never await a thenable that is not blocking. We had a bug a while back that caused the notification to show, then hide after 10s and this promise was left hanging (VS Code API uses a thenable so it does not adhere to promises and as such might never throw)

const doc = await vscode.workspace.openTextDocument(uri);
await vscode.window.showTextDocument(doc);
}
}

/**
* View the logs for the currently connected workspace.
*/
Expand Down
20 changes: 20 additions & 0 deletions src/core/cliUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,26 @@ export async function version(binPath: string): Promise<string> {
return json.version;
}

/**
* Run a speed test against the specified workspace and return the raw output.
* Throw if unable to execute the binary.
*/
export async function speedtest(
binPath: string,
globalFlags: string[],
workspaceName: string,
options?: { signal?: AbortSignal; duration?: string },
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Options are always passed no?

): Promise<string> {
const args = [...globalFlags, "speedtest", workspaceName, "--output", "json"];
if (options?.duration) {
args.push("-t", options.duration);
}
const result = await promisify(execFile)(binPath, args, {
signal: options?.signal,
});
return result.stdout;
}

export interface RemovalResult {
fileName: string;
error: unknown;
Expand Down
4 changes: 4 additions & 0 deletions src/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -281,6 +281,10 @@ export async function activate(ctx: vscode.ExtensionContext): Promise<void> {
void myWorkspacesProvider.fetchAndRefresh();
void allWorkspacesProvider.fetchAndRefresh();
}),
vscode.commands.registerCommand(
"coder.speedTest",
commands.speedTest.bind(commands),
),
vscode.commands.registerCommand(
"coder.viewLogs",
commands.viewLogs.bind(commands),
Expand Down
3 changes: 3 additions & 0 deletions test/fixtures/scripts/echo-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
/* eslint-env node */
// Prints each argument on its own line, so tests can verify exact args.
process.argv.slice(2).forEach((arg) => console.log(arg));
38 changes: 38 additions & 0 deletions test/unit/cliConfig.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { it, expect, describe, vi } from "vitest";
import {
type CliAuth,
getGlobalFlags,
getGlobalFlagsForExec,
getGlobalFlagsRaw,
getSshFlags,
isKeyringEnabled,
Expand Down Expand Up @@ -172,6 +173,43 @@ describe("cliConfig", () => {
);
});

describe("getGlobalFlagsForExec", () => {
const urlAuth: CliAuth = { mode: "url", url: "https://dev.coder.com" };

it("should not escape auth flags", () => {
const config = new MockConfigurationProvider();
expect(getGlobalFlagsForExec(config, globalConfigAuth)).toStrictEqual([
"--global-config",
"/config/dir",
]);
expect(getGlobalFlagsForExec(config, urlAuth)).toStrictEqual([
"--url",
"https://dev.coder.com",
]);
});

it("should still escape header-command flags", () => {
const config = new MockConfigurationProvider();
config.set("coder.headerCommand", "echo test");
expect(getGlobalFlagsForExec(config, globalConfigAuth)).toStrictEqual([
"--global-config",
"/config/dir",
"--header-command",
quoteCommand("echo test"),
]);
});

it("should include user global flags", () => {
const config = new MockConfigurationProvider();
config.set("coder.globalFlags", ["--verbose"]);
expect(getGlobalFlagsForExec(config, globalConfigAuth)).toStrictEqual([
"--verbose",
"--global-config",
"/config/dir",
]);
});
});

describe("getGlobalFlagsRaw", () => {
it("returns empty array when no global flags configured", () => {
const config = new MockConfigurationProvider();
Expand Down
81 changes: 81 additions & 0 deletions test/unit/core/cliUtils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,87 @@ describe("CliUtils", () => {
]);
});

describe("speedtest", () => {
const echoArgsBin = isWindows()
? path.join(tmp, "echo-args.cmd")
: path.join(tmp, "echo-args");

beforeAll(async () => {
const scriptPath = getFixturePath("scripts", "echo-args.js");
if (isWindows()) {
await fs.writeFile(echoArgsBin, `@node "${scriptPath}" %*\r\n`);
} else {
const content = await fs.readFile(scriptPath, "utf8");
await fs.writeFile(echoArgsBin, `#!/usr/bin/env node\n${content}`);
await fs.chmod(echoArgsBin, "755");
}
Comment on lines +152 to +158
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this? Can't we execute the JS file the same way?

});

it("passes global flags", async () => {
const result = await cliUtils.speedtest(
echoArgsBin,
["--global-config", "/tmp/test-config"],
"owner/workspace",
);
const args = result.trim().split("\n");
expect(args).toEqual([
"--global-config",
"/tmp/test-config",
"speedtest",
"owner/workspace",
"--output",
"json",
]);
});

it("passes url flags", async () => {
const result = await cliUtils.speedtest(
echoArgsBin,
["--url", "http://localhost:3000"],
"owner/workspace",
);
const args = result.trim().split("\n");
expect(args).toEqual([
"--url",
"http://localhost:3000",
"speedtest",
"owner/workspace",
"--output",
"json",
]);
});

it("passes duration flag", async () => {
const result = await cliUtils.speedtest(
echoArgsBin,
["--url", "http://localhost:3000"],
"owner/workspace",
{ duration: "10s" },
);
const args = result.trim().split("\n");
expect(args).toEqual([
"--url",
"http://localhost:3000",
"speedtest",
"owner/workspace",
"--output",
"json",
"-t",
"10s",
]);
});

it("throws when binary does not exist", async () => {
await expect(
cliUtils.speedtest(
"/nonexistent/binary",
["--global-config", "/tmp"],
"owner/workspace",
),
).rejects.toThrow("ENOENT");
});
});

it("ETag", async () => {
const binPath = path.join(tmp, "hash");

Expand Down