mirror of
https://github.com/coder/coder.git
synced 2026-10-16 14:48:56 +08:00
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
<details>
<summary>Context and findings</summary>
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.
</details>
_Generated by Coder Agents on behalf of @jeremyruppel._
This commit is contained in:
@@ -110,6 +110,7 @@
|
||||
"rules-of-hooks": "error",
|
||||
"refs": "error",
|
||||
"erasing-op": "error",
|
||||
"no-unused-expressions": "error",
|
||||
|
||||
// --- security ---
|
||||
"jsx-no-target-blank": "error",
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -541,7 +541,9 @@ export const MultiSelectCombobox: React.FC<MultiSelectComboboxProps> = ({
|
||||
}}
|
||||
onFocus={(event) => {
|
||||
setOpen(true);
|
||||
triggerSearchOnFocus && onSearch?.(debouncedSearchTerm);
|
||||
if (triggerSearchOnFocus) {
|
||||
onSearch?.(debouncedSearchTerm);
|
||||
}
|
||||
inputProps?.onFocus?.(event);
|
||||
}}
|
||||
placeholder={
|
||||
|
||||
@@ -103,7 +103,11 @@ export const WorkspaceTerminal = ({
|
||||
const fitAddonRef = useRef<FitAddon | undefined>(undefined);
|
||||
const websocketRef = useRef<Websocket | undefined>(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);
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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()}`;
|
||||
|
||||
Reference in New Issue
Block a user