From abc2faed6004a928dc4fe7a4c76c2f4391685e9d Mon Sep 17 00:00:00 2001 From: Jeremy Ruppel Date: Mon, 12 Oct 2026 10:31:07 -0400 Subject: [PATCH] fix(site): keep path separator in terminal websocket URL (#30573) Enables oxlint's `no-unused-expressions` rule and fixes its violations, two of which were bugs. - `terminalWebsocketUrl` built `` `${url.pathname}/` `` without assigning it, so a base URL with a path (for example a proxy at `/prefix`) produced `/prefixapi/v2/...`. It now assigns `url.pathname`, and a test covers it (the test fails without the fix). - The deployment `experiments` e2e test called `expect(...).toBeVisible` without `()` or `await`, so those two assertions never ran. The remaining three hits were `a && f()` and ternaries used as statements. These are rewritten as `if` statements with no behavior change. Depends on #30572
Context and findings Part of evaluating oxlint's `correctness` category one rule at a time (see #30572). This rule had 6 hits in 5 files. The e2e fix makes two assertions active for the first time. Enabled experiments render with `option-enabled` in `OptionValue`, so they are expected to pass, but they were not run locally. Validation: `pnpm run lint` (Biome, oxlint, types, circular deps, React Compiler, knip) and the full unit suite.
_Generated by Coder Agents on behalf of @jeremyruppel._ --- site/.oxlintrc.jsonc | 1 + site/e2e/tests/deployment/general.spec.ts | 8 +++---- .../MultiSelectCombobox.tsx | 4 +++- .../modules/terminal/WorkspaceTerminal.tsx | 6 ++++- site/src/testHelpers/storybook.tsx | 8 ++++--- site/src/utils/terminal.test.ts | 24 ++++++++++++++++++- site/src/utils/terminal.ts | 2 +- 7 files changed, 42 insertions(+), 11 deletions(-) diff --git a/site/.oxlintrc.jsonc b/site/.oxlintrc.jsonc index c8632c91a4..397291f0a4 100644 --- a/site/.oxlintrc.jsonc +++ b/site/.oxlintrc.jsonc @@ -110,6 +110,7 @@ "rules-of-hooks": "error", "refs": "error", "erasing-op": "error", + "no-unused-expressions": "error", // --- security --- "jsx-no-target-blank": "error", diff --git a/site/e2e/tests/deployment/general.spec.ts b/site/e2e/tests/deployment/general.spec.ts index e4f570bb66..ea5156dbb9 100644 --- a/site/e2e/tests/deployment/general.spec.ts +++ b/site/e2e/tests/deployment/general.spec.ts @@ -24,16 +24,16 @@ test("experiments", async ({ page }) => { await expect(experimentsLocator).toBeVisible(); // Firstly, check if all enabled experiments are listed - expect( + await expect( experimentsLocator.locator( `li.option-array-item-${e2eFakeExperiment1}.option-enabled`, ), - ).toBeVisible; - expect( + ).toBeVisible(); + await expect( experimentsLocator.locator( `li.option-array-item-${e2eFakeExperiment2}.option-enabled`, ), - ).toBeVisible; + ).toBeVisible(); // Secondly, check if available experiments are listed for (const experiment of availableExperiments.safe) { diff --git a/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx b/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx index c201027eec..8e44001869 100644 --- a/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx +++ b/site/src/components/MultiSelectCombobox/MultiSelectCombobox.tsx @@ -541,7 +541,9 @@ export const MultiSelectCombobox: React.FC = ({ }} onFocus={(event) => { setOpen(true); - triggerSearchOnFocus && onSearch?.(debouncedSearchTerm); + if (triggerSearchOnFocus) { + onSearch?.(debouncedSearchTerm); + } inputProps?.onFocus?.(event); }} placeholder={ diff --git a/site/src/modules/terminal/WorkspaceTerminal.tsx b/site/src/modules/terminal/WorkspaceTerminal.tsx index 8be8af2e8b..29e34127ce 100644 --- a/site/src/modules/terminal/WorkspaceTerminal.tsx +++ b/site/src/modules/terminal/WorkspaceTerminal.tsx @@ -103,7 +103,11 @@ export const WorkspaceTerminal = ({ const fitAddonRef = useRef(undefined); const websocketRef = useRef(undefined); const handleOpenLink = useEffectEvent((uri: string) => { - onOpenLink ? onOpenLink(uri) : window.open(uri, "_blank", "noopener"); + if (onOpenLink) { + onOpenLink(uri); + } else { + window.open(uri, "_blank", "noopener"); + } }); const handleStatusChange = useEffectEvent((status: ConnectionStatus) => { onStatusChange?.(status); diff --git a/site/src/testHelpers/storybook.tsx b/site/src/testHelpers/storybook.tsx index da7cc82e66..f84b81db4d 100644 --- a/site/src/testHelpers/storybook.tsx +++ b/site/src/testHelpers/storybook.tsx @@ -142,9 +142,11 @@ export const withWebSocket = ( const callback = this.#listeners.get(entry.event); if (callback) { - entry.event === "message" - ? callback({ data: entry.data }) - : callback(); + if (entry.event === "message") { + callback({ data: entry.data }); + } else { + callback(); + } } } }, 0); diff --git a/site/src/utils/terminal.test.ts b/site/src/utils/terminal.test.ts index 44897f9c1e..caf59a3812 100644 --- a/site/src/utils/terminal.test.ts +++ b/site/src/utils/terminal.test.ts @@ -1,4 +1,5 @@ -import { describe, expect, it } from "vitest"; +import { describe, expect, it, vi } from "vitest"; +import { API } from "#/api/api"; import { terminalWebsocketUrl } from "./terminal"; describe("terminalWebsocketUrl", () => { @@ -21,4 +22,25 @@ describe("terminalWebsocketUrl", () => { ); expect(parsed.searchParams.get("reconnect")).toBe("reconnect-token"); }); + + it("adds a path separator after a base URL path", async () => { + vi.spyOn(API, "issueReconnectingPTYSignedToken").mockResolvedValue({ + signed_token: "token", + }); + const url = await terminalWebsocketUrl( + "https://proxy.example.com/prefix", + "reconnect-token", + "agent-id", + undefined, + 24, + 80, + undefined, + undefined, + "session-id", + ); + + expect(new URL(url).pathname).toBe( + "/prefix/api/v2/workspaceagents/agent-id/pty", + ); + }); }); diff --git a/site/src/utils/terminal.ts b/site/src/utils/terminal.ts index ef355be6d9..325a39487b 100644 --- a/site/src/utils/terminal.ts +++ b/site/src/utils/terminal.ts @@ -28,7 +28,7 @@ export const terminalWebsocketUrl = async ( const url = new URL(baseUrl || `${location.protocol}//${location.host}`); url.protocol = url.protocol === "https:" ? "wss:" : "ws:"; if (!url.pathname.endsWith("/")) { - `${url.pathname}/`; + url.pathname = `${url.pathname}/`; } url.pathname += `api/v2/workspaceagents/${agentId}/pty`; url.search = `?${query.toString()}`;