From 97474bb28bbb066c5ac74e5a1f258d82027da180 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Tue, 17 Jun 2025 16:06:47 +0300 Subject: [PATCH] feat: support devcontainer agents in ui and unify backend (#18332) This commit consolidates two container endpoints on the backend and improves the frontend devcontainer support by showing names and displaying apps as appropriate. With this change, the frontend now has knowledge of the subagent and we can also display things like port forwards. The frontend was updated to show dev container labels on the border as well as subagent connection status. The recreation flow was also adjusted a bit to show placeholder app icons when relevant. Support for apps was also added, although these are still WIP on the backend. And the port forwarding utility was added in since the sub agents now provide the necessary info. Fixes coder/internal#666 --- agent/agentcontainers/api.go | 354 +++++++------- agent/agentcontainers/api_test.go | 196 +++++--- coderd/apidoc/docs.go | 69 ++- coderd/apidoc/swagger.json | 69 ++- coderd/workspaceagents_test.go | 16 +- codersdk/workspaceagents.go | 25 +- docs/reference/api/agents.md | 41 +- docs/reference/api/schemas.md | 153 +++++- site/src/api/typesGenerated.ts | 16 +- .../modules/resources/AgentApps/AgentApps.tsx | 100 ++++ .../AgentDevcontainerCard.stories.tsx | 104 ++++- .../resources/AgentDevcontainerCard.tsx | 442 +++++++++++++----- .../modules/resources/AgentRow.stories.tsx | 1 + site/src/modules/resources/AgentRow.test.tsx | 2 +- site/src/modules/resources/AgentRow.tsx | 129 +---- site/src/modules/resources/AgentStatus.tsx | 26 ++ .../modules/resources/SSHButton/SSHButton.tsx | 60 +-- .../resources/SubAgentOutdatedTooltip.tsx | 67 +++ .../VSCodeDevContainerButton.tsx | 4 +- .../pages/WorkspacePage/Workspace.stories.tsx | 2 +- site/src/pages/WorkspacePage/Workspace.tsx | 3 + site/src/testHelpers/entities.ts | 52 +-- 22 files changed, 1265 insertions(+), 666 deletions(-) create mode 100644 site/src/modules/resources/AgentApps/AgentApps.tsx create mode 100644 site/src/modules/resources/SubAgentOutdatedTooltip.tsx diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index 1dddcc7098..71b5267f40 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" ) @@ -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. @@ -129,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 @@ -403,8 +404,9 @@ func (api *API) Routes() http.Handler { r.Use(ensureInitialUpdateDoneMW) r.Get("/", api.handleList) + // 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.Get("/", api.handleDevcontainersList) r.Post("/container/{container}/recreate", api.handleDevcontainerRecreate) }) @@ -486,8 +488,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] @@ -513,10 +513,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 { @@ -564,12 +564,10 @@ 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) } - dc.Container.DevcontainerStatus = dc.Status - dc.Container.DevcontainerDirty = dc.Dirty } switch { @@ -584,16 +582,14 @@ 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) + if dc.Status == codersdk.WorkspaceAgentDevcontainerStatusRunning { + err := api.maybeInjectSubAgentIntoContainerLocked(ctx, dc) if err != nil { logger.Error(ctx, "inject subagent into container failed", slog.Error(err)) } @@ -661,9 +657,32 @@ func (api *API) getContainers() (codersdk.WorkspaceAgentListContainersResponse, if api.containersErr != nil { return codersdk.WorkspaceAgentListContainersResponse{}, api.containersErr } + + var devcontainers []codersdk.WorkspaceAgentDevcontainer + 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 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, + Directory: proc.agent.Directory, + } + } + + devcontainers = append(devcontainers, dc) + } + slices.SortFunc(devcontainers, func(a, b codersdk.WorkspaceAgentDevcontainer) int { + return strings.Compare(a.Name, b.Name) + }) + } + 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 +759,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 - if dc.Container != nil { - dc.Container.DevcontainerStatus = dc.Status - } + dc.Container = nil api.knownDevcontainers[dc.WorkspaceFolder] = dc api.asyncWg.Add(1) go api.recreateDevcontainer(dc, configPath) @@ -815,9 +832,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 +852,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") @@ -852,39 +865,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) { @@ -914,10 +894,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 } @@ -964,13 +940,14 @@ 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. +// 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. -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() @@ -979,17 +956,44 @@ func (api *API) injectSubAgentIntoContainerLocked(ctx context.Context, dc coders return xerrors.New("container is nil, cannot inject subagent") } - // Skip if subagent already exists for this container. - if _, injected := api.injectedSubAgentProcs[container.ID]; injected || api.closed { - return nil + 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), + ) + + // 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 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 @@ -1001,12 +1005,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 @@ -1014,13 +1019,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) @@ -1035,7 +1033,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() @@ -1095,59 +1094,91 @@ 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 && 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) + } } + + 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)) } - // 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) + 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 } - - logger.Info(ctx, "created subagent record", slog.F("agent_id", createdAgent.ID)) + // 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 @@ -1157,59 +1188,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() - delete(api.injectedSubAgentProcs, container.ID) - 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[container.ID] = subAgentProcess{ - agent: agent, - 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...), ) @@ -1229,14 +1227,38 @@ func (api *API) Close() error { api.logger.Debug(api.ctx, "closing API") 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)) + // 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 821117685b..92a697b6e2 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,20 +592,19 @@ 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") 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) @@ -626,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) @@ -637,7 +632,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 } @@ -649,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) @@ -662,7 +656,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 +750,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 +794,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 +835,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 +873,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) }, }, { @@ -995,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) @@ -1006,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") @@ -1081,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) @@ -1105,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) @@ -1171,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) @@ -1185,8 +1172,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, @@ -1207,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,8 +1205,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" @@ -1233,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) @@ -1246,8 +1229,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) { @@ -1289,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), @@ -1300,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), @@ -1308,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) @@ -1342,18 +1329,12 @@ 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...") - // Expect the agent to be reinjected. - 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), - mCCLI.EXPECT().Copy(gomock.Any(), "test-container-id", coderBin, "/.coder-agent/coder").Return(nil), - 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. + // 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 { @@ -1361,13 +1342,71 @@ func TestAPI(t *testing.T) { } return errTestTermination }) + <-terminated - // Allow cleanup to proceed. + 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), + mCCLI.EXPECT().Copy(gomock.Any(), "test-container-id", coderBin, "/.coder-agent/coder").Return(nil), + mCCLI.EXPECT().ExecAs(gomock.Any(), "test-container-id", "root", "chmod", "0755", "/.coder-agent", "/.coder-agent/coder").Return(nil, nil), + ) + + // 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), + ) + + // 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 + + // 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) @@ -1375,20 +1414,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/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index b6a35b1738..f2a7dd2dee 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -17501,18 +17501,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" @@ -17577,6 +17565,56 @@ const docTemplate = `{ } } }, + "codersdk.WorkspaceAgentDevcontainer": { + "type": "object", + "properties": { + "agent": { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerAgent" + }, + "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.WorkspaceAgentDevcontainerAgent": { + "type": "object", + "properties": { + "directory": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + } + } + }, "codersdk.WorkspaceAgentDevcontainerStatus": { "type": "string", "enum": [ @@ -17642,6 +17680,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 e789ffb059..74b5aad0af 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,56 @@ } } }, + "codersdk.WorkspaceAgentDevcontainer": { + "type": "object", + "properties": { + "agent": { + "$ref": "#/definitions/codersdk.WorkspaceAgentDevcontainerAgent" + }, + "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.WorkspaceAgentDevcontainerAgent": { + "type": "object", + "properties": { + "directory": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + } + } + }, "codersdk.WorkspaceAgentDevcontainerStatus": { "type": "string", "enum": ["running", "stopped", "starting", "error"], @@ -16127,6 +16165,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 ec0b692886..6d53bd3df1 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 6a4380fed4..5fe648ce15 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 @@ -422,6 +416,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 @@ -450,14 +453,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 +481,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 5ef747066b..1c0534ad4c 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,45 @@ 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", + "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 993334e9e9..dd6f162f83 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -9012,8 +9012,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": { @@ -9040,21 +9038,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 @@ -9076,6 +9072,79 @@ 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 +{ + "agent": { + "directory": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string" + }, + "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 | +|--------------------|----------------------------------------------------------------------------------------|----------|--------------|----------------------------| +| `agent` | [codersdk.WorkspaceAgentDevcontainerAgent](#codersdkworkspaceagentdevcontaineragent) | false | | | +| `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.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 @@ -9138,8 +9207,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": { @@ -9163,6 +9230,45 @@ 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", + "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" ] @@ -9171,10 +9277,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 234e841615..06acdfed6e 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 @@ -3375,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 @@ -3387,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; @@ -3424,6 +3425,7 @@ export const WorkspaceAgentLifecycles: WorkspaceAgentLifecycle[] = [ // From codersdk/workspaceagents.go export interface WorkspaceAgentListContainersResponse { + readonly devcontainers: readonly WorkspaceAgentDevcontainer[]; readonly containers: readonly WorkspaceAgentContainer[]; readonly warnings?: readonly string[]; } diff --git a/site/src/modules/resources/AgentApps/AgentApps.tsx b/site/src/modules/resources/AgentApps/AgentApps.tsx new file mode 100644 index 0000000000..75793ef7a8 --- /dev/null +++ b/site/src/modules/resources/AgentApps/AgentApps.tsx @@ -0,0 +1,100 @@ +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 AgentAppsProps = { + section: AgentAppSection; + agent: WorkspaceAgent; + workspace: Workspace; +}; + +export const AgentApps: FC = ({ + section, + agent, + workspace, +}) => { + return section.group ? ( + + + + + {section.group} + + + + {section.apps.map((app) => ( + + + + ))} + + + ) : ( + <> + {section.apps.map((app) => ( + + ))} + + ); +}; + +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. + */ + 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[], +): AgentAppSection[] { + let currentSection: AgentAppSection | undefined = undefined; + const appGroups: AgentAppSection[] = []; + 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.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index fdd85d95c4..1f798b7540 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -1,20 +1,45 @@ import type { Meta, StoryObj } from "@storybook/react"; +import { getPreferredProxy } from "contexts/ProxyContext"; +import { chromatic } from "testHelpers/chromatic"; import { + MockListeningPortsResponse, + MockPrimaryWorkspaceProxy, + MockTemplate, MockWorkspace, MockWorkspaceAgent, MockWorkspaceAgentContainer, MockWorkspaceAgentContainerPorts, + MockWorkspaceAgentDevcontainer, + MockWorkspaceApp, + MockWorkspaceProxies, + MockWorkspaceSubAgent, } from "testHelpers/entities"; +import { + withDashboardProvider, + withProxyProvider, +} from "testHelpers/storybook"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; const meta: Meta = { title: "modules/resources/AgentDevcontainerCard", component: AgentDevcontainerCard, args: { - container: MockWorkspaceAgentContainer, + devcontainer: MockWorkspaceAgentDevcontainer, workspace: MockWorkspace, wildcardHostname: "*.wildcard.hostname", - agent: MockWorkspaceAgent, + parentAgent: MockWorkspaceAgent, + template: MockTemplate, + subAgents: [MockWorkspaceSubAgent], + }, + decorators: [withProxyProvider(), withDashboardProvider], + parameters: { + chromatic, + queries: [ + { + key: ["portForward", MockWorkspaceSubAgent.id], + data: MockListeningPortsResponse, + }, + ], }, }; @@ -25,30 +50,81 @@ 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, }, }, }; export const Recreating: Story = { args: { - container: { - ...MockWorkspaceAgentContainer, - devcontainer_dirty: true, - devcontainer_status: "starting", - ports: MockWorkspaceAgentContainerPorts, + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + dirty: true, + status: "starting", + container: undefined, }, + subAgents: [], }, }; + +export const NoSubAgent: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + agent: undefined, + }, + subAgents: [], + }, +}; + +export const SubAgentConnecting: Story = { + args: { + subAgents: [ + { + ...MockWorkspaceSubAgent, + status: "connecting", + }, + ], + }, +}; + +export const WithAppsAndPorts: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + container: { + ...MockWorkspaceAgentContainer, + ports: MockWorkspaceAgentContainerPorts, + }, + }, + subAgents: [ + { + ...MockWorkspaceSubAgent, + apps: [MockWorkspaceApp], + }, + ], + }, +}; + +export const WithPortForwarding: Story = { + decorators: [ + withProxyProvider({ + proxy: getPreferredProxy(MockWorkspaceProxies, MockPrimaryWorkspaceProxy), + proxies: MockWorkspaceProxies, + }), + ], +}; diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index 65b32593c1..9ba6e26c5d 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -1,117 +1,245 @@ +import Skeleton from "@mui/material/Skeleton"; import type { + Template, Workspace, WorkspaceAgent, - WorkspaceAgentContainer, + WorkspaceAgentDevcontainer, + WorkspaceAgentListContainersResponse, } from "api/typesGenerated"; 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 { Stack } from "components/Stack/Stack"; import { Tooltip, TooltipContent, TooltipProvider, TooltipTrigger, } from "components/Tooltip/Tooltip"; -import { ExternalLinkIcon } 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 { useEffect } from "react"; +import { useMutation, useQueryClient } from "react-query"; import { portForwardURL } from "utils/portForward"; +import { AgentApps, organizeAgentApps } from "./AgentApps/AgentApps"; import { AgentButton } from "./AgentButton"; -import { AgentDevcontainerSSHButton } from "./SSHButton/SSHButton"; +import { AgentLatency } from "./AgentLatency"; +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"; type AgentDevcontainerCardProps = { - agent: WorkspaceAgent; - container: WorkspaceAgentContainer; + parentAgent: WorkspaceAgent; + subAgents: WorkspaceAgent[]; + devcontainer: WorkspaceAgentDevcontainer; workspace: Workspace; + template: Template; wildcardHostname: string; }; export const AgentDevcontainerCard: FC = ({ - agent, - container, + parentAgent, + subAgents, + devcontainer, workspace, + template, wildcardHostname, }) => { - const folderPath = container.labels["devcontainer.local_folder"]; - const containerFolder = container.volumes[folderPath]; - const [isRecreating, setIsRecreating] = useState(false); + const { browser_only } = useFeatureVisibility(); + const { proxy } = useProxy(); + const queryClient = useQueryClient(); - const handleRecreateDevcontainer = async () => { - setIsRecreating(true); - let recreateSucceeded = false; - try { + // 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 appSections = (subAgent && organizeAgentApps(subAgent.apps)) || []; + const displayApps = + subAgent?.display_apps.filter((app) => { + if (browser_only) { + return ["web_terminal", "port_forwarding_helper"].includes(app); + } + 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 rebuildDevcontainerMutation = useMutation({ + mutationFn: async () => { const response = await fetch( - `/api/v2/workspaceagents/${agent.id}/containers/devcontainers/container/${container.id}/recreate`, - { - method: "POST", - }, + `/api/v2/workspaceagents/${parentAgent.id}/containers/devcontainers/container/${devcontainer.container?.id}/recreate`, + { 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) { - recreateSucceeded = 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 (!recreateSucceeded) { - setIsRecreating(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. 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, + ); + useEffect(() => { + if (!latestSubAgentByName?.id || !latestSubAgentByName?.status) { + return; } + queryClient.invalidateQueries({ + queryKey: ["agents", parentAgent.id, "containers"], + }); + }, [ + latestSubAgentByName?.id, + latestSubAgentByName?.status, + queryClient, + parentAgent.id, + ]); + + 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(); }; - // If the container is starting, reflect this in the recreate button. - useEffect(() => { - if (container.devcontainer_status === "starting") { - setIsRecreating(true); - } else { - setIsRecreating(false); - } - }, [container.devcontainer_status]); + const appsClasses = "flex flex-wrap gap-4 empty:hidden md:justify-start"; return ( -
-
-
-

- dev container:{" "} - {container.name} -

- {container.devcontainer_dirty && ( - - - Outdated - - - Devcontainer Outdated - - Devcontainer configuration has been modified and is outdated. - Recreate to get an up-to-date container. - - - +
+ + dev container +
+
+
+
+ + + {subAgent?.name ?? devcontainer.name} + {devcontainer.container && ( + + {" "} + ({devcontainer.container.name}) + + )} + +
+ {subAgent?.status === "connected" && ( + <> + + + + )} + {subAgent?.status === "connecting" && ( + <> + + + )}
@@ -119,73 +247,129 @@ export const AgentDevcontainerCard: FC = ({ - + {showDevcontainerControls && displayApps.includes("ssh_helper") && ( + + )} + {showDevcontainerControls && + displayApps.includes("port_forwarding_helper") && + proxy.preferredWildcardHostname !== "" && ( + + )}
-

Forwarded ports

+ {(showSubAgentApps || showSubAgentAppsPlaceholders) && ( +
+ {subAgent && + workspace.latest_app_status?.agent_id === subAgent.id && ( +
+

App statuses

+ +
+ )} -
- + {showSubAgentApps && ( +
+ <> + {showVSCode && ( + + )} + {appSections.map((section, i) => ( + + ))} + - - {wildcardHostname !== "" && - 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} - - - ); - })} -
-
+ {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} + + + ); + })} + + )} + + {showSubAgentAppsPlaceholders && ( +
+ + +
+ )} + + )} + ); }; diff --git a/site/src/modules/resources/AgentRow.stories.tsx b/site/src/modules/resources/AgentRow.stories.tsx index afeb95d0d2..a5ad16ae9f 100644 --- a/site/src/modules/resources/AgentRow.stories.tsx +++ b/site/src/modules/resources/AgentRow.stories.tsx @@ -288,6 +288,7 @@ export const GroupApp: Story = { export const Devcontainer: Story = { beforeEach: () => { spyOn(API, "getAgentContainers").mockResolvedValue({ + devcontainers: [M.MockWorkspaceAgentDevcontainer], containers: [M.MockWorkspaceAgentContainer], }); }, diff --git a/site/src/modules/resources/AgentRow.test.tsx b/site/src/modules/resources/AgentRow.test.tsx index 55b14704ad..3af0575890 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 d7545ff5c8..54ffe229b2 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 { AgentApps, 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"; @@ -54,6 +45,7 @@ import { useAgentLogs } from "./useAgentLogs"; interface AgentRowProps { agent: WorkspaceAgent; + subAgents?: WorkspaceAgent[]; workspace: Workspace; template: Template; initialMetadata?: WorkspaceAgentMetadata[]; @@ -62,6 +54,7 @@ interface AgentRowProps { export const AgentRow: FC = ({ agent, + subAgents, workspace, template, onUpdateAgent, @@ -140,16 +133,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 +152,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 +188,7 @@ export const AgentRow: FC = ({
- {containers && containers.length > 0 && ( + {devcontainers && devcontainers.length > 0 && ( - - - - - Run the following commands to connect with SSH: - - -
    - - - -
- - - - Install Coder CLI - - - SSH configuration - - - - - ); -}; - interface SSHStepProps { helpText: string; codeExample: string; @@ -151,11 +101,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/modules/resources/SubAgentOutdatedTooltip.tsx b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx new file mode 100644 index 0000000000..c32b4c30c8 --- /dev/null +++ b/site/src/modules/resources/SubAgentOutdatedTooltip.tsx @@ -0,0 +1,67 @@ +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 { RotateCcwIcon } from "lucide-react"; +import type { FC } from "react"; + +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 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 + + + + + + ); +}; diff --git a/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx b/site/src/modules/resources/VSCodeDevContainerButton/VSCodeDevContainerButton.tsx index 42e0a5bd75..ffaef3e130 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 = ({ diff --git a/site/src/pages/WorkspacePage/Workspace.stories.tsx b/site/src/pages/WorkspacePage/Workspace.stories.tsx index 978a58e8cb..4fb197e6b5 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/pages/WorkspacePage/Workspace.tsx b/site/src/pages/WorkspacePage/Workspace.tsx index 65c924354c..5c032c04ef 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 diff --git a/site/src/testHelpers/entities.ts b/site/src/testHelpers/entities.ts index cbd0a8bd45..c73f009c77 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", @@ -4397,9 +4374,24 @@ export const MockWorkspaceAgentContainer: TypesGen.WorkspaceAgentContainer = { volumes: { "/mnt/volume1": "/volume1", }, - devcontainer_dirty: false, }; +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)