From 765c2cfd0467fac8117fb24d191ef63c9d384299 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 10 Jun 2025 11:08:46 +0000 Subject: [PATCH 01/29] backend --- agent/agentcontainers/api.go | 31 +++----- agent/agentcontainers/api_test.go | 15 ---- coderd/apidoc/docs.go | 51 +++++++++--- coderd/apidoc/swagger.json | 51 +++++++++--- coderd/workspaceagents_test.go | 16 ++-- codersdk/workspaceagents.go | 10 +-- docs/reference/api/agents.md | 36 ++++++++- docs/reference/api/schemas.md | 124 ++++++++++++++++++++++++------ site/src/api/typesGenerated.ts | 3 +- 9 files changed, 235 insertions(+), 102 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 1dddcc709848e..9d706f7fed8c1 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -486,8 +486,6 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code // Check if the container is running and update the known devcontainers. for i := range updated.Containers { container := &updated.Containers[i] // Grab a reference to the container to allow mutating it. - container.DevcontainerStatus = "" // Reset the status for the container (updated later). - container.DevcontainerDirty = false // Reset dirty state for the container (updated later). workspaceFolder := container.Labels[DevcontainerLocalFolderLabel] configFile := container.Labels[DevcontainerConfigFileLabel] @@ -568,8 +566,6 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code // TODO(mafredri): Parse the container label (i.e. devcontainer.json) for customization. dc.Name = safeFriendlyName(dc.Container.FriendlyName) } - dc.Container.DevcontainerStatus = dc.Status - dc.Container.DevcontainerDirty = dc.Dirty } switch { @@ -584,13 +580,11 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code if dc.Container.Running { dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning } - dc.Container.DevcontainerStatus = dc.Status dc.Dirty = false if lastModified, hasModTime := api.configFileModifiedTimes[dc.ConfigPath]; hasModTime && dc.Container.CreatedAt.Before(lastModified) { dc.Dirty = true } - dc.Container.DevcontainerDirty = dc.Dirty if _, injected := api.injectedSubAgentProcs[dc.Container.ID]; !injected && dc.Status == codersdk.WorkspaceAgentDevcontainerStatusRunning { err := api.injectSubAgentIntoContainerLocked(ctx, dc) @@ -661,9 +655,19 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse, if api.containersErr != nil { return codersdk.WorkspaceAgentListContainersResponse{}, api.containersErr } + + devcontainers := make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers)) + for _, dc := range api.knownDevcontainers { + devcontainers = append(devcontainers, dc) + } + slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { + return strings.Compare(a.ID.String(), b.ID.String()) + }) + return codersdk.WorkspaceAgentListContainersResponse{ - Containers: slices.Clone(api.containers.Containers), - Warnings: slices.Clone(api.containers.Warnings), + Devcontainers: devcontainers, + Containers: slices.Clone(api.containers.Containers), + Warnings: slices.Clone(api.containers.Warnings), }, nil } @@ -740,9 +744,6 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques // Update the status so that we don't try to recreate the // devcontainer multiple times in parallel. dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting - if dc.Container != nil { - dc.Container.DevcontainerStatus = dc.Status - } api.knownDevcontainers[dc.WorkspaceFolder] = dc api.asyncWg.Add(1) go api.recreateDevcontainer(dc, configPath) @@ -815,9 +816,6 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con api.mu.Lock() dc = api.knownDevcontainers[dc.WorkspaceFolder] dc.Status = codersdk.WorkspaceAgentDevcontainerStatusError - if dc.Container != nil { - dc.Container.DevcontainerStatus = dc.Status - } api.knownDevcontainers[dc.WorkspaceFolder] = dc api.recreateErrorTimes[dc.WorkspaceFolder] = api.clock.Now("agentcontainers", "recreate", "errorTimes") api.mu.Unlock() @@ -838,7 +836,6 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con if dc.Container.Running { dc.Status = codersdk.WorkspaceAgentDevcontainerStatusRunning } - dc.Container.DevcontainerStatus = dc.Status } dc.Dirty = false api.recreateSuccessTimes[dc.WorkspaceFolder] = api.clock.Now("agentcontainers", "recreate", "successTimes") @@ -914,10 +911,6 @@ func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) { logger.Info(api.ctx, "marking devcontainer as dirty") dc.Dirty = true } - if dc.Container != nil && !dc.Container.DevcontainerDirty { - logger.Info(api.ctx, "marking devcontainer container as dirty") - dc.Container.DevcontainerDirty = true - } api.knownDevcontainers[dc.WorkspaceFolder] = dc } diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 821117685b50e..5f9929a8430f2 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -609,7 +609,6 @@ func TestAPI(t *testing.T) { require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Status, "devcontainer is not starting") require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference") - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStarting, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not starting") // Allow the devcontainer CLI to continue the up process. close(tt.devcontainerCLI.upErrC) @@ -637,7 +636,6 @@ func TestAPI(t *testing.T) { require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response after error") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusError, resp.Devcontainers[0].Status, "devcontainer is not in an error state after up failure") require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference after up failure") - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusError, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not error after up failure") return } @@ -662,7 +660,6 @@ func TestAPI(t *testing.T) { require.Len(t, resp.Devcontainers, 1, "expected one devcontainer in response after recreation") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Status, "devcontainer is not running after recreation") require.NotNil(t, resp.Devcontainers[0].Container, "devcontainer should have container reference after recreation") - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, resp.Devcontainers[0].Container.DevcontainerStatus, "container dc status is not running after recreation") }) } }) @@ -757,7 +754,6 @@ func TestAPI(t *testing.T) { assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc.Status) require.NotNil(t, dc.Container) assert.Equal(t, "runtime-container-1", dc.Container.ID) - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc.Container.DevcontainerStatus) }, }, { @@ -802,10 +798,8 @@ func TestAPI(t *testing.T) { require.NotNil(t, known1.Container) assert.Equal(t, "known-container-1", known1.Container.ID) - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, known1.Container.DevcontainerStatus) require.NotNil(t, runtime1.Container) assert.Equal(t, "runtime-container-1", runtime1.Container.ID) - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, runtime1.Container.DevcontainerStatus) }, }, { @@ -845,11 +839,9 @@ func TestAPI(t *testing.T) { require.NotNil(t, running.Container, "running container should have container reference") assert.Equal(t, "running-container", running.Container.ID) - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, running.Container.DevcontainerStatus) require.NotNil(t, nonRunning.Container, "non-running container should have container reference") assert.Equal(t, "non-running-container", nonRunning.Container.ID) - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusStopped, nonRunning.Container.DevcontainerStatus) }, }, { @@ -885,7 +877,6 @@ func TestAPI(t *testing.T) { assert.NotEmpty(t, dc2.ConfigPath) require.NotNil(t, dc2.Container) assert.Equal(t, "known-container-2", dc2.Container.ID) - assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, dc2.Container.DevcontainerStatus) }, }, { @@ -1185,8 +1176,6 @@ func TestAPI(t *testing.T) { "devcontainer should not be marked as dirty initially") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, response.Devcontainers[0].Status, "devcontainer should be running initially") require.NotNil(t, response.Devcontainers[0].Container, "container should not be nil") - assert.False(t, response.Devcontainers[0].Container.DevcontainerDirty, - "container should not be marked as dirty initially") // Verify the watcher is watching the config file. assert.Contains(t, fWatcher.addedPaths, configPath, @@ -1220,8 +1209,6 @@ func TestAPI(t *testing.T) { "container should be marked as dirty after config file was modified") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, response.Devcontainers[0].Status, "devcontainer should be running after config file was modified") require.NotNil(t, response.Devcontainers[0].Container, "container should not be nil") - assert.True(t, response.Devcontainers[0].Container.DevcontainerDirty, - "container should be marked as dirty after config file was modified") container.ID = "new-container-id" // Simulate a new container ID after recreation. container.FriendlyName = "new-container-name" @@ -1246,8 +1233,6 @@ func TestAPI(t *testing.T) { "dirty flag should be cleared on the devcontainer after container recreation") assert.Equal(t, codersdk.WorkspaceAgentDevcontainerStatusRunning, response.Devcontainers[0].Status, "devcontainer should be running after recreation") require.NotNil(t, response.Devcontainers[0].Container, "container should not be nil") - assert.False(t, response.Devcontainers[0].Container.DevcontainerDirty, - "dirty flag should be cleared on the container after container recreation") }) t.Run("SubAgentLifecycle", func(t *testing.T) { diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 5dc293e2e706e..3f1af0a388cb5 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -17500,18 +17500,6 @@ const docTemplate = `{ "type": "string", "format": "date-time" }, - "devcontainer_dirty": { - "description": "DevcontainerDirty is true if the devcontainer configuration has changed\nsince the container was created. This is used to determine if the\ncontainer needs to be rebuilt.", - "type": "boolean" - }, - "devcontainer_status": { - "description": "DevcontainerStatus is the status of the devcontainer, if this\ncontainer is a devcontainer. This is used to determine if the\ndevcontainer is running, stopped, starting, or in an error state.", - "allOf": [ - { - "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerStatus" - } - ] - }, "id": { "description": "ID is the unique identifier of the container.", "type": "string" @@ -17576,6 +17564,38 @@ const docTemplate = `{ } } }, + "codersdk.WorkspaceAgentDevcontainer": { + "type": "object", + "properties": { + "config_path": { + "type": "string" + }, + "container": { + "$ref": "#/definitions/codersdk.WorkspaceAgentContainer" + }, + "dirty": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "status": { + "description": "Additional runtime fields.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerStatus" + } + ] + }, + "workspace_folder": { + "type": "string" + } + } + }, "codersdk.WorkspaceAgentDevcontainerStatus": { "type": "string", "enum": [ @@ -17641,6 +17661,13 @@ const docTemplate = `{ "$ref": "#/definitions/codersdk.WorkspaceAgentContainer" } }, + "devcontainers": { + "description": "Devcontainers is a list of devcontainers visible to the workspace agent.", + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainer" + } + }, "warnings": { "description": "Warnings is a list of warnings that may have occurred during the\nprocess of listing containers. This should not include fatal errors.", "type": "array", diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index ff48e99d393fc..a95ac5fc55000 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -15991,18 +15991,6 @@ "type": "string", "format": "date-time" }, - "devcontainer_dirty": { - "description": "DevcontainerDirty is true if the devcontainer configuration has changed\nsince the container was created. This is used to determine if the\ncontainer needs to be rebuilt.", - "type": "boolean" - }, - "devcontainer_status": { - "description": "DevcontainerStatus is the status of the devcontainer, if this\ncontainer is a devcontainer. This is used to determine if the\ndevcontainer is running, stopped, starting, or in an error state.", - "allOf": [ - { - "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerStatus" - } - ] - }, "id": { "description": "ID is the unique identifier of the container.", "type": "string" @@ -16067,6 +16055,38 @@ } } }, + "codersdk.WorkspaceAgentDevcontainer": { + "type": "object", + "properties": { + "config_path": { + "type": "string" + }, + "container": { + "$ref": "#/definitions/codersdk.WorkspaceAgentContainer" + }, + "dirty": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "status": { + "description": "Additional runtime fields.", + "allOf": [ + { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerStatus" + } + ] + }, + "workspace_folder": { + "type": "string" + } + } + }, "codersdk.WorkspaceAgentDevcontainerStatus": { "type": "string", "enum": ["running", "stopped", "starting", "error"], @@ -16127,6 +16147,13 @@ "$ref": "#/definitions/codersdk.WorkspaceAgentContainer" } }, + "devcontainers": { + "description": "Devcontainers is a list of devcontainers visible to the workspace agent.", + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainer" + } + }, "warnings": { "description": "Warnings is a list of warnings that may have occurred during the\nprocess of listing containers. This should not include fatal errors.", "type": "array", diff --git a/coderd/workspaceagents_test.go b/coderd/workspaceagents_test.go index ec0b692886918..6d53bd3df1140 100644 --- a/coderd/workspaceagents_test.go +++ b/coderd/workspaceagents_test.go @@ -1403,15 +1403,13 @@ func TestWorkspaceAgentRecreateDevcontainer(t *testing.T) { agentcontainers.DevcontainerConfigFileLabel: configFile, } devContainer = codersdk.WorkspaceAgentContainer{ - ID: uuid.NewString(), - CreatedAt: dbtime.Now(), - FriendlyName: testutil.GetRandomName(t), - Image: "busybox:latest", - Labels: dcLabels, - Running: true, - Status: "running", - DevcontainerDirty: true, - DevcontainerStatus: codersdk.WorkspaceAgentDevcontainerStatusRunning, + ID: uuid.NewString(), + CreatedAt: dbtime.Now(), + FriendlyName: testutil.GetRandomName(t), + Image: "busybox:latest", + Labels: dcLabels, + Running: true, + Status: "running", } plainContainer = codersdk.WorkspaceAgentContainer{ ID: uuid.NewString(), diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index 6a4380fed47ac..44e421ec88aa1 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -450,14 +450,6 @@ type WorkspaceAgentContainer struct { // Volumes is a map of "things" mounted into the container. Again, this // is somewhat implementation-dependent. Volumes map[string]string `json:"volumes"` - // DevcontainerStatus is the status of the devcontainer, if this - // container is a devcontainer. This is used to determine if the - // devcontainer is running, stopped, starting, or in an error state. - DevcontainerStatus WorkspaceAgentDevcontainerStatus `json:"devcontainer_status,omitempty"` - // DevcontainerDirty is true if the devcontainer configuration has changed - // since the container was created. This is used to determine if the - // container needs to be rebuilt. - DevcontainerDirty bool `json:"devcontainer_dirty"` } func (c *WorkspaceAgentContainer) Match(idOrName string) bool { @@ -486,6 +478,8 @@ type WorkspaceAgentContainerPort struct { // WorkspaceAgentListContainersResponse is the response to the list containers // request. type WorkspaceAgentListContainersResponse struct { + // Devcontainers is a list of devcontainers visible to the workspace agent. + Devcontainers []WorkspaceAgentDevcontainer `json:"devcontainers"` // Containers is a list of containers visible to the workspace agent. Containers []WorkspaceAgentContainer `json:"containers"` // Warnings is a list of warnings that may have occurred during the diff --git a/docs/reference/api/agents.md b/docs/reference/api/agents.md index 5ef747066b6ab..db4a99db67a8b 100644 --- a/docs/reference/api/agents.md +++ b/docs/reference/api/agents.md @@ -777,8 +777,6 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con "containers": [ { "created_at": "2019-08-24T14:15:22Z", - "devcontainer_dirty": true, - "devcontainer_status": "running", "id": "string", "image": "string", "labels": { @@ -802,6 +800,40 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con } } ], + "devcontainers": [ + { + "config_path": "string", + "container": { + "created_at": "2019-08-24T14:15:22Z", + "id": "string", + "image": "string", + "labels": { + "property1": "string", + "property2": "string" + }, + "name": "string", + "ports": [ + { + "host_ip": "string", + "host_port": 0, + "network": "string", + "port": 0 + } + ], + "running": true, + "status": "string", + "volumes": { + "property1": "string", + "property2": "string" + } + }, + "dirty": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string", + "status": "running", + "workspace_folder": "string" + } + ], "warnings": [ "string" ] diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index a5b759e5dfb0c..04a2472916490 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -9011,8 +9011,6 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { "created_at": "2019-08-24T14:15:22Z", - "devcontainer_dirty": true, - "devcontainer_status": "running", "id": "string", "image": "string", "labels": { @@ -9039,21 +9037,19 @@ If the schedule is empty, the user will be updated to use the default schedule.| ### Properties -| Name | Type | Required | Restrictions | Description | -|-----------------------|----------------------------------------------------------------------------------------|----------|--------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `created_at` | string | false | | Created at is the time the container was created. | -| `devcontainer_dirty` | boolean | false | | Devcontainer dirty is true if the devcontainer configuration has changed since the container was created. This is used to determine if the container needs to be rebuilt. | -| `devcontainer_status` | [codersdk.WorkspaceAgentDevcontainerStatus](#codersdkworkspaceagentdevcontainerstatus) | false | | Devcontainer status is the status of the devcontainer, if this container is a devcontainer. This is used to determine if the devcontainer is running, stopped, starting, or in an error state. | -| `id` | string | false | | ID is the unique identifier of the container. | -| `image` | string | false | | Image is the name of the container image. | -| `labels` | object | false | | Labels is a map of key-value pairs of container labels. | -| » `[any property]` | string | false | | | -| `name` | string | false | | Name is the human-readable name of the container. | -| `ports` | array of [codersdk.WorkspaceAgentContainerPort](#codersdkworkspaceagentcontainerport) | false | | Ports includes ports exposed by the container. | -| `running` | boolean | false | | Running is true if the container is currently running. | -| `status` | string | false | | Status is the current status of the container. This is somewhat implementation-dependent, but should generally be a human-readable string. | -| `volumes` | object | false | | Volumes is a map of "things" mounted into the container. Again, this is somewhat implementation-dependent. | -| » `[any property]` | string | false | | | +| Name | Type | Required | Restrictions | Description | +|--------------------|---------------------------------------------------------------------------------------|----------|--------------|--------------------------------------------------------------------------------------------------------------------------------------------| +| `created_at` | string | false | | Created at is the time the container was created. | +| `id` | string | false | | ID is the unique identifier of the container. | +| `image` | string | false | | Image is the name of the container image. | +| `labels` | object | false | | Labels is a map of key-value pairs of container labels. | +| » `[any property]` | string | false | | | +| `name` | string | false | | Name is the human-readable name of the container. | +| `ports` | array of [codersdk.WorkspaceAgentContainerPort](#codersdkworkspaceagentcontainerport) | false | | Ports includes ports exposed by the container. | +| `running` | boolean | false | | Running is true if the container is currently running. | +| `status` | string | false | | Status is the current status of the container. This is somewhat implementation-dependent, but should generally be a human-readable string. | +| `volumes` | object | false | | Volumes is a map of "things" mounted into the container. Again, this is somewhat implementation-dependent. | +| » `[any property]` | string | false | | | ## codersdk.WorkspaceAgentContainerPort @@ -9075,6 +9071,55 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `network` | string | false | | Network is the network protocol used by the port (tcp, udp, etc). | | `port` | integer | false | | Port is the port number *inside* the container. | +## codersdk.WorkspaceAgentDevcontainer + +```json +{ + "config_path": "string", + "container": { + "created_at": "2019-08-24T14:15:22Z", + "id": "string", + "image": "string", + "labels": { + "property1": "string", + "property2": "string" + }, + "name": "string", + "ports": [ + { + "host_ip": "string", + "host_port": 0, + "network": "string", + "port": 0 + } + ], + "running": true, + "status": "string", + "volumes": { + "property1": "string", + "property2": "string" + } + }, + "dirty": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string", + "status": "running", + "workspace_folder": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|--------------------|----------------------------------------------------------------------------------------|----------|--------------|----------------------------| +| `config_path` | string | false | | | +| `container` | [codersdk.WorkspaceAgentContainer](#codersdkworkspaceagentcontainer) | false | | | +| `dirty` | boolean | false | | | +| `id` | string | false | | | +| `name` | string | false | | | +| `status` | [codersdk.WorkspaceAgentDevcontainerStatus](#codersdkworkspaceagentdevcontainerstatus) | false | | Additional runtime fields. | +| `workspace_folder` | string | false | | | + ## codersdk.WorkspaceAgentDevcontainerStatus ```json @@ -9137,8 +9182,6 @@ If the schedule is empty, the user will be updated to use the default schedule.| "containers": [ { "created_at": "2019-08-24T14:15:22Z", - "devcontainer_dirty": true, - "devcontainer_status": "running", "id": "string", "image": "string", "labels": { @@ -9162,6 +9205,40 @@ If the schedule is empty, the user will be updated to use the default schedule.| } } ], + "devcontainers": [ + { + "config_path": "string", + "container": { + "created_at": "2019-08-24T14:15:22Z", + "id": "string", + "image": "string", + "labels": { + "property1": "string", + "property2": "string" + }, + "name": "string", + "ports": [ + { + "host_ip": "string", + "host_port": 0, + "network": "string", + "port": 0 + } + ], + "running": true, + "status": "string", + "volumes": { + "property1": "string", + "property2": "string" + } + }, + "dirty": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string", + "status": "running", + "workspace_folder": "string" + } + ], "warnings": [ "string" ] @@ -9170,10 +9247,11 @@ If the schedule is empty, the user will be updated to use the default schedule.| ### Properties -| Name | Type | Required | Restrictions | Description | -|--------------|-------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------| -| `containers` | array of [codersdk.WorkspaceAgentContainer](#codersdkworkspaceagentcontainer) | false | | Containers is a list of containers visible to the workspace agent. | -| `warnings` | array of string | false | | Warnings is a list of warnings that may have occurred during the process of listing containers. This should not include fatal errors. | +| Name | Type | Required | Restrictions | Description | +|-----------------|-------------------------------------------------------------------------------------|----------|--------------|---------------------------------------------------------------------------------------------------------------------------------------| +| `containers` | array of [codersdk.WorkspaceAgentContainer](#codersdkworkspaceagentcontainer) | false | | Containers is a list of containers visible to the workspace agent. | +| `devcontainers` | array of [codersdk.WorkspaceAgentDevcontainer](#codersdkworkspaceagentdevcontainer) | false | | Devcontainers is a list of devcontainers visible to the workspace agent. | +| `warnings` | array of string | false | | Warnings is a list of warnings that may have occurred during the process of listing containers. This should not include fatal errors. | ## codersdk.WorkspaceAgentListeningPort diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index a512305c489d3..c6cacea7cdfcd 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3354,8 +3354,6 @@ export interface WorkspaceAgentContainer { readonly ports: readonly WorkspaceAgentContainerPort[]; readonly status: string; readonly volumes: Record; - readonly devcontainer_status?: WorkspaceAgentDevcontainerStatus; - readonly devcontainer_dirty: boolean; } // From codersdk/workspaceagents.go @@ -3424,6 +3422,7 @@ export const WorkspaceAgentLifecycles: WorkspaceAgentLifecycle[] = [ // From codersdk/workspaceagents.go export interface WorkspaceAgentListContainersResponse { + readonly devcontainers: readonly WorkspaceAgentDevcontainer[]; readonly containers: readonly WorkspaceAgentContainer[]; readonly warnings?: readonly string[]; } From 4019358a5ad7c91bf8aed7e9043f10072c95d15f Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 10 Jun 2025 16:39:53 +0000 Subject: [PATCH 02/29] ui-1 --- agent/agentcontainers/api.go | 15 +++-- .../AgentDevcontainerCard.stories.tsx | 46 ++++++++++---- .../resources/AgentDevcontainerCard.tsx | 61 ++++++++++++------- .../modules/resources/AgentRow.stories.tsx | 11 ++++ site/src/modules/resources/AgentRow.tsx | 23 +++---- .../modules/resources/SSHButton/SSHButton.tsx | 18 +++--- site/src/testHelpers/entities.ts | 1 - 7 files changed, 109 insertions(+), 66 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 9d706f7fed8c1..e12caa96a322a 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -656,13 +656,16 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse, return codersdk.WorkspaceAgentListContainersResponse{}, api.containersErr } - devcontainers := make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers)) - for _, dc := range api.knownDevcontainers { - devcontainers = append(devcontainers, dc) + var devcontainers []codersdk.WorkspaceAgentDevcontainer + if len(api.knownDevcontainers) > 0 { + devcontainers = make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers)) + for _, dc := range api.knownDevcontainers { + devcontainers = append(devcontainers, dc) + } + slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { + return strings.Compare(a.ID.String(), b.ID.String()) + }) } - slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { - return strings.Compare(a.ID.String(), b.ID.String()) - }) return codersdk.WorkspaceAgentListContainersResponse{ Devcontainers: devcontainers, diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index fdd85d95c4849..5f93860873dc3 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -6,12 +6,23 @@ import { MockWorkspaceAgentContainerPorts, } from "testHelpers/entities"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; +import type { WorkspaceAgentDevcontainer } from "api/typesGenerated"; + +const MockWorkspaceAgentDevcontainer: WorkspaceAgentDevcontainer = { + id: "test-devcontainer-id", + name: "test-devcontainer", + workspace_folder: "/workspace/test", + config_path: "/workspace/test/.devcontainer/devcontainer.json", + status: "running", + dirty: false, + container: MockWorkspaceAgentContainer, +}; const meta: Meta = { title: "modules/resources/AgentDevcontainerCard", component: AgentDevcontainerCard, args: { - container: MockWorkspaceAgentContainer, + devcontainer: MockWorkspaceAgentDevcontainer, workspace: MockWorkspace, wildcardHostname: "*.wildcard.hostname", agent: MockWorkspaceAgent, @@ -25,30 +36,39 @@ export const NoPorts: Story = {}; export const WithPorts: Story = { args: { - container: { - ...MockWorkspaceAgentContainer, - ports: MockWorkspaceAgentContainerPorts, + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + container: { + ...MockWorkspaceAgentContainer, + ports: MockWorkspaceAgentContainerPorts, + }, }, }, }; export const Dirty: Story = { args: { - container: { - ...MockWorkspaceAgentContainer, - devcontainer_dirty: true, - ports: MockWorkspaceAgentContainerPorts, + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + dirty: true, + container: { + ...MockWorkspaceAgentContainer, + ports: MockWorkspaceAgentContainerPorts, + }, }, }, }; export const Recreating: Story = { args: { - container: { - ...MockWorkspaceAgentContainer, - devcontainer_dirty: true, - devcontainer_status: "starting", - ports: MockWorkspaceAgentContainerPorts, + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + dirty: true, + status: "starting", + container: { + ...MockWorkspaceAgentContainer, + ports: MockWorkspaceAgentContainerPorts, + }, }, }, }; diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 65b32593c1418..003a229e5d4e9 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -1,7 +1,7 @@ import type { Workspace, WorkspaceAgent, - WorkspaceAgentContainer, + WorkspaceAgentDevcontainer, } from "api/typesGenerated"; import { Button } from "components/Button/Button"; import { displayError } from "components/GlobalSnackbar/utils"; @@ -24,25 +24,26 @@ import type { FC } from "react"; import { useEffect, useState } from "react"; import { portForwardURL } from "utils/portForward"; import { AgentButton } from "./AgentButton"; -import { AgentDevcontainerSSHButton } from "./SSHButton/SSHButton"; +import { + AgentDevcontainerSSHButton, + AgentSSHButton, +} from "./SSHButton/SSHButton"; import { TerminalLink } from "./TerminalLink/TerminalLink"; import { VSCodeDevContainerButton } from "./VSCodeDevContainerButton/VSCodeDevContainerButton"; type AgentDevcontainerCardProps = { agent: WorkspaceAgent; - container: WorkspaceAgentContainer; + devcontainer: WorkspaceAgentDevcontainer; workspace: Workspace; wildcardHostname: string; }; export const AgentDevcontainerCard: FC = ({ agent, - container, + devcontainer, workspace, wildcardHostname, }) => { - const folderPath = container.labels["devcontainer.local_folder"]; - const containerFolder = container.volumes[folderPath]; const [isRecreating, setIsRecreating] = useState(false); const handleRecreateDevcontainer = async () => { @@ -50,7 +51,7 @@ export const AgentDevcontainerCard: FC = ({ let recreateSucceeded = false; try { const response = await fetch( - `/api/v2/workspaceagents/${agent.id}/containers/devcontainers/container/${container.id}/recreate`, + `/api/v2/workspaceagents/${agent.id}/containers/devcontainers/container/${devcontainer.container?.id}/recreate`, { method: "POST", }, @@ -79,27 +80,35 @@ export const AgentDevcontainerCard: FC = ({ } }; - // If the container is starting, reflect this in the recreate button. + // If the devcontainer is starting, reflect this in the recreate button. useEffect(() => { - if (container.devcontainer_status === "starting") { + if (devcontainer.status === "starting") { setIsRecreating(true); } else { setIsRecreating(false); } - }, [container.devcontainer_status]); + }, [devcontainer.status]); return (

