Fix not being able to specify agent when connecting to terminal

The `workspace.agent` syntax was only used when fetching the agent and not the workspace so it would try to fetch a workspace called `workspace.agent` instead of just `workspace`.
coder · code-asher · May 18, 2022 · May 17, 2022 · May 17, 2022 · May 17, 2022
commit b1dbbccdaf00250876f8a2d84103c17777abe031
@@ -6,7 +6,7 @@ import React from "react"
 import { Route, Routes } from "react-router-dom"
 import { TextDecoder, TextEncoder } from "util"
 import { ReconnectingPTYRequest } from "../../api/types"
-import { history, MockWorkspaceAgent, render } from "../../testHelpers/renderHelpers"
+import { history, MockWorkspace, MockWorkspaceAgent, render } from "../../testHelpers/renderHelpers"
 import { server } from "../../testHelpers/server"
 import TerminalPage, { Language } from "./TerminalPage"
 
@@ -52,7 +52,7 @@ const expectTerminalText = (container: HTMLElement, text: string) => {
 
 describe("TerminalPage", () => {
   beforeEach(() => {
-    history.push("/some-user/my-workspace/terminal")
+    history.push(`/some-user/${MockWorkspace.name}/terminal`)
   })
 
   it("shows an error if fetching organizations fails", async () => {
@@ -146,4 +146,20 @@ describe("TerminalPage", () => {
     expect(req.width).toBeGreaterThan(0)
     server.close()
   })
+
+  it("supports workspace.agent syntax", async () => {
+    // Given
+    const server = new WS("ws://localhost/api/v2/workspaceagents/" + MockWorkspaceAgent.id + "/pty")
+    const text = "something to render"
+
+    // When
+    history.push(`/some-user/${MockWorkspace.name}.${MockWorkspaceAgent.name}/terminal`)
+    const { container } = renderTerminal()
+
+    // Then
+    await server.connected
+    server.send(text)
+    await expectTerminalText(container, text)
+    server.close()
+  })
 })
@@ -34,10 +34,14 @@ const TerminalPage: React.FC<{
     const search = new URLSearchParams(location.search)
     return search.get("reconnect") ?? uuidv4()
   })
+  // The workspace name is in the format:
+  // <workspace name>[.<agent name>]
+  const workspaceNameParts = workspace?.split(".")
   const [terminalState, sendEvent] = useMachine(terminalMachine, {
     context: {
+      agentName: workspaceNameParts?.[1],
       reconnection: reconnectionToken,
-      workspaceName: workspace,
+      workspaceName: workspaceNameParts?.[0],
       username: username,
     },
     actions: {

@@ -77,7 +77,16 @@ export const handlers = [
 
   // workspaces
   rest.get("/api/v2/organizations/:organizationId/workspaces/:userName/:workspaceName", (req, res, ctx) => {
-    return res(ctx.status(200), ctx.json(M.MockWorkspace))
+    if (req.params.workspaceName !== M.MockWorkspace.name) {
+      return res(
+        ctx.status(404),
+        ctx.json({
+          message: "workspace not found",
+        }),
+      )
+    } else {
+      return res(ctx.status(200), ctx.json(M.MockWorkspace))
+    }
   }),
   rest.get("/api/v2/workspaces/:workspaceId", async (req, res, ctx) => {
     return res(ctx.status(200), ctx.json(M.MockWorkspace))

@@ -14,13 +14,16 @@ export interface TerminalContext {
   websocketError?: Error | unknown
 
   // Assigned by connecting!
+  // The workspace agent is entirely optional.  If the agent is omitted the
+  // first agent will be used.
+  agentName?: string
   username?: string
   workspaceName?: string
   reconnection?: string
 }
 
 export type TerminalEvent =
-  | { type: "CONNECT"; reconnection?: string; workspaceName?: string; username?: string }
+  | { type: "CONNECT"; agentName?: string; reconnection?: string; workspaceName?: string; username?: string }
   | { type: "WRITE"; request: Types.ReconnectingPTYRequest }
   | { type: "READ"; data: ArrayBuffer }
   | { type: "DISCONNECT" }
@@ -153,19 +156,14 @@ export const terminalMachine =
         getOrganizations: API.getOrganizations,
         getWorkspace: async (context) => {
           if (!context.organizations || !context.workspaceName) {
-            throw new Error("organizations or workspace not set")
+            throw new Error("organizations or workspace name not set")
           }
           return API.getWorkspaceByOwnerAndName(context.organizations[0].id, context.username, context.workspaceName)
         },
         getWorkspaceAgent: async (context) => {
           if (!context.workspace || !context.workspaceName) {
             throw new Error("workspace or workspace name is not set")
           }
-          // The workspace name is in the format:
-          // <workspace name>[.<agent name>]
-          // The workspace agent is entirely optional.
-          const workspaceNameParts = context.workspaceName.split(".")
-          const agentName = workspaceNameParts[1]
 
           const resources = await API.getWorkspaceResources(context.workspace.latest_build.id)
 
@@ -174,10 +172,10 @@ export const terminalMachine =
               if (!resource.agents || resource.agents.length < 1) {
               }
-              if (!agentName) {
+              if (!context.agentName) {
                 return resource.agents[0]
               }
-              return resource.agents.find((agent) => agent.name === agentName)
+              return resource.agents.find((agent) => agent.name === context.agentName)
             })
             .filter((a) => a)[0]
           if (!agent) {
@@ -218,6 +216,7 @@ export const terminalMachine =
       actions: {
         assignConnection: assign((context, event) => ({
           ...context,
+          agentName: event.agentName ?? context.agentName,
           reconnection: event.reconnection ?? context.reconnection,
           workspaceName: event.workspaceName ?? context.workspaceName,
         })),