From 7f937cf7be4453e111208e1dc7ac249a038c8305 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 15 Jul 2025 22:19:22 +0000 Subject: [PATCH 1/5] fix(site): only attempt to watch when dev containers enabled --- .../resources/useAgentContainers.test.tsx | 76 +++++++++++++++---- .../modules/resources/useAgentContainers.ts | 30 +++++++- 2 files changed, 87 insertions(+), 19 deletions(-) diff --git a/site/src/modules/resources/useAgentContainers.test.tsx b/site/src/modules/resources/useAgentContainers.test.tsx index dbdcdf6f21293..d3bde93dfa1d3 100644 --- a/site/src/modules/resources/useAgentContainers.test.tsx +++ b/site/src/modules/resources/useAgentContainers.test.tsx @@ -4,23 +4,19 @@ import type { WorkspaceAgentListContainersResponse } from "api/typesGenerated"; import * as GlobalSnackbar from "components/GlobalSnackbar/utils"; import { http, HttpResponse } from "msw"; import type { FC, PropsWithChildren } from "react"; -import { QueryClient, QueryClientProvider } from "react-query"; +import { act } from "react"; +import { QueryClientProvider } from "react-query"; import { MockWorkspaceAgent, MockWorkspaceAgentDevcontainer, } from "testHelpers/entities"; +import { createTestQueryClient } from "testHelpers/renderHelpers"; import { server } from "testHelpers/server"; import type { OneWayWebSocket } from "utils/OneWayWebSocket"; import { useAgentContainers } from "./useAgentContainers"; const createWrapper = (): FC => { - const queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, - }); + const queryClient = createTestQueryClient(); return ({ children }) => ( {children} ); @@ -111,22 +107,29 @@ describe("useAgentContainers", () => { ), ); - const { unmount } = renderHook( + const { result, unmount } = renderHook( () => useAgentContainers(MockWorkspaceAgent), { wrapper: createWrapper(), }, ); - // Simulate message event with parsing error + // Wait for initial query to complete + await waitFor(() => { + expect(result.current).toEqual([MockWorkspaceAgentDevcontainer]); + }); + + // Now simulate message event with parsing error const messageHandler = mockSocket.addEventListener.mock.calls.find( (call) => call[0] === "message", )?.[1]; if (messageHandler) { - messageHandler({ - parseError: new Error("Parse error"), - parsedMessage: null, + act(() => { + messageHandler({ + parseError: new Error("Parse error"), + parsedMessage: null, + }); }); } @@ -166,20 +169,27 @@ describe("useAgentContainers", () => { ), ); - const { unmount } = renderHook( + const { result, unmount } = renderHook( () => useAgentContainers(MockWorkspaceAgent), { wrapper: createWrapper(), }, ); - // Simulate error event + // Wait for initial query to complete + await waitFor(() => { + expect(result.current).toEqual([MockWorkspaceAgentDevcontainer]); + }); + + // Now simulate error event const errorHandler = mockSocket.addEventListener.mock.calls.find( (call) => call[0] === "error", )?.[1]; if (errorHandler) { - errorHandler(new Error("WebSocket error")); + act(() => { + errorHandler(new Error("WebSocket error")); + }); } await waitFor(() => { @@ -211,4 +221,38 @@ describe("useAgentContainers", () => { watchAgentContainersSpy.mockRestore(); }); + + it("does not establish WebSocket connection when dev container feature is not enabled", async () => { + const watchAgentContainersSpy = jest.spyOn(API, "watchAgentContainers"); + + server.use( + http.get( + `/api/v2/workspaceagents/${MockWorkspaceAgent.id}/containers`, + () => { + return HttpResponse.json( + { + message: "Dev Container feature not enabled.", + }, + { status: 403 }, + ); + }, + ), + ); + + const { result } = renderHook( + () => useAgentContainers(MockWorkspaceAgent), + { + wrapper: createWrapper(), + }, + ); + + // Wait for the query to complete and error to be processed + await waitFor(() => { + expect(result.current).toBeUndefined(); + }); + + expect(watchAgentContainersSpy).not.toHaveBeenCalled(); + + watchAgentContainersSpy.mockRestore(); + }); }); diff --git a/site/src/modules/resources/useAgentContainers.ts b/site/src/modules/resources/useAgentContainers.ts index e2239fe4666f1..d0962c814b6b1 100644 --- a/site/src/modules/resources/useAgentContainers.ts +++ b/site/src/modules/resources/useAgentContainers.ts @@ -4,6 +4,7 @@ import type { WorkspaceAgentDevcontainer, WorkspaceAgentListContainersResponse, } from "api/typesGenerated"; +import { AxiosError } from "axios"; import { displayError } from "components/GlobalSnackbar/utils"; import { useEffectEvent } from "hooks/hookPolyfills"; import { useEffect } from "react"; @@ -14,7 +15,11 @@ export function useAgentContainers( ): readonly WorkspaceAgentDevcontainer[] | undefined { const queryClient = useQueryClient(); - const { data: devcontainers } = useQuery({ + const { + data: devcontainers, + error: queryError, + isLoading: queryIsLoading, + } = useQuery({ queryKey: ["agents", agent.id, "containers"], queryFn: () => API.getAgentContainers(agent.id), enabled: agent.status === "connected", @@ -31,7 +36,20 @@ export function useAgentContainers( ); useEffect(() => { - if (agent.status !== "connected") { + const devcontainerFeatureDisabled = + queryError instanceof AxiosError && + queryError.status === 403 && + queryError.response?.data.message === + "Dev Container feature not enabled."; + + // We do not want to attempt to call the `/watch` endpoint + // if the agent isn't connected yet as the endpoint will + // not be ready to call. + if ( + agent.status !== "connected" || + queryIsLoading || + devcontainerFeatureDisabled + ) { return; } @@ -57,7 +75,13 @@ export function useAgentContainers( }); return () => socket.close(); - }, [agent.id, agent.status, updateDevcontainersCache]); + }, [ + agent.id, + agent.status, + queryIsLoading, + queryError, + updateDevcontainersCache, + ]); return devcontainers; } From c2f43efcc35c38a1c6176e6def54d5bf42a0ce3f Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Tue, 15 Jul 2025 22:29:18 +0000 Subject: [PATCH 2/5] refactor: slightly --- .../modules/resources/useAgentContainers.ts | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/site/src/modules/resources/useAgentContainers.ts b/site/src/modules/resources/useAgentContainers.ts index d0962c814b6b1..88d04a544943d 100644 --- a/site/src/modules/resources/useAgentContainers.ts +++ b/site/src/modules/resources/useAgentContainers.ts @@ -37,19 +37,13 @@ export function useAgentContainers( useEffect(() => { const devcontainerFeatureDisabled = - queryError instanceof AxiosError && - queryError.status === 403 && - queryError.response?.data.message === - "Dev Container feature not enabled."; - - // We do not want to attempt to call the `/watch` endpoint - // if the agent isn't connected yet as the endpoint will - // not be ready to call. - if ( - agent.status !== "connected" || queryIsLoading || - devcontainerFeatureDisabled - ) { + (queryError instanceof AxiosError && + queryError.status === 403 && + queryError.response?.data.message === + "Dev Container feature not enabled."); + + if (agent.status !== "connected" || devcontainerFeatureDisabled) { return; } From 5b3836db0f2e02277cbd1db5a15f27f65fffba3c Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 16 Jul 2025 08:41:49 +0000 Subject: [PATCH 3/5] chore: simplify based on kayla's feedback <3 --- site/src/modules/resources/useAgentContainers.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/site/src/modules/resources/useAgentContainers.ts b/site/src/modules/resources/useAgentContainers.ts index 88d04a544943d..4d3d1e679d058 100644 --- a/site/src/modules/resources/useAgentContainers.ts +++ b/site/src/modules/resources/useAgentContainers.ts @@ -36,14 +36,7 @@ export function useAgentContainers( ); useEffect(() => { - const devcontainerFeatureDisabled = - queryIsLoading || - (queryError instanceof AxiosError && - queryError.status === 403 && - queryError.response?.data.message === - "Dev Container feature not enabled."); - - if (agent.status !== "connected" || devcontainerFeatureDisabled) { + if (agent.status !== "connected" || queryIsLoading || queryError) { return; } From 3b37f1731dc4cd551eaa07e7249d5ff270e90994 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 16 Jul 2025 08:42:43 +0000 Subject: [PATCH 4/5] chore: appease kayla's formatting --- site/src/modules/resources/useAgentContainers.test.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/site/src/modules/resources/useAgentContainers.test.tsx b/site/src/modules/resources/useAgentContainers.test.tsx index d3bde93dfa1d3..363f8d93223c8 100644 --- a/site/src/modules/resources/useAgentContainers.test.tsx +++ b/site/src/modules/resources/useAgentContainers.test.tsx @@ -230,9 +230,7 @@ describe("useAgentContainers", () => { `/api/v2/workspaceagents/${MockWorkspaceAgent.id}/containers`, () => { return HttpResponse.json( - { - message: "Dev Container feature not enabled.", - }, + { message: "Dev Container feature not enabled." }, { status: 403 }, ); }, From a1675d28bc8aede83e3c1b481700dc5e624a9231 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Wed, 16 Jul 2025 09:07:02 +0000 Subject: [PATCH 5/5] chore: appease linter --- site/src/modules/resources/useAgentContainers.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/site/src/modules/resources/useAgentContainers.ts b/site/src/modules/resources/useAgentContainers.ts index 4d3d1e679d058..8437fbaed6075 100644 --- a/site/src/modules/resources/useAgentContainers.ts +++ b/site/src/modules/resources/useAgentContainers.ts @@ -4,7 +4,6 @@ import type { WorkspaceAgentDevcontainer, WorkspaceAgentListContainersResponse, } from "api/typesGenerated"; -import { AxiosError } from "axios"; import { displayError } from "components/GlobalSnackbar/utils"; import { useEffectEvent } from "hooks/hookPolyfills"; import { useEffect } from "react";