dev container:{" "} - {container.name} + + {devcontainer.name} + {devcontainer.container && ( + + {" "} + ({devcontainer.container.name}) + + )} +

- {container.devcontainer_dirty && ( + {devcontainer.dirty && ( Outdated @@ -126,10 +135,17 @@ export const AgentDevcontainerCard: FC = ({ Recreate - + container={devcontainer.container?.name || devcontainer.name} + /> */} + {agent.display_apps.includes("ssh_helper") && ( // TODO agent + + )}
@@ -139,20 +155,19 @@ export const AgentDevcontainerCard: FC = ({ {wildcardHostname !== "" && - container.ports.map((port) => { + devcontainer.container?.ports.map((port) => { const portLabel = `${port.port}/${port.network.toUpperCase()}`; const hasHostBind = port.host_port !== undefined && port.host_ip !== undefined; diff --git a/site/src/modules/resources/AgentRow.stories.tsx b/site/src/modules/resources/AgentRow.stories.tsx index afeb95d0d2177..d6ab3dc45a96a 100644 --- a/site/src/modules/resources/AgentRow.stories.tsx +++ b/site/src/modules/resources/AgentRow.stories.tsx @@ -288,6 +288,17 @@ export const GroupApp: Story = { export const Devcontainer: Story = { beforeEach: () => { spyOn(API, "getAgentContainers").mockResolvedValue({ + devcontainers: [ + { + id: "test-devcontainer-id", + name: "test-devcontainer", + workspace_folder: "/workspace/test", + config_path: "/workspace/test/.devcontainer/devcontainer.json", + status: "running", + dirty: false, + container: M.MockWorkspaceAgentContainer, + }, + ], containers: [M.MockWorkspaceAgentContainer], }); }, diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index d7545ff5c8430..fbb522b125177 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -140,16 +140,11 @@ export const AgentRow: FC = ({ setBottomOfLogs(distanceFromBottom < AGENT_LOG_LINE_HEIGHT); }, []); - const { data: containers } = useQuery({ + const { data: devcontainers } = useQuery({ queryKey: ["agents", agent.id, "containers"], - queryFn: () => - // Only return devcontainers - API.getAgentContainers(agent.id, [ - "devcontainer.config_file=", - "devcontainer.local_folder=", - ]), + queryFn: () => API.getAgentContainers(agent.id), enabled: agent.status === "connected", - select: (res) => res.containers.filter((c) => c.status === "running"), + select: (res) => res.devcontainers, // TODO: Implement a websocket connection to get updates on containers // without having to poll. refetchInterval: ({ state }) => { @@ -164,7 +159,7 @@ export const AgentRow: FC = ({ const [showParentApps, setShowParentApps] = useState(false); let shouldDisplayAppsSection = shouldDisplayAgentApps; - if (containers && containers.length > 0 && !showParentApps) { + if (devcontainers && devcontainers.length > 0 && !showParentApps) { shouldDisplayAppsSection = false; } @@ -200,7 +195,7 @@ export const AgentRow: FC = ({
- {containers && containers.length > 0 && ( + {devcontainers && devcontainers.length > 0 && (
)} - {containers && containers.length > 0 && ( + {devcontainers && devcontainers.length > 0 && (
- {containers.map((container) => { + {devcontainers.map((devcontainer) => { return ( = ({ interface AgentDevcontainerSSHButtonProps { workspace: string; - container: string; + agentName: string; } export const AgentDevcontainerSSHButton: FC< AgentDevcontainerSSHButtonProps -> = ({ workspace, container }) => { +> = ({ workspace, agentName }) => { const paper = useClassName(classNames.paper, []); return ( @@ -116,8 +116,8 @@ export const AgentDevcontainerSSHButton: FC<
@@ -151,11 +151,11 @@ const SSHStep: FC = ({ helpText, codeExample }) => ( const classNames = { paper: (css, theme) => css` - padding: 16px 24px 24px; - width: 304px; - color: ${theme.palette.text.secondary}; - margin-top: 2px; - `, + padding: 16px 24px 24px; + width: 304px; + color: ${theme.palette.text.secondary}; + margin-top: 2px; + `, } satisfies Record; const styles = { diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 0201e4b563efc..c5a6823ff3828 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -4390,7 +4390,6 @@ export const MockWorkspaceAgentContainer: TypesGen.WorkspaceAgentContainer = { volumes: { "/mnt/volume1": "/volume1", }, - devcontainer_dirty: false, }; export const MockWorkspaceAppStatuses: TypesGen.WorkspaceAppStatus[] = [ From 452dbc9b7545c391f14c071dc5ef20226c26fe8b Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 10 Jun 2025 16:41:23 +0000 Subject: [PATCH 03/29] backend-2 --- agent/agentcontainers/api.go | 75 ++++++++++++++++++++++++---------- coderd/apidoc/docs.go | 18 ++++++++ coderd/apidoc/swagger.json | 18 ++++++++ codersdk/workspaceagents.go | 9 ++++ docs/reference/api/agents.md | 5 +++ docs/reference/api/schemas.md | 29 +++++++++++++ site/src/api/typesGenerated.ts | 8 ++++ 7 files changed, 141 insertions(+), 21 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index e12caa96a322a..336f1753abe57 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -72,16 +72,17 @@ type API struct { configFileModifiedTimes map[string]time.Time // By config file path. recreateSuccessTimes map[string]time.Time // By workspace folder. recreateErrorTimes map[string]time.Time // By workspace folder. - injectedSubAgentProcs map[string]subAgentProcess // By container ID. + injectedSubAgentProcs map[string]subAgentProcess // By workspace folder. asyncWg sync.WaitGroup devcontainerLogSourceIDs map[string]uuid.UUID // By workspace folder. } type subAgentProcess struct { - agent SubAgent - ctx context.Context - stop context.CancelFunc + agent SubAgent + containerID string + ctx context.Context + stop context.CancelFunc } // Option is a functional option for API. @@ -586,7 +587,11 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code dc.Dirty = true } - if _, injected := api.injectedSubAgentProcs[dc.Container.ID]; !injected && dc.Status == codersdk.WorkspaceAgentDevcontainerStatusRunning { + proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder] + if injected && proc.containerID != dc.Container.ID { + injected = false // The container ID changed, we need to re-inject. + } + if !injected && dc.Status == codersdk.WorkspaceAgentDevcontainerStatusRunning { err := api.injectSubAgentIntoContainerLocked(ctx, dc) if err != nil { logger.Error(ctx, "inject subagent into container failed", slog.Error(err)) @@ -660,10 +665,22 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse, if len(api.knownDevcontainers) > 0 { devcontainers = make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers)) for _, dc := range api.knownDevcontainers { + // Include the agent if it's been created (we're iterating over + // copies, so mutating is fine). + if dc.Container != nil { + if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil && proc.containerID == dc.Container.ID { + dc.Agent = &codersdk.WorkspaceAgentDevcontainerAgent{ + ID: proc.agent.ID, + Name: proc.agent.Name, + Directory: proc.agent.Directory, + } + } + } + devcontainers = append(devcontainers, dc) } slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { - return strings.Compare(a.ID.String(), b.ID.String()) + return strings.Compare(a.Name, b.Name) }) } @@ -975,9 +992,25 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders return xerrors.New("container is nil, cannot inject subagent") } + logger := api.logger.With( + slog.F("devcontainer_id", dc.ID), + slog.F("devcontainer_name", dc.Name), + slog.F("workspace_folder", dc.WorkspaceFolder), + slog.F("config_path", dc.ConfigPath), + slog.F("container_id", container.ID), + slog.F("container_name", container.FriendlyName), + ) + // Skip if subagent already exists for this container. - if _, injected := api.injectedSubAgentProcs[container.ID]; injected || api.closed { - return nil + if proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder]; injected || api.closed { + if proc.containerID == container.ID { + return nil + } + + // If the subagent is already injected but the container ID has + // changed, we need to inject it into the new container. + logger.Debug(ctx, "injecting subagent into new container") + proc.stop() } // Mark subagent as being injected immediately with a placeholder. @@ -1010,13 +1043,6 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders api.mu.Unlock() defer api.mu.Lock() // Re-lock. - logger := api.logger.With( - slog.F("devcontainer_id", dc.ID), - slog.F("devcontainer_name", dc.Name), - slog.F("workspace_folder", dc.WorkspaceFolder), - slog.F("config_path", dc.ConfigPath), - ) - arch, err := api.ccli.DetectArchitecture(ctx, container.ID) if err != nil { return xerrors.Errorf("detect architecture: %w", err) @@ -1176,7 +1202,9 @@ func (api *API) runSubAgentInContainer(ctx context.Context, dc codersdk.Workspac } api.mu.Lock() - delete(api.injectedSubAgentProcs, container.ID) + if api.injectedSubAgentProcs[dc.WorkspaceFolder].containerID == container.ID { + delete(api.injectedSubAgentProcs, dc.WorkspaceFolder) + } api.mu.Unlock() logger.Debug(ctx, "agent process cleanup complete") @@ -1191,10 +1219,11 @@ func (api *API) runSubAgentInContainer(ctx context.Context, dc codersdk.Workspac return } // Update the placeholder with a valid subagent, context and stop. - api.injectedSubAgentProcs[container.ID] = subAgentProcess{ - agent: agent, - ctx: agentCtx, - stop: agentStop, + api.injectedSubAgentProcs[dc.WorkspaceFolder] = subAgentProcess{ + agent: agent, + containerID: container.ID, + ctx: agentCtx, + stop: agentStop, } api.mu.Unlock() @@ -1226,7 +1255,11 @@ func (api *API) Close() error { api.closed = true for _, proc := range api.injectedSubAgentProcs { - api.logger.Debug(api.ctx, "canceling subagent process", slog.F("agent_name", proc.agent.Name), slog.F("agent_id", proc.agent.ID)) + api.logger.Debug(api.ctx, "canceling subagent process", + slog.F("agent_name", proc.agent.Name), + slog.F("agent_id", proc.agent.ID), + slog.F("container_id", proc.containerID), + ) proc.stop() } diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 3f1af0a388cb5..975e094b52fc6 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -17567,6 +17567,9 @@ const docTemplate = `{ "codersdk.WorkspaceAgentDevcontainer": { "type": "object", "properties": { + "agent": { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerAgent" + }, "config_path": { "type": "string" }, @@ -17596,6 +17599,21 @@ const docTemplate = `{ } } }, + "codersdk.WorkspaceAgentDevcontainerAgent": { + "type": "object", + "properties": { + "directory": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + } + } + }, "codersdk.WorkspaceAgentDevcontainerStatus": { "type": "string", "enum": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index a95ac5fc55000..7b71741a205d5 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -16058,6 +16058,9 @@ "codersdk.WorkspaceAgentDevcontainer": { "type": "object", "properties": { + "agent": { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerAgent" + }, "config_path": { "type": "string" }, @@ -16087,6 +16090,21 @@ } } }, + "codersdk.WorkspaceAgentDevcontainerAgent": { + "type": "object", + "properties": { + "directory": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + } + } + }, "codersdk.WorkspaceAgentDevcontainerStatus": { "type": "string", "enum": ["running", "stopped", "starting", "error"], diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index 44e421ec88aa1..90bd6e67dcf38 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -422,6 +422,15 @@ type WorkspaceAgentDevcontainer struct { Status WorkspaceAgentDevcontainerStatus `json:"status"` Dirty bool `json:"dirty"` Container *WorkspaceAgentContainer `json:"container,omitempty"` + Agent *WorkspaceAgentDevcontainerAgent `json:"agent,omitempty"` +} + +// WorkspaceAgentDevcontainerAgent represents the sub agent for a +// devcontainer. +type WorkspaceAgentDevcontainerAgent struct { + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + Directory string `json:"directory"` } // WorkspaceAgentContainer describes a devcontainer of some sort diff --git a/docs/reference/api/agents.md b/docs/reference/api/agents.md index db4a99db67a8b..1c0534ad4c2bf 100644 --- a/docs/reference/api/agents.md +++ b/docs/reference/api/agents.md @@ -802,6 +802,11 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con ], "devcontainers": [ { + "agent": { + "directory": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string" + }, "config_path": "string", "container": { "created_at": "2019-08-24T14:15:22Z", diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 04a2472916490..b4bb668a8a89d 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -9075,6 +9075,11 @@ If the schedule is empty, the user will be updated to use the default schedule.| ```json { + "agent": { + "directory": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string" + }, "config_path": "string", "container": { "created_at": "2019-08-24T14:15:22Z", @@ -9112,6 +9117,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| | Name | Type | Required | Restrictions | Description | |--------------------|----------------------------------------------------------------------------------------|----------|--------------|----------------------------| +| `agent` | [codersdk.WorkspaceAgentDevcontainerAgent](#codersdkworkspaceagentdevcontaineragent) | false | | | | `config_path` | string | false | | | | `container` | [codersdk.WorkspaceAgentContainer](#codersdkworkspaceagentcontainer) | false | | | | `dirty` | boolean | false | | | @@ -9120,6 +9126,24 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `status` | [codersdk.WorkspaceAgentDevcontainerStatus](#codersdkworkspaceagentdevcontainerstatus) | false | | Additional runtime fields. | | `workspace_folder` | string | false | | | +## codersdk.WorkspaceAgentDevcontainerAgent + +```json +{ + "directory": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|-------------|--------|----------|--------------|-------------| +| `directory` | string | false | | | +| `id` | string | false | | | +| `name` | string | false | | | + ## codersdk.WorkspaceAgentDevcontainerStatus ```json @@ -9207,6 +9231,11 @@ If the schedule is empty, the user will be updated to use the default schedule.| ], "devcontainers": [ { + "agent": { + "directory": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string" + }, "config_path": "string", "container": { "created_at": "2019-08-24T14:15:22Z", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index c6cacea7cdfcd..250a9f33c101b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3373,6 +3373,14 @@ export interface WorkspaceAgentDevcontainer { readonly status: WorkspaceAgentDevcontainerStatus; readonly dirty: boolean; readonly container?: WorkspaceAgentContainer; + readonly agent?: WorkspaceAgentDevcontainerAgent; +} + +// From codersdk/workspaceagents.go +export interface WorkspaceAgentDevcontainerAgent { + readonly id: string; + readonly name: string; + readonly directory: string; } // From codersdk/workspaceagents.go From 6a23998ae0315eb7b1eac7726f99482d474b8547 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 10 Jun 2025 16:41:41 +0000 Subject: [PATCH 04/29] ui-2 --- .../AgentDevcontainerCard.stories.tsx | 5 + .../resources/AgentDevcontainerCard.tsx | 113 ++++++++++-------- 2 files changed, 66 insertions(+), 52 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index 5f93860873dc3..b494f0711bf05 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -16,6 +16,11 @@ const MockWorkspaceAgentDevcontainer: WorkspaceAgentDevcontainer = { status: "running", dirty: false, container: MockWorkspaceAgentContainer, + agent: { + id: "test-agent-id", + name: "test-devcontainer-agent", + directory: "/workspace/test", + }, }; const meta: Meta = { diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 003a229e5d4e9..e6c4169fa691b 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -139,68 +139,77 @@ export const AgentDevcontainerCard: FC = ({ workspace={workspace.name} container={devcontainer.container?.name || devcontainer.name} /> */} - {agent.display_apps.includes("ssh_helper") && ( // TODO agent + {/* TODO(mafredri): Sub agent display apps. */} + {devcontainer.agent && agent.display_apps.includes("ssh_helper") && ( )} -

Forwarded ports

+ {devcontainer.agent && ( + <> +

Forwarded ports

+
+ {devcontainer.container && ( + + )} -
- + {devcontainer.agent && ( + + )} - - {wildcardHostname !== "" && - devcontainer.container?.ports.map((port) => { - const portLabel = `${port.port}/${port.network.toUpperCase()}`; - const hasHostBind = - port.host_port !== undefined && port.host_ip !== undefined; - const helperText = hasHostBind - ? `${port.host_ip}:${port.host_port}` - : "Not bound to host"; - const linkDest = hasHostBind - ? portForwardURL( - wildcardHostname, - port.host_port, - agent.name, - workspace.name, - workspace.owner_name, - location.protocol === "https" ? "https" : "http", - ) - : ""; - return ( - - - - - - - {portLabel} - - - - {helperText} - - - ); - })} -
+ {wildcardHostname !== "" && + devcontainer.container?.ports.map((port) => { + const portLabel = `${port.port}/${port.network.toUpperCase()}`; + const hasHostBind = + port.host_port !== undefined && port.host_ip !== undefined; + const helperText = hasHostBind + ? `${port.host_ip}:${port.host_port}` + : "Not bound to host"; + const linkDest = hasHostBind + ? portForwardURL( + wildcardHostname, + port.host_port, + devcontainer.agent?.name || agent.name, + workspace.name, + workspace.owner_name, + location.protocol === "https" ? "https" : "http", + ) + : ""; + return ( + + + + + + + {portLabel} + + + + {helperText} + + + ); + })} +
+ + )}
); }; From ee3ed36e1a27c5b815b5802a38da6dd95cc385d4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 11 Jun 2025 14:12:15 +0000 Subject: [PATCH 05/29] backend-3 --- agent/agentcontainers/api.go | 298 +++++++++++++++--------------- agent/agentcontainers/api_test.go | 167 +++++++++++------ codersdk/workspaceagents.go | 6 - 3 files changed, 259 insertions(+), 212 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 336f1753abe57..5806349a4358b 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -37,7 +37,7 @@ const ( // Destination path inside the container, we store it in a fixed location // under /.coder-agent/coder to avoid conflicts and avoid being shadowed // by tmpfs or other mounts. This assumes the container root filesystem is - // read-write, which seems sensible for dev containers. + // read-write, which seems sensible for devcontainers. coderPathInsideContainer = "/.coder-agent/coder" ) @@ -130,7 +130,7 @@ func WithDevcontainerCLI(dccli DevcontainerCLI) Option { } // WithSubAgentClient sets the SubAgentClient implementation to use. -// This is used to list, create and delete Dev Container agents. +// This is used to list, create and delete devcontainer agents. func WithSubAgentClient(client SubAgentClient) Option { return func(api *API) { api.subAgentClient = client @@ -404,8 +404,8 @@ func (api *API) Routes() http.Handler { r.Use(ensureInitialUpdateDoneMW) r.Get("/", api.handleList) + // TODO(mafredri): Simplify this route. r.Route("/devcontainers", func(r chi.Router) { - r.Get("/", api.handleDevcontainersList) r.Post("/container/{container}/recreate", api.handleDevcontainerRecreate) }) @@ -512,10 +512,10 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code // Verbose debug logging is fine here since typically filters // are only used in development or testing environments. if !ok { - logger.Debug(ctx, "container does not match include filter, ignoring dev container", slog.F("container_labels", container.Labels), slog.F("include_filter", api.containerLabelIncludeFilter)) + logger.Debug(ctx, "container does not match include filter, ignoring devcontainer", slog.F("container_labels", container.Labels), slog.F("include_filter", api.containerLabelIncludeFilter)) continue } - logger.Debug(ctx, "container matches include filter, processing dev container", slog.F("container_labels", container.Labels), slog.F("include_filter", api.containerLabelIncludeFilter)) + logger.Debug(ctx, "container matches include filter, processing devcontainer", slog.F("container_labels", container.Labels), slog.F("include_filter", api.containerLabelIncludeFilter)) } if dc, ok := api.knownDevcontainers[workspaceFolder]; ok { @@ -563,7 +563,7 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code if !api.devcontainerNames[dc.Name] { // If the devcontainer name wasn't set via terraform, we // use the containers friendly name as a fallback which - // will keep changing as the dev container is recreated. + // will keep changing as the devcontainer is recreated. // TODO(mafredri): Parse the container label (i.e. devcontainer.json) for customization. dc.Name = safeFriendlyName(dc.Container.FriendlyName) } @@ -587,12 +587,8 @@ func (api *API) processUpdatedContainersLocked(ctx context.Context, updated code dc.Dirty = true } - proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder] - if injected && proc.containerID != dc.Container.ID { - injected = false // The container ID changed, we need to re-inject. - } - if !injected && dc.Status == codersdk.WorkspaceAgentDevcontainerStatusRunning { - err := api.injectSubAgentIntoContainerLocked(ctx, dc) + if dc.Status == codersdk.WorkspaceAgentDevcontainerStatusRunning { + err := api.maybeInjectSubAgentIntoContainerLocked(ctx, dc) if err != nil { logger.Error(ctx, "inject subagent into container failed", slog.Error(err)) } @@ -665,15 +661,13 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse, if len(api.knownDevcontainers) > 0 { devcontainers = make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers)) for _, dc := range api.knownDevcontainers { - // Include the agent if it's been created (we're iterating over - // copies, so mutating is fine). - if dc.Container != nil { - if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil && proc.containerID == dc.Container.ID { - dc.Agent = &codersdk.WorkspaceAgentDevcontainerAgent{ - ID: proc.agent.ID, - Name: proc.agent.Name, - Directory: proc.agent.Directory, - } + // Include the agent if it's been created (we're iterating + // over copies, so mutating is fine). + if agent := api.injectedSubAgentProcs[dc.WorkspaceFolder].agent; agent.ID != uuid.Nil { + dc.Agent = &codersdk.WorkspaceAgentDevcontainerAgent{ + ID: agent.ID, + Name: agent.Name, + Directory: agent.Directory, } } @@ -869,39 +863,6 @@ func (api *API) recreateDevcontainer(dc codersdk.WorkspaceAgentDevcontainer, con } } -// handleDevcontainersList handles the HTTP request to list known devcontainers. -func (api *API) handleDevcontainersList(w http.ResponseWriter, r *http.Request) { - ctx := r.Context() - - api.mu.RLock() - err := api.containersErr - devcontainers := make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers)) - for _, dc := range api.knownDevcontainers { - devcontainers = append(devcontainers, dc) - } - api.mu.RUnlock() - if err != nil { - httpapi.Write(ctx, w, http.StatusInternalServerError, codersdk.Response{ - Message: "Could not list containers", - Detail: err.Error(), - }) - return - } - - slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { - if cmp := strings.Compare(a.WorkspaceFolder, b.WorkspaceFolder); cmp != 0 { - return cmp - } - return strings.Compare(a.ConfigPath, b.ConfigPath) - }) - - response := codersdk.WorkspaceAgentDevcontainersResponse{ - Devcontainers: devcontainers, - } - - httpapi.Write(ctx, w, http.StatusOK, response) -} - // markDevcontainerDirty finds the devcontainer with the given config file path // and marks it as dirty. It acquires the lock before modifying the state. func (api *API) markDevcontainerDirty(configPath string, modifiedAt time.Time) { @@ -977,13 +938,13 @@ func (api *API) cleanupSubAgents(ctx context.Context) error { return nil } -// injectSubAgentIntoContainerLocked injects a subagent into a dev +// maybeInjectSubAgentIntoContainerLocked injects a subagent into a dev // container and starts the subagent process. This method assumes that // api.mu is held. // // This method uses an internal timeout to prevent blocking indefinitely // if something goes wrong with the injection. -func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc codersdk.WorkspaceAgentDevcontainer) (err error) { +func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc codersdk.WorkspaceAgentDevcontainer) (err error) { ctx, cancel := context.WithTimeout(ctx, defaultOperationTimeout) defer cancel() @@ -1001,24 +962,35 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders slog.F("container_name", container.FriendlyName), ) - // Skip if subagent already exists for this container. - if proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder]; injected || api.closed { - if proc.containerID == container.ID { + // Check if subagent already exists for this devcontainer. + recreateSubAgent := false + proc, injected := api.injectedSubAgentProcs[dc.WorkspaceFolder] + if injected { + if proc.containerID == container.ID && proc.ctx.Err() == nil { + // Same container and running, no need to reinject. return nil } - // If the subagent is already injected but the container ID has - // changed, we need to inject it into the new container. - logger.Debug(ctx, "injecting subagent into new container") + if proc.containerID != container.ID { + // Always recreate the subagent if the container ID changed + // for now, in the future we can inspect e.g. if coder_apps + // remain the same and avoid unnecessary recreation. + logger.Debug(ctx, "container ID changed, injecting subagent into new container", + slog.F("old_container_id", proc.containerID), + ) + recreateSubAgent = true + } + + // Container ID changed or the subagent process is not running, + // stop the existing subagent context to replace it. proc.stop() } - // Mark subagent as being injected immediately with a placeholder. - subAgent := subAgentProcess{ - ctx: context.Background(), - stop: func() {}, - } - api.injectedSubAgentProcs[container.ID] = subAgent + // Prepare the subAgentProcess to be used when running the subagent. + // We use api.ctx here to ensure that the process keeps running + // after this method returns. + proc.ctx, proc.stop = context.WithCancel(api.ctx) + api.injectedSubAgentProcs[dc.WorkspaceFolder] = proc // This is used to track the goroutine that will run the subagent // process inside the container. It will be decremented when the @@ -1030,12 +1002,13 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders // Clean up if injection fails. defer func() { if !ranSubAgent { + proc.stop() + if !api.closed { + // Ensure sure state modifications are reflected. + api.injectedSubAgentProcs[dc.WorkspaceFolder] = proc + } api.asyncWg.Done() } - if err != nil { - // Mutex is held (defer re-lock). - delete(api.injectedSubAgentProcs, container.ID) - } }() // Unlock the mutex to allow other operations while we @@ -1057,7 +1030,8 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders if arch != hostArch { logger.Warn(ctx, "skipping subagent injection for unsupported architecture", slog.F("container_arch", arch), - slog.F("host_arch", hostArch)) + slog.F("host_arch", hostArch), + ) return nil } agentBinaryPath, err := os.Executable() @@ -1117,59 +1091,93 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders directory := strings.TrimSpace(pwdBuf.String()) if directory == "" { logger.Warn(ctx, "detected workspace folder is empty, using default workspace folder", - slog.F("default_workspace_folder", DevcontainerDefaultContainerWorkspaceFolder)) + slog.F("default_workspace_folder", DevcontainerDefaultContainerWorkspaceFolder), + ) directory = DevcontainerDefaultContainerWorkspaceFolder } - displayAppsMap := map[codersdk.DisplayApp]bool{ - // NOTE(DanielleMaywood): - // We use the same defaults here as set in terraform-provider-coder. - // https://github.com/coder/terraform-provider-coder/blob/c1c33f6d556532e75662c0ca373ed8fdea220eb5/provider/agent.go#L38-L51 - codersdk.DisplayAppVSCodeDesktop: true, - codersdk.DisplayAppVSCodeInsiders: false, - codersdk.DisplayAppWebTerminal: true, - codersdk.DisplayAppSSH: true, - codersdk.DisplayAppPortForward: true, + if proc.agent.ID != uuid.Nil { + if recreateSubAgent { + logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID)) + err = api.subAgentClient.Delete(ctx, proc.agent.ID) + if err != nil { + return xerrors.Errorf("delete existing subagent failed: %w", err) + } + proc.agent = SubAgent{} + } } + if proc.agent.ID == uuid.Nil { + displayAppsMap := map[codersdk.DisplayApp]bool{ + // NOTE(DanielleMaywood): + // We use the same defaults here as set in terraform-provider-coder. + // https://github.com/coder/terraform-provider-coder/blob/c1c33f6d556532e75662c0ca373ed8fdea220eb5/provider/agent.go#L38-L51 + codersdk.DisplayAppVSCodeDesktop: true, + codersdk.DisplayAppVSCodeInsiders: false, + codersdk.DisplayAppWebTerminal: true, + codersdk.DisplayAppSSH: true, + codersdk.DisplayAppPortForward: true, + } - if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath); err != nil { - api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err)) - } else { - coderCustomization := config.MergedConfiguration.Customizations.Coder + if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath); err != nil { + api.logger.Error(ctx, "unable to read devcontainer config", slog.Error(err)) + } else { + coderCustomization := config.MergedConfiguration.Customizations.Coder - for _, customization := range coderCustomization { - for app, enabled := range customization.DisplayApps { - displayAppsMap[app] = enabled + for _, customization := range coderCustomization { + for app, enabled := range customization.DisplayApps { + displayAppsMap[app] = enabled + } } } - } - displayApps := make([]codersdk.DisplayApp, 0, len(displayAppsMap)) - for app, enabled := range displayAppsMap { - if enabled { - displayApps = append(displayApps, app) + displayApps := make([]codersdk.DisplayApp, 0, len(displayAppsMap)) + for app, enabled := range displayAppsMap { + if enabled { + displayApps = append(displayApps, app) + } } - } - // The preparation of the subagent is done, now we can create the - // subagent record in the database to receive the auth token. - createdAgent, err := api.subAgentClient.Create(ctx, SubAgent{ - Name: dc.Name, - Directory: directory, - OperatingSystem: "linux", // Assuming Linux for dev containers. - Architecture: arch, - DisplayApps: displayApps, - }) - if err != nil { - return xerrors.Errorf("create agent: %w", err) + logger.Debug(ctx, "creating new subagent", + slog.F("directory", directory), + slog.F("display_apps", displayApps), + ) + + // Create new subagent record in the database to receive the auth token. + proc.agent, err = api.subAgentClient.Create(ctx, SubAgent{ + Name: dc.Name, + Directory: directory, + OperatingSystem: "linux", // Assuming Linux for devcontainers. + Architecture: arch, + DisplayApps: displayApps, + }) + if err != nil { + return xerrors.Errorf("create subagent failed: %w", err) + } + + logger.Info(ctx, "created new subagent", slog.F("agent_id", proc.agent.ID)) } - logger.Info(ctx, "created subagent record", slog.F("agent_id", createdAgent.ID)) + api.mu.Lock() // Re-lock to update the agent. + defer api.mu.Unlock() + if api.closed { + deleteCtx, deleteCancel := context.WithTimeout(context.Background(), defaultOperationTimeout) + defer deleteCancel() + err := api.subAgentClient.Delete(deleteCtx, proc.agent.ID) + if err != nil { + return xerrors.Errorf("delete existing subagent failed after API closed: %w", err) + } + return nil + } + // If we got this far, we should update the container ID to make + // sure we don't retry. If we update it too soon we may end up + // using an old subagent if e.g. delete failed previously. + proc.containerID = container.ID + api.injectedSubAgentProcs[dc.WorkspaceFolder] = proc // Start the subagent in the container in a new goroutine to avoid // blocking. Note that we pass the api.ctx to the subagent process // so that it isn't affected by the timeout. - go api.runSubAgentInContainer(api.ctx, dc, createdAgent, coderPathInsideContainer) + go api.runSubAgentInContainer(api.ctx, logger, dc, proc, coderPathInsideContainer) ranSubAgent = true return nil @@ -1179,62 +1187,26 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders // container. The api.asyncWg must be incremented before calling this // function, and it will be decremented when the subagent process // completes or if an error occurs. -func (api *API) runSubAgentInContainer(ctx context.Context, dc codersdk.WorkspaceAgentDevcontainer, agent SubAgent, agentPath string) { +func (api *API) runSubAgentInContainer(ctx context.Context, logger slog.Logger, dc codersdk.WorkspaceAgentDevcontainer, proc subAgentProcess, agentPath string) { container := dc.Container // Must not be nil. - logger := api.logger.With( - slog.F("container_name", container.FriendlyName), - slog.F("agent_id", agent.ID), + logger = logger.With( + slog.F("agent_id", proc.agent.ID), ) - agentCtx, agentStop := context.WithCancel(ctx) defer func() { - agentStop() - - // Best effort cleanup of the agent record after the process - // completes. Note that we use the background context here - // because the api.ctx will be canceled when the API is closed. - // This may delay shutdown of the agent by the given timeout. - deleteCtx, cancel := context.WithTimeout(context.Background(), defaultOperationTimeout) - defer cancel() - err := api.subAgentClient.Delete(deleteCtx, agent.ID) - if err != nil { - logger.Error(deleteCtx, "failed to delete agent record after process completion", slog.Error(err)) - } - - api.mu.Lock() - if api.injectedSubAgentProcs[dc.WorkspaceFolder].containerID == container.ID { - delete(api.injectedSubAgentProcs, dc.WorkspaceFolder) - } - api.mu.Unlock() - + proc.stop() logger.Debug(ctx, "agent process cleanup complete") api.asyncWg.Done() }() - api.mu.Lock() - if api.closed { - api.mu.Unlock() - // If the API is closed, we should not run the agent. - logger.Debug(ctx, "the API is closed, not running subagent in container") - return - } - // Update the placeholder with a valid subagent, context and stop. - api.injectedSubAgentProcs[dc.WorkspaceFolder] = subAgentProcess{ - agent: agent, - containerID: container.ID, - ctx: agentCtx, - stop: agentStop, - } - api.mu.Unlock() - - logger.Info(ctx, "starting subagent in dev container") + logger.Info(ctx, "starting subagent in devcontainer") env := []string{ "CODER_AGENT_URL=" + api.subAgentURL, - "CODER_AGENT_TOKEN=" + agent.AuthToken.String(), + "CODER_AGENT_TOKEN=" + proc.agent.AuthToken.String(), } env = append(env, api.subAgentEnv...) - err := api.dccli.Exec(agentCtx, dc.WorkspaceFolder, dc.ConfigPath, agentPath, []string{"agent"}, + err := api.dccli.Exec(proc.ctx, dc.WorkspaceFolder, dc.ConfigPath, agentPath, []string{"agent"}, WithExecContainerID(container.ID), WithRemoteEnv(env...), ) @@ -1254,18 +1226,38 @@ func (api *API) Close() error { api.logger.Debug(api.ctx, "closing API") api.closed = true - for _, proc := range api.injectedSubAgentProcs { + // Stop all running subagent processes and clean up. + subAgentIDs := make([]uuid.UUID, 0, len(api.injectedSubAgentProcs)) + for workspaceFolder, proc := range api.injectedSubAgentProcs { api.logger.Debug(api.ctx, "canceling subagent process", slog.F("agent_name", proc.agent.Name), slog.F("agent_id", proc.agent.ID), slog.F("container_id", proc.containerID), + slog.F("workspace_folder", workspaceFolder), ) proc.stop() + if proc.agent.ID != uuid.Nil { + subAgentIDs = append(subAgentIDs, proc.agent.ID) + } } + api.injectedSubAgentProcs = make(map[string]subAgentProcess) api.cancel() // Interrupt all routines. api.mu.Unlock() // Release lock before waiting for goroutines. + // Note: We can't use api.ctx here because it's canceled. + deleteCtx, deleteCancel := context.WithTimeout(context.Background(), defaultOperationTimeout) + defer deleteCancel() + for _, id := range subAgentIDs { + err := api.subAgentClient.Delete(deleteCtx, id) + if err != nil { + api.logger.Error(api.ctx, "delete subagent record during shutdown failed", + slog.Error(err), + slog.F("agent_id", id), + ) + } + } + // Close the watcher to ensure its loop finishes. err := api.watcher.Close() diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 5f9929a8430f2..31e35d400025c 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -9,6 +9,7 @@ import ( "os" "runtime" "strings" + "sync" "testing" "time" @@ -186,7 +187,7 @@ func (w *fakeWatcher) Next(ctx context.Context) (*fsnotify.Event, error) { case <-ctx.Done(): return nil, ctx.Err() case <-w.closeNotify: - return nil, xerrors.New("watcher closed") + return nil, watcher.ErrClosed case event := <-w.events: return event, nil } @@ -212,7 +213,6 @@ func (w *fakeWatcher) sendEventWaitNextCalled(ctx context.Context, event fsnotif // fakeSubAgentClient implements SubAgentClient for testing purposes. type fakeSubAgentClient struct { agents map[uuid.UUID]agentcontainers.SubAgent - nextID int listErrC chan error // If set, send to return error, close to return nil. created []agentcontainers.SubAgent @@ -222,14 +222,13 @@ type fakeSubAgentClient struct { } func (m *fakeSubAgentClient) List(ctx context.Context) ([]agentcontainers.SubAgent, error) { - var listErr error if m.listErrC != nil { select { case <-ctx.Done(): return nil, ctx.Err() - case err, ok := <-m.listErrC: - if ok { - listErr = err + case err := <-m.listErrC: + if err != nil { + return nil, err } } } @@ -237,22 +236,20 @@ func (m *fakeSubAgentClient) List(ctx context.Context) ([]agentcontainers.SubAge for _, agent := range m.agents { agents = append(agents, agent) } - return agents, listErr + return agents, nil } func (m *fakeSubAgentClient) Create(ctx context.Context, agent agentcontainers.SubAgent) (agentcontainers.SubAgent, error) { - var createErr error if m.createErrC != nil { select { case <-ctx.Done(): return agentcontainers.SubAgent{}, ctx.Err() - case err, ok := <-m.createErrC: - if ok { - createErr = err + case err := <-m.createErrC: + if err != nil { + return agentcontainers.SubAgent{}, err } } } - m.nextID++ agent.ID = uuid.New() agent.AuthToken = uuid.New() if m.agents == nil { @@ -260,18 +257,17 @@ func (m *fakeSubAgentClient) Create(ctx context.Context, agent agentcontainers.S } m.agents[agent.ID] = agent m.created = append(m.created, agent) - return agent, createErr + return agent, nil } func (m *fakeSubAgentClient) Delete(ctx context.Context, id uuid.UUID) error { - var deleteErr error if m.deleteErrC != nil { select { case <-ctx.Done(): return ctx.Err() - case err, ok := <-m.deleteErrC: - if ok { - deleteErr = err + case err := <-m.deleteErrC: + if err != nil { + return err } } } @@ -280,7 +276,7 @@ func (m *fakeSubAgentClient) Delete(ctx context.Context, id uuid.UUID) error { } delete(m.agents, id) m.deleted = append(m.deleted, id) - return deleteErr + return nil } func TestAPI(t *testing.T) { @@ -596,13 +592,13 @@ func TestAPI(t *testing.T) { // Verify the devcontainer is in starting state after recreation // request is made. - req := httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req := httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec := httptest.NewRecorder() r.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code, "status code mismatch") - var resp codersdk.WorkspaceAgentDevcontainersResponse + var resp codersdk.WorkspaceAgentListContainersResponse t.Log(rec.Body.String()) err := json.NewDecoder(rec.Body).Decode(&resp) require.NoError(t, err, "unmarshal response failed") @@ -625,7 +621,7 @@ func TestAPI(t *testing.T) { _, aw = mClock.AdvanceNext() aw.MustWait(ctx) - req = httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req = httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec = httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -647,7 +643,7 @@ func TestAPI(t *testing.T) { _, aw = mClock.AdvanceNext() aw.MustWait(ctx) - req = httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req = httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec = httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -986,7 +982,7 @@ func TestAPI(t *testing.T) { _, aw := mClock.AdvanceNext() aw.MustWait(ctx) - req := httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req := httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec := httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -997,7 +993,7 @@ func TestAPI(t *testing.T) { return } - var response codersdk.WorkspaceAgentDevcontainersResponse + var response codersdk.WorkspaceAgentListContainersResponse err := json.NewDecoder(rec.Body).Decode(&response) require.NoError(t, err, "unmarshal response failed") @@ -1072,13 +1068,13 @@ func TestAPI(t *testing.T) { }) // Initially the devcontainer should be running and dirty. - req := httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req := httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec := httptest.NewRecorder() api.Routes().ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) - var resp1 codersdk.WorkspaceAgentDevcontainersResponse + var resp1 codersdk.WorkspaceAgentListContainersResponse err := json.NewDecoder(rec.Body).Decode(&resp1) require.NoError(t, err) require.Len(t, resp1.Devcontainers, 1) @@ -1096,13 +1092,13 @@ func TestAPI(t *testing.T) { aw.MustWait(ctx) // Afterwards the devcontainer should not be running and not dirty. - req = httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req = httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec = httptest.NewRecorder() api.Routes().ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) - var resp2 codersdk.WorkspaceAgentDevcontainersResponse + var resp2 codersdk.WorkspaceAgentListContainersResponse err = json.NewDecoder(rec.Body).Decode(&resp2) require.NoError(t, err) require.Len(t, resp2.Devcontainers, 1) @@ -1162,13 +1158,13 @@ func TestAPI(t *testing.T) { // Call the list endpoint first to ensure config files are // detected and watched. - req := httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req := httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec := httptest.NewRecorder() r.ServeHTTP(rec, req) require.Equal(t, http.StatusOK, rec.Code) - var response codersdk.WorkspaceAgentDevcontainersResponse + var response codersdk.WorkspaceAgentListContainersResponse err := json.NewDecoder(rec.Body).Decode(&response) require.NoError(t, err) require.Len(t, response.Devcontainers, 1) @@ -1196,7 +1192,7 @@ func TestAPI(t *testing.T) { aw.MustWait(ctx) // Check if the container is marked as dirty. - req = httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req = httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec = httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -1220,7 +1216,7 @@ func TestAPI(t *testing.T) { aw.MustWait(ctx) // Check if dirty flag is cleared. - req = httptest.NewRequest(http.MethodGet, "/devcontainers", nil). + req = httptest.NewRequest(http.MethodGet, "/", nil). WithContext(ctx) rec = httptest.NewRecorder() r.ServeHTTP(rec, req) @@ -1274,7 +1270,7 @@ func TestAPI(t *testing.T) { mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{ Containers: []codersdk.WorkspaceAgentContainer{testContainer}, - }, nil).AnyTimes() + }, nil).Times(1 + 3) // 1 initial call + 3 updates. gomock.InOrder( mCCLI.EXPECT().DetectArchitecture(gomock.Any(), "test-container-id").Return(runtime.GOARCH, nil), mCCLI.EXPECT().ExecAs(gomock.Any(), "test-container-id", "root", "mkdir", "-p", "/.coder-agent").Return(nil, nil), @@ -1285,6 +1281,7 @@ func TestAPI(t *testing.T) { mClock.Set(time.Now()).MustWait(ctx) tickerTrap := mClock.Trap().TickerFunc("updaterLoop") + var closeOnce sync.Once api := agentcontainers.NewAPI(logger, agentcontainers.WithClock(mClock), agentcontainers.WithContainerCLI(mCCLI), @@ -1293,12 +1290,17 @@ func TestAPI(t *testing.T) { agentcontainers.WithSubAgentURL("test-subagent-url"), agentcontainers.WithDevcontainerCLI(fakeDCCLI), ) - defer api.Close() + apiClose := func() { + closeOnce.Do(func() { + // Close before api.Close() defer to avoid deadlock after test. + close(fakeSAC.createErrC) + close(fakeSAC.deleteErrC) + close(fakeDCCLI.execErrC) - // Close before api.Close() defer to avoid deadlock after test. - defer close(fakeSAC.createErrC) - defer close(fakeSAC.deleteErrC) - defer close(fakeDCCLI.execErrC) + _ = api.Close() + }) + } + defer apiClose() // Allow initial agent creation and injection to succeed. testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil) @@ -1327,9 +1329,27 @@ func TestAPI(t *testing.T) { assert.Len(t, fakeSAC.deleted, 0) } - t.Log("Agent injected successfully, now testing cleanup and reinjection...") + t.Log("Agent injected successfully, now testing reinjection into the same container...") + + // Terminate the agent and verify it can be reinjected. + terminated := make(chan struct{}) + testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(_ string, args ...string) error { + defer close(terminated) + if len(args) > 0 { + assert.Equal(t, "agent", args[0]) + } else { + assert.Fail(t, `want "agent" command argument`) + } + return errTestTermination + }) + <-terminated + + t.Log("Waiting for agent reinjection...") // Expect the agent to be reinjected. + mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{testContainer}, + }, nil).Times(3) // 3 updates. gomock.InOrder( mCCLI.EXPECT().DetectArchitecture(gomock.Any(), "test-container-id").Return(runtime.GOARCH, nil), mCCLI.EXPECT().ExecAs(gomock.Any(), "test-container-id", "root", "mkdir", "-p", "/.coder-agent").Return(nil, nil), @@ -1337,8 +1357,46 @@ func TestAPI(t *testing.T) { mCCLI.EXPECT().ExecAs(gomock.Any(), "test-container-id", "root", "chmod", "0755", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil), ) - // Terminate the agent and verify it is deleted. + // Allow agent reinjection to succeed. + testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error { + assert.Equal(t, "pwd", cmd) + assert.Empty(t, args) + return nil + }) // Exec pwd. + + // Ensure we only inject the agent once. + for i := range 3 { + _, aw := mClock.AdvanceNext() + aw.MustWait(ctx) + + t.Logf("Iteration %d: agents created: %d", i+1, len(fakeSAC.created)) + + // Verify that the agent was reused. + require.Len(t, fakeSAC.created, 1) + assert.Len(t, fakeSAC.deleted, 0) + } + + t.Log("Agent reinjected successfully, now testing agent deletion and recreation...") + + // New container ID means the agent will be recreated. + testContainer.ID = "new-test-container-id" // Simulate a new container ID after recreation. + // Expect the agent to be injected. + mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{testContainer}, + }, nil).Times(3) // 3 updates. + gomock.InOrder( + mCCLI.EXPECT().DetectArchitecture(gomock.Any(), "new-test-container-id").Return(runtime.GOARCH, nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "mkdir", "-p", "/.coder-agent").Return(nil, nil), + mCCLI.EXPECT().Copy(gomock.Any(), "new-test-container-id", coderBin, "/.coder-agent/coder").Return(nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "chmod", "0755", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "chown", "0:0", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "setcap", "cap_net_admin+ep", "/.coder-agent/coder").Return(nil, nil), + ) + + // Terminate the agent and verify it can be reinjected. + terminated = make(chan struct{}) testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(_ string, args ...string) error { + defer close(terminated) if len(args) > 0 { assert.Equal(t, "agent", args[0]) } else { @@ -1346,13 +1404,11 @@ func TestAPI(t *testing.T) { } return errTestTermination }) + <-terminated - // Allow cleanup to proceed. + // Simulate the agent deletion. testutil.RequireSend(ctx, t, fakeSAC.deleteErrC, nil) - - t.Log("Waiting for agent recreation...") - - // Allow agent recreation and reinjection to succeed. + // Expect the agent to be recreated. testutil.RequireSend(ctx, t, fakeSAC.createErrC, nil) testutil.RequireSend(ctx, t, fakeDCCLI.execErrC, func(cmd string, args ...string) error { assert.Equal(t, "pwd", cmd) @@ -1360,20 +1416,25 @@ func TestAPI(t *testing.T) { return nil }) // Exec pwd. - // Wait until the agent recreation is started. - for len(fakeSAC.createErrC) > 0 { + // Advance the clock to run updaterLoop. + for i := range 3 { _, aw := mClock.AdvanceNext() aw.MustWait(ctx) + + t.Logf("Iteration %d: agents created: %d, deleted: %d", i+1, len(fakeSAC.created), len(fakeSAC.deleted)) } - t.Log("Agent recreated successfully.") + // Verify the agent was deleted and recreated. + require.Len(t, fakeSAC.deleted, 1, "there should be one deleted agent after recreation") + assert.Len(t, fakeSAC.created, 2, "there should be two created agents after recreation") + assert.Equal(t, fakeSAC.created[0].ID, fakeSAC.deleted[0], "the deleted agent should match the first created agent") - // Verify agent was deleted. - require.Len(t, fakeSAC.deleted, 1) - assert.Equal(t, fakeSAC.created[0].ID, fakeSAC.deleted[0]) + t.Log("Agent deleted and recreated successfully.") - // Verify the agent recreated. - require.Len(t, fakeSAC.created, 2) + apiClose() + require.Len(t, fakeSAC.created, 2, "API close should not create more agents") + require.Len(t, fakeSAC.deleted, 2, "API close should delete the agent") + assert.Equal(t, fakeSAC.created[1].ID, fakeSAC.deleted[1], "the second created agent should be deleted on API close") }) t.Run("SubAgentCleanup", func(t *testing.T) { diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index 90bd6e67dcf38..5fe648ce15045 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -393,12 +393,6 @@ func (c *Client) WorkspaceAgentListeningPorts(ctx context.Context, agentID uuid. return listeningPorts, json.NewDecoder(res.Body).Decode(&listeningPorts) } -// WorkspaceAgentDevcontainersResponse is the response to the devcontainers -// request. -type WorkspaceAgentDevcontainersResponse struct { - Devcontainers []WorkspaceAgentDevcontainer `json:"devcontainers"` -} - // WorkspaceAgentDevcontainerStatus is the status of a devcontainer. type WorkspaceAgentDevcontainerStatus string From fc1a90a5005817d145008195105c1cd17cc2db00 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 11 Jun 2025 15:01:21 +0000 Subject: [PATCH 06/29] ui-3 --- .../AgentDevcontainerCard.stories.tsx | 2 +- .../resources/AgentDevcontainerCard.tsx | 66 ++++++++++--------- site/src/modules/resources/AgentRow.tsx | 5 +- site/src/pages/WorkspacePage/Workspace.tsx | 3 + 4 files changed, 43 insertions(+), 33 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index b494f0711bf05..3d4211442a536 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -30,7 +30,7 @@ const meta: Meta = { devcontainer: MockWorkspaceAgentDevcontainer, workspace: MockWorkspace, wildcardHostname: "*.wildcard.hostname", - agent: MockWorkspaceAgent, + parentAgent: MockWorkspaceAgent, }, }; diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index e6c4169fa691b..e28c6f05239c3 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -32,26 +32,29 @@ import { TerminalLink } from "./TerminalLink/TerminalLink"; import { VSCodeDevContainerButton } from "./VSCodeDevContainerButton/VSCodeDevContainerButton"; type AgentDevcontainerCardProps = { - agent: WorkspaceAgent; + parentAgent: WorkspaceAgent; + subAgents: WorkspaceAgent[]; devcontainer: WorkspaceAgentDevcontainer; workspace: Workspace; wildcardHostname: string; }; export const AgentDevcontainerCard: FC = ({ - agent, + parentAgent, + subAgents, devcontainer, workspace, wildcardHostname, }) => { const [isRecreating, setIsRecreating] = useState(false); + const [subAgent, setSubAgent] = useState(null); const handleRecreateDevcontainer = async () => { setIsRecreating(true); let recreateSucceeded = false; try { const response = await fetch( - `/api/v2/workspaceagents/${agent.id}/containers/devcontainers/container/${devcontainer.container?.id}/recreate`, + `/api/v2/workspaceagents/${parentAgent.id}/containers/devcontainers/container/${devcontainer.container?.id}/recreate`, { method: "POST", }, @@ -87,7 +90,17 @@ export const AgentDevcontainerCard: FC = ({ } else { setIsRecreating(false); } - }, [devcontainer.status]); + }, [devcontainer.id, devcontainer.status]); + + const shouldDisplayAgentApps = + subAgent?.status === "connected" || subAgent?.status === "connecting"; + + // Woot! We have a sub agent, so we can display the forwarded ports. + useEffect(() => { + setSubAgent( + subAgents.find((sub) => sub.id === devcontainer.agent?.id) || null, + ); + }, [subAgents, devcontainer.agent?.id]); return (
= ({ Recreate - {/* */} - {/* TODO(mafredri): Sub agent display apps. */} - {devcontainer.agent && agent.display_apps.includes("ssh_helper") && ( + {subAgent && subAgent.display_apps.includes("ssh_helper") && ( )} - {devcontainer.agent && ( + {subAgent && devcontainer.container && ( <>

Forwarded ports

- {devcontainer.container && ( - - )} + - {devcontainer.agent && ( - - )} + {wildcardHostname !== "" && - devcontainer.container?.ports.map((port) => { + devcontainer.container.ports.map((port) => { const portLabel = `${port.port}/${port.network.toUpperCase()}`; const hasHostBind = port.host_port !== undefined && port.host_ip !== undefined; @@ -185,7 +189,7 @@ export const AgentDevcontainerCard: FC = ({ ? portForwardURL( wildcardHostname, port.host_port, - devcontainer.agent?.name || agent.name, + subAgent.name, workspace.name, workspace.owner_name, location.protocol === "https" ? "https" : "http", diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index fbb522b125177..d97ec546949a8 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -54,6 +54,7 @@ import { useAgentLogs } from "./useAgentLogs"; interface AgentRowProps { agent: WorkspaceAgent; + subAgents?: WorkspaceAgent[]; workspace: Workspace; template: Template; initialMetadata?: WorkspaceAgentMetadata[]; @@ -62,6 +63,7 @@ interface AgentRowProps { export const AgentRow: FC = ({ agent, + subAgents, workspace, template, onUpdateAgent, @@ -293,7 +295,8 @@ export const AgentRow: FC = ({ devcontainer={devcontainer} workspace={workspace} wildcardHostname={proxy.preferredWildcardHostname} - agent={agent} + parentAgent={agent} + subAgents={subAgents ?? []} /> ); })} diff --git a/site/src/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index 65c924354ceb0..5c032c04efbdf 100644 --- a/site/src/pages/WorkspacePage/Workspace.tsx +++ b/site/src/pages/WorkspacePage/Workspace.tsx @@ -242,6 +242,9 @@ export const Workspace: FC = ({ a.parent_id === agent.id, + )} workspace={workspace} template={template} onUpdateAgent={handleUpdate} // On updating the workspace the agent version is also updated From 88ce78f30fa279c288d209498145a47d3cf6b44e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 11 Jun 2025 16:23:13 +0000 Subject: [PATCH 07/29] ui-wip --- site/src/api/typesGenerated.ts | 5 -- .../resources/AgentDevcontainerCard.tsx | 67 +++++++++++++------ site/src/modules/resources/AgentRow.tsx | 1 + .../VSCodeDevContainerButton.tsx | 4 +- 4 files changed, 48 insertions(+), 29 deletions(-) diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 250a9f33c101b..96a4fbe75e834 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -3393,11 +3393,6 @@ export type WorkspaceAgentDevcontainerStatus = export const WorkspaceAgentDevcontainerStatuses: WorkspaceAgentDevcontainerStatus[] = ["error", "running", "starting", "stopped"]; -// From codersdk/workspaceagents.go -export interface WorkspaceAgentDevcontainersResponse { - readonly devcontainers: readonly WorkspaceAgentDevcontainer[]; -} - // From codersdk/workspaceagents.go export interface WorkspaceAgentHealth { readonly healthy: boolean; diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index e28c6f05239c3..5bf8ce31e93ed 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -1,4 +1,5 @@ import type { + Template, Workspace, WorkspaceAgent, WorkspaceAgentDevcontainer, @@ -30,12 +31,16 @@ import { } from "./SSHButton/SSHButton"; import { TerminalLink } from "./TerminalLink/TerminalLink"; import { VSCodeDevContainerButton } from "./VSCodeDevContainerButton/VSCodeDevContainerButton"; +import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; +import { useProxy } from "contexts/ProxyContext"; +import { PortForwardButton } from "./PortForwardButton"; type AgentDevcontainerCardProps = { parentAgent: WorkspaceAgent; subAgents: WorkspaceAgent[]; devcontainer: WorkspaceAgentDevcontainer; workspace: Workspace; + template: Template; wildcardHostname: string; }; @@ -44,10 +49,13 @@ export const AgentDevcontainerCard: FC = ({ subAgents, devcontainer, workspace, + template, wildcardHostname, }) => { + const { browser_only } = useFeatureVisibility(); + const { proxy } = useProxy(); + const [isRecreating, setIsRecreating] = useState(false); - const [subAgent, setSubAgent] = useState(null); const handleRecreateDevcontainer = async () => { setIsRecreating(true); @@ -83,24 +91,24 @@ export const AgentDevcontainerCard: FC = ({ } }; + const subAgent = subAgents.find((sub) => sub.id === devcontainer.agent?.id); + const shouldDisplaySubAgentApps = + subAgent?.status === "connected" || subAgent?.status === "connecting"; + // If the devcontainer is starting, reflect this in the recreate button. useEffect(() => { + console.log( + "Devcontainer status:", + devcontainer.status, + "Sub agent status:", + subAgent?.status, + ); if (devcontainer.status === "starting") { setIsRecreating(true); } else { setIsRecreating(false); } - }, [devcontainer.id, devcontainer.status]); - - const shouldDisplayAgentApps = - subAgent?.status === "connected" || subAgent?.status === "connecting"; - - // Woot! We have a sub agent, so we can display the forwarded ports. - useEffect(() => { - setSubAgent( - subAgents.find((sub) => sub.id === devcontainer.agent?.id) || null, - ); - }, [subAgents, devcontainer.agent?.id]); + }, [devcontainer]); return (
= ({ Recreate - {subAgent && subAgent.display_apps.includes("ssh_helper") && ( - - )} + {shouldDisplaySubAgentApps && + !browser_only && + // TODO(mafredri): We could use subAgent display apps here but we currently set none. + parentAgent.display_apps.includes("ssh_helper") && ( + + )} + + {shouldDisplaySubAgentApps && + proxy.preferredWildcardHostname === "" && + // TODO(mafredri): We could use subAgent display apps here but we currently set none. + parentAgent.display_apps.includes("port_forwarding_helper") && ( + + )}
- {subAgent && devcontainer.container && ( + {shouldDisplaySubAgentApps && devcontainer.container && ( <>

Forwarded ports

@@ -166,8 +189,8 @@ export const AgentDevcontainerCard: FC = ({ userName={workspace.owner_name} workspaceName={workspace.name} devContainerName={devcontainer.container.name} - devContainerFolder={subAgent.directory ?? ""} // This will always be set. - displayApps={subAgent.display_apps} + devContainerFolder={subAgent.directory ?? "/"} // This will always be set on subagents but provide fallback anyway. + displayApps={parentAgent.display_apps} // TODO(mafredri): We could use subAgent display apps here but we currently set none. agentName={parentAgent.name} /> diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index d97ec546949a8..528fa7a6937f1 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -294,6 +294,7 @@ export const AgentRow: FC = ({ key={devcontainer.id} devcontainer={devcontainer} workspace={workspace} + template={template} wildcardHostname={proxy.preferredWildcardHostname} parentAgent={agent} subAgents={subAgents ?? []} diff --git a/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx b/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx index 42e0a5bd75db4..ffaef3e13016c 100644 --- a/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx +++ b/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx @@ -101,9 +101,9 @@ export const VSCodeDevContainerButton: FC = ( ) : includesVSCodeDesktop ? ( - ) : ( + ) : includesVSCodeInsiders ? ( - ); + ) : null; }; const VSCodeButton: FC = ({ From b1431a3384cf875e865a0e4cff6c14a44e02a158 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 09:05:47 +0000 Subject: [PATCH 08/29] ui-wip2 --- .../resources/AgentDevcontainerCard.tsx | 485 +++++++++++++++--- site/src/modules/resources/AgentStatus.tsx | 26 + .../resources/SubAgentOutdatedTooltip.tsx | 75 +++ 3 files changed, 510 insertions(+), 76 deletions(-) create mode 100644 site/src/modules/resources/SubAgentOutdatedTooltip.tsx diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 5bf8ce31e93ed..58880880586f7 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -1,9 +1,12 @@ +import type { Interpolation, Theme } from "@emotion/react"; +import { alpha } from "@mui/material/styles"; import type { Template, Workspace, WorkspaceAgent, WorkspaceAgentDevcontainer, } from "api/typesGenerated"; +import { Stack } from "components/Stack/Stack"; import { Button } from "components/Button/Button"; import { displayError } from "components/GlobalSnackbar/utils"; import { @@ -20,7 +23,7 @@ import { TooltipProvider, TooltipTrigger, } from "components/Tooltip/Tooltip"; -import { ExternalLinkIcon } from "lucide-react"; +import { ExternalLinkIcon, Container } from "lucide-react"; import type { FC } from "react"; import { useEffect, useState } from "react"; import { portForwardURL } from "utils/portForward"; @@ -34,6 +37,13 @@ import { VSCodeDevContainerButton } from "./VSCodeDevContainerButton/VSCodeDevCo import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { useProxy } from "contexts/ProxyContext"; import { PortForwardButton } from "./PortForwardButton"; +import { AgentStatus, SubAgentStatus } from "./AgentStatus"; +import { AgentVersion } from "./AgentVersion"; +import { AgentLatency } from "./AgentLatency"; +import Skeleton from "@mui/material/Skeleton"; +import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; +import { VSCodeDesktopButton } from "./VSCodeDesktopButton/VSCodeDesktopButton"; +import { SubAgentOutdatedTooltip } from "./SubAgentOutdatedTooltip"; type AgentDevcontainerCardProps = { parentAgent: WorkspaceAgent; @@ -52,14 +62,39 @@ export const AgentDevcontainerCard: FC = ({ template, wildcardHostname, }) => { + const [isRebuilding, setIsRebuilding] = useState(false); + const [subAgentRemoved, setSubAgentRemoved] = useState(false); + const { browser_only } = useFeatureVisibility(); const { proxy } = useProxy(); - const [isRecreating, setIsRecreating] = useState(false); + const subAgent = subAgents.find((sub) => sub.id === devcontainer.agent?.id); + const shouldDisplaySubAgentApps = + devcontainer.container && subAgent?.status === "connected"; + const shouldNotDisplaySubAgentApps = !devcontainer.container || !subAgent; + + const showSubAgentAppsAndElements = + !subAgentRemoved && + subAgent && + subAgent.status === "connected" && + devcontainer.container; + + // const appSections = organizeAgentApps(subAgent?.apps); + // const hasAppsToDisplay = + // !browser_only || appSections.some((it) => it.apps.length > 0); + const hasAppsToDisplay = true; + const shouldDisplayAgentApps = + (subAgent?.status === "connected" && hasAppsToDisplay) || + subAgent?.status === "connecting"; + const hasVSCodeApp = + subAgent?.display_apps.includes("vscode") || + subAgent?.display_apps.includes("vscode_insiders"); + const showVSCode = hasVSCodeApp && !browser_only; - const handleRecreateDevcontainer = async () => { - setIsRecreating(true); - let recreateSucceeded = false; + const handleRebuildDevcontainer = async () => { + setIsRebuilding(true); + setSubAgentRemoved(true); + let rebuildSucceeded = false; try { const response = await fetch( `/api/v2/workspaceagents/${parentAgent.id}/containers/devcontainers/container/${devcontainer.container?.id}/recreate`, @@ -77,7 +112,7 @@ export const AgentDevcontainerCard: FC = ({ // Once complete, the component will unmount, so the spinner will // disappear with it. if (response.status === 202) { - recreateSucceeded = true; + rebuildSucceeded = true; } } catch (error) { const errorMessage = @@ -85,15 +120,20 @@ export const AgentDevcontainerCard: FC = ({ displayError(`Failed to recreate devcontainer: ${errorMessage}`); console.error("Failed to recreate devcontainer:", error); } finally { - if (!recreateSucceeded) { - setIsRecreating(false); + if (!rebuildSucceeded) { + setIsRebuilding(false); } } }; - const subAgent = subAgents.find((sub) => sub.id === devcontainer.agent?.id); - const shouldDisplaySubAgentApps = - subAgent?.status === "connected" || subAgent?.status === "connecting"; + useEffect(() => { + // If the sub agent is removed, we set the state to true to avoid rendering it. + if (!subAgent?.id) { + setSubAgentRemoved(true); + } else { + setSubAgentRemoved(false); + } + }, [subAgent?.id]); // If the devcontainer is starting, reflect this in the recreate button. useEffect(() => { @@ -104,18 +144,91 @@ export const AgentDevcontainerCard: FC = ({ subAgent?.status, ); if (devcontainer.status === "starting") { - setIsRecreating(true); + setIsRebuilding(true); } else { - setIsRecreating(false); + setIsRebuilding(false); } }, [devcontainer]); return ( -
-
+
+ + dev container +
+
+
+
+ + + {subAgent?.name ?? devcontainer.name} + {!isRebuilding && devcontainer.container && ( + + {" "} + ({devcontainer.container.name}) + + )} + +
+ {!subAgentRemoved && subAgent?.status === "connected" && ( + <> + + + + )} + {!subAgentRemoved && subAgent?.status === "connecting" && ( + <> + + + + )} +
+ +
+ + + {showSubAgentAppsAndElements && + subAgent.display_apps.includes("ssh_helper") && ( + + )} + {showSubAgentAppsAndElements && + proxy.preferredWildcardHostname !== "" && + subAgent.display_apps.includes("port_forwarding_helper") && ( + + )} +
+
+ {/*

dev container:{" "} @@ -138,7 +251,7 @@ export const AgentDevcontainerCard: FC = ({ Devcontainer Outdated Devcontainer configuration has been modified and is outdated. - Recreate to get an up-to-date container. + Rebuild to get an up-to-date container. @@ -149,11 +262,11 @@ export const AgentDevcontainerCard: FC = ({ {shouldDisplaySubAgentApps && @@ -179,64 +292,284 @@ export const AgentDevcontainerCard: FC = ({ /> )}

-
+
*/} - {shouldDisplaySubAgentApps && devcontainer.container && ( - <> -

Forwarded ports

-
- +
+ {subAgent && workspace.latest_app_status?.agent_id === subAgent.id && ( +
+

App statuses

+ +
+ )} - + {showSubAgentAppsAndElements && ( + <> + {showVSCode && ( + + )} + {/* {appSections.map((section, i) => ( + + ))} */} + + )} + + {showSubAgentAppsAndElements && + subAgent.display_apps.includes("web_terminal") && ( + + )} +
+ + {/*
*/} + {/* {showApps && subAgent && devcontainer.container && ( + + )} + + {showApps && parentAgent.display_apps.includes("web_terminal") && ( + + )} */} + + {showSubAgentAppsAndElements && + wildcardHostname !== "" && + devcontainer.container?.ports.map((port) => { + const portLabel = `${port.port}/${port.network.toUpperCase()}`; + const hasHostBind = + port.host_port !== undefined && port.host_ip !== undefined; + const helperText = hasHostBind + ? `${port.host_ip}:${port.host_port}` + : "Not bound to host"; + const linkDest = hasHostBind + ? portForwardURL( + wildcardHostname, + port.host_port, + subAgent.name, + workspace.name, + workspace.owner_name, + location.protocol === "https" ? "https" : "http", + ) + : ""; + return ( + + + + + + + {portLabel} + + + + {helperText} + + + ); + })} + + {!showSubAgentAppsAndElements && ( +
+ + +
+ )} +
- {wildcardHostname !== "" && - devcontainer.container.ports.map((port) => { - const portLabel = `${port.port}/${port.network.toUpperCase()}`; - const hasHostBind = - port.host_port !== undefined && port.host_ip !== undefined; - const helperText = hasHostBind - ? `${port.host_ip}:${port.host_port}` - : "Not bound to host"; - const linkDest = hasHostBind - ? portForwardURL( - wildcardHostname, - port.host_port, - subAgent.name, - workspace.name, - workspace.owner_name, - location.protocol === "https" ? "https" : "http", - ) - : ""; - return ( - - - - - - - {portLabel} - - - - {helperText} - - - ); - })} -
- - )} -
+ {/* */} + ); }; + +const styles = { + // Many of these styles are borrowed or mimic those from AgentRow.tsx. + subAgentRow: (theme) => ({ + fontSize: 14 - 2, + // TODO(mafredri): Not sure which color to use here, this comes + // from the border css classes. + border: `1px dashed hsl(var(--border-default))`, + borderRadius: 8, + position: "relative", + }), + + header: (theme) => ({ + padding: "16px 16px 0 32px", + display: "flex", + gap: 24, + alignItems: "center", + justifyContent: "space-between", + flexWrap: "wrap", + lineHeight: "1.5", + + "&:has(+ [role='alert'])": { + paddingBottom: 16, + }, + + [theme.breakpoints.down("md")]: { + gap: 16, + }, + }), + + devContainerLabel: (theme) => ({ + backgroundColor: theme.palette.background.default, + fontSize: 12, + lineHeight: 1, + padding: "4px 8px", + position: "absolute", + top: -11, + left: 19, + }), + devContainerIcon: { + marginRight: 5, + }, + + agentInfo: (theme) => ({ + display: "flex", + alignItems: "center", + gap: 24, + color: theme.palette.text.secondary, + fontSize: 14 - 2, + }), + + agentNameAndInfo: (theme) => ({ + display: "flex", + alignItems: "center", + gap: 24, + flexWrap: "wrap", + + [theme.breakpoints.down("md")]: { + gap: 12, + }, + }), + + content: { + padding: 32, + display: "flex", + flexDirection: "column", + gap: 32, + }, + + apps: (theme) => ({ + display: "flex", + gap: 16, + flexWrap: "wrap", + + "&:empty": { + display: "none", + }, + + [theme.breakpoints.down("md")]: { + marginLeft: 0, + justifyContent: "flex-start", + }, + }), + + agentDescription: (theme) => ({ + fontSize: 14 - 2, + color: theme.palette.text.secondary, + }), + + agentNameAndStatus: (theme) => ({ + display: "flex", + alignItems: "center", + gap: 16, + + [theme.breakpoints.down("md")]: { + width: "100%", + }, + }), + + agentName: (theme) => ({ + whiteSpace: "nowrap", + overflow: "hidden", + textOverflow: "ellipsis", + maxWidth: 260, + fontWeight: 600, + flexShrink: 0, + width: "fit-content", + fontSize: 16 - 2, + color: theme.palette.text.primary, + + [theme.breakpoints.down("md")]: { + overflow: "unset", + }, + }), + + agentDataGroup: { + display: "flex", + alignItems: "baseline", + gap: 48, + }, + + agentData: (theme) => ({ + display: "flex", + flexDirection: "column", + fontSize: 12 - 2, + + "& > *:first-of-type": { + fontWeight: 500, + color: theme.palette.text.secondary, + }, + }), + + buttonSkeleton: { + borderRadius: 4, + }, + + agentErrorMessage: (theme) => ({ + fontSize: 12 - 2, + fontWeight: 400, + marginTop: 4, + color: theme.palette.warning.light, + }), + + agentOS: { + textTransform: "capitalize", + }, + + startupLogs: (theme) => ({ + maxHeight: 256, + borderBottom: `1px solid ${theme.palette.divider}`, + backgroundColor: theme.palette.background.paper, + paddingTop: 16, + + // We need this to be able to apply the padding top from startupLogs + "& > div": { + position: "relative", + }, + }), +} satisfies Record>; diff --git a/site/src/modules/resources/AgentStatus.tsx b/site/src/modules/resources/AgentStatus.tsx index 2f80ab5dab16a..eb113f55fed4a 100644 --- a/site/src/modules/resources/AgentStatus.tsx +++ b/site/src/modules/resources/AgentStatus.tsx @@ -46,6 +46,10 @@ interface AgentStatusProps { agent: WorkspaceAgent; } +interface SubAgentStatusProps { + agent?: WorkspaceAgent; +} + const StartTimeoutLifecycle: FC = ({ agent }) => { return ( @@ -270,6 +274,28 @@ export const AgentStatus: FC = ({ agent }) => { ); }; +export const SubAgentStatus: FC = ({ agent }) => { + if (!agent) { + return ; + } + return ( + + + + + + + + + + + + + + + ); +}; + const styles = { status: { width: 6, diff --git a/site/src/modules/resources/SubAgentOutdatedTooltip.tsx b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx new file mode 100644 index 0000000000000..c02b3942b47ac --- /dev/null +++ b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx @@ -0,0 +1,75 @@ +import { useTheme } from "@emotion/react"; +import type { + WorkspaceAgent, + WorkspaceAgentDevcontainer, +} from "api/typesGenerated"; +import { + HelpTooltip, + HelpTooltipAction, + HelpTooltipContent, + HelpTooltipLinksGroup, + HelpTooltipText, + HelpTooltipTitle, + HelpTooltipTrigger, +} from "components/HelpTooltip/HelpTooltip"; +import { Stack } from "components/Stack/Stack"; +import { PopoverTrigger } from "components/deprecated/Popover/Popover"; +import { RotateCcwIcon } from "lucide-react"; +import type { FC } from "react"; +import { agentVersionStatus } from "../../utils/workspace"; + +type SubAgentOutdatedTooltipProps = { + devcontainer: WorkspaceAgentDevcontainer; + agent: WorkspaceAgent; + onUpdate: () => void; +}; + +export const SubAgentOutdatedTooltip: FC = ({ + devcontainer, + agent, + onUpdate, +}) => { + if (!devcontainer.agent || devcontainer.agent.id != agent.id) { + return null; + } + if (!devcontainer.dirty) { + return null; + } + + const theme = useTheme(); + const versionLabelStyles = { + fontWeight: 600, + color: theme.palette.text.primary, + }; + const title = "Dev Container Outdated"; + const opener = "This Dev Container is outdated."; + const text = `${opener} This can happen if you modify your devcontainer.json file after the Dev Container has been created. To fix this, you can rebuild the Dev Container.`; + + return ( + + + + Outdated + + + + +
+ {title} + {text} +
+ + + + Rebuild Dev Container + + + + + + ); +}; From 2c2bf28ff71b74a4e794a63a636f2ad01d0980aa Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 09:11:50 +0000 Subject: [PATCH 09/29] backend fix test after rebase --- agent/agentcontainers/api_test.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index 31e35d400025c..92a697b6e23b4 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -1389,8 +1389,6 @@ func TestAPI(t *testing.T) { mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "mkdir", "-p", "/.coder-agent").Return(nil, nil), mCCLI.EXPECT().Copy(gomock.Any(), "new-test-container-id", coderBin, "/.coder-agent/coder").Return(nil), mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "chmod", "0755", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil), - mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "chown", "0:0", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil), - mCCLI.EXPECT().ExecAs(gomock.Any(), "new-test-container-id", "root", "setcap", "cap_net_admin+ep", "/.coder-agent/coder").Return(nil, nil), ) // Terminate the agent and verify it can be reinjected. From 18e1593c30dd0f6f2d350531abe22bc0180f30ab Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 17:29:42 +0000 Subject: [PATCH 10/29] ui-final? --- agent/agentcontainers/api.go | 16 +- .../resources/AgentDevcontainerCard.tsx | 395 ++++++------------ site/src/modules/resources/AgentRow.tsx | 6 +- site/src/modules/resources/AgentStatus.tsx | 2 +- .../modules/resources/SSHButton/SSHButton.tsx | 50 --- 5 files changed, 133 insertions(+), 336 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 5806349a4358b..daa55ab70bc7e 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -661,13 +661,17 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse, if len(api.knownDevcontainers) > 0 { devcontainers = make([]codersdk.WorkspaceAgentDevcontainer, 0, len(api.knownDevcontainers)) for _, dc := range api.knownDevcontainers { - // Include the agent if it's been created (we're iterating - // over copies, so mutating is fine). - if agent := api.injectedSubAgentProcs[dc.WorkspaceFolder].agent; agent.ID != uuid.Nil { + // Include the agent if it's been created (we're iterating over + // copies, so mutating is fine). + // + // NOTE(mafredri): We could filter on "proc.containerID == dc.Container.ID" + // here but not doing so allows us to do some tricks in the UI to + // make the experience more responsive for now. + if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil { dc.Agent = &codersdk.WorkspaceAgentDevcontainerAgent{ - ID: agent.ID, - Name: agent.Name, - Directory: agent.Directory, + ID: proc.agent.ID, + Name: proc.agent.Name, + Directory: proc.agent.Directory, } } diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 58880880586f7..85ae871f15171 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -1,5 +1,4 @@ import type { Interpolation, Theme } from "@emotion/react"; -import { alpha } from "@mui/material/styles"; import type { Template, Workspace, @@ -9,13 +8,6 @@ import type { import { Stack } from "components/Stack/Stack"; import { Button } from "components/Button/Button"; import { displayError } from "components/GlobalSnackbar/utils"; -import { - HelpTooltip, - HelpTooltipContent, - HelpTooltipText, - HelpTooltipTitle, - HelpTooltipTrigger, -} from "components/HelpTooltip/HelpTooltip"; import { Spinner } from "components/Spinner/Spinner"; import { Tooltip, @@ -28,22 +20,18 @@ import type { FC } from "react"; import { useEffect, useState } from "react"; import { portForwardURL } from "utils/portForward"; import { AgentButton } from "./AgentButton"; -import { - AgentDevcontainerSSHButton, - AgentSSHButton, -} from "./SSHButton/SSHButton"; +import { AgentSSHButton } from "./SSHButton/SSHButton"; import { TerminalLink } from "./TerminalLink/TerminalLink"; import { VSCodeDevContainerButton } from "./VSCodeDevContainerButton/VSCodeDevContainerButton"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { useProxy } from "contexts/ProxyContext"; import { PortForwardButton } from "./PortForwardButton"; -import { AgentStatus, SubAgentStatus } from "./AgentStatus"; -import { AgentVersion } from "./AgentVersion"; +import { SubAgentStatus } from "./AgentStatus"; import { AgentLatency } from "./AgentLatency"; import Skeleton from "@mui/material/Skeleton"; import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; -import { VSCodeDesktopButton } from "./VSCodeDesktopButton/VSCodeDesktopButton"; import { SubAgentOutdatedTooltip } from "./SubAgentOutdatedTooltip"; +import { organizeAgentApps, Apps } from "./AgentRow"; type AgentDevcontainerCardProps = { parentAgent: WorkspaceAgent; @@ -62,34 +50,47 @@ export const AgentDevcontainerCard: FC = ({ template, wildcardHostname, }) => { - const [isRebuilding, setIsRebuilding] = useState(false); - const [subAgentRemoved, setSubAgentRemoved] = useState(false); - const { browser_only } = useFeatureVisibility(); const { proxy } = useProxy(); + const [isRebuilding, setIsRebuilding] = useState(false); + + // Track sub agent removal state to improve UX. This will not be needed once + // the devcontainer and agent responses are aligned. + const [subAgentRemoved, setSubAgentRemoved] = useState(false); + + // The sub agent comes from the workspace response whereas the devcontainer + // comes from the agent containers endpoint. We need alignment between the + // two, so if the sub agent is not present or the IDs do not match, we + // assume it has been removed. const subAgent = subAgents.find((sub) => sub.id === devcontainer.agent?.id); - const shouldDisplaySubAgentApps = - devcontainer.container && subAgent?.status === "connected"; - const shouldNotDisplaySubAgentApps = !devcontainer.container || !subAgent; - const showSubAgentAppsAndElements = + const appSections = (subAgent && organizeAgentApps(subAgent.apps)) || []; + const displayApps = + subAgent?.display_apps.filter((app) => { + switch (true) { + case browser_only: + return ["web_terminal", "port_forwarding_helper"].includes(app); + default: + return true; + } + }) || []; + const showVSCode = + devcontainer.container && + (displayApps.includes("vscode") || displayApps.includes("vscode_insiders")); + const hasAppsToDisplay = + displayApps.includes("web_terminal") || + showVSCode || + appSections.some((it) => it.apps.length > 0); + + const showDevcontainerControls = + !subAgentRemoved && subAgent && devcontainer.container; + const showSubAgentApps = !subAgentRemoved && - subAgent && - subAgent.status === "connected" && - devcontainer.container; - - // const appSections = organizeAgentApps(subAgent?.apps); - // const hasAppsToDisplay = - // !browser_only || appSections.some((it) => it.apps.length > 0); - const hasAppsToDisplay = true; - const shouldDisplayAgentApps = - (subAgent?.status === "connected" && hasAppsToDisplay) || - subAgent?.status === "connecting"; - const hasVSCodeApp = - subAgent?.display_apps.includes("vscode") || - subAgent?.display_apps.includes("vscode_insiders"); - const showVSCode = hasVSCodeApp && !browser_only; + ((subAgent?.status === "connected" && hasAppsToDisplay) || + subAgent?.status === "connecting"); + const showSubAgentAppsPlaceholders = + subAgentRemoved || subAgent?.status === "connecting"; const handleRebuildDevcontainer = async () => { setIsRebuilding(true); @@ -127,22 +128,15 @@ export const AgentDevcontainerCard: FC = ({ }; useEffect(() => { - // If the sub agent is removed, we set the state to true to avoid rendering it. - if (!subAgent?.id) { - setSubAgentRemoved(true); - } else { + if (subAgent?.id) { setSubAgentRemoved(false); + } else { + setSubAgentRemoved(true); } }, [subAgent?.id]); // If the devcontainer is starting, reflect this in the recreate button. useEffect(() => { - console.log( - "Devcontainer status:", - devcontainer.status, - "Sub agent status:", - subAgent?.status, - ); if (devcontainer.status === "starting") { setIsRebuilding(true); } else { @@ -155,12 +149,12 @@ export const AgentDevcontainerCard: FC = ({ key={devcontainer.id} direction="column" spacing={0} - css={[styles.subAgentRow]} - className="border border-border border-dashed rounded" + css={styles.devcontainerRow} + className="border border-border border-dashed rounded relative" >
dev container @@ -208,82 +202,16 @@ export const AgentDevcontainerCard: FC = ({ Rebuild - {showSubAgentAppsAndElements && - subAgent.display_apps.includes("ssh_helper") && ( - - )} - {showSubAgentAppsAndElements && - proxy.preferredWildcardHostname !== "" && - subAgent.display_apps.includes("port_forwarding_helper") && ( - - )} -
- - {/*
-
-

- dev container:{" "} - - {devcontainer.name} - {devcontainer.container && ( - - {" "} - ({devcontainer.container.name}) - - )} - -

- {devcontainer.dirty && ( - - - Outdated - - - Devcontainer Outdated - - Devcontainer configuration has been modified and is outdated. - Rebuild to get an up-to-date container. - - - + {showDevcontainerControls && displayApps.includes("ssh_helper") && ( + )} -
- -
- - - {shouldDisplaySubAgentApps && - !browser_only && - // TODO(mafredri): We could use subAgent display apps here but we currently set none. - parentAgent.display_apps.includes("ssh_helper") && ( - - )} - - {shouldDisplaySubAgentApps && - proxy.preferredWildcardHostname === "" && - // TODO(mafredri): We could use subAgent display apps here but we currently set none. - parentAgent.display_apps.includes("port_forwarding_helper") && ( + {showDevcontainerControls && + displayApps.includes("port_forwarding_helper") && + proxy.preferredWildcardHostname !== "" && ( = ({ /> )}
-
*/} +
{subAgent && workspace.latest_app_status?.agent_id === subAgent.id && ( @@ -302,97 +230,75 @@ export const AgentDevcontainerCard: FC = ({ )} -
- {showSubAgentAppsAndElements && ( + {showSubAgentApps && ( +
<> {showVSCode && ( )} - {/* {appSections.map((section, i) => ( + {appSections.map((section, i) => ( - ))} */} + ))} - )} - {showSubAgentAppsAndElements && - subAgent.display_apps.includes("web_terminal") && ( + {displayApps.includes("web_terminal") && ( )} -
- - {/*
*/} - {/* {showApps && subAgent && devcontainer.container && ( - + + {wildcardHostname !== "" && + devcontainer.container?.ports.map((port) => { + const portLabel = `${port.port}/${port.network.toUpperCase()}`; + const hasHostBind = + port.host_port !== undefined && port.host_ip !== undefined; + const helperText = hasHostBind + ? `${port.host_ip}:${port.host_port}` + : "Not bound to host"; + const linkDest = hasHostBind + ? portForwardURL( + wildcardHostname, + port.host_port, + subAgent.name, + workspace.name, + workspace.owner_name, + location.protocol === "https" ? "https" : "http", + ) + : ""; + return ( + + + + + + + {portLabel} + + + + {helperText} + + + ); + })} +
)} - {showApps && parentAgent.display_apps.includes("web_terminal") && ( - - )} */} - - {showSubAgentAppsAndElements && - wildcardHostname !== "" && - devcontainer.container?.ports.map((port) => { - const portLabel = `${port.port}/${port.network.toUpperCase()}`; - const hasHostBind = - port.host_port !== undefined && port.host_ip !== undefined; - const helperText = hasHostBind - ? `${port.host_ip}:${port.host_port}` - : "Not bound to host"; - const linkDest = hasHostBind - ? portForwardURL( - wildcardHostname, - port.host_port, - subAgent.name, - workspace.name, - workspace.owner_name, - location.protocol === "https" ? "https" : "http", - ) - : ""; - return ( - - - - - - - {portLabel} - - - - {helperText} - - - ); - })} - - {!showSubAgentAppsAndElements && ( + {showSubAgentAppsPlaceholders && (
= ({
)}
- - {/* */} ); }; const styles = { - // Many of these styles are borrowed or mimic those from AgentRow.tsx. - subAgentRow: (theme) => ({ - fontSize: 14 - 2, - // TODO(mafredri): Not sure which color to use here, this comes - // from the border css classes. - border: `1px dashed hsl(var(--border-default))`, - borderRadius: 8, - position: "relative", + devContainerLabel: (theme) => ({ + backgroundColor: theme.palette.background.default, + fontSize: 12, + lineHeight: 1, + padding: "4px 8px", + position: "absolute", + top: -11, + left: 20, }), + devContainerIcon: { + marginRight: 5, + }, + devcontainerRow: { + padding: "16px 0px", + }, + + // Many of these styles are borrowed or mimic those from AgentRow.tsx. header: (theme) => ({ - padding: "16px 16px 0 32px", + padding: "0px 16px 0px 32px", display: "flex", gap: 24, alignItems: "center", @@ -444,40 +356,16 @@ const styles = { }, }), - devContainerLabel: (theme) => ({ - backgroundColor: theme.palette.background.default, - fontSize: 12, - lineHeight: 1, - padding: "4px 8px", - position: "absolute", - top: -11, - left: 19, - }), - devContainerIcon: { - marginRight: 5, - }, - agentInfo: (theme) => ({ display: "flex", alignItems: "center", gap: 24, color: theme.palette.text.secondary, - fontSize: 14 - 2, - }), - - agentNameAndInfo: (theme) => ({ - display: "flex", - alignItems: "center", - gap: 24, - flexWrap: "wrap", - - [theme.breakpoints.down("md")]: { - gap: 12, - }, + fontSize: 12, }), content: { - padding: 32, + padding: "16px 32px 0px 32px", display: "flex", flexDirection: "column", gap: 32, @@ -498,11 +386,6 @@ const styles = { }, }), - agentDescription: (theme) => ({ - fontSize: 14 - 2, - color: theme.palette.text.secondary, - }), - agentNameAndStatus: (theme) => ({ display: "flex", alignItems: "center", @@ -521,7 +404,7 @@ const styles = { fontWeight: 600, flexShrink: 0, width: "fit-content", - fontSize: 16 - 2, + fontSize: 14, color: theme.palette.text.primary, [theme.breakpoints.down("md")]: { @@ -529,47 +412,7 @@ const styles = { }, }), - agentDataGroup: { - display: "flex", - alignItems: "baseline", - gap: 48, - }, - - agentData: (theme) => ({ - display: "flex", - flexDirection: "column", - fontSize: 12 - 2, - - "& > *:first-of-type": { - fontWeight: 500, - color: theme.palette.text.secondary, - }, - }), - buttonSkeleton: { borderRadius: 4, }, - - agentErrorMessage: (theme) => ({ - fontSize: 12 - 2, - fontWeight: 400, - marginTop: 4, - color: theme.palette.warning.light, - }), - - agentOS: { - textTransform: "capitalize", - }, - - startupLogs: (theme) => ({ - maxHeight: 256, - borderBottom: `1px solid ${theme.palette.divider}`, - backgroundColor: theme.palette.background.paper, - paddingTop: 16, - - // We need this to be able to apply the padding top from startupLogs - "& > div": { - position: "relative", - }, - }), } satisfies Record>; diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index 528fa7a6937f1..b374bdb648de6 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -354,7 +354,7 @@ export const AgentRow: FC = ({ ); }; -type AppSection = { +export type AppSection = { /** * If there is no `group`, just render all of the apps inline. If there is a * group name, show them all in a dropdown. @@ -409,13 +409,13 @@ export function organizeAgentApps(apps: readonly WorkspaceApp[]): AppSection[] { return appGroups; } -type AppsProps = { +export type AppsProps = { section: AppSection; agent: WorkspaceAgent; workspace: Workspace; }; -const Apps: FC = ({ section, agent, workspace }) => { +export const Apps: FC = ({ section, agent, workspace }) => { return section.group ? ( diff --git a/site/src/modules/resources/AgentStatus.tsx b/site/src/modules/resources/AgentStatus.tsx index eb113f55fed4a..eeae1d8fcd88f 100644 --- a/site/src/modules/resources/AgentStatus.tsx +++ b/site/src/modules/resources/AgentStatus.tsx @@ -276,7 +276,7 @@ export const AgentStatus: FC = ({ agent }) => { export const SubAgentStatus: FC = ({ agent }) => { if (!agent) { - return ; + return ; } return ( diff --git a/site/src/modules/resources/SSHButton/SSHButton.tsx b/site/src/modules/resources/SSHButton/SSHButton.tsx index 27d124b7c9443..a1ac3c6819361 100644 --- a/site/src/modules/resources/SSHButton/SSHButton.tsx +++ b/site/src/modules/resources/SSHButton/SSHButton.tsx @@ -85,56 +85,6 @@ export const AgentSSHButton: FC = ({ ); }; -interface AgentDevcontainerSSHButtonProps { - workspace: string; - agentName: string; -} - -export const AgentDevcontainerSSHButton: FC< - AgentDevcontainerSSHButtonProps -> = ({ workspace, agentName }) => { - const paper = useClassName(classNames.paper, []); - - return ( - - - - - - - - Run the following commands to connect with SSH: - - -
    - - - -
- - - - Install Coder CLI - - - SSH configuration - - - - - ); -}; - interface SSHStepProps { helpText: string; codeExample: string; From df28ff9ed14efc83964404a0bf7992e58315e02f Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 19:05:56 +0000 Subject: [PATCH 11/29] oh no I accidentally the coercion --- .../AgentDevcontainerCard.stories.tsx | 2 +- .../resources/AgentDevcontainerCard.tsx | 22 +++++++++---------- .../resources/SubAgentOutdatedTooltip.tsx | 4 +--- 3 files changed, 13 insertions(+), 15 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index 3d4211442a536..9103e4246a70f 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,4 +1,5 @@ import type { Meta, StoryObj } from "@storybook/react"; +import type { WorkspaceAgentDevcontainer } from "api/typesGenerated"; import { MockWorkspace, MockWorkspaceAgent, @@ -6,7 +7,6 @@ import { MockWorkspaceAgentContainerPorts, } from "testHelpers/entities"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; -import type { WorkspaceAgentDevcontainer } from "api/typesGenerated"; const MockWorkspaceAgentDevcontainer: WorkspaceAgentDevcontainer = { id: "test-devcontainer-id", diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 85ae871f15171..ef5ada169845d 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -1,37 +1,37 @@ import type { Interpolation, Theme } from "@emotion/react"; +import Skeleton from "@mui/material/Skeleton"; import type { Template, Workspace, WorkspaceAgent, WorkspaceAgentDevcontainer, } from "api/typesGenerated"; -import { Stack } from "components/Stack/Stack"; import { Button } from "components/Button/Button"; import { displayError } from "components/GlobalSnackbar/utils"; import { Spinner } from "components/Spinner/Spinner"; +import { Stack } from "components/Stack/Stack"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger, } from "components/Tooltip/Tooltip"; -import { ExternalLinkIcon, Container } from "lucide-react"; +import { useProxy } from "contexts/ProxyContext"; +import { Container, ExternalLinkIcon } from "lucide-react"; +import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; +import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; import type { FC } from "react"; import { useEffect, useState } from "react"; import { portForwardURL } from "utils/portForward"; import { AgentButton } from "./AgentButton"; +import { AgentLatency } from "./AgentLatency"; +import { Apps, organizeAgentApps } from "./AgentRow"; +import { SubAgentStatus } from "./AgentStatus"; +import { PortForwardButton } from "./PortForwardButton"; import { AgentSSHButton } from "./SSHButton/SSHButton"; +import { SubAgentOutdatedTooltip } from "./SubAgentOutdatedTooltip"; import { TerminalLink } from "./TerminalLink/TerminalLink"; import { VSCodeDevContainerButton } from "./VSCodeDevContainerButton/VSCodeDevContainerButton"; -import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; -import { useProxy } from "contexts/ProxyContext"; -import { PortForwardButton } from "./PortForwardButton"; -import { SubAgentStatus } from "./AgentStatus"; -import { AgentLatency } from "./AgentLatency"; -import Skeleton from "@mui/material/Skeleton"; -import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; -import { SubAgentOutdatedTooltip } from "./SubAgentOutdatedTooltip"; -import { organizeAgentApps, Apps } from "./AgentRow"; type AgentDevcontainerCardProps = { parentAgent: WorkspaceAgent; diff --git a/site/src/modules/resources/SubAgentOutdatedTooltip.tsx b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx index c02b3942b47ac..ab16c227e1b73 100644 --- a/site/src/modules/resources/SubAgentOutdatedTooltip.tsx +++ b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx @@ -13,10 +13,8 @@ import { HelpTooltipTrigger, } from "components/HelpTooltip/HelpTooltip"; import { Stack } from "components/Stack/Stack"; -import { PopoverTrigger } from "components/deprecated/Popover/Popover"; import { RotateCcwIcon } from "lucide-react"; import type { FC } from "react"; -import { agentVersionStatus } from "../../utils/workspace"; type SubAgentOutdatedTooltipProps = { devcontainer: WorkspaceAgentDevcontainer; @@ -29,7 +27,7 @@ export const SubAgentOutdatedTooltip: FC = ({ agent, onUpdate, }) => { - if (!devcontainer.agent || devcontainer.agent.id != agent.id) { + if (!devcontainer.agent || devcontainer.agent.id !== agent.id) { return null; } if (!devcontainer.dirty) { From 9f69f692182058012d0bfbc83882716c948b851b Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 18:08:12 +0000 Subject: [PATCH 12/29] site: remove circ dependency --- .../modules/resources/AgentApps/AgentApps.tsx | 94 ++++++++++++++++++ .../resources/AgentDevcontainerCard.tsx | 2 +- site/src/modules/resources/AgentRow.test.tsx | 2 +- site/src/modules/resources/AgentRow.tsx | 98 +------------------ 4 files changed, 97 insertions(+), 99 deletions(-) create mode 100644 site/src/modules/resources/AgentApps/AgentApps.tsx diff --git a/site/src/modules/resources/AgentApps/AgentApps.tsx b/site/src/modules/resources/AgentApps/AgentApps.tsx new file mode 100644 index 0000000000000..a59444419d704 --- /dev/null +++ b/site/src/modules/resources/AgentApps/AgentApps.tsx @@ -0,0 +1,94 @@ +import type { WorkspaceApp } from "api/typesGenerated"; +import type { Workspace, WorkspaceAgent } from "api/typesGenerated"; +import { + DropdownMenu, + DropdownMenuContent, + DropdownMenuItem, + DropdownMenuTrigger, +} from "components/DropdownMenu/DropdownMenu"; +import { Folder } from "lucide-react"; +import type { FC } from "react"; +import { AgentButton } from "../AgentButton"; +import { AppLink } from "../AppLink/AppLink"; + +type AppsProps = { + section: AppSection; + agent: WorkspaceAgent; + workspace: Workspace; +}; + +export const Apps: FC = ({ section, agent, workspace }) => { + return section.group ? ( + + + + + {section.group} + + + + {section.apps.map((app) => ( + + + + ))} + + + ) : ( + <> + {section.apps.map((app) => ( + + ))} + + ); +}; + +type AppSection = { + /** + * If there is no `group`, just render all of the apps inline. If there is a + * group name, show them all in a dropdown. + */ + group?: string; + + apps: WorkspaceApp[]; +}; + +/** + * Groups apps by their `group` property. Apps with the same group are placed + * in the same section. Apps without a group are placed in their own section. + * + * The algorithm assumes that apps are already sorted by group, meaning that + * every ungrouped section is expected to have a group in between, to make the + * algorithm a little simpler to implement. + */ +export function organizeAgentApps(apps: readonly WorkspaceApp[]): AppSection[] { + let currentSection: AppSection | undefined = undefined; + const appGroups: AppSection[] = []; + const groupsByName = new Map(); + + for (const app of apps) { + if (app.hidden) { + continue; + } + + if (!currentSection || app.group !== currentSection.group) { + const existingSection = groupsByName.get(app.group!); + if (existingSection) { + currentSection = existingSection; + } else { + currentSection = { + group: app.group, + apps: [], + }; + appGroups.push(currentSection); + if (app.group) { + groupsByName.set(app.group, currentSection); + } + } + } + + currentSection.apps.push(app); + } + + return appGroups; +} diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index ef5ada169845d..a0eef87e094d8 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -23,9 +23,9 @@ import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; import type { FC } from "react"; import { useEffect, useState } from "react"; import { portForwardURL } from "utils/portForward"; +import { Apps, organizeAgentApps } from "./AgentApps/AgentApps"; import { AgentButton } from "./AgentButton"; import { AgentLatency } from "./AgentLatency"; -import { Apps, organizeAgentApps } from "./AgentRow"; import { SubAgentStatus } from "./AgentStatus"; import { PortForwardButton } from "./PortForwardButton"; import { AgentSSHButton } from "./SSHButton/SSHButton"; diff --git a/site/src/modules/resources/AgentRow.test.tsx b/site/src/modules/resources/AgentRow.test.tsx index 55b14704ad7a6..3af0575890320 100644 --- a/site/src/modules/resources/AgentRow.test.tsx +++ b/site/src/modules/resources/AgentRow.test.tsx @@ -1,5 +1,5 @@ import { MockWorkspaceApp } from "testHelpers/entities"; -import { organizeAgentApps } from "./AgentRow"; +import { organizeAgentApps } from "./AgentApps/AgentApps"; describe("organizeAgentApps", () => { test("returns one ungrouped app", () => { diff --git a/site/src/modules/resources/AgentRow.tsx b/site/src/modules/resources/AgentRow.tsx index b374bdb648de6..19c820b436aff 100644 --- a/site/src/modules/resources/AgentRow.tsx +++ b/site/src/modules/resources/AgentRow.tsx @@ -8,20 +8,12 @@ import type { Workspace, WorkspaceAgent, WorkspaceAgentMetadata, - WorkspaceApp, } from "api/typesGenerated"; import { isAxiosError } from "axios"; import { Button } from "components/Button/Button"; import { DropdownArrow } from "components/DropdownArrow/DropdownArrow"; -import { - DropdownMenu, - DropdownMenuContent, - DropdownMenuItem, - DropdownMenuTrigger, -} from "components/DropdownMenu/DropdownMenu"; import { Stack } from "components/Stack/Stack"; import { useProxy } from "contexts/ProxyContext"; -import { Folder } from "lucide-react"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; import { @@ -36,7 +28,7 @@ import { import { useQuery } from "react-query"; import AutoSizer from "react-virtualized-auto-sizer"; import type { FixedSizeList as List, ListOnScrollProps } from "react-window"; -import { AgentButton } from "./AgentButton"; +import { Apps, organizeAgentApps } from "./AgentApps/AgentApps"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; import { AgentLatency } from "./AgentLatency"; import { AGENT_LOG_LINE_HEIGHT } from "./AgentLogs/AgentLogLine"; @@ -44,7 +36,6 @@ import { AgentLogs } from "./AgentLogs/AgentLogs"; import { AgentMetadata } from "./AgentMetadata"; import { AgentStatus } from "./AgentStatus"; import { AgentVersion } from "./AgentVersion"; -import { AppLink } from "./AppLink/AppLink"; import { DownloadAgentLogsButton } from "./DownloadAgentLogsButton"; import { PortForwardButton } from "./PortForwardButton"; import { AgentSSHButton } from "./SSHButton/SSHButton"; @@ -354,93 +345,6 @@ export const AgentRow: FC = ({ ); }; -export type AppSection = { - /** - * If there is no `group`, just render all of the apps inline. If there is a - * group name, show them all in a dropdown. - */ - group?: string; - - apps: WorkspaceApp[]; -}; - -/** - * organizeAgentApps returns an ordering of agent apps that accounts for - * grouping. When we receive the list of apps from the backend, they have - * already been "ordered" by their `order` attribute, but we are not given that - * value. We must be careful to preserve that ordering, while also properly - * grouping together all apps of any given group. - * - * The position of the group overall is determined by the `order` position of - * the first app in the group. There may be several sections returned without - * a group name, to allow placing grouped apps in between non-grouped apps. Not - * every ungrouped section is expected to have a group in between, to make the - * algorithm a little simpler to implement. - */ -export function organizeAgentApps(apps: readonly WorkspaceApp[]): AppSection[] { - let currentSection: AppSection | undefined = undefined; - const appGroups: AppSection[] = []; - const groupsByName = new Map(); - - for (const app of apps) { - if (app.hidden) { - continue; - } - - if (!currentSection || app.group !== currentSection.group) { - const existingSection = groupsByName.get(app.group!); - if (existingSection) { - currentSection = existingSection; - } else { - currentSection = { - group: app.group, - apps: [], - }; - appGroups.push(currentSection); - if (app.group) { - groupsByName.set(app.group, currentSection); - } - } - } - - currentSection.apps.push(app); - } - - return appGroups; -} - -export type AppsProps = { - section: AppSection; - agent: WorkspaceAgent; - workspace: Workspace; -}; - -export const Apps: FC = ({ section, agent, workspace }) => { - return section.group ? ( - - - - - {section.group} - - - - {section.apps.map((app) => ( - - - - ))} - - - ) : ( - <> - {section.apps.map((app) => ( - - ))} - - ); -}; - const styles = { agentRow: (theme) => ({ fontSize: 14, From ecfe483f74856e922447db5cf16cc09da85b1740 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 18:42:57 +0000 Subject: [PATCH 13/29] site: add tests --- .../AgentDevcontainerCard.stories.tsx | 59 ++++++++++++++++++- .../pages/WorkspacePage/Workspace.stories.tsx | 2 +- site/src/testHelpers/entities.ts | 35 ++--------- 3 files changed, 63 insertions(+), 33 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index 9103e4246a70f..ccdf47c15e980 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,10 +1,13 @@ import type { Meta, StoryObj } from "@storybook/react"; import type { WorkspaceAgentDevcontainer } from "api/typesGenerated"; import { + MockTemplate, MockWorkspace, MockWorkspaceAgent, MockWorkspaceAgentContainer, MockWorkspaceAgentContainerPorts, + MockWorkspaceApp, + MockWorkspaceSubAgent, } from "testHelpers/entities"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; @@ -17,9 +20,9 @@ const MockWorkspaceAgentDevcontainer: WorkspaceAgentDevcontainer = { dirty: false, container: MockWorkspaceAgentContainer, agent: { - id: "test-agent-id", - name: "test-devcontainer-agent", - directory: "/workspace/test", + id: MockWorkspaceSubAgent.id, + name: MockWorkspaceSubAgent.name, + directory: MockWorkspaceSubAgent?.directory ?? "/workspace/test", }, }; @@ -31,6 +34,8 @@ const meta: Meta = { workspace: MockWorkspace, wildcardHostname: "*.wildcard.hostname", parentAgent: MockWorkspaceAgent, + template: MockTemplate, + subAgents: [MockWorkspaceSubAgent], }, }; @@ -48,6 +53,7 @@ export const WithPorts: Story = { ports: MockWorkspaceAgentContainerPorts, }, }, + subAgents: [MockWorkspaceSubAgent], }, }; @@ -61,6 +67,7 @@ export const Dirty: Story = { ports: MockWorkspaceAgentContainerPorts, }, }, + subAgents: [MockWorkspaceSubAgent], }, }; @@ -75,5 +82,51 @@ export const Recreating: Story = { ports: MockWorkspaceAgentContainerPorts, }, }, + subAgents: [], + }, +}; + +export const NoSubAgent: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + agent: undefined, + }, + subAgents: [], + }, +}; + +export const SubAgentConnecting: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + container: { + ...MockWorkspaceAgentContainer, + }, + }, + subAgents: [ + { + ...MockWorkspaceSubAgent, + status: "connecting", + }, + ], + }, +}; + +export const WithAppsAndPorts: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + container: { + ...MockWorkspaceAgentContainer, + ports: MockWorkspaceAgentContainerPorts, + }, + }, + subAgents: [ + { + ...MockWorkspaceSubAgent, + apps: [MockWorkspaceApp], + }, + ], }, }; diff --git a/site/src/pages/WorkspacePage/Workspace.stories.tsx b/site/src/pages/WorkspacePage/Workspace.stories.tsx index 978a58e8cb0e1..4fb197e6b5146 100644 --- a/site/src/pages/WorkspacePage/Workspace.stories.tsx +++ b/site/src/pages/WorkspacePage/Workspace.stories.tsx @@ -97,7 +97,7 @@ export const RunningWithChildAgent: Story = { lifecycle_state: "ready", }, { - ...Mocks.MockWorkspaceChildAgent, + ...Mocks.MockWorkspaceSubAgent, lifecycle_state: "ready", }, ], diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index c5a6823ff3828..491c43d50906b 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -970,38 +970,15 @@ export const MockWorkspaceAgent: TypesGen.WorkspaceAgent = { ], }; -export const MockWorkspaceChildAgent: TypesGen.WorkspaceAgent = { +export const MockWorkspaceSubAgent: TypesGen.WorkspaceAgent = { + ...MockWorkspaceAgent, apps: [], - architecture: "amd64", - created_at: "", - environment_variables: {}, - id: "test-workspace-child-agent", + id: "test-workspace-sub-agent", parent_id: "test-workspace-agent", - name: "a-workspace-child-agent", - operating_system: "linux", - resource_id: "", - status: "connected", - updated_at: "", - version: MockBuildInfo.version, - api_version: MockBuildInfo.agent_api_version, - latency: { - "Coder Embedded DERP": { - latency_ms: 32.55, - preferred: true, - }, - }, - connection_timeout_seconds: 120, - troubleshooting_url: "https://coder.com/troubleshoot", - lifecycle_state: "starting", - logs_length: 0, - logs_overflowed: false, - log_sources: [MockWorkspaceAgentLogSource], + name: "a-workspace-sub-agent", + log_sources: [], scripts: [], - startup_script_behavior: "non-blocking", - subsystems: ["envbox", "exectrace"], - health: { - healthy: true, - }, + directory: "/workspace/test", display_apps: [ "ssh_helper", "port_forwarding_helper", From 71c61b6b373fbe7ca9fdebed197902b3452a8514 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 19:12:47 +0000 Subject: [PATCH 14/29] whelp --- .../AgentDevcontainerCard.stories.tsx | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index ccdf47c15e980..740107008ba49 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,6 +1,8 @@ import type { Meta, StoryObj } from "@storybook/react"; import type { WorkspaceAgentDevcontainer } from "api/typesGenerated"; +import { chromatic } from "testHelpers/chromatic"; import { + MockListeningPortsResponse, MockTemplate, MockWorkspace, MockWorkspaceAgent, @@ -9,6 +11,10 @@ import { MockWorkspaceApp, MockWorkspaceSubAgent, } from "testHelpers/entities"; +import { + withDashboardProvider, + withProxyProvider, +} from "testHelpers/storybook"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; const MockWorkspaceAgentDevcontainer: WorkspaceAgentDevcontainer = { @@ -37,6 +43,16 @@ const meta: Meta = { template: MockTemplate, subAgents: [MockWorkspaceSubAgent], }, + decorators: [withProxyProvider(), withDashboardProvider], + parameters: { + chromatic, + queries: [ + { + key: ["portForward", MockWorkspaceSubAgent.id], + data: MockListeningPortsResponse, + }, + ], + }, }; export default meta; @@ -53,7 +69,6 @@ export const WithPorts: Story = { ports: MockWorkspaceAgentContainerPorts, }, }, - subAgents: [MockWorkspaceSubAgent], }, }; @@ -67,7 +82,6 @@ export const Dirty: Story = { ports: MockWorkspaceAgentContainerPorts, }, }, - subAgents: [MockWorkspaceSubAgent], }, }; From 2e1c31fb634bef6499225f5644c810691d20761b Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 19:33:44 +0000 Subject: [PATCH 15/29] tweak app display --- site/src/modules/resources/AgentDevcontainerCard.tsx | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index a0eef87e094d8..77a36c9b10948 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -86,9 +86,7 @@ export const AgentDevcontainerCard: FC = ({ const showDevcontainerControls = !subAgentRemoved && subAgent && devcontainer.container; const showSubAgentApps = - !subAgentRemoved && - ((subAgent?.status === "connected" && hasAppsToDisplay) || - subAgent?.status === "connecting"); + !subAgentRemoved && subAgent?.status === "connected" && hasAppsToDisplay; const showSubAgentAppsPlaceholders = subAgentRemoved || subAgent?.status === "connecting"; From 7e41c152487c1200a5f27da6e2dd4bf3b5b8c21e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 19:36:03 +0000 Subject: [PATCH 16/29] add port forward test --- .../resources/AgentDevcontainerCard.stories.tsx | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index 740107008ba49..e4c01c68ec213 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,14 +1,17 @@ import type { Meta, StoryObj } from "@storybook/react"; import type { WorkspaceAgentDevcontainer } from "api/typesGenerated"; +import { getPreferredProxy } from "contexts/ProxyContext"; import { chromatic } from "testHelpers/chromatic"; import { MockListeningPortsResponse, + MockPrimaryWorkspaceProxy, MockTemplate, MockWorkspace, MockWorkspaceAgent, MockWorkspaceAgentContainer, MockWorkspaceAgentContainerPorts, MockWorkspaceApp, + MockWorkspaceProxies, MockWorkspaceSubAgent, } from "testHelpers/entities"; import { @@ -144,3 +147,12 @@ export const WithAppsAndPorts: Story = { ], }, }; + +export const ShowingPortForward: Story = { + decorators: [ + withProxyProvider({ + proxy: getPreferredProxy(MockWorkspaceProxies, MockPrimaryWorkspaceProxy), + proxies: MockWorkspaceProxies, + }), + ], +}; From 79e1844d25c97b3e51e76fe585a7dce8ecf7fa14 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 13 Jun 2025 20:06:03 +0000 Subject: [PATCH 17/29] cleanup --- .../AgentDevcontainerCard.stories.tsx | 34 ++----------------- .../modules/resources/AgentRow.stories.tsx | 12 +------ site/src/testHelpers/entities.ts | 16 +++++++++ 3 files changed, 20 insertions(+), 42 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index e4c01c68ec213..1f798b7540f79 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,5 +1,4 @@ import type { Meta, StoryObj } from "@storybook/react"; -import type { WorkspaceAgentDevcontainer } from "api/typesGenerated"; import { getPreferredProxy } from "contexts/ProxyContext"; import { chromatic } from "testHelpers/chromatic"; import { @@ -10,6 +9,7 @@ import { MockWorkspaceAgent, MockWorkspaceAgentContainer, MockWorkspaceAgentContainerPorts, + MockWorkspaceAgentDevcontainer, MockWorkspaceApp, MockWorkspaceProxies, MockWorkspaceSubAgent, @@ -20,21 +20,6 @@ import { } from "testHelpers/storybook"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; -const MockWorkspaceAgentDevcontainer: WorkspaceAgentDevcontainer = { - id: "test-devcontainer-id", - name: "test-devcontainer", - workspace_folder: "/workspace/test", - config_path: "/workspace/test/.devcontainer/devcontainer.json", - status: "running", - dirty: false, - container: MockWorkspaceAgentContainer, - agent: { - id: MockWorkspaceSubAgent.id, - name: MockWorkspaceSubAgent.name, - directory: MockWorkspaceSubAgent?.directory ?? "/workspace/test", - }, -}; - const meta: Meta = { title: "modules/resources/AgentDevcontainerCard", component: AgentDevcontainerCard, @@ -80,10 +65,6 @@ export const Dirty: Story = { devcontainer: { ...MockWorkspaceAgentDevcontainer, dirty: true, - container: { - ...MockWorkspaceAgentContainer, - ports: MockWorkspaceAgentContainerPorts, - }, }, }, }; @@ -94,10 +75,7 @@ export const Recreating: Story = { ...MockWorkspaceAgentDevcontainer, dirty: true, status: "starting", - container: { - ...MockWorkspaceAgentContainer, - ports: MockWorkspaceAgentContainerPorts, - }, + container: undefined, }, subAgents: [], }, @@ -115,12 +93,6 @@ export const NoSubAgent: Story = { export const SubAgentConnecting: Story = { args: { - devcontainer: { - ...MockWorkspaceAgentDevcontainer, - container: { - ...MockWorkspaceAgentContainer, - }, - }, subAgents: [ { ...MockWorkspaceSubAgent, @@ -148,7 +120,7 @@ export const WithAppsAndPorts: Story = { }, }; -export const ShowingPortForward: Story = { +export const WithPortForwarding: Story = { decorators: [ withProxyProvider({ proxy: getPreferredProxy(MockWorkspaceProxies, MockPrimaryWorkspaceProxy), diff --git a/site/src/modules/resources/AgentRow.stories.tsx b/site/src/modules/resources/AgentRow.stories.tsx index d6ab3dc45a96a..a5ad16ae9f97b 100644 --- a/site/src/modules/resources/AgentRow.stories.tsx +++ b/site/src/modules/resources/AgentRow.stories.tsx @@ -288,17 +288,7 @@ export const GroupApp: Story = { export const Devcontainer: Story = { beforeEach: () => { spyOn(API, "getAgentContainers").mockResolvedValue({ - devcontainers: [ - { - id: "test-devcontainer-id", - name: "test-devcontainer", - workspace_folder: "/workspace/test", - config_path: "/workspace/test/.devcontainer/devcontainer.json", - status: "running", - dirty: false, - container: M.MockWorkspaceAgentContainer, - }, - ], + devcontainers: [M.MockWorkspaceAgentDevcontainer], containers: [M.MockWorkspaceAgentContainer], }); }, diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index 491c43d50906b..5b391f359f2ad 100644 --- a/site/src/testHelpers/entities.ts +++ b/site/src/testHelpers/entities.ts @@ -4369,6 +4369,22 @@ export const MockWorkspaceAgentContainer: TypesGen.WorkspaceAgentContainer = { }, }; +export const MockWorkspaceAgentDevcontainer: TypesGen.WorkspaceAgentDevcontainer = + { + id: "test-devcontainer-id", + name: "test-devcontainer", + workspace_folder: "/workspace/test", + config_path: "/workspace/test/.devcontainer/devcontainer.json", + status: "running", + dirty: false, + container: MockWorkspaceAgentContainer, + agent: { + id: MockWorkspaceSubAgent.id, + name: MockWorkspaceSubAgent.name, + directory: MockWorkspaceSubAgent?.directory ?? "/workspace/test", + }, + }; + export const MockWorkspaceAppStatuses: TypesGen.WorkspaceAppStatus[] = [ { // This is the latest status chronologically (15:04:38) From 8ce0aeccd01b798bfde02b4f5f866a2a0d46237a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Jun 2025 08:01:12 +0000 Subject: [PATCH 18/29] dont show content (due to margin) when there are no apps --- .../resources/AgentDevcontainerCard.tsx | 179 +++++++++--------- 1 file changed, 91 insertions(+), 88 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 77a36c9b10948..a73410bd17620 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -220,99 +220,102 @@ export const AgentDevcontainerCard: FC = ({ -
- {subAgent && workspace.latest_app_status?.agent_id === subAgent.id && ( -
-

App statuses

- -
- )} - - {showSubAgentApps && ( -
- <> - {showVSCode && ( - + {subAgent && + workspace.latest_app_status?.agent_id === subAgent.id && ( +
+

App statuses

+ +
+ )} + + {showSubAgentApps && ( +
+ <> + {showVSCode && ( + + )} + {appSections.map((section, i) => ( + + ))} + + + {displayApps.includes("web_terminal") && ( + )} - {appSections.map((section, i) => ( - - ))} - - {displayApps.includes("web_terminal") && ( - - )} + {wildcardHostname !== "" && + devcontainer.container?.ports.map((port) => { + const portLabel = `${port.port}/${port.network.toUpperCase()}`; + const hasHostBind = + port.host_port !== undefined && port.host_ip !== undefined; + const helperText = hasHostBind + ? `${port.host_ip}:${port.host_port}` + : "Not bound to host"; + const linkDest = hasHostBind + ? portForwardURL( + wildcardHostname, + port.host_port, + subAgent.name, + workspace.name, + workspace.owner_name, + location.protocol === "https" ? "https" : "http", + ) + : ""; + return ( + + + + + + + {portLabel} + + + + {helperText} + + + ); + })} +
+ )} - {wildcardHostname !== "" && - devcontainer.container?.ports.map((port) => { - const portLabel = `${port.port}/${port.network.toUpperCase()}`; - const hasHostBind = - port.host_port !== undefined && port.host_ip !== undefined; - const helperText = hasHostBind - ? `${port.host_ip}:${port.host_port}` - : "Not bound to host"; - const linkDest = hasHostBind - ? portForwardURL( - wildcardHostname, - port.host_port, - subAgent.name, - workspace.name, - workspace.owner_name, - location.protocol === "https" ? "https" : "http", - ) - : ""; - return ( - - - - - - - {portLabel} - - - - {helperText} - - - ); - })} -
- )} - - {showSubAgentAppsPlaceholders && ( -
- - -
- )} -
+ {showSubAgentAppsPlaceholders && ( +
+ + +
+ )} + + )} ); }; From d3bda050528b63e7c1baffff74a2760e34190f12 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Jun 2025 08:23:02 +0000 Subject: [PATCH 19/29] rewrite styles as Tailwind CSS --- .../resources/AgentDevcontainerCard.tsx | 139 ++++-------------- 1 file changed, 26 insertions(+), 113 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index a73410bd17620..cd9b43d22193e 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -1,4 +1,3 @@ -import type { Interpolation, Theme } from "@emotion/react"; import Skeleton from "@mui/material/Skeleton"; import type { Template, @@ -142,26 +141,38 @@ export const AgentDevcontainerCard: FC = ({ } }, [devcontainer]); + const appsClasses = "flex flex-wrap gap-4 empty:hidden md:justify-start"; + return (
- + dev container
-
-
-
+
+
+
- + {subAgent?.name ?? devcontainer.name} {!isRebuilding && devcontainer.container && ( @@ -221,7 +232,7 @@ export const AgentDevcontainerCard: FC = ({
{(showSubAgentApps || showSubAgentAppsPlaceholders) && ( -
+
{subAgent && workspace.latest_app_status?.agent_id === subAgent.id && (
@@ -231,7 +242,7 @@ export const AgentDevcontainerCard: FC = ({ )} {showSubAgentApps && ( -
+
<> {showVSCode && ( = ({ )} {showSubAgentAppsPlaceholders && ( -
+
)} @@ -319,101 +330,3 @@ export const AgentDevcontainerCard: FC = ({ ); }; - -const styles = { - devContainerLabel: (theme) => ({ - backgroundColor: theme.palette.background.default, - fontSize: 12, - lineHeight: 1, - padding: "4px 8px", - position: "absolute", - top: -11, - left: 20, - }), - devContainerIcon: { - marginRight: 5, - }, - - devcontainerRow: { - padding: "16px 0px", - }, - - // Many of these styles are borrowed or mimic those from AgentRow.tsx. - header: (theme) => ({ - padding: "0px 16px 0px 32px", - display: "flex", - gap: 24, - alignItems: "center", - justifyContent: "space-between", - flexWrap: "wrap", - lineHeight: "1.5", - - "&:has(+ [role='alert'])": { - paddingBottom: 16, - }, - - [theme.breakpoints.down("md")]: { - gap: 16, - }, - }), - - agentInfo: (theme) => ({ - display: "flex", - alignItems: "center", - gap: 24, - color: theme.palette.text.secondary, - fontSize: 12, - }), - - content: { - padding: "16px 32px 0px 32px", - display: "flex", - flexDirection: "column", - gap: 32, - }, - - apps: (theme) => ({ - display: "flex", - gap: 16, - flexWrap: "wrap", - - "&:empty": { - display: "none", - }, - - [theme.breakpoints.down("md")]: { - marginLeft: 0, - justifyContent: "flex-start", - }, - }), - - agentNameAndStatus: (theme) => ({ - display: "flex", - alignItems: "center", - gap: 16, - - [theme.breakpoints.down("md")]: { - width: "100%", - }, - }), - - agentName: (theme) => ({ - whiteSpace: "nowrap", - overflow: "hidden", - textOverflow: "ellipsis", - maxWidth: 260, - fontWeight: 600, - flexShrink: 0, - width: "fit-content", - fontSize: 14, - color: theme.palette.text.primary, - - [theme.breakpoints.down("md")]: { - overflow: "unset", - }, - }), - - buttonSkeleton: { - borderRadius: 4, - }, -} satisfies Record>; From d4f208b2f2d4deebb0f492d9321d3564c35edcd5 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Jun 2025 08:40:32 +0000 Subject: [PATCH 20/29] review comments --- site/src/modules/resources/SubAgentOutdatedTooltip.tsx | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/site/src/modules/resources/SubAgentOutdatedTooltip.tsx b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx index ab16c227e1b73..c32b4c30c863b 100644 --- a/site/src/modules/resources/SubAgentOutdatedTooltip.tsx +++ b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx @@ -1,4 +1,3 @@ -import { useTheme } from "@emotion/react"; import type { WorkspaceAgent, WorkspaceAgentDevcontainer, @@ -34,11 +33,6 @@ export const SubAgentOutdatedTooltip: FC = ({ return null; } - const theme = useTheme(); - const versionLabelStyles = { - fontWeight: 600, - color: theme.palette.text.primary, - }; const title = "Dev Container Outdated"; const opener = "This Dev Container is outdated."; const text = `${opener} This can happen if you modify your devcontainer.json file after the Dev Container has been created. To fix this, you can rebuild the Dev Container.`; @@ -46,7 +40,7 @@ export const SubAgentOutdatedTooltip: FC = ({ return ( - + Outdated From 7e5ede0a9d999c8c89e40264a788a8f0e53520b5 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Jun 2025 09:36:50 +0000 Subject: [PATCH 21/29] review comments --- agent/agentcontainers/api.go | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index daa55ab70bc7e..ca948f73da7b6 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -1100,15 +1100,13 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c directory = DevcontainerDefaultContainerWorkspaceFolder } - if proc.agent.ID != uuid.Nil { - if recreateSubAgent { - logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID)) - err = api.subAgentClient.Delete(ctx, proc.agent.ID) - if err != nil { - return xerrors.Errorf("delete existing subagent failed: %w", err) - } - proc.agent = SubAgent{} + if proc.agent.ID != uuid.Nil && recreateSubAgent { + logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID)) + err = api.subAgentClient.Delete(ctx, proc.agent.ID) + if err != nil { + return xerrors.Errorf("delete existing subagent failed: %w", err) } + proc.agent = SubAgent{} } if proc.agent.ID == uuid.Nil { displayAppsMap := map[codersdk.DisplayApp]bool{ From 7a3674aa1bbd74fc9a1287ed131fbade43d270e7 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Jun 2025 09:40:52 +0000 Subject: [PATCH 22/29] add comment about idempotency --- agent/agentcontainers/api.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index ca948f73da7b6..416b0f689a0ba 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -944,7 +944,8 @@ func (api *API) cleanupSubAgents(ctx context.Context) error { // maybeInjectSubAgentIntoContainerLocked injects a subagent into a dev // container and starts the subagent process. This method assumes that -// api.mu is held. +// api.mu is held. This method is idempotent and will not re-inject the +// subagent if it is already/still running in the container. // // This method uses an internal timeout to prevent blocking indefinitely // if something goes wrong with the injection. From c0607b1a03385417dfd10b1e3d0dd691c058b83d Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Jun 2025 13:33:36 +0000 Subject: [PATCH 23/29] switch -> if --- site/src/modules/resources/AgentDevcontainerCard.tsx | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index cd9b43d22193e..fe488eeac85bc 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -67,12 +67,10 @@ export const AgentDevcontainerCard: FC = ({ const appSections = (subAgent && organizeAgentApps(subAgent.apps)) || []; const displayApps = subAgent?.display_apps.filter((app) => { - switch (true) { - case browser_only: - return ["web_terminal", "port_forwarding_helper"].includes(app); - default: - return true; + if (browser_only) { + return ["web_terminal", "port_forwarding_helper"].includes(app); } + return true; }) || []; const showVSCode = devcontainer.container && From e1ea4bf0a33e5ef7cca57f2d1251193c8a474f25 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Jun 2025 13:39:03 +0000 Subject: [PATCH 24/29] rename apps to agentapps --- .../modules/resources/AgentApps/AgentApps.tsx | 22 ++++++++++++------- .../resources/AgentDevcontainerCard.tsx | 4 ++-- site/src/modules/resources/AgentRow.tsx | 4 ++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/site/src/modules/resources/AgentApps/AgentApps.tsx b/site/src/modules/resources/AgentApps/AgentApps.tsx index a59444419d704..75793ef7a82c7 100644 --- a/site/src/modules/resources/AgentApps/AgentApps.tsx +++ b/site/src/modules/resources/AgentApps/AgentApps.tsx @@ -11,13 +11,17 @@ import type { FC } from "react"; import { AgentButton } from "../AgentButton"; import { AppLink } from "../AppLink/AppLink"; -type AppsProps = { - section: AppSection; +type AgentAppsProps = { + section: AgentAppSection; agent: WorkspaceAgent; workspace: Workspace; }; -export const Apps: FC = ({ section, agent, workspace }) => { +export const AgentApps: FC = ({ + section, + agent, + workspace, +}) => { return section.group ? ( @@ -43,7 +47,7 @@ export const Apps: FC = ({ section, agent, workspace }) => { ); }; -type AppSection = { +type AgentAppSection = { /** * If there is no `group`, just render all of the apps inline. If there is a * group name, show them all in a dropdown. @@ -61,10 +65,12 @@ type AppSection = { * every ungrouped section is expected to have a group in between, to make the * algorithm a little simpler to implement. */ -export function organizeAgentApps(apps: readonly WorkspaceApp[]): AppSection[] { - let currentSection: AppSection | undefined = undefined; - const appGroups: AppSection[] = []; - const groupsByName = new Map(); +export function organizeAgentApps( + apps: readonly WorkspaceApp[], +): AgentAppSection[] { + let currentSection: AgentAppSection | undefined = undefined; + const appGroups: AgentAppSection[] = []; + const groupsByName = new Map(); for (const app of apps) { if (app.hidden) { diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index fe488eeac85bc..9385987b171df 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -22,7 +22,7 @@ import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; import type { FC } from "react"; import { useEffect, useState } from "react"; import { portForwardURL } from "utils/portForward"; -import { Apps, organizeAgentApps } from "./AgentApps/AgentApps"; +import { AgentApps, organizeAgentApps } from "./AgentApps/AgentApps"; import { AgentButton } from "./AgentButton"; import { AgentLatency } from "./AgentLatency"; import { SubAgentStatus } from "./AgentStatus"; @@ -253,7 +253,7 @@ export const AgentDevcontainerCard: FC = ({ /> )} {appSections.map((section, i) => ( - = ({ /> )} {appSections.map((section, i) => ( - Date: Mon, 16 Jun 2025 15:39:39 +0000 Subject: [PATCH 25/29] refactor to use mutation --- agent/agentcontainers/api.go | 7 +- .../resources/AgentDevcontainerCard.tsx | 148 +++++++++++------- 2 files changed, 95 insertions(+), 60 deletions(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 416b0f689a0ba..bb7d29be1895c 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -663,11 +663,7 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse, for _, dc := range api.knownDevcontainers { // Include the agent if it's been created (we're iterating over // copies, so mutating is fine). - // - // NOTE(mafredri): We could filter on "proc.containerID == dc.Container.ID" - // here but not doing so allows us to do some tricks in the UI to - // make the experience more responsive for now. - if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil { + if proc := api.injectedSubAgentProcs[dc.WorkspaceFolder]; proc.agent.ID != uuid.Nil && dc.Container != nil && proc.containerID == dc.Container.ID { dc.Agent = &codersdk.WorkspaceAgentDevcontainerAgent{ ID: proc.agent.ID, Name: proc.agent.Name, @@ -762,6 +758,7 @@ func (api *API) handleDevcontainerRecreate(w http.ResponseWriter, r *http.Reques // Update the status so that we don't try to recreate the // devcontainer multiple times in parallel. dc.Status = codersdk.WorkspaceAgentDevcontainerStatusStarting + dc.Container = nil api.knownDevcontainers[dc.WorkspaceFolder] = dc api.asyncWg.Add(1) go api.recreateDevcontainer(dc, configPath) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 9385987b171df..37badd05ae85d 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -4,6 +4,7 @@ import type { Workspace, WorkspaceAgent, WorkspaceAgentDevcontainer, + WorkspaceAgentListContainersResponse, } from "api/typesGenerated"; import { Button } from "components/Button/Button"; import { displayError } from "components/GlobalSnackbar/utils"; @@ -20,7 +21,8 @@ import { Container, ExternalLinkIcon } from "lucide-react"; import { useFeatureVisibility } from "modules/dashboard/useFeatureVisibility"; import { AppStatuses } from "pages/WorkspacePage/AppStatuses"; import type { FC } from "react"; -import { useEffect, useState } from "react"; +import { useEffect } from "react"; +import { useMutation, useQueryClient } from "react-query"; import { portForwardURL } from "utils/portForward"; import { AgentApps, organizeAgentApps } from "./AgentApps/AgentApps"; import { AgentButton } from "./AgentButton"; @@ -51,12 +53,7 @@ export const AgentDevcontainerCard: FC = ({ }) => { const { browser_only } = useFeatureVisibility(); const { proxy } = useProxy(); - - const [isRebuilding, setIsRebuilding] = useState(false); - - // Track sub agent removal state to improve UX. This will not be needed once - // the devcontainer and agent responses are aligned. - const [subAgentRemoved, setSubAgentRemoved] = useState(false); + const queryClient = useQueryClient(); // The sub agent comes from the workspace response whereas the devcontainer // comes from the agent containers endpoint. We need alignment between the @@ -80,64 +77,105 @@ export const AgentDevcontainerCard: FC = ({ showVSCode || appSections.some((it) => it.apps.length > 0); - const showDevcontainerControls = - !subAgentRemoved && subAgent && devcontainer.container; - const showSubAgentApps = - !subAgentRemoved && subAgent?.status === "connected" && hasAppsToDisplay; - const showSubAgentAppsPlaceholders = - subAgentRemoved || subAgent?.status === "connecting"; - - const handleRebuildDevcontainer = async () => { - setIsRebuilding(true); - setSubAgentRemoved(true); - let rebuildSucceeded = false; - try { + const rebuildDevcontainerMutation = useMutation({ + mutationFn: async () => { const response = await fetch( `/api/v2/workspaceagents/${parentAgent.id}/containers/devcontainers/container/${devcontainer.container?.id}/recreate`, - { - method: "POST", - }, + { method: "POST" }, ); if (!response.ok) { const errorData = await response.json().catch(() => ({})); throw new Error( - errorData.message || `Failed to recreate: ${response.statusText}`, + errorData.message || `Failed to rebuild: ${response.statusText}`, ); } - // If the request was accepted (e.g. 202), we mark it as succeeded. - // Once complete, the component will unmount, so the spinner will - // disappear with it. - if (response.status === 202) { - rebuildSucceeded = true; + return response; + }, + onMutate: async () => { + await queryClient.cancelQueries({ + queryKey: ["agents", parentAgent.id, "containers"], + }); + + // Snapshot the previous data for rollback in case of error. + const previousData = queryClient.getQueryData([ + "agents", + parentAgent.id, + "containers", + ]); + + // Optimistically update the devcontainer status to + // "starting" and zero the agent and container to mimic what + // the API does. + queryClient.setQueryData( + ["agents", parentAgent.id, "containers"], + (oldData?: WorkspaceAgentListContainersResponse) => { + if (!oldData?.devcontainers) return oldData; + return { + ...oldData, + devcontainers: oldData.devcontainers.map((dc) => { + if (dc.id === devcontainer.id) { + return { + ...dc, + agent: null, + container: null, + status: "starting", + }; + } + return dc; + }), + }; + }, + ); + + return { previousData }; + }, + onSuccess: async () => { + // Invalidate the containers query to refetch updated data. + await queryClient.invalidateQueries({ + queryKey: ["agents", parentAgent.id, "containers"], + }); + }, + onError: (error, _, context) => { + // If the mutation fails, use the context returned from + // onMutate to roll back. + if (context?.previousData) { + queryClient.setQueryData( + ["agents", parentAgent.id, "containers"], + context.previousData, + ); } - } catch (error) { const errorMessage = error instanceof Error ? error.message : "An unknown error occurred."; - displayError(`Failed to recreate devcontainer: ${errorMessage}`); - console.error("Failed to recreate devcontainer:", error); - } finally { - if (!rebuildSucceeded) { - setIsRebuilding(false); - } - } - }; + displayError(`Failed to rebuild devcontainer: ${errorMessage}`); + console.error("Failed to rebuild devcontainer:", error); + }, + }); + // Re-fetch containers when the subAgent changes to ensure data is + // in sync. + const latestSubAgentByName = subAgents.find( + (agent) => agent.name === devcontainer.name, + ); useEffect(() => { - if (subAgent?.id) { - setSubAgentRemoved(false); - } else { - setSubAgentRemoved(true); + if (!latestSubAgentByName) { + return; } - }, [subAgent?.id]); + queryClient.invalidateQueries({ + queryKey: ["agents", parentAgent.id, "containers"], + }); + }, [latestSubAgentByName, queryClient, parentAgent.id]); - // If the devcontainer is starting, reflect this in the recreate button. - useEffect(() => { - if (devcontainer.status === "starting") { - setIsRebuilding(true); - } else { - setIsRebuilding(false); - } - }, [devcontainer]); + const showDevcontainerControls = subAgent && devcontainer.container; + const showSubAgentApps = + devcontainer.status !== "starting" && + subAgent?.status === "connected" && + hasAppsToDisplay; + const showSubAgentAppsPlaceholders = + devcontainer.status === "starting" || subAgent?.status === "connecting"; + + const handleRebuildDevcontainer = () => { + rebuildDevcontainerMutation.mutate(); + }; const appsClasses = "flex flex-wrap gap-4 empty:hidden md:justify-start"; @@ -172,7 +210,7 @@ export const AgentDevcontainerCard: FC = ({ md:overflow-visible" > {subAgent?.name ?? devcontainer.name} - {!isRebuilding && devcontainer.container && ( + {devcontainer.container && ( {" "} ({devcontainer.container.name}) @@ -180,7 +218,7 @@ export const AgentDevcontainerCard: FC = ({ )}
- {!subAgentRemoved && subAgent?.status === "connected" && ( + {subAgent?.status === "connected" && ( <> = ({ )} - {!subAgentRemoved && subAgent?.status === "connecting" && ( + {subAgent?.status === "connecting" && ( <> @@ -203,9 +241,9 @@ export const AgentDevcontainerCard: FC = ({ variant="outline" size="sm" onClick={handleRebuildDevcontainer} - disabled={isRebuilding} + disabled={devcontainer.status === "starting"} > - + Rebuild From 2b22ee23e335ac17862aabbca39bddb8923c3070 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 17 Jun 2025 07:32:52 +0000 Subject: [PATCH 26/29] adjust refetch trigger --- site/src/modules/resources/AgentDevcontainerCard.tsx | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 37badd05ae85d..2cd41f9ebf787 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -152,7 +152,9 @@ export const AgentDevcontainerCard: FC = ({ }); // Re-fetch containers when the subAgent changes to ensure data is - // in sync. + // in sync. This relies on agent updates being pushed to the client + // to trigger the re-fetch. That is why we match on name here + // instead of ID as we need to fetch to get an up-to-date ID. const latestSubAgentByName = subAgents.find( (agent) => agent.name === devcontainer.name, ); @@ -163,7 +165,12 @@ export const AgentDevcontainerCard: FC = ({ queryClient.invalidateQueries({ queryKey: ["agents", parentAgent.id, "containers"], }); - }, [latestSubAgentByName, queryClient, parentAgent.id]); + }, [ + latestSubAgentByName?.id, + latestSubAgentByName?.status, + queryClient, + parentAgent.id, + ]); const showDevcontainerControls = subAgent && devcontainer.container; const showSubAgentApps = From 4b9d2189082c04569e662710bc2db2ca780cc41e Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 17 Jun 2025 09:58:15 +0000 Subject: [PATCH 27/29] fix linter --- site/src/modules/resources/AgentDevcontainerCard.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 2cd41f9ebf787..9ba6e26c5d46a 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -159,7 +159,7 @@ export const AgentDevcontainerCard: FC = ({ (agent) => agent.name === devcontainer.name, ); useEffect(() => { - if (!latestSubAgentByName) { + if (!latestSubAgentByName?.id || !latestSubAgentByName?.status) { return; } queryClient.invalidateQueries({ From e021c8d6406f75d8ee9cb2f0a3b65b3d210dfb9a Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 17 Jun 2025 12:59:03 +0300 Subject: [PATCH 28/29] Update agent/agentcontainers/api.go Co-authored-by: Cian Johnston --- agent/agentcontainers/api.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index bb7d29be1895c..8ed8e586f3986 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -130,7 +130,7 @@ func WithDevcontainerCLI(dccli DevcontainerCLI) Option { } // WithSubAgentClient sets the SubAgentClient implementation to use. -// This is used to list, create and delete devcontainer agents. +// This is used to list, create, and delete devcontainer agents. func WithSubAgentClient(client SubAgentClient) Option { return func(api *API) { api.subAgentClient = client From 0c44de8084a34872be73b20e4f0172271709d4b4 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 17 Jun 2025 10:01:22 +0000 Subject: [PATCH 29/29] clarify todo --- agent/agentcontainers/api.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 8ed8e586f3986..71b5267f40fec 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -404,7 +404,8 @@ func (api *API) Routes() http.Handler { r.Use(ensureInitialUpdateDoneMW) r.Get("/", api.handleList) - // TODO(mafredri): Simplify this route. + // TODO(mafredri): Simplify this route as the previous /devcontainers + // /-route was dropped. We can drop the /devcontainers prefix here too. r.Route("/devcontainers", func(r chi.Router) { r.Post("/container/{container}/recreate", api.handleDevcontainerRecreate) })