From 6ccd20d45fb9abcea18db69f8ffd110842176077 Mon Sep 17 00:00:00 2001 From: Danielle Maywood Date: Fri, 6 Feb 2026 15:52:54 +0000 Subject: [PATCH] feat(agent): populate subagent ID for terraform-defined devcontainers (#21942) Completes the final piece of the puzzle. Support the pre-creation flow from the agent side. --- agent/agentcontainers/acmock/acmock.go | 73 +++- agent/agentcontainers/acmock/doc.go | 2 +- agent/agentcontainers/api.go | 62 ++- agent/agentcontainers/api_test.go | 362 +++++++++++++++++- agent/agentcontainers/subagent.go | 12 +- agent/agentcontainers/subagent_test.go | 125 ++++++ coderd/apidoc/docs.go | 8 + coderd/apidoc/swagger.json | 8 + codersdk/agentsdk/convert.go | 15 + codersdk/agentsdk/convert_test.go | 1 + codersdk/workspaceagents.go | 16 +- codersdk/workspaceagents_test.go | 139 +++++++ docs/reference/api/agents.md | 8 + docs/reference/api/schemas.md | 9 + site/src/api/typesGenerated.ts | 1 + .../AgentDevcontainerCard.stories.tsx | 33 +- .../resources/AgentDevcontainerCard.tsx | 15 +- .../resources/SubAgentOutdatedTooltip.tsx | 17 +- 18 files changed, 869 insertions(+), 37 deletions(-) diff --git a/agent/agentcontainers/acmock/acmock.go b/agent/agentcontainers/acmock/acmock.go index af18e88045..05efa1ab12 100644 --- a/agent/agentcontainers/acmock/acmock.go +++ b/agent/agentcontainers/acmock/acmock.go @@ -1,9 +1,9 @@ // Code generated by MockGen. DO NOT EDIT. -// Source: .. (interfaces: ContainerCLI,DevcontainerCLI) +// Source: .. (interfaces: ContainerCLI,DevcontainerCLI,SubAgentClient) // // Generated by this command: // -// mockgen -destination ./acmock.go -package acmock .. ContainerCLI,DevcontainerCLI +// mockgen -destination ./acmock.go -package acmock .. ContainerCLI,DevcontainerCLI,SubAgentClient // // Package acmock is a generated GoMock package. @@ -15,6 +15,7 @@ import ( agentcontainers "github.com/coder/coder/v2/agent/agentcontainers" codersdk "github.com/coder/coder/v2/codersdk" + uuid "github.com/google/uuid" gomock "go.uber.org/mock/gomock" ) @@ -216,3 +217,71 @@ func (mr *MockDevcontainerCLIMockRecorder) Up(ctx, workspaceFolder, configPath a varargs := append([]any{ctx, workspaceFolder, configPath}, opts...) return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Up", reflect.TypeOf((*MockDevcontainerCLI)(nil).Up), varargs...) } + +// MockSubAgentClient is a mock of SubAgentClient interface. +type MockSubAgentClient struct { + ctrl *gomock.Controller + recorder *MockSubAgentClientMockRecorder + isgomock struct{} +} + +// MockSubAgentClientMockRecorder is the mock recorder for MockSubAgentClient. +type MockSubAgentClientMockRecorder struct { + mock *MockSubAgentClient +} + +// NewMockSubAgentClient creates a new mock instance. +func NewMockSubAgentClient(ctrl *gomock.Controller) *MockSubAgentClient { + mock := &MockSubAgentClient{ctrl: ctrl} + mock.recorder = &MockSubAgentClientMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockSubAgentClient) EXPECT() *MockSubAgentClientMockRecorder { + return m.recorder +} + +// Create mocks base method. +func (m *MockSubAgentClient) Create(ctx context.Context, agent agentcontainers.SubAgent) (agentcontainers.SubAgent, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Create", ctx, agent) + ret0, _ := ret[0].(agentcontainers.SubAgent) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Create indicates an expected call of Create. +func (mr *MockSubAgentClientMockRecorder) Create(ctx, agent any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Create", reflect.TypeOf((*MockSubAgentClient)(nil).Create), ctx, agent) +} + +// Delete mocks base method. +func (m *MockSubAgentClient) Delete(ctx context.Context, id uuid.UUID) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Delete", ctx, id) + ret0, _ := ret[0].(error) + return ret0 +} + +// Delete indicates an expected call of Delete. +func (mr *MockSubAgentClientMockRecorder) Delete(ctx, id any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Delete", reflect.TypeOf((*MockSubAgentClient)(nil).Delete), ctx, id) +} + +// List mocks base method. +func (m *MockSubAgentClient) List(ctx context.Context) ([]agentcontainers.SubAgent, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "List", ctx) + ret0, _ := ret[0].([]agentcontainers.SubAgent) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// List indicates an expected call of List. +func (mr *MockSubAgentClientMockRecorder) List(ctx any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "List", reflect.TypeOf((*MockSubAgentClient)(nil).List), ctx) +} diff --git a/agent/agentcontainers/acmock/doc.go b/agent/agentcontainers/acmock/doc.go index d0951fc848..08b5d32921 100644 --- a/agent/agentcontainers/acmock/doc.go +++ b/agent/agentcontainers/acmock/doc.go @@ -1,4 +1,4 @@ // Package acmock contains a mock implementation of agentcontainers.Lister for use in tests. package acmock -//go:generate mockgen -destination ./acmock.go -package acmock .. ContainerCLI,DevcontainerCLI +//go:generate mockgen -destination ./acmock.go -package acmock .. ContainerCLI,DevcontainerCLI,SubAgentClient diff --git a/agent/agentcontainers/api.go b/agent/agentcontainers/api.go index e69e3f2d99..e2d9dad7e4 100644 --- a/agent/agentcontainers/api.go +++ b/agent/agentcontainers/api.go @@ -562,12 +562,9 @@ func (api *API) discoverDevcontainersInProject(projectPath string) error { api.broadcastUpdatesLocked() if dc.Status == codersdk.WorkspaceAgentDevcontainerStatusStarting { - api.asyncWg.Add(1) - go func() { - defer api.asyncWg.Done() - + api.asyncWg.Go(func() { _ = api.CreateDevcontainer(dc.WorkspaceFolder, dc.ConfigPath) - }() + }) } } api.mu.Unlock() @@ -1627,16 +1624,25 @@ func (api *API) cleanupSubAgents(ctx context.Context) error { api.mu.Lock() defer api.mu.Unlock() - injected := make(map[uuid.UUID]bool, len(api.injectedSubAgentProcs)) + // Collect all subagent IDs that should be kept: + // 1. Subagents currently tracked by injectedSubAgentProcs + // 2. Subagents referenced by known devcontainers from the manifest + var keep []uuid.UUID for _, proc := range api.injectedSubAgentProcs { - injected[proc.agent.ID] = true + keep = append(keep, proc.agent.ID) + } + for _, dc := range api.knownDevcontainers { + if dc.SubagentID.Valid { + keep = append(keep, dc.SubagentID.UUID) + } } ctx, cancel := context.WithTimeout(ctx, defaultOperationTimeout) defer cancel() + var errs []error for _, agent := range agents { - if injected[agent.ID] { + if slices.Contains(keep, agent.ID) { continue } client := *api.subAgentClient.Load() @@ -1647,10 +1653,11 @@ func (api *API) cleanupSubAgents(ctx context.Context) error { slog.F("agent_id", agent.ID), slog.F("agent_name", agent.Name), ) + errs = append(errs, xerrors.Errorf("delete agent %s (%s): %w", agent.Name, agent.ID, err)) } } - return nil + return errors.Join(errs...) } // maybeInjectSubAgentIntoContainerLocked injects a subagent into a dev @@ -2001,7 +2008,20 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c // logger.Warn(ctx, "set CAP_NET_ADMIN on agent binary failed", slog.Error(err)) // } - deleteSubAgent := proc.agent.ID != uuid.Nil && maybeRecreateSubAgent && !proc.agent.EqualConfig(subAgentConfig) + // Only delete and recreate subagents that were dynamically created + // (ID == uuid.Nil). Terraform-defined subagents (subAgentConfig.ID != + // uuid.Nil) must not be deleted because they have attached resources + // managed by terraform. + isTerraformManaged := subAgentConfig.ID != uuid.Nil + configHasChanged := !proc.agent.EqualConfig(subAgentConfig) + + logger.Debug(ctx, "checking if sub agent should be deleted", + slog.F("is_terraform_managed", isTerraformManaged), + slog.F("maybe_recreate_sub_agent", maybeRecreateSubAgent), + slog.F("config_has_changed", configHasChanged), + ) + + deleteSubAgent := !isTerraformManaged && maybeRecreateSubAgent && configHasChanged if deleteSubAgent { logger.Debug(ctx, "deleting existing subagent for recreation", slog.F("agent_id", proc.agent.ID)) client := *api.subAgentClient.Load() @@ -2012,11 +2032,23 @@ func (api *API) maybeInjectSubAgentIntoContainerLocked(ctx context.Context, dc c proc.agent = SubAgent{} // Clear agent to signal that we need to create a new one. } - if proc.agent.ID == uuid.Nil { - logger.Debug(ctx, "creating new subagent", - slog.F("directory", subAgentConfig.Directory), - slog.F("display_apps", subAgentConfig.DisplayApps), - ) + // Re-create (upsert) terraform-managed subagents when the config + // changes so that display apps and other settings are updated + // without deleting the agent. + recreateTerraformSubAgent := isTerraformManaged && maybeRecreateSubAgent && configHasChanged + + if proc.agent.ID == uuid.Nil || recreateTerraformSubAgent { + if recreateTerraformSubAgent { + logger.Debug(ctx, "updating existing subagent", + slog.F("directory", subAgentConfig.Directory), + slog.F("display_apps", subAgentConfig.DisplayApps), + ) + } else { + logger.Debug(ctx, "creating new subagent", + slog.F("directory", subAgentConfig.Directory), + slog.F("display_apps", subAgentConfig.DisplayApps), + ) + } // Create new subagent record in the database to receive the auth token. // If we get a unique constraint violation, try with expanded names that diff --git a/agent/agentcontainers/api_test.go b/agent/agentcontainers/api_test.go index ee6775b5c5..faf91d06ae 100644 --- a/agent/agentcontainers/api_test.go +++ b/agent/agentcontainers/api_test.go @@ -437,7 +437,11 @@ func (m *fakeSubAgentClient) Create(ctx context.Context, agent agentcontainers.S } } - agent.ID = uuid.New() + // Only generate a new ID if one wasn't provided. Terraform-defined + // subagents have pre-existing IDs that should be preserved. + if agent.ID == uuid.Nil { + agent.ID = uuid.New() + } agent.AuthToken = uuid.New() if m.agents == nil { m.agents = make(map[uuid.UUID]agentcontainers.SubAgent) @@ -1035,6 +1039,30 @@ func TestAPI(t *testing.T) { wantStatus: []int{http.StatusAccepted, http.StatusConflict}, wantBody: []string{"Devcontainer recreation initiated", "is currently starting and cannot be restarted"}, }, + { + name: "Terraform-defined devcontainer can be rebuilt", + devcontainerID: devcontainerID1.String(), + setupDevcontainers: []codersdk.WorkspaceAgentDevcontainer{ + { + ID: devcontainerID1, + Name: "test-devcontainer-terraform", + WorkspaceFolder: workspaceFolder1, + ConfigPath: configPath1, + Status: codersdk.WorkspaceAgentDevcontainerStatusRunning, + Container: &devContainer1, + SubagentID: uuid.NullUUID{UUID: uuid.New(), Valid: true}, + }, + }, + lister: &fakeContainerCLI{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{devContainer1}, + }, + arch: "", + }, + devcontainerCLI: &fakeDevcontainerCLI{}, + wantStatus: []int{http.StatusAccepted, http.StatusConflict}, + wantBody: []string{"Devcontainer recreation initiated", "is currently starting and cannot be restarted"}, + }, } for _, tt := range tests { @@ -2490,6 +2518,338 @@ func TestAPI(t *testing.T) { assert.Empty(t, fakeSAC.agents) }) + t.Run("SubAgentCleanupPreservesTerraformDefined", func(t *testing.T) { + t.Parallel() + + var ( + // Given: A terraform-defined agent and devcontainer that should be preserved + terraformAgentID = uuid.New() + terraformAgentToken = uuid.New() + terraformAgent = agentcontainers.SubAgent{ + ID: terraformAgentID, + Name: "terraform-defined-agent", + Directory: "/workspace", + AuthToken: terraformAgentToken, + } + terraformDevcontainer = codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "terraform-devcontainer", + WorkspaceFolder: "/workspace/project", + SubagentID: uuid.NullUUID{UUID: terraformAgentID, Valid: true}, + } + + // Given: An orphaned agent that should be cleaned up + orphanedAgentID = uuid.New() + orphanedAgentToken = uuid.New() + orphanedAgent = agentcontainers.SubAgent{ + ID: orphanedAgentID, + Name: "orphaned-agent", + Directory: "/tmp", + AuthToken: orphanedAgentToken, + } + + ctx = testutil.Context(t, testutil.WaitMedium) + logger = slog.Make() + mClock = quartz.NewMock(t) + mCCLI = acmock.NewMockContainerCLI(gomock.NewController(t)) + + fakeSAC = &fakeSubAgentClient{ + logger: logger.Named("fakeSubAgentClient"), + agents: map[uuid.UUID]agentcontainers.SubAgent{ + terraformAgentID: terraformAgent, + orphanedAgentID: orphanedAgent, + }, + } + ) + + mCCLI.EXPECT().List(gomock.Any()).Return(codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{}, + }, nil).AnyTimes() + + mClock.Set(time.Now()).MustWait(ctx) + tickerTrap := mClock.Trap().TickerFunc("updaterLoop") + + api := agentcontainers.NewAPI(logger, + agentcontainers.WithClock(mClock), + agentcontainers.WithContainerCLI(mCCLI), + agentcontainers.WithSubAgentClient(fakeSAC), + agentcontainers.WithDevcontainerCLI(&fakeDevcontainerCLI{}), + agentcontainers.WithDevcontainers([]codersdk.WorkspaceAgentDevcontainer{terraformDevcontainer}, nil), + ) + api.Start() + defer api.Close() + + tickerTrap.MustWait(ctx).MustRelease(ctx) + tickerTrap.Close() + + // When: We advance the clock, allowing cleanup to occur + _, aw := mClock.AdvanceNext() + aw.MustWait(ctx) + + // Then: The orphaned agent should be deleted + assert.Contains(t, fakeSAC.deleted, orphanedAgentID, "orphaned agent should be deleted") + + // And: The terraform-defined agent should not be deleted + assert.NotContains(t, fakeSAC.deleted, terraformAgentID, "terraform-defined agent should be preserved") + assert.Len(t, fakeSAC.agents, 1, "only terraform agent should remain") + assert.Contains(t, fakeSAC.agents, terraformAgentID, "terraform agent should still exist") + }) + + t.Run("TerraformDefinedSubAgentNotRecreatedOnConfigChange", func(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("Dev Container tests are not supported on Windows (this test uses mocks but fails due to Windows paths)") + } + + var ( + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + mCtrl = gomock.NewController(t) + + // Given: A terraform-defined devcontainer with a pre-assigned subagent ID. + terraformAgentID = uuid.New() + terraformContainer = codersdk.WorkspaceAgentContainer{ + ID: "test-container-id", + FriendlyName: "test-container", + Image: "test-image", + Running: true, + CreatedAt: time.Now(), + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/project/.devcontainer/devcontainer.json", + }, + } + terraformDevcontainer = codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "terraform-devcontainer", + WorkspaceFolder: "/workspace/project", + ConfigPath: "/workspace/project/.devcontainer/devcontainer.json", + SubagentID: uuid.NullUUID{UUID: terraformAgentID, Valid: true}, + } + + fCCLI = &fakeContainerCLI{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{terraformContainer}, + }, + arch: runtime.GOARCH, + } + + fDCCLI = &fakeDevcontainerCLI{ + upID: terraformContainer.ID, + readConfig: agentcontainers.DevcontainerConfig{ + MergedConfiguration: agentcontainers.DevcontainerMergedConfiguration{ + Customizations: agentcontainers.DevcontainerMergedCustomizations{ + Coder: []agentcontainers.CoderCustomization{{ + Apps: []agentcontainers.SubAgentApp{{Slug: "app1"}}, + }}, + }, + }, + }, + } + + mSAC = acmock.NewMockSubAgentClient(mCtrl) + closed bool + ) + + mSAC.EXPECT().List(gomock.Any()).Return([]agentcontainers.SubAgent{}, nil).AnyTimes() + + // EXPECT: Create is called twice with the terraform-defined ID: + // once for the initial creation and once after the rebuild with + // config changes (upsert). + mSAC.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, agent agentcontainers.SubAgent) (agentcontainers.SubAgent, error) { + assert.Equal(t, terraformAgentID, agent.ID, "agent should have terraform-defined ID") + agent.AuthToken = uuid.New() + return agent, nil + }, + ).Times(2) + + // EXPECT: Delete may be called during Close, but not before. + mSAC.EXPECT().Delete(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, _ uuid.UUID) error { + assert.True(t, closed, "Delete should only be called after Close, not during recreation") + return nil + }).AnyTimes() + + api := agentcontainers.NewAPI(logger, + agentcontainers.WithContainerCLI(fCCLI), + agentcontainers.WithDevcontainerCLI(fDCCLI), + agentcontainers.WithDevcontainers( + []codersdk.WorkspaceAgentDevcontainer{terraformDevcontainer}, + []codersdk.WorkspaceAgentScript{{ID: terraformDevcontainer.ID, LogSourceID: uuid.New()}}, + ), + agentcontainers.WithSubAgentClient(mSAC), + agentcontainers.WithSubAgentURL("test-subagent-url"), + agentcontainers.WithWatcher(watcher.NewNoop()), + ) + api.Start() + + // Given: We create the devcontainer for the first time. + err := api.CreateDevcontainer(terraformDevcontainer.WorkspaceFolder, terraformDevcontainer.ConfigPath) + require.NoError(t, err) + + // When: The container is recreated (new container ID) with config changes. + terraformContainer.ID = "new-container-id" + fCCLI.containers.Containers = []codersdk.WorkspaceAgentContainer{terraformContainer} + fDCCLI.upID = terraformContainer.ID + fDCCLI.readConfig.MergedConfiguration.Customizations.Coder = []agentcontainers.CoderCustomization{{ + Apps: []agentcontainers.SubAgentApp{{Slug: "app2"}}, // Changed app triggers recreation logic. + }} + + err = api.CreateDevcontainer(terraformDevcontainer.WorkspaceFolder, terraformDevcontainer.ConfigPath, agentcontainers.WithRemoveExistingContainer()) + require.NoError(t, err) + + // Then: Mock expectations verify that Create was called once and Delete was not called during recreation. + closed = true + api.Close() + }) + + // Verify that rebuilding a terraform-defined devcontainer via the + // HTTP API does not delete the sub agent. The sub agent should be + // preserved (Create called again with the same terraform ID) and + // display app changes should be picked up. + t.Run("TerraformDefinedSubAgentRebuildViaHTTP", func(t *testing.T) { + t.Parallel() + + if runtime.GOOS == "windows" { + t.Skip("Dev Container tests are not supported on Windows (this test uses mocks but fails due to Windows paths)") + } + + var ( + ctx = testutil.Context(t, testutil.WaitMedium) + logger = slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}).Leveled(slog.LevelDebug) + mCtrl = gomock.NewController(t) + + terraformAgentID = uuid.New() + containerID = "test-container-id" + + terraformContainer = codersdk.WorkspaceAgentContainer{ + ID: containerID, + FriendlyName: "test-container", + Image: "test-image", + Running: true, + CreatedAt: time.Now(), + Labels: map[string]string{ + agentcontainers.DevcontainerLocalFolderLabel: "/workspace/project", + agentcontainers.DevcontainerConfigFileLabel: "/workspace/project/.devcontainer/devcontainer.json", + }, + } + terraformDevcontainer = codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "terraform-devcontainer", + WorkspaceFolder: "/workspace/project", + ConfigPath: "/workspace/project/.devcontainer/devcontainer.json", + SubagentID: uuid.NullUUID{UUID: terraformAgentID, Valid: true}, + } + + fCCLI = &fakeContainerCLI{ + containers: codersdk.WorkspaceAgentListContainersResponse{ + Containers: []codersdk.WorkspaceAgentContainer{terraformContainer}, + }, + arch: runtime.GOARCH, + } + + fDCCLI = &fakeDevcontainerCLI{ + upID: containerID, + readConfig: agentcontainers.DevcontainerConfig{ + MergedConfiguration: agentcontainers.DevcontainerMergedConfiguration{ + Customizations: agentcontainers.DevcontainerMergedCustomizations{ + Coder: []agentcontainers.CoderCustomization{{ + DisplayApps: map[codersdk.DisplayApp]bool{ + codersdk.DisplayAppSSH: true, + codersdk.DisplayAppWebTerminal: true, + }, + }}, + }, + }, + }, + } + + mSAC = acmock.NewMockSubAgentClient(mCtrl) + closed bool + + createCalled = make(chan agentcontainers.SubAgent, 2) + ) + + mSAC.EXPECT().List(gomock.Any()).Return([]agentcontainers.SubAgent{}, nil).AnyTimes() + + // Create should be called twice: once for the initial injection + // and once after the rebuild picks up the new container. + mSAC.EXPECT().Create(gomock.Any(), gomock.Any()).DoAndReturn( + func(_ context.Context, agent agentcontainers.SubAgent) (agentcontainers.SubAgent, error) { + assert.Equal(t, terraformAgentID, agent.ID, "agent should always use terraform-defined ID") + agent.AuthToken = uuid.New() + createCalled <- agent + return agent, nil + }, + ).Times(2) + + // Delete must only be called during Close, never during rebuild. + mSAC.EXPECT().Delete(gomock.Any(), gomock.Any()).DoAndReturn(func(_ context.Context, _ uuid.UUID) error { + assert.True(t, closed, "Delete should only be called after Close, not during rebuild") + return nil + }).AnyTimes() + + api := agentcontainers.NewAPI(logger, + agentcontainers.WithContainerCLI(fCCLI), + agentcontainers.WithDevcontainerCLI(fDCCLI), + agentcontainers.WithDevcontainers( + []codersdk.WorkspaceAgentDevcontainer{terraformDevcontainer}, + []codersdk.WorkspaceAgentScript{{ID: terraformDevcontainer.ID, LogSourceID: uuid.New()}}, + ), + agentcontainers.WithSubAgentClient(mSAC), + agentcontainers.WithSubAgentURL("test-subagent-url"), + agentcontainers.WithWatcher(watcher.NewNoop()), + ) + api.Start() + defer func() { + closed = true + api.Close() + }() + + r := chi.NewRouter() + r.Mount("/", api.Routes()) + + // Perform the initial devcontainer creation directly to set up + // the subagent (mirrors the TerraformDefinedSubAgentNotRecreatedOnConfigChange + // test pattern). + err := api.CreateDevcontainer(terraformDevcontainer.WorkspaceFolder, terraformDevcontainer.ConfigPath) + require.NoError(t, err) + + initialAgent := testutil.RequireReceive(ctx, t, createCalled) + assert.Equal(t, terraformAgentID, initialAgent.ID) + + // Simulate container rebuild: new container ID, changed display apps. + newContainerID := "new-container-id" + terraformContainer.ID = newContainerID + fCCLI.containers.Containers = []codersdk.WorkspaceAgentContainer{terraformContainer} + fDCCLI.upID = newContainerID + fDCCLI.readConfig.MergedConfiguration.Customizations.Coder = []agentcontainers.CoderCustomization{{ + DisplayApps: map[codersdk.DisplayApp]bool{ + codersdk.DisplayAppSSH: true, + codersdk.DisplayAppWebTerminal: true, + codersdk.DisplayAppVSCodeDesktop: true, + codersdk.DisplayAppVSCodeInsiders: true, + }, + }} + + // Issue the rebuild request via the HTTP API. + req := httptest.NewRequest(http.MethodPost, "/devcontainers/"+terraformDevcontainer.ID.String()+"/recreate", nil). + WithContext(ctx) + rec := httptest.NewRecorder() + r.ServeHTTP(rec, req) + require.Equal(t, http.StatusAccepted, rec.Code) + + // Wait for the post-rebuild injection to complete. + rebuiltAgent := testutil.RequireReceive(ctx, t, createCalled) + assert.Equal(t, terraformAgentID, rebuiltAgent.ID, "rebuilt agent should preserve terraform ID") + + // Verify that the display apps were updated. + assert.Contains(t, rebuiltAgent.DisplayApps, codersdk.DisplayAppVSCodeDesktop, + "rebuilt agent should include updated display apps") + assert.Contains(t, rebuiltAgent.DisplayApps, codersdk.DisplayAppVSCodeInsiders, + "rebuilt agent should include updated display apps") + }) + t.Run("Error", func(t *testing.T) { t.Parallel() diff --git a/agent/agentcontainers/subagent.go b/agent/agentcontainers/subagent.go index 51259948ac..b23bb7a878 100644 --- a/agent/agentcontainers/subagent.go +++ b/agent/agentcontainers/subagent.go @@ -24,10 +24,12 @@ type SubAgent struct { DisplayApps []codersdk.DisplayApp } -// CloneConfig makes a copy of SubAgent without ID and AuthToken. The -// name is inherited from the devcontainer. +// CloneConfig makes a copy of SubAgent using configuration from the +// devcontainer. The ID is inherited from dc.SubagentID if present, and +// the name is inherited from the devcontainer. AuthToken is not copied. func (s SubAgent) CloneConfig(dc codersdk.WorkspaceAgentDevcontainer) SubAgent { return SubAgent{ + ID: dc.SubagentID.UUID, Name: dc.Name, Directory: s.Directory, Architecture: s.Architecture, @@ -190,6 +192,11 @@ func (a *subAgentAPIClient) List(ctx context.Context) ([]SubAgent, error) { func (a *subAgentAPIClient) Create(ctx context.Context, agent SubAgent) (_ SubAgent, err error) { a.logger.Debug(ctx, "creating sub agent", slog.F("name", agent.Name), slog.F("directory", agent.Directory)) + var id []byte + if agent.ID != uuid.Nil { + id = agent.ID[:] + } + displayApps := make([]agentproto.CreateSubAgentRequest_DisplayApp, 0, len(agent.DisplayApps)) for _, displayApp := range agent.DisplayApps { var app agentproto.CreateSubAgentRequest_DisplayApp @@ -228,6 +235,7 @@ func (a *subAgentAPIClient) Create(ctx context.Context, agent SubAgent) (_ SubAg OperatingSystem: agent.OperatingSystem, DisplayApps: displayApps, Apps: apps, + Id: id, }) if err != nil { return SubAgent{}, err diff --git a/agent/agentcontainers/subagent_test.go b/agent/agentcontainers/subagent_test.go index 041103f20e..855ec47769 100644 --- a/agent/agentcontainers/subagent_test.go +++ b/agent/agentcontainers/subagent_test.go @@ -306,3 +306,128 @@ func TestSubAgentClient_CreateWithDisplayApps(t *testing.T) { } }) } + +func TestSubAgent_CloneConfig(t *testing.T) { + t.Parallel() + + t.Run("CopiesIDFromDevcontainer", func(t *testing.T) { + t.Parallel() + + subAgent := agentcontainers.SubAgent{ + ID: uuid.New(), + Name: "original-name", + Directory: "/workspace", + Architecture: "amd64", + OperatingSystem: "linux", + DisplayApps: []codersdk.DisplayApp{codersdk.DisplayAppVSCodeDesktop}, + Apps: []agentcontainers.SubAgentApp{{Slug: "app1"}}, + } + expectedID := uuid.MustParse("550e8400-e29b-41d4-a716-446655440000") + dc := codersdk.WorkspaceAgentDevcontainer{ + Name: "devcontainer-name", + SubagentID: uuid.NullUUID{UUID: expectedID, Valid: true}, + } + + cloned := subAgent.CloneConfig(dc) + + assert.Equal(t, expectedID, cloned.ID) + assert.Equal(t, dc.Name, cloned.Name) + assert.Equal(t, subAgent.Directory, cloned.Directory) + assert.Zero(t, cloned.AuthToken, "AuthToken should not be copied") + }) + + t.Run("HandlesNilSubagentID", func(t *testing.T) { + t.Parallel() + + subAgent := agentcontainers.SubAgent{ + ID: uuid.New(), + Name: "original-name", + Directory: "/workspace", + Architecture: "amd64", + OperatingSystem: "linux", + } + dc := codersdk.WorkspaceAgentDevcontainer{ + Name: "devcontainer-name", + SubagentID: uuid.NullUUID{Valid: false}, + } + + cloned := subAgent.CloneConfig(dc) + + assert.Equal(t, uuid.Nil, cloned.ID) + }) +} + +func TestSubAgent_EqualConfig(t *testing.T) { + t.Parallel() + + base := agentcontainers.SubAgent{ + ID: uuid.New(), + Name: "test-agent", + Directory: "/workspace", + Architecture: "amd64", + OperatingSystem: "linux", + DisplayApps: []codersdk.DisplayApp{codersdk.DisplayAppVSCodeDesktop}, + Apps: []agentcontainers.SubAgentApp{ + {Slug: "test-app", DisplayName: "Test App"}, + }, + } + + tests := []struct { + name string + modify func(*agentcontainers.SubAgent) + wantEqual bool + }{ + { + name: "identical", + modify: func(s *agentcontainers.SubAgent) {}, + wantEqual: true, + }, + { + name: "different ID", + modify: func(s *agentcontainers.SubAgent) { s.ID = uuid.New() }, + wantEqual: true, + }, + { + name: "different Name", + modify: func(s *agentcontainers.SubAgent) { s.Name = "different-name" }, + wantEqual: false, + }, + { + name: "different Directory", + modify: func(s *agentcontainers.SubAgent) { s.Directory = "/different/path" }, + wantEqual: false, + }, + { + name: "different Architecture", + modify: func(s *agentcontainers.SubAgent) { s.Architecture = "arm64" }, + wantEqual: false, + }, + { + name: "different OperatingSystem", + modify: func(s *agentcontainers.SubAgent) { s.OperatingSystem = "windows" }, + wantEqual: false, + }, + { + name: "different DisplayApps", + modify: func(s *agentcontainers.SubAgent) { s.DisplayApps = []codersdk.DisplayApp{codersdk.DisplayAppSSH} }, + wantEqual: false, + }, + { + name: "different Apps", + modify: func(s *agentcontainers.SubAgent) { + s.Apps = []agentcontainers.SubAgentApp{{Slug: "different-app", DisplayName: "Different App"}} + }, + wantEqual: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + modified := base + tt.modify(&modified) + assert.Equal(t, tt.wantEqual, base.EqualConfig(modified)) + }) + } +} diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index ed86bd78c4..6ba016c4a9 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -20837,6 +20837,14 @@ const docTemplate = `{ } ] }, + "subagent_id": { + "format": "uuid", + "allOf": [ + { + "$ref": "#/definitions/uuid.NullUUID" + } + ] + }, "workspace_folder": { "type": "string" } diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 719b6b3aca..24187a50ab 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -19141,6 +19141,14 @@ } ] }, + "subagent_id": { + "format": "uuid", + "allOf": [ + { + "$ref": "#/definitions/uuid.NullUUID" + } + ] + }, "workspace_folder": { "type": "string" } diff --git a/codersdk/agentsdk/convert.go b/codersdk/agentsdk/convert.go index 775ce06c73..a9e41fb4b3 100644 --- a/codersdk/agentsdk/convert.go +++ b/codersdk/agentsdk/convert.go @@ -425,11 +425,20 @@ func DevcontainerFromProto(pdc *proto.WorkspaceAgentDevcontainer) (codersdk.Work if err != nil { return codersdk.WorkspaceAgentDevcontainer{}, xerrors.Errorf("parse id: %w", err) } + var subagentID uuid.NullUUID + if pdc.SubagentId != nil { + subagentID.Valid = true + subagentID.UUID, err = uuid.FromBytes(pdc.SubagentId) + if err != nil { + return codersdk.WorkspaceAgentDevcontainer{}, xerrors.Errorf("parse subagent id: %w", err) + } + } return codersdk.WorkspaceAgentDevcontainer{ ID: id, Name: pdc.Name, WorkspaceFolder: pdc.WorkspaceFolder, ConfigPath: pdc.ConfigPath, + SubagentID: subagentID, }, nil } @@ -442,10 +451,16 @@ func ProtoFromDevcontainers(dcs []codersdk.WorkspaceAgentDevcontainer) []*proto. } func ProtoFromDevcontainer(dc codersdk.WorkspaceAgentDevcontainer) *proto.WorkspaceAgentDevcontainer { + var subagentID []byte + if dc.SubagentID.Valid { + subagentID = dc.SubagentID.UUID[:] + } + return &proto.WorkspaceAgentDevcontainer{ Id: dc.ID[:], Name: dc.Name, WorkspaceFolder: dc.WorkspaceFolder, ConfigPath: dc.ConfigPath, + SubagentId: subagentID, } } diff --git a/codersdk/agentsdk/convert_test.go b/codersdk/agentsdk/convert_test.go index f324d504b8..15f063f4e4 100644 --- a/codersdk/agentsdk/convert_test.go +++ b/codersdk/agentsdk/convert_test.go @@ -136,6 +136,7 @@ func TestManifest(t *testing.T) { ID: uuid.New(), WorkspaceFolder: "/home/coder/coder", ConfigPath: "/home/coder/coder/.devcontainer/devcontainer.json", + SubagentID: uuid.NullUUID{Valid: true, UUID: uuid.New()}, }, }, } diff --git a/codersdk/workspaceagents.go b/codersdk/workspaceagents.go index a9026baf27..3e2fafa7c4 100644 --- a/codersdk/workspaceagents.go +++ b/codersdk/workspaceagents.go @@ -440,10 +440,11 @@ func (s WorkspaceAgentDevcontainerStatus) Transitioning() bool { // WorkspaceAgentDevcontainer defines the location of a devcontainer // configuration in a workspace that is visible to the workspace agent. type WorkspaceAgentDevcontainer struct { - ID uuid.UUID `json:"id" format:"uuid"` - Name string `json:"name"` - WorkspaceFolder string `json:"workspace_folder"` - ConfigPath string `json:"config_path,omitempty"` + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + WorkspaceFolder string `json:"workspace_folder"` + ConfigPath string `json:"config_path,omitempty"` + SubagentID uuid.NullUUID `json:"subagent_id,omitempty" format:"uuid"` // Additional runtime fields. Status WorkspaceAgentDevcontainerStatus `json:"status"` @@ -458,6 +459,7 @@ func (d WorkspaceAgentDevcontainer) Equals(other WorkspaceAgentDevcontainer) boo return d.ID == other.ID && d.Name == other.Name && d.WorkspaceFolder == other.WorkspaceFolder && + d.SubagentID == other.SubagentID && d.Status == other.Status && d.Dirty == other.Dirty && (d.Container == nil && other.Container == nil || @@ -467,6 +469,12 @@ func (d WorkspaceAgentDevcontainer) Equals(other WorkspaceAgentDevcontainer) boo d.Error == other.Error } +// IsTerraformDefined returns true if this devcontainer has resources defined +// in Terraform. +func (d WorkspaceAgentDevcontainer) IsTerraformDefined() bool { + return d.SubagentID.Valid +} + // WorkspaceAgentDevcontainerAgent represents the sub agent for a // devcontainer. type WorkspaceAgentDevcontainerAgent struct { diff --git a/codersdk/workspaceagents_test.go b/codersdk/workspaceagents_test.go index b44508f06a..0d4a9816ae 100644 --- a/codersdk/workspaceagents_test.go +++ b/codersdk/workspaceagents_test.go @@ -110,3 +110,142 @@ func TestWorkspaceAgentLogTextSpecialChars(t *testing.T) { result := log.Text("main", "startup_script") require.Equal(t, "2024-01-28T10:30:00Z [debug] [agent.main|startup_script] \033[31mError!\033[0m 🚀 Unicode: 日本語", result) } + +func TestWorkspaceAgentDevcontainerEquals(t *testing.T) { + t.Parallel() + + agentID := uuid.New() + + base := codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "test-dc", + WorkspaceFolder: "/workspace", + Status: codersdk.WorkspaceAgentDevcontainerStatusRunning, + Dirty: false, + Container: &codersdk.WorkspaceAgentContainer{ID: "container-123"}, + Agent: &codersdk.WorkspaceAgentDevcontainerAgent{ID: agentID, Name: "agent-1"}, + Error: "", + } + + tests := []struct { + name string + modify func(*codersdk.WorkspaceAgentDevcontainer) + wantEqual bool + }{ + { + name: "identical", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) {}, + wantEqual: true, + }, + { + name: "different ID", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { d.ID = uuid.New() }, + wantEqual: false, + }, + { + name: "different Name", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { d.Name = "other-dc" }, + wantEqual: false, + }, + { + name: "different WorkspaceFolder", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { d.WorkspaceFolder = "/other" }, + wantEqual: false, + }, + { + name: "different SubagentID (one valid, one nil)", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { + d.SubagentID = uuid.NullUUID{Valid: true, UUID: uuid.New()} + }, + wantEqual: false, + }, + { + name: "different SubagentID UUIDs", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { + d.SubagentID = uuid.NullUUID{Valid: true, UUID: uuid.New()} + }, + wantEqual: false, + }, + { + name: "different Status", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { + d.Status = codersdk.WorkspaceAgentDevcontainerStatusStopped + }, + wantEqual: false, + }, + { + name: "different Dirty", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { d.Dirty = true }, + wantEqual: false, + }, + { + name: "different Container (one nil)", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { d.Container = nil }, + wantEqual: false, + }, + { + name: "different Container IDs", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { + d.Container = &codersdk.WorkspaceAgentContainer{ID: "different-container"} + }, + wantEqual: false, + }, + { + name: "different Agent (one nil)", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { d.Agent = nil }, + wantEqual: false, + }, + { + name: "different Agent values", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { + d.Agent = &codersdk.WorkspaceAgentDevcontainerAgent{ID: agentID, Name: "agent-2"} + }, + wantEqual: false, + }, + { + name: "different Error", + modify: func(d *codersdk.WorkspaceAgentDevcontainer) { d.Error = "some error" }, + wantEqual: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + modified := base + tt.modify(&modified) + require.Equal(t, tt.wantEqual, base.Equals(modified)) + }) + } +} + +func TestWorkspaceAgentDevcontainerIsTerraformDefined(t *testing.T) { + t.Parallel() + + t.Run("SubagentID Valid", func(t *testing.T) { + t.Parallel() + + dc := codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "test-dc", + WorkspaceFolder: "/workspace", + SubagentID: uuid.NullUUID{Valid: true, UUID: uuid.New()}, + } + + require.True(t, dc.IsTerraformDefined()) + }) + + t.Run("SubagentID Null", func(t *testing.T) { + t.Parallel() + + dc := codersdk.WorkspaceAgentDevcontainer{ + ID: uuid.New(), + Name: "test-dc", + WorkspaceFolder: "/workspace", + SubagentID: uuid.NullUUID{Valid: false}, + } + + require.False(t, dc.IsTerraformDefined()) + }) +} diff --git a/docs/reference/api/agents.md b/docs/reference/api/agents.md index 1db9c4cc74..8252582093 100644 --- a/docs/reference/api/agents.md +++ b/docs/reference/api/agents.md @@ -838,6 +838,10 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "name": "string", "status": "running", + "subagent_id": { + "uuid": "string", + "valid": true + }, "workspace_folder": "string" } ], @@ -1015,6 +1019,10 @@ curl -X GET http://coder-server:8080/api/v2/workspaceagents/{workspaceagent}/con "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "name": "string", "status": "running", + "subagent_id": { + "uuid": "string", + "valid": true + }, "workspace_folder": "string" } ], diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index d4d6fa4f16..983f28987a 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -10518,6 +10518,10 @@ If the schedule is empty, the user will be updated to use the default schedule.| "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "name": "string", "status": "running", + "subagent_id": { + "uuid": "string", + "valid": true + }, "workspace_folder": "string" } ``` @@ -10534,6 +10538,7 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `id` | string | false | | | | `name` | string | false | | | | `status` | [codersdk.WorkspaceAgentDevcontainerStatus](#codersdkworkspaceagentdevcontainerstatus) | false | | Additional runtime fields. | +| `subagent_id` | [uuid.NullUUID](#uuidnulluuid) | false | | | | `workspace_folder` | string | false | | | ## codersdk.WorkspaceAgentDevcontainerAgent @@ -10665,6 +10670,10 @@ If the schedule is empty, the user will be updated to use the default schedule.| "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", "name": "string", "status": "running", + "subagent_id": { + "uuid": "string", + "valid": true + }, "workspace_folder": "string" } ], diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index cceedca129..aa3548d444 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -6306,6 +6306,7 @@ export interface WorkspaceAgentDevcontainer { readonly name: string; readonly workspace_folder: string; readonly config_path?: string; + readonly subagent_id?: string; /** * Additional runtime fields. */ diff --git a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx index b8be38a2ad..df6f030cb9 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.stories.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.stories.tsx @@ -20,7 +20,7 @@ import { import type { Meta, StoryObj } from "@storybook/react-vite"; import { API } from "api/api"; import { getPreferredProxy } from "contexts/ProxyContext"; -import { spyOn, userEvent, within } from "storybook/test"; +import { screen, spyOn, userEvent, within } from "storybook/test"; import { AgentDevcontainerCard } from "./AgentDevcontainerCard"; const meta: Meta = { @@ -185,6 +185,37 @@ export const WithPortForwarding: Story = { ], }; +export const TerraformManaged: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + subagent_id: "precreated-subagent-id", + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const label = canvas.getByText("dev container (terraform agent)"); + await userEvent.hover(label); + await screen.findByRole("tooltip"); + }, +}; + +export const TerraformManagedDirty: Story = { + args: { + devcontainer: { + ...MockWorkspaceAgentDevcontainer, + subagent_id: "precreated-subagent-id", + dirty: true, + }, + }, + play: async ({ canvasElement }) => { + const canvas = within(canvasElement); + const outdatedStatus = canvas.getByText("Outdated"); + await userEvent.hover(outdatedStatus); + await screen.findByRole("tooltip"); + }, +}; + export const WithDeleteError: Story = { beforeEach: () => { spyOn(API, "deleteDevContainer").mockRejectedValue( diff --git a/site/src/modules/resources/AgentDevcontainerCard.tsx b/site/src/modules/resources/AgentDevcontainerCard.tsx index e3f1bfcf57..5fecb2d80e 100644 --- a/site/src/modules/resources/AgentDevcontainerCard.tsx +++ b/site/src/modules/resources/AgentDevcontainerCard.tsx @@ -183,7 +183,19 @@ export const AgentDevcontainerCard: FC = ({ text-xs text-content-secondary" > - dev container + {devcontainer.subagent_id ? ( + + + dev container (terraform agent) + + + This dev container agent is defined in Terraform and has limited + configurability via the devcontainer.json file. + + + ) : ( + dev container + )}
= ({ 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 ( @@ -45,10 +40,14 @@ export const SubAgentOutdatedTooltip: FC = ({ - +
- {title} - {text} + Dev Container Outdated + + This Dev Container is outdated. 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. +
@@ -60,7 +59,7 @@ export const SubAgentOutdatedTooltip: FC = ({ Rebuild Dev Container - +
);