From 91876505b2bd9e445477d8c2cbc92a3deceb6303 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Tue, 14 Apr 2026 17:56:07 +0300 Subject: [PATCH 1/2] feat: add network slowness detection with configurable thresholds (#887) Extract status bar presentation and threshold logic into networkStatus.ts with a NetworkStatusReporter class. SshProcessMonitor delegates status bar updates to the reporter. Warn users when latency, download, or upload cross configurable thresholds (debounced over 3 consecutive polls). Clicking the warning runs relevant diagnostics. Tooltip shows live metrics with codicons and a link to configure thresholds. --- package.json | 15 ++ src/remote/networkStatus.ts | 231 +++++++++++++++++++++++ src/remote/sshProcess.ts | 68 +------ test/mocks/vscode.runtime.ts | 21 ++- test/unit/remote/networkStatus.test.ts | 242 +++++++++++++++++++++++++ test/unit/remote/sshProcess.test.ts | 168 ++++++++++++++--- 6 files changed, 658 insertions(+), 87 deletions(-) create mode 100644 src/remote/networkStatus.ts create mode 100644 test/unit/remote/networkStatus.test.ts diff --git a/package.json b/package.json index 1b63145e..805ad9d7 100644 --- a/package.json +++ b/package.json @@ -167,6 +167,21 @@ "type": "boolean", "default": false }, + "coder.networkThreshold.latencyMs": { + "markdownDescription": "Latency threshold in milliseconds. A warning indicator appears in the status bar when latency exceeds this value. Set to `0` to disable.", + "type": "number", + "default": 200 + }, + "coder.networkThreshold.downloadMbps": { + "markdownDescription": "Download speed threshold in Mbps. A warning indicator appears in the status bar when download speed drops below this value. Set to `0` to disable.", + "type": "number", + "default": 5 + }, + "coder.networkThreshold.uploadMbps": { + "markdownDescription": "Upload speed threshold in Mbps. A warning indicator appears in the status bar when upload speed drops below this value. Set to `0` to disable.", + "type": "number", + "default": 0 + }, "coder.httpClientLogLevel": { "markdownDescription": "Controls the verbosity of HTTP client logging. This affects what details are logged for each HTTP request and response.", "type": "string", diff --git a/src/remote/networkStatus.ts b/src/remote/networkStatus.ts new file mode 100644 index 00000000..edc70757 --- /dev/null +++ b/src/remote/networkStatus.ts @@ -0,0 +1,231 @@ +import prettyBytes from "pretty-bytes"; +import * as vscode from "vscode"; + +import type { NetworkInfo } from "./sshProcess"; + +/** Bytes per second in 1 Mbps */ +const BYTES_PER_MBPS = 125_000; + +/** Number of consecutive polls required to trigger or clear a warning */ +const WARNING_DEBOUNCE_THRESHOLD = 3; + +export interface ThresholdViolations { + latency: boolean; + download: boolean; + upload: boolean; +} + +const NO_VIOLATIONS: ThresholdViolations = { + latency: false, + download: false, + upload: false, +}; + +export function getThresholdConfig(): { + latencyMs: number; + downloadMbps: number; + uploadMbps: number; +} { + const cfg = vscode.workspace.getConfiguration("coder"); + return { + latencyMs: cfg.get("networkThreshold.latencyMs", 200), + downloadMbps: cfg.get("networkThreshold.downloadMbps", 5), + uploadMbps: cfg.get("networkThreshold.uploadMbps", 0), + }; +} + +export function checkThresholdViolations( + network: NetworkInfo, + thresholds: { latencyMs: number; downloadMbps: number; uploadMbps: number }, +): ThresholdViolations { + return { + latency: thresholds.latencyMs > 0 && network.latency > thresholds.latencyMs, + download: + thresholds.downloadMbps > 0 && + network.download_bytes_sec / BYTES_PER_MBPS < thresholds.downloadMbps, + upload: + thresholds.uploadMbps > 0 && + network.upload_bytes_sec / BYTES_PER_MBPS < thresholds.uploadMbps, + }; +} + +export function hasAnyViolation(violations: ThresholdViolations): boolean { + return violations.latency || violations.download || violations.upload; +} + +export function getWarningCommand(violations: ThresholdViolations): string { + const latencyOnly = + violations.latency && !violations.download && !violations.upload; + const throughputOnly = + !violations.latency && (violations.download || violations.upload); + + if (latencyOnly) { + return "coder.pingWorkspace"; + } + if (throughputOnly) { + return "coder.speedTest"; + } + // Multiple types of violations โ€” let the user choose + return "coder.showNetworkDiagnostics"; +} + +export function buildNetworkTooltip( + network: NetworkInfo, + violations: ThresholdViolations, + thresholds: { latencyMs: number; downloadMbps: number; uploadMbps: number }, +): vscode.MarkdownString { + const fmt = (bytesPerSec: number) => + prettyBytes(bytesPerSec * 8, { bits: true }) + "/s"; + + const lines: string[] = []; + + let latencyLine = `Latency: ${network.latency.toFixed(2)}ms`; + if (violations.latency) { + latencyLine += ` $(warning) (threshold: ${thresholds.latencyMs}ms)`; + } + lines.push(latencyLine); + + let downloadLine = `Download: ${fmt(network.download_bytes_sec)}`; + if (violations.download) { + downloadLine += ` $(warning) (threshold: ${thresholds.downloadMbps} Mbit/s)`; + } + lines.push(downloadLine); + + let uploadLine = `Upload: ${fmt(network.upload_bytes_sec)}`; + if (violations.upload) { + uploadLine += ` $(warning) (threshold: ${thresholds.uploadMbps} Mbit/s)`; + } + lines.push(uploadLine); + + if (network.using_coder_connect) { + lines.push("Connection: Coder Connect"); + } else if (network.p2p) { + lines.push("Connection: Direct (P2P)"); + } else { + lines.push(`Connection: ${network.preferred_derp} (relay)`); + } + + if (hasAnyViolation(violations)) { + lines.push(""); + lines.push( + "_Click for diagnostics_ | [Configure thresholds](command:workbench.action.openSettings?%22coder.networkThreshold%22)", + ); + } + + const md = new vscode.MarkdownString(lines.join("\n\n")); + md.isTrusted = true; + md.supportThemeIcons = true; + return md; +} + +/** + * Manages network status bar presentation and slowness warning state. + * Owns the warning debounce logic, status bar updates, and the + * diagnostics command registration. + */ +export class NetworkStatusReporter implements vscode.Disposable { + private warningCounter = 0; + private isWarningActive = false; + private readonly diagnosticsCommand: vscode.Disposable; + + constructor(private readonly statusBarItem: vscode.StatusBarItem) { + this.diagnosticsCommand = vscode.commands.registerCommand( + "coder.showNetworkDiagnostics", + async () => { + const pick = await vscode.window.showQuickPick( + [ + { label: "Run Ping", commandId: "coder.pingWorkspace" }, + { label: "Run Speed Test", commandId: "coder.speedTest" }, + { + label: "Create Support Bundle", + commandId: "coder.supportBundle", + }, + ], + { placeHolder: "Select a diagnostic to run" }, + ); + if (pick) { + await vscode.commands.executeCommand(pick.commandId); + } + }, + ); + } + + update(network: NetworkInfo, isStale: boolean): void { + let statusText = "$(globe) "; + + // Coder Connect doesn't populate any other stats + if (network.using_coder_connect) { + this.warningCounter = 0; + this.isWarningActive = false; + this.statusBarItem.text = statusText + "Coder Connect "; + this.statusBarItem.tooltip = "You're connected using Coder Connect."; + this.statusBarItem.backgroundColor = undefined; + this.statusBarItem.command = undefined; + this.statusBarItem.show(); + return; + } + + const thresholds = getThresholdConfig(); + const violations = checkThresholdViolations(network, thresholds); + const activeViolations = this.updateWarningState(violations); + + if (network.p2p) { + statusText += "Direct "; + } else { + statusText += network.preferred_derp + " "; + } + + const latencyText = isStale + ? `(~${network.latency.toFixed(2)}ms)` + : `(${network.latency.toFixed(2)}ms)`; + statusText += latencyText; + this.statusBarItem.text = statusText; + + if (this.isWarningActive) { + this.statusBarItem.backgroundColor = new vscode.ThemeColor( + "statusBarItem.warningBackground", + ); + this.statusBarItem.command = getWarningCommand(activeViolations); + } else { + this.statusBarItem.backgroundColor = undefined; + this.statusBarItem.command = undefined; + } + + this.statusBarItem.tooltip = buildNetworkTooltip( + network, + activeViolations, + thresholds, + ); + + this.statusBarItem.show(); + } + + /** + * Updates the debounce counter and returns the effective violations + * (current violations when warning is active, all-clear otherwise). + */ + private updateWarningState( + violations: ThresholdViolations, + ): ThresholdViolations { + if (hasAnyViolation(violations)) { + this.warningCounter = Math.min( + this.warningCounter + 1, + WARNING_DEBOUNCE_THRESHOLD, + ); + } else { + this.warningCounter = Math.max(this.warningCounter - 1, 0); + } + + if (this.warningCounter >= WARNING_DEBOUNCE_THRESHOLD) { + this.isWarningActive = true; + } else if (this.warningCounter === 0) { + this.isWarningActive = false; + } + + return this.isWarningActive ? violations : NO_VIOLATIONS; + } + + dispose(): void { + this.diagnosticsCommand.dispose(); + } +} diff --git a/src/remote/sshProcess.ts b/src/remote/sshProcess.ts index c53c31a8..f4a4307f 100644 --- a/src/remote/sshProcess.ts +++ b/src/remote/sshProcess.ts @@ -1,12 +1,14 @@ import find from "find-process"; import * as fs from "node:fs/promises"; import * as path from "node:path"; -import prettyBytes from "pretty-bytes"; import * as vscode from "vscode"; -import { type Logger } from "../logging/logger"; import { findPort } from "../util"; +import { NetworkStatusReporter } from "./networkStatus"; + +import type { Logger } from "../logging/logger"; + /** * Network information from the Coder CLI. */ @@ -76,6 +78,7 @@ export class SshProcessMonitor implements vscode.Disposable { private logFilePath: string | undefined; private pendingTimeout: NodeJS.Timeout | undefined; private lastStaleSearchTime = 0; + private readonly reporter: NetworkStatusReporter; /** * Helper to clean up files in a directory. @@ -195,6 +198,7 @@ export class SshProcessMonitor implements vscode.Disposable { vscode.StatusBarAlignment.Left, 1000, ); + this.reporter = new NetworkStatusReporter(this.statusBarItem); } /** @@ -251,6 +255,7 @@ export class SshProcessMonitor implements vscode.Disposable { this.pendingTimeout = undefined; } this.statusBarItem.dispose(); + this.reporter.dispose(); this._onLogFilePathChange.dispose(); this._onPidChange.dispose(); } @@ -475,7 +480,7 @@ export class SshProcessMonitor implements vscode.Disposable { const content = await fs.readFile(filePath, "utf8"); const network = JSON.parse(content) as NetworkInfo; const isStale = ageMs > networkPollInterval * 2; - this.updateStatusBar(network, isStale); + this.reporter.update(network, isStale); } } catch (error) { readFailures++; @@ -508,63 +513,6 @@ export class SshProcessMonitor implements vscode.Disposable { await this.delay(networkPollInterval); } } - - /** - * Updates the status bar with network information. - */ - private updateStatusBar(network: NetworkInfo, isStale: boolean): void { - let statusText = "$(globe) "; - - // Coder Connect doesn't populate any other stats - if (network.using_coder_connect) { - this.statusBarItem.text = statusText + "Coder Connect "; - this.statusBarItem.tooltip = "You're connected using Coder Connect."; - this.statusBarItem.show(); - return; - } - - if (network.p2p) { - statusText += "Direct "; - this.statusBarItem.tooltip = "You're connected peer-to-peer โœจ."; - } else { - statusText += network.preferred_derp + " "; - this.statusBarItem.tooltip = - "You're connected through a relay ๐Ÿ•ต.\nWe'll switch over to peer-to-peer when available."; - } - - let tooltip = this.statusBarItem.tooltip; - tooltip += - "\n\nDownload โ†“ " + - prettyBytes(network.download_bytes_sec, { bits: true }) + - "/s โ€ข Upload โ†‘ " + - prettyBytes(network.upload_bytes_sec, { bits: true }) + - "/s\n"; - - if (!network.p2p) { - const derpLatency = network.derp_latency[network.preferred_derp]; - tooltip += `You โ†” ${derpLatency.toFixed(2)}ms โ†” ${network.preferred_derp} โ†” ${(network.latency - derpLatency).toFixed(2)}ms โ†” Workspace`; - - let first = true; - for (const region of Object.keys(network.derp_latency)) { - if (region === network.preferred_derp) { - continue; - } - if (first) { - tooltip += `\n\nOther regions:`; - first = false; - } - tooltip += `\n${region}: ${Math.round(network.derp_latency[region] * 100) / 100}ms`; - } - } - - this.statusBarItem.tooltip = tooltip; - const latencyText = isStale - ? `(~${network.latency.toFixed(2)}ms)` - : `(${network.latency.toFixed(2)}ms)`; - statusText += latencyText; - this.statusBarItem.text = statusText; - this.statusBarItem.show(); - } } /** diff --git a/test/mocks/vscode.runtime.ts b/test/mocks/vscode.runtime.ts index f8e3b490..633155e5 100644 --- a/test/mocks/vscode.runtime.ts +++ b/test/mocks/vscode.runtime.ts @@ -40,6 +40,23 @@ export const InputBoxValidationSeverity = E({ Error: 3, }); +export class MarkdownString { + value: string; + isTrusted = false; + supportThemeIcons = false; + + constructor(value = "") { + this.value = value; + } +} + +export class ThemeColor { + id: string; + constructor(id: string) { + this.id = id; + } +} + export class Uri { constructor( public scheme: string, @@ -118,7 +135,7 @@ export const window = { }; export const commands = { - registerCommand: vi.fn(), + registerCommand: vi.fn(() => ({ dispose: vi.fn() })), executeCommand: vi.fn(), }; @@ -170,6 +187,8 @@ const vscode = { InputBoxValidationSeverity, Uri, EventEmitter, + MarkdownString, + ThemeColor, window, commands, workspace, diff --git a/test/unit/remote/networkStatus.test.ts b/test/unit/remote/networkStatus.test.ts new file mode 100644 index 00000000..c0cc2b0c --- /dev/null +++ b/test/unit/remote/networkStatus.test.ts @@ -0,0 +1,242 @@ +import { describe, expect, it } from "vitest"; + +import { + buildNetworkTooltip, + checkThresholdViolations, + getWarningCommand, + hasAnyViolation, + type ThresholdViolations, +} from "@/remote/networkStatus"; + +import type { NetworkInfo } from "@/remote/sshProcess"; + +function makeNetwork(overrides: Partial = {}): NetworkInfo { + return { + p2p: true, + latency: 50, + preferred_derp: "NYC", + derp_latency: { NYC: 10 }, + upload_bytes_sec: 1_250_000, // 10 Mbps + download_bytes_sec: 6_250_000, // 50 Mbps + using_coder_connect: false, + ...overrides, + }; +} + +const defaultThresholds = { latencyMs: 200, downloadMbps: 5, uploadMbps: 0 }; + +const noViolations: ThresholdViolations = { + latency: false, + download: false, + upload: false, +}; + +function tooltip( + overrides: Partial = {}, + options: { + violations?: ThresholdViolations; + thresholds?: { + latencyMs: number; + downloadMbps: number; + uploadMbps: number; + }; + } = {}, +) { + return buildNetworkTooltip( + makeNetwork(overrides), + options.violations ?? noViolations, + options.thresholds ?? defaultThresholds, + ); +} + +describe("checkThresholdViolations", () => { + interface TestCase { + desc: string; + network: Partial; + thresholds?: typeof defaultThresholds; + expected: ThresholdViolations; + } + + it.each([ + { + desc: "no violations when within thresholds", + network: {}, + expected: { latency: false, download: false, upload: false }, + }, + { + desc: "detects high latency", + network: { latency: 250 }, + expected: { latency: true, download: false, upload: false }, + }, + { + desc: "detects low download (4 Mbps < 5 Mbps threshold)", + network: { download_bytes_sec: 500_000 }, + expected: { latency: false, download: true, upload: false }, + }, + { + desc: "detects low upload when threshold enabled", + network: { upload_bytes_sec: 100_000 }, + thresholds: { ...defaultThresholds, uploadMbps: 1 }, + expected: { latency: false, download: false, upload: true }, + }, + { + desc: "ignores upload when threshold is 0", + network: { upload_bytes_sec: 0 }, + expected: { latency: false, download: false, upload: false }, + }, + { + desc: "ignores latency when threshold is 0", + network: { latency: 9999 }, + thresholds: { ...defaultThresholds, latencyMs: 0 }, + expected: { latency: false, download: false, upload: false }, + }, + { + desc: "detects multiple simultaneous violations", + network: { latency: 300, download_bytes_sec: 100_000 }, + expected: { latency: true, download: true, upload: false }, + }, + ])("$desc", ({ network, thresholds, expected }) => { + expect( + checkThresholdViolations( + makeNetwork(network), + thresholds ?? defaultThresholds, + ), + ).toEqual(expected); + }); + + it("handles exact boundary (5 Mbps = 625,000 bytes/sec)", () => { + const at = checkThresholdViolations( + makeNetwork({ download_bytes_sec: 625_000 }), + defaultThresholds, + ); + expect(at.download).toBe(false); + + const below = checkThresholdViolations( + makeNetwork({ download_bytes_sec: 624_999 }), + defaultThresholds, + ); + expect(below.download).toBe(true); + }); +}); + +describe("hasAnyViolation", () => { + it.each<{ violations: ThresholdViolations; expected: boolean }>([ + { + violations: { latency: false, download: false, upload: false }, + expected: false, + }, + { + violations: { latency: true, download: false, upload: false }, + expected: true, + }, + { + violations: { latency: false, download: true, upload: false }, + expected: true, + }, + { + violations: { latency: false, download: false, upload: true }, + expected: true, + }, + ])("returns $expected for %j", ({ violations, expected }) => { + expect(hasAnyViolation(violations)).toBe(expected); + }); +}); + +describe("getWarningCommand", () => { + it.each<{ desc: string; violations: ThresholdViolations; expected: string }>([ + { + desc: "ping for latency only", + violations: { latency: true, download: false, upload: false }, + expected: "coder.pingWorkspace", + }, + { + desc: "speedtest for download only", + violations: { latency: false, download: true, upload: false }, + expected: "coder.speedTest", + }, + { + desc: "speedtest for upload only", + violations: { latency: false, download: false, upload: true }, + expected: "coder.speedTest", + }, + { + desc: "speedtest for download + upload", + violations: { latency: false, download: true, upload: true }, + expected: "coder.speedTest", + }, + { + desc: "diagnostics for latency + throughput", + violations: { latency: true, download: true, upload: false }, + expected: "coder.showNetworkDiagnostics", + }, + { + desc: "diagnostics for all violations", + violations: { latency: true, download: true, upload: true }, + expected: "coder.showNetworkDiagnostics", + }, + ])("returns $expected for $desc", ({ violations, expected }) => { + expect(getWarningCommand(violations)).toBe(expected); + }); +}); + +describe("buildNetworkTooltip", () => { + it("shows all metrics without warnings in normal state", () => { + const t = tooltip(); + expect(t.value).toContain("Latency: 50.00ms"); + expect(t.value).toContain("Download: 50 Mbit/s"); + expect(t.value).toContain("Upload: 10 Mbit/s"); + expect(t.value).not.toContain("$(warning)"); + expect(t.value).not.toContain("Click for diagnostics"); + expect(t.value).not.toContain("Configure thresholds"); + }); + + it("shows warning markers and actions when thresholds violated", () => { + const violations: ThresholdViolations = { + latency: true, + download: false, + upload: false, + }; + const t = tooltip({ latency: 350 }, { violations }); + expect(t.value).toContain( + "Latency: 350.00ms $(warning) (threshold: 200ms)", + ); + expect(t.value).not.toContain("Download:$(warning)"); + expect(t.value).toContain("Click for diagnostics"); + expect(t.value).toContain("Configure thresholds"); + }); + + it("shows multiple warning markers when multiple thresholds crossed", () => { + const violations: ThresholdViolations = { + latency: true, + download: true, + upload: false, + }; + const t = tooltip( + { latency: 300, download_bytes_sec: 100_000 }, + { violations }, + ); + expect(t.value).toContain("Latency: 300.00ms $(warning)"); + expect(t.value).toContain("Download: 800 kbit/s $(warning)"); + expect(t.value).toContain("Click for diagnostics"); + }); + + it.each<{ desc: string; overrides: Partial; expected: string }>([ + { + desc: "P2P", + overrides: { p2p: true }, + expected: "Connection: Direct (P2P)", + }, + { + desc: "relay", + overrides: { p2p: false, preferred_derp: "SFO" }, + expected: "Connection: SFO (relay)", + }, + { + desc: "Coder Connect", + overrides: { using_coder_connect: true }, + expected: "Connection: Coder Connect", + }, + ])("shows $desc connection type", ({ overrides, expected }) => { + expect(tooltip(overrides).value).toContain(expected); + }); +}); diff --git a/test/unit/remote/sshProcess.test.ts b/test/unit/remote/sshProcess.test.ts index b8726b33..0263eafc 100644 --- a/test/unit/remote/sshProcess.test.ts +++ b/test/unit/remote/sshProcess.test.ts @@ -3,16 +3,35 @@ import { vol } from "memfs"; import * as fsPromises from "node:fs/promises"; import path from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { ThemeColor } from "vscode"; import { SshProcessMonitor, + type NetworkInfo, type SshProcessMonitorOptions, } from "@/remote/sshProcess"; -import { createMockLogger, MockStatusBar } from "../../mocks/testHelpers"; +import { + createMockLogger, + MockConfigurationProvider, + MockStatusBar, +} from "../../mocks/testHelpers"; import type * as fs from "node:fs"; +function makeNetworkJson(overrides: Partial = {}): string { + return JSON.stringify({ + p2p: true, + latency: 50, + preferred_derp: "NYC", + derp_latency: { NYC: 10 }, + upload_bytes_sec: 1_000_000, + download_bytes_sec: 5_000_000, + using_coder_connect: false, + ...overrides, + }); +} + vi.mock("find-process", () => ({ default: vi.fn() })); vi.mock("node:fs/promises", async () => { @@ -29,6 +48,8 @@ describe("SshProcessMonitor", () => { vol.reset(); activeMonitors = []; statusBar = new MockStatusBar(); + // Provide default threshold config so getThresholdConfig() works + new MockConfigurationProvider(); // Default: process found immediately vi.mocked(find).mockResolvedValue([ @@ -402,20 +423,20 @@ describe("SshProcessMonitor", () => { }); }); + function tooltipText(): string { + const t = statusBar.tooltip; + if (typeof t === "string") { + return t; + } + return t?.value ?? ""; + } + describe("network status", () => { it("shows P2P connection in status bar", async () => { vol.fromJSON({ "/logs/ms-vscode-remote.remote-ssh/1-Remote - SSH.log": "-> socksPort 12345 ->", - "/network/999.json": JSON.stringify({ - p2p: true, - latency: 25.5, - preferred_derp: "NYC", - derp_latency: { NYC: 10 }, - upload_bytes_sec: 1024, - download_bytes_sec: 2048, - using_coder_connect: false, - }), + "/network/999.json": makeNetworkJson({ latency: 25.5 }), }); createMonitor({ @@ -426,21 +447,17 @@ describe("SshProcessMonitor", () => { expect(statusBar.text).toContain("Direct"); expect(statusBar.text).toContain("25.50ms"); - expect(statusBar.tooltip).toContain("peer-to-peer"); + expect(tooltipText()).toContain("Direct (P2P)"); }); it("shows relay connection with DERP region", async () => { vol.fromJSON({ "/logs/ms-vscode-remote.remote-ssh/1-Remote - SSH.log": "-> socksPort 12345 ->", - "/network/999.json": JSON.stringify({ + "/network/999.json": makeNetworkJson({ p2p: false, - latency: 50, preferred_derp: "SFO", derp_latency: { SFO: 20, NYC: 40 }, - upload_bytes_sec: 512, - download_bytes_sec: 1024, - using_coder_connect: false, }), }); @@ -451,22 +468,14 @@ describe("SshProcessMonitor", () => { await waitFor(() => statusBar.text.includes("SFO")); expect(statusBar.text).toContain("SFO"); - expect(statusBar.tooltip).toContain("relay"); + expect(tooltipText()).toContain("relay"); }); it("shows Coder Connect status", async () => { vol.fromJSON({ "/logs/ms-vscode-remote.remote-ssh/1-Remote - SSH.log": "-> socksPort 12345 ->", - "/network/999.json": JSON.stringify({ - p2p: false, - latency: 0, - preferred_derp: "", - derp_latency: {}, - upload_bytes_sec: 0, - download_bytes_sec: 0, - using_coder_connect: true, - }), + "/network/999.json": makeNetworkJson({ using_coder_connect: true }), }); createMonitor({ @@ -768,6 +777,113 @@ describe("SshProcessMonitor", () => { }); }); + describe("slowness detection", () => { + const sshLog = { + "/logs/ms-vscode-remote.remote-ssh/1-Remote - SSH.log": + "-> socksPort 12345 ->", + }; + + function setThresholds( + latencyMs: number, + downloadMbps = 0, + uploadMbps = 0, + ) { + const mockConfig = new MockConfigurationProvider(); + mockConfig.set("coder.networkThreshold.latencyMs", latencyMs); + mockConfig.set("coder.networkThreshold.downloadMbps", downloadMbps); + mockConfig.set("coder.networkThreshold.uploadMbps", uploadMbps); + } + + function startWithNetwork(networkOverrides: Partial = {}) { + vol.fromJSON({ + ...sshLog, + "/network/999.json": makeNetworkJson(networkOverrides), + }); + return createMonitor({ + codeLogDir: "/logs/window1", + networkInfoPath: "/network", + networkPollInterval: 10, + }); + } + + it("shows warning background after 3 consecutive slow polls", async () => { + setThresholds(100); + startWithNetwork({ latency: 200 }); + + await waitFor( + () => statusBar.backgroundColor instanceof ThemeColor, + 2000, + ); + expect(statusBar.backgroundColor).toBeInstanceOf(ThemeColor); + }); + + it("clears warning after 3 consecutive healthy polls", async () => { + setThresholds(100); + startWithNetwork({ latency: 200 }); + + await waitFor( + () => statusBar.backgroundColor instanceof ThemeColor, + 2000, + ); + + // Improve latency โ€” warning should clear after 3 healthy polls + vol.fromJSON({ + ...sshLog, + "/network/999.json": makeNetworkJson({ latency: 50 }), + }); + await waitFor(() => statusBar.backgroundColor === undefined, 2000); + expect(statusBar.backgroundColor).toBeUndefined(); + }); + + it("sets ping command when only latency is slow", async () => { + setThresholds(100); + startWithNetwork({ latency: 200 }); + + await waitFor(() => statusBar.command === "coder.pingWorkspace", 2000); + expect(statusBar.command).toBe("coder.pingWorkspace"); + }); + + it("sets speedtest command when only download is slow", async () => { + setThresholds(0, 10); + startWithNetwork({ download_bytes_sec: 500_000 }); // 4 Mbps < 10 + + await waitFor(() => statusBar.command === "coder.speedTest", 2000); + expect(statusBar.command).toBe("coder.speedTest"); + }); + + it("sets diagnostics command when multiple thresholds are crossed", async () => { + setThresholds(100, 10); + startWithNetwork({ latency: 200, download_bytes_sec: 500_000 }); + + await waitFor( + () => statusBar.command === "coder.showNetworkDiagnostics", + 2000, + ); + expect(statusBar.command).toBe("coder.showNetworkDiagnostics"); + }); + + it("does not show warning for Coder Connect connections", async () => { + setThresholds(100); + startWithNetwork({ using_coder_connect: true }); + + await waitFor(() => statusBar.text.includes("Coder Connect"), 2000); + expect(statusBar.backgroundColor).toBeUndefined(); + expect(statusBar.command).toBeUndefined(); + }); + + it("includes threshold info in tooltip when warning is active", async () => { + setThresholds(100); + startWithNetwork({ latency: 200 }); + + await waitFor( + () => statusBar.backgroundColor instanceof ThemeColor, + 2000, + ); + expect(tooltipText()).toContain("threshold: 100ms"); + expect(tooltipText()).toContain("Click for diagnostics"); + }); + }); + function createMonitor(overrides: Partial = {}) { const monitor = SshProcessMonitor.start({ sshHost: "coder-vscode--user--workspace", From 411f5f857fe1ce9d61b4ff22db6b31618ee6b278 Mon Sep 17 00:00:00 2001 From: Ehab Younes Date: Thu, 16 Apr 2026 19:41:36 +0300 Subject: [PATCH 2/2] fix: drop throughput-based slowness detection, polish tooltip The download_bytes_sec/upload_bytes_sec fields from the Coder CLI measure actual tunnel traffic during the poll window, not link capacity. Idle SSH sessions always dipped below the 5 Mbps default even on fast networks, producing spurious warnings. Keep latency as the only slowness signal. Display throughput in the tooltip as informational data only. Replace the warning-icon-mid-line layout with a bold header, tight metric rows, and an action row containing explicit Ping workspace and Configure threshold links. --- package.json | 10 - src/remote/networkStatus.ts | 172 +++++----------- src/remote/sshProcess.ts | 20 +- test/unit/remote/networkStatus.test.ts | 263 +++++++++---------------- test/unit/remote/sshProcess.test.ts | 31 +-- 5 files changed, 147 insertions(+), 349 deletions(-) diff --git a/package.json b/package.json index 805ad9d7..6609fe95 100644 --- a/package.json +++ b/package.json @@ -172,16 +172,6 @@ "type": "number", "default": 200 }, - "coder.networkThreshold.downloadMbps": { - "markdownDescription": "Download speed threshold in Mbps. A warning indicator appears in the status bar when download speed drops below this value. Set to `0` to disable.", - "type": "number", - "default": 5 - }, - "coder.networkThreshold.uploadMbps": { - "markdownDescription": "Upload speed threshold in Mbps. A warning indicator appears in the status bar when upload speed drops below this value. Set to `0` to disable.", - "type": "number", - "default": 0 - }, "coder.httpClientLogLevel": { "markdownDescription": "Controls the verbosity of HTTP client logging. This affects what details are logged for each HTTP request and response.", "type": "string", diff --git a/src/remote/networkStatus.ts b/src/remote/networkStatus.ts index edc70757..a6ff653c 100644 --- a/src/remote/networkStatus.ts +++ b/src/remote/networkStatus.ts @@ -3,116 +3,74 @@ import * as vscode from "vscode"; import type { NetworkInfo } from "./sshProcess"; -/** Bytes per second in 1 Mbps */ -const BYTES_PER_MBPS = 125_000; - /** Number of consecutive polls required to trigger or clear a warning */ const WARNING_DEBOUNCE_THRESHOLD = 3; -export interface ThresholdViolations { - latency: boolean; - download: boolean; - upload: boolean; -} - -const NO_VIOLATIONS: ThresholdViolations = { - latency: false, - download: false, - upload: false, -}; +const WARNING_BACKGROUND = new vscode.ThemeColor( + "statusBarItem.warningBackground", +); -export function getThresholdConfig(): { +export interface NetworkThresholds { latencyMs: number; - downloadMbps: number; - uploadMbps: number; -} { +} + +function getThresholdConfig(): NetworkThresholds { const cfg = vscode.workspace.getConfiguration("coder"); return { latencyMs: cfg.get("networkThreshold.latencyMs", 200), - downloadMbps: cfg.get("networkThreshold.downloadMbps", 5), - uploadMbps: cfg.get("networkThreshold.uploadMbps", 0), }; } -export function checkThresholdViolations( +export function isLatencySlow( network: NetworkInfo, - thresholds: { latencyMs: number; downloadMbps: number; uploadMbps: number }, -): ThresholdViolations { - return { - latency: thresholds.latencyMs > 0 && network.latency > thresholds.latencyMs, - download: - thresholds.downloadMbps > 0 && - network.download_bytes_sec / BYTES_PER_MBPS < thresholds.downloadMbps, - upload: - thresholds.uploadMbps > 0 && - network.upload_bytes_sec / BYTES_PER_MBPS < thresholds.uploadMbps, - }; -} - -export function hasAnyViolation(violations: ThresholdViolations): boolean { - return violations.latency || violations.download || violations.upload; -} - -export function getWarningCommand(violations: ThresholdViolations): string { - const latencyOnly = - violations.latency && !violations.download && !violations.upload; - const throughputOnly = - !violations.latency && (violations.download || violations.upload); - - if (latencyOnly) { - return "coder.pingWorkspace"; - } - if (throughputOnly) { - return "coder.speedTest"; - } - // Multiple types of violations โ€” let the user choose - return "coder.showNetworkDiagnostics"; + thresholds: NetworkThresholds, +): boolean { + return thresholds.latencyMs > 0 && network.latency > thresholds.latencyMs; } export function buildNetworkTooltip( network: NetworkInfo, - violations: ThresholdViolations, - thresholds: { latencyMs: number; downloadMbps: number; uploadMbps: number }, + latencySlow: boolean, + thresholds: NetworkThresholds, ): vscode.MarkdownString { const fmt = (bytesPerSec: number) => prettyBytes(bytesPerSec * 8, { bits: true }) + "/s"; - const lines: string[] = []; - - let latencyLine = `Latency: ${network.latency.toFixed(2)}ms`; - if (violations.latency) { - latencyLine += ` $(warning) (threshold: ${thresholds.latencyMs}ms)`; - } - lines.push(latencyLine); + const sections: string[] = []; - let downloadLine = `Download: ${fmt(network.download_bytes_sec)}`; - if (violations.download) { - downloadLine += ` $(warning) (threshold: ${thresholds.downloadMbps} Mbit/s)`; + if (latencySlow) { + sections.push("$(warning) **Slow connection detected**"); } - lines.push(downloadLine); - let uploadLine = `Upload: ${fmt(network.upload_bytes_sec)}`; - if (violations.upload) { - uploadLine += ` $(warning) (threshold: ${thresholds.uploadMbps} Mbit/s)`; - } - lines.push(uploadLine); + const metrics: string[] = []; + metrics.push( + latencySlow + ? `Latency: ${network.latency.toFixed(2)}ms (threshold: ${thresholds.latencyMs}ms)` + : `Latency: ${network.latency.toFixed(2)}ms`, + ); + metrics.push(`Download: ${fmt(network.download_bytes_sec)}`); + metrics.push(`Upload: ${fmt(network.upload_bytes_sec)}`); if (network.using_coder_connect) { - lines.push("Connection: Coder Connect"); + metrics.push("Connection: Coder Connect"); } else if (network.p2p) { - lines.push("Connection: Direct (P2P)"); + metrics.push("Connection: Direct (P2P)"); } else { - lines.push(`Connection: ${network.preferred_derp} (relay)`); + metrics.push(`Connection: ${network.preferred_derp} (relay)`); } - if (hasAnyViolation(violations)) { - lines.push(""); - lines.push( - "_Click for diagnostics_ | [Configure thresholds](command:workbench.action.openSettings?%22coder.networkThreshold%22)", + // Two trailing spaces + \n = hard line break (tight rows within a section). + sections.push(metrics.join(" \n")); + + if (latencySlow) { + sections.push( + "[$(pulse) Ping workspace](command:coder.pingWorkspace) ยท " + + "[$(gear) Configure threshold](command:workbench.action.openSettings?%22coder.networkThreshold%22)", ); } - const md = new vscode.MarkdownString(lines.join("\n\n")); + // Blank line between sections = paragraph break. + const md = new vscode.MarkdownString(sections.join("\n\n")); md.isTrusted = true; md.supportThemeIcons = true; return md; @@ -120,35 +78,13 @@ export function buildNetworkTooltip( /** * Manages network status bar presentation and slowness warning state. - * Owns the warning debounce logic, status bar updates, and the - * diagnostics command registration. + * Owns the warning debounce logic and status bar updates. */ -export class NetworkStatusReporter implements vscode.Disposable { +export class NetworkStatusReporter { private warningCounter = 0; private isWarningActive = false; - private readonly diagnosticsCommand: vscode.Disposable; - - constructor(private readonly statusBarItem: vscode.StatusBarItem) { - this.diagnosticsCommand = vscode.commands.registerCommand( - "coder.showNetworkDiagnostics", - async () => { - const pick = await vscode.window.showQuickPick( - [ - { label: "Run Ping", commandId: "coder.pingWorkspace" }, - { label: "Run Speed Test", commandId: "coder.speedTest" }, - { - label: "Create Support Bundle", - commandId: "coder.supportBundle", - }, - ], - { placeHolder: "Select a diagnostic to run" }, - ); - if (pick) { - await vscode.commands.executeCommand(pick.commandId); - } - }, - ); - } + + constructor(private readonly statusBarItem: vscode.StatusBarItem) {} update(network: NetworkInfo, isStale: boolean): void { let statusText = "$(globe) "; @@ -166,8 +102,8 @@ export class NetworkStatusReporter implements vscode.Disposable { } const thresholds = getThresholdConfig(); - const violations = checkThresholdViolations(network, thresholds); - const activeViolations = this.updateWarningState(violations); + const latencySlow = isLatencySlow(network, thresholds); + this.updateWarningState(latencySlow); if (network.p2p) { statusText += "Direct "; @@ -182,10 +118,8 @@ export class NetworkStatusReporter implements vscode.Disposable { this.statusBarItem.text = statusText; if (this.isWarningActive) { - this.statusBarItem.backgroundColor = new vscode.ThemeColor( - "statusBarItem.warningBackground", - ); - this.statusBarItem.command = getWarningCommand(activeViolations); + this.statusBarItem.backgroundColor = WARNING_BACKGROUND; + this.statusBarItem.command = "coder.pingWorkspace"; } else { this.statusBarItem.backgroundColor = undefined; this.statusBarItem.command = undefined; @@ -193,21 +127,15 @@ export class NetworkStatusReporter implements vscode.Disposable { this.statusBarItem.tooltip = buildNetworkTooltip( network, - activeViolations, + this.isWarningActive, thresholds, ); this.statusBarItem.show(); } - /** - * Updates the debounce counter and returns the effective violations - * (current violations when warning is active, all-clear otherwise). - */ - private updateWarningState( - violations: ThresholdViolations, - ): ThresholdViolations { - if (hasAnyViolation(violations)) { + private updateWarningState(latencySlow: boolean): void { + if (latencySlow) { this.warningCounter = Math.min( this.warningCounter + 1, WARNING_DEBOUNCE_THRESHOLD, @@ -221,11 +149,5 @@ export class NetworkStatusReporter implements vscode.Disposable { } else if (this.warningCounter === 0) { this.isWarningActive = false; } - - return this.isWarningActive ? violations : NO_VIOLATIONS; - } - - dispose(): void { - this.diagnosticsCommand.dispose(); } } diff --git a/src/remote/sshProcess.ts b/src/remote/sshProcess.ts index f4a4307f..df88938a 100644 --- a/src/remote/sshProcess.ts +++ b/src/remote/sshProcess.ts @@ -255,7 +255,6 @@ export class SshProcessMonitor implements vscode.Disposable { this.pendingTimeout = undefined; } this.statusBarItem.dispose(); - this.reporter.dispose(); this._onLogFilePathChange.dispose(); this._onPidChange.dispose(); } @@ -462,9 +461,7 @@ export class SshProcessMonitor implements vscode.Disposable { while (!this.disposed && this.currentPid !== undefined) { const filePath = path.join(networkInfoPath, `${this.currentPid}.json`); - let search: { needed: true; reason: string } | { needed: false } = { - needed: false, - }; + let searchReason: string | undefined; try { const stats = await fs.stat(filePath); @@ -472,10 +469,7 @@ export class SshProcessMonitor implements vscode.Disposable { readFailures = 0; if (ageMs > staleThreshold) { - search = { - needed: true, - reason: `Network info stale (${Math.round(ageMs / 1000)}s old)`, - }; + searchReason = `Network info stale (${Math.round(ageMs / 1000)}s old)`; } else { const content = await fs.readFile(filePath, "utf8"); const network = JSON.parse(content) as NetworkInfo; @@ -488,22 +482,18 @@ export class SshProcessMonitor implements vscode.Disposable { `Failed to read network info (attempt ${readFailures}): ${(error as Error).message}`, ); if (readFailures >= maxReadFailures) { - search = { - needed: true, - reason: `Network info missing for ${readFailures} attempts`, - }; + searchReason = `Network info missing for ${readFailures} attempts`; } } - // Search for new process if needed (with throttling) - if (search.needed) { + if (searchReason !== undefined) { const timeSinceLastSearch = Date.now() - this.lastStaleSearchTime; if (timeSinceLastSearch < staleThreshold) { await this.delay(staleThreshold - timeSinceLastSearch); continue; } - logger.debug(`${search.reason}, searching for new SSH process`); + logger.debug(`${searchReason}, searching for new SSH process`); // searchForProcess will update PID if a different process is found this.lastStaleSearchTime = Date.now(); await this.searchForProcess(); diff --git a/test/unit/remote/networkStatus.test.ts b/test/unit/remote/networkStatus.test.ts index c0cc2b0c..4a9dcb96 100644 --- a/test/unit/remote/networkStatus.test.ts +++ b/test/unit/remote/networkStatus.test.ts @@ -1,13 +1,18 @@ import { describe, expect, it } from "vitest"; +import { ThemeColor } from "vscode"; import { buildNetworkTooltip, - checkThresholdViolations, - getWarningCommand, - hasAnyViolation, - type ThresholdViolations, + isLatencySlow, + NetworkStatusReporter, + type NetworkThresholds, } from "@/remote/networkStatus"; +import { + MockConfigurationProvider, + MockStatusBar, +} from "../../mocks/testHelpers"; + import type { NetworkInfo } from "@/remote/sshProcess"; function makeNetwork(overrides: Partial = {}): NetworkInfo { @@ -16,208 +21,73 @@ function makeNetwork(overrides: Partial = {}): NetworkInfo { latency: 50, preferred_derp: "NYC", derp_latency: { NYC: 10 }, - upload_bytes_sec: 1_250_000, // 10 Mbps - download_bytes_sec: 6_250_000, // 50 Mbps + upload_bytes_sec: 1_250_000, + download_bytes_sec: 6_250_000, using_coder_connect: false, ...overrides, }; } -const defaultThresholds = { latencyMs: 200, downloadMbps: 5, uploadMbps: 0 }; - -const noViolations: ThresholdViolations = { - latency: false, - download: false, - upload: false, -}; +const defaultThresholds: NetworkThresholds = { latencyMs: 200 }; function tooltip( overrides: Partial = {}, options: { - violations?: ThresholdViolations; - thresholds?: { - latencyMs: number; - downloadMbps: number; - uploadMbps: number; - }; + latencySlow?: boolean; + thresholds?: NetworkThresholds; } = {}, ) { return buildNetworkTooltip( makeNetwork(overrides), - options.violations ?? noViolations, + options.latencySlow ?? false, options.thresholds ?? defaultThresholds, ); } -describe("checkThresholdViolations", () => { - interface TestCase { - desc: string; - network: Partial; - thresholds?: typeof defaultThresholds; - expected: ThresholdViolations; - } - - it.each([ - { - desc: "no violations when within thresholds", - network: {}, - expected: { latency: false, download: false, upload: false }, - }, - { - desc: "detects high latency", - network: { latency: 250 }, - expected: { latency: true, download: false, upload: false }, - }, - { - desc: "detects low download (4 Mbps < 5 Mbps threshold)", - network: { download_bytes_sec: 500_000 }, - expected: { latency: false, download: true, upload: false }, - }, - { - desc: "detects low upload when threshold enabled", - network: { upload_bytes_sec: 100_000 }, - thresholds: { ...defaultThresholds, uploadMbps: 1 }, - expected: { latency: false, download: false, upload: true }, - }, - { - desc: "ignores upload when threshold is 0", - network: { upload_bytes_sec: 0 }, - expected: { latency: false, download: false, upload: false }, - }, - { - desc: "ignores latency when threshold is 0", - network: { latency: 9999 }, - thresholds: { ...defaultThresholds, latencyMs: 0 }, - expected: { latency: false, download: false, upload: false }, - }, - { - desc: "detects multiple simultaneous violations", - network: { latency: 300, download_bytes_sec: 100_000 }, - expected: { latency: true, download: true, upload: false }, - }, - ])("$desc", ({ network, thresholds, expected }) => { - expect( - checkThresholdViolations( - makeNetwork(network), - thresholds ?? defaultThresholds, - ), - ).toEqual(expected); - }); - - it("handles exact boundary (5 Mbps = 625,000 bytes/sec)", () => { - const at = checkThresholdViolations( - makeNetwork({ download_bytes_sec: 625_000 }), - defaultThresholds, - ); - expect(at.download).toBe(false); - - const below = checkThresholdViolations( - makeNetwork({ download_bytes_sec: 624_999 }), - defaultThresholds, +describe("isLatencySlow", () => { + it("returns false when latency is within threshold", () => { + expect(isLatencySlow(makeNetwork({ latency: 50 }), defaultThresholds)).toBe( + false, ); - expect(below.download).toBe(true); }); -}); -describe("hasAnyViolation", () => { - it.each<{ violations: ThresholdViolations; expected: boolean }>([ - { - violations: { latency: false, download: false, upload: false }, - expected: false, - }, - { - violations: { latency: true, download: false, upload: false }, - expected: true, - }, - { - violations: { latency: false, download: true, upload: false }, - expected: true, - }, - { - violations: { latency: false, download: false, upload: true }, - expected: true, - }, - ])("returns $expected for %j", ({ violations, expected }) => { - expect(hasAnyViolation(violations)).toBe(expected); + it("returns true when latency exceeds threshold", () => { + expect( + isLatencySlow(makeNetwork({ latency: 250 }), defaultThresholds), + ).toBe(true); }); -}); -describe("getWarningCommand", () => { - it.each<{ desc: string; violations: ThresholdViolations; expected: string }>([ - { - desc: "ping for latency only", - violations: { latency: true, download: false, upload: false }, - expected: "coder.pingWorkspace", - }, - { - desc: "speedtest for download only", - violations: { latency: false, download: true, upload: false }, - expected: "coder.speedTest", - }, - { - desc: "speedtest for upload only", - violations: { latency: false, download: false, upload: true }, - expected: "coder.speedTest", - }, - { - desc: "speedtest for download + upload", - violations: { latency: false, download: true, upload: true }, - expected: "coder.speedTest", - }, - { - desc: "diagnostics for latency + throughput", - violations: { latency: true, download: true, upload: false }, - expected: "coder.showNetworkDiagnostics", - }, - { - desc: "diagnostics for all violations", - violations: { latency: true, download: true, upload: true }, - expected: "coder.showNetworkDiagnostics", - }, - ])("returns $expected for $desc", ({ violations, expected }) => { - expect(getWarningCommand(violations)).toBe(expected); + it("ignores latency when threshold is 0", () => { + expect( + isLatencySlow(makeNetwork({ latency: 9999 }), { latencyMs: 0 }), + ).toBe(false); }); }); describe("buildNetworkTooltip", () => { - it("shows all metrics without warnings in normal state", () => { + it("shows all metrics without warning or actions in normal state", () => { const t = tooltip(); expect(t.value).toContain("Latency: 50.00ms"); expect(t.value).toContain("Download: 50 Mbit/s"); expect(t.value).toContain("Upload: 10 Mbit/s"); expect(t.value).not.toContain("$(warning)"); - expect(t.value).not.toContain("Click for diagnostics"); - expect(t.value).not.toContain("Configure thresholds"); + expect(t.value).not.toContain("Slow connection"); + expect(t.value).not.toContain("command:coder.pingWorkspace"); }); - it("shows warning markers and actions when thresholds violated", () => { - const violations: ThresholdViolations = { - latency: true, - download: false, - upload: false, - }; - const t = tooltip({ latency: 350 }, { violations }); - expect(t.value).toContain( - "Latency: 350.00ms $(warning) (threshold: 200ms)", - ); - expect(t.value).not.toContain("Download:$(warning)"); - expect(t.value).toContain("Click for diagnostics"); - expect(t.value).toContain("Configure thresholds"); + it("shows warning header, threshold, and action links when latency is slow", () => { + const t = tooltip({ latency: 350 }, { latencySlow: true }); + expect(t.value).toContain("$(warning) **Slow connection detected**"); + expect(t.value).toContain("Latency: 350.00ms (threshold: 200ms)"); + expect(t.value).toContain("command:coder.pingWorkspace"); + expect(t.value).toContain("command:workbench.action.openSettings"); + expect(t.value).toContain("Ping workspace"); + expect(t.value).toContain("Configure threshold"); }); - it("shows multiple warning markers when multiple thresholds crossed", () => { - const violations: ThresholdViolations = { - latency: true, - download: true, - upload: false, - }; - const t = tooltip( - { latency: 300, download_bytes_sec: 100_000 }, - { violations }, - ); - expect(t.value).toContain("Latency: 300.00ms $(warning)"); - expect(t.value).toContain("Download: 800 kbit/s $(warning)"); - expect(t.value).toContain("Click for diagnostics"); + it("does not mark throughput lines with warnings", () => { + const t = tooltip({ download_bytes_sec: 100_000 }, { latencySlow: true }); + expect(t.value).not.toContain("Download: 800 kbit/s $(warning)"); }); it.each<{ desc: string; overrides: Partial; expected: string }>([ @@ -240,3 +110,54 @@ describe("buildNetworkTooltip", () => { expect(tooltip(overrides).value).toContain(expected); }); }); + +describe("NetworkStatusReporter hysteresis", () => { + function setup(latencyMs: number) { + const cfg = new MockConfigurationProvider(); + cfg.set("coder.networkThreshold.latencyMs", latencyMs); + const bar = new MockStatusBar(); + const reporter = new NetworkStatusReporter( + bar as unknown as import("vscode").StatusBarItem, + ); + return { bar, reporter }; + } + + const slow = makeNetwork({ latency: 500 }); + const healthy = makeNetwork({ latency: 50 }); + + it("does not warn if slow polls never reach the debounce threshold", () => { + const { bar, reporter } = setup(100); + reporter.update(slow, false); + reporter.update(slow, false); + reporter.update(healthy, false); + reporter.update(healthy, false); + expect(bar.backgroundColor).toBeUndefined(); + expect(bar.command).toBeUndefined(); + }); + + it("stays warning if a single healthy poll appears mid-streak", () => { + const { bar, reporter } = setup(100); + for (let i = 0; i < 3; i++) { + reporter.update(slow, false); + } + expect(bar.backgroundColor).toBeInstanceOf(ThemeColor); + + reporter.update(healthy, false); + expect(bar.backgroundColor).toBeInstanceOf(ThemeColor); + + reporter.update(slow, false); + expect(bar.backgroundColor).toBeInstanceOf(ThemeColor); + }); + + it("clears immediately when Coder Connect takes over", () => { + const { bar, reporter } = setup(100); + for (let i = 0; i < 3; i++) { + reporter.update(slow, false); + } + expect(bar.backgroundColor).toBeInstanceOf(ThemeColor); + + reporter.update(makeNetwork({ using_coder_connect: true }), false); + expect(bar.backgroundColor).toBeUndefined(); + expect(bar.command).toBeUndefined(); + }); +}); diff --git a/test/unit/remote/sshProcess.test.ts b/test/unit/remote/sshProcess.test.ts index 0263eafc..021b9424 100644 --- a/test/unit/remote/sshProcess.test.ts +++ b/test/unit/remote/sshProcess.test.ts @@ -783,15 +783,9 @@ describe("SshProcessMonitor", () => { "-> socksPort 12345 ->", }; - function setThresholds( - latencyMs: number, - downloadMbps = 0, - uploadMbps = 0, - ) { + function setThresholds(latencyMs: number) { const mockConfig = new MockConfigurationProvider(); mockConfig.set("coder.networkThreshold.latencyMs", latencyMs); - mockConfig.set("coder.networkThreshold.downloadMbps", downloadMbps); - mockConfig.set("coder.networkThreshold.uploadMbps", uploadMbps); } function startWithNetwork(networkOverrides: Partial = {}) { @@ -835,7 +829,7 @@ describe("SshProcessMonitor", () => { expect(statusBar.backgroundColor).toBeUndefined(); }); - it("sets ping command when only latency is slow", async () => { + it("sets ping command when latency is slow", async () => { setThresholds(100); startWithNetwork({ latency: 200 }); @@ -843,25 +837,6 @@ describe("SshProcessMonitor", () => { expect(statusBar.command).toBe("coder.pingWorkspace"); }); - it("sets speedtest command when only download is slow", async () => { - setThresholds(0, 10); - startWithNetwork({ download_bytes_sec: 500_000 }); // 4 Mbps < 10 - - await waitFor(() => statusBar.command === "coder.speedTest", 2000); - expect(statusBar.command).toBe("coder.speedTest"); - }); - - it("sets diagnostics command when multiple thresholds are crossed", async () => { - setThresholds(100, 10); - startWithNetwork({ latency: 200, download_bytes_sec: 500_000 }); - - await waitFor( - () => statusBar.command === "coder.showNetworkDiagnostics", - 2000, - ); - expect(statusBar.command).toBe("coder.showNetworkDiagnostics"); - }); - it("does not show warning for Coder Connect connections", async () => { setThresholds(100); startWithNetwork({ using_coder_connect: true }); @@ -880,7 +855,7 @@ describe("SshProcessMonitor", () => { 2000, ); expect(tooltipText()).toContain("threshold: 100ms"); - expect(tooltipText()).toContain("Click for diagnostics"); + expect(tooltipText()).toContain("Ping workspace"); }); });