feat(coderd/agentapi): support terraform-defined subagent ids by DanielleMaywood · Pull Request #21837 · coder/coder

DanielleMaywood · 2026-02-02T10:59:24Z

Update coderd/agentapi to handle pre-created sub agents

…ontainers

coder-tasks

Code Review

Reviewed PR #21837: Support for terraform-defined subagent IDs. The changes enable updating pre-created subagents from terraform rather than always creating new ones. Found 3 issues requiring attention.

Summary

Strengths:

Good separation of create vs. update logic paths
Comprehensive test coverage with 6 test cases covering various scenarios
Proper authorization checks via dbauthz layer
Clear validation of parent-child relationships

Issues:

Logic reordering breaks existing behavior (validation happens after operations)
Missing validation when ID is provided in request
Security: No validation that provided ID actually belongs to current workspace

General Comments

The new update path is well-designed and properly validates parent relationships
Test cases are thorough and test both success and error scenarios
Database query and authorization changes follow project patterns correctly

coderd/agentapi/subagent.go

mafredri

The agentapi look fine to me. 👍🏻 Only potential blocker I see is rbac.

coderd/agentapi/subagent.go

coderd/agentapi/subagent_test.go

coderd/database/dbauthz/dbauthz.go

johnstcn · 2026-02-04T09:27:55Z

coderd/database/dbauthz/dbauthz_test.go

+	s.Run("UpdateWorkspaceAgentDisplayAppsByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) {
+		w := testutil.Fake(s.T(), faker, database.Workspace{})
+		agt := testutil.Fake(s.T(), faker, database.WorkspaceAgent{})
+		arg := database.UpdateWorkspaceAgentDisplayAppsByIDParams{
+			ID:          agt.ID,
+			DisplayApps: []database.DisplayApp{database.DisplayAppVscode},
+		}
+		dbm.EXPECT().GetWorkspaceByAgentID(gomock.Any(), agt.ID).Return(w, nil).AnyTimes()
+		dbm.EXPECT().UpdateWorkspaceAgentDisplayAppsByID(gomock.Any(), arg).Return(nil).AnyTimes()
+		check.Args(arg).Asserts(w, policy.ActionUpdate).Returns()
+	}))


This might be a good place to ensure that we don't allow sub-agents to update across workspaces?

Re-reading this, I'm unsure what you mean. Are these check.Args(...).Asserts(...) capable of testing this explicitly?

I think you're right. I zoomed in on WithNotAuthorized() but appear to have misunderstood what it actually does.

coderd/agentapi/subagent.go

Emyrk · 2026-02-04T14:16:37Z

coderd/database/dbauthz/dbauthz.go

+func (q *querier) UpdateWorkspaceAgentDisplayAppsByID(ctx context.Context, arg database.UpdateWorkspaceAgentDisplayAppsByIDParams) error {
+	workspace, err := q.db.GetWorkspaceByAgentID(ctx, arg.ID)
+	if err != nil {
+		return err
+	}
+
+	if err := q.authorizeContext(ctx, policy.ActionUpdateAgent, workspace); err != nil {
+		return err
+	}
+
+	return q.db.UpdateWorkspaceAgentDisplayAppsByID(ctx, arg)
+}


johnstcn · 2026-02-04T15:10:46Z

codersdk/apikey_scopes_gen.go

 	APIKeyScopeWorkspaceDormantStart               APIKeyScope = "workspace_dormant:start"
 	APIKeyScopeWorkspaceDormantStop                APIKeyScope = "workspace_dormant:stop"
 	APIKeyScopeWorkspaceDormantUpdate              APIKeyScope = "workspace_dormant:update"
+	APIKeyScopeWorkspaceDormantUpdateAgent         APIKeyScope = "workspace_dormant:update_agent"


This is an interesting question - when is this going to happen?

I think these scopes are internal by-default and have to be explicitly exported? I'm a little unsure about API Key Scopes, so I might need to do some reading to get more context.

mafredri

Looks good! One last question/suggestion, but I leave it up to you as it's not a blocker.

mafredri · 2026-02-04T15:19:30Z

coderd/agentapi/subagent_test.go

+				check: func(t *testing.T, ctx context.Context, db database.Store, resp *proto.CreateSubAgentResponse, agent database.WorkspaceAgent) {
+					// Then: The response contains the original agent name, not the new one.
+					require.NotNil(t, resp.Agent)
+					require.Equal(t, baseChildAgent.Name, resp.Agent.Name)


Will the client side see that some inputs were discarded? If not, that might lead to some confusion.

Should we consider introducing an errors field (like app creation errors)? Or perhaps erroring out if the name is changed? (Forcing the client to provide valid input for existing agents.)

Not at the moment, no.

I think I'll address this as a follow-up once the feature is fully implemented, and will follow the pattern introduced with app creation errors.

github-actions bot assigned DanielleMaywood Feb 2, 2026

DanielleMaywood force-pushed the danielle/devcontainer-resources/agent branch from ca6855c to 2b48d69 Compare February 2, 2026 15:11

DanielleMaywood changed the base branch from main to danielle/devcontainer-resources/agent-proto February 2, 2026 15:11

DanielleMaywood mentioned this pull request Feb 2, 2026

feat(agent): add subagent ID fields to devcontainers in manifest #21848

Merged

DanielleMaywood force-pushed the danielle/devcontainer-resources/agent branch from 2b48d69 to fa0ccbc Compare February 3, 2026 11:36

Base automatically changed from danielle/devcontainer-resources/agent-proto to main February 3, 2026 12:37

DanielleMaywood force-pushed the danielle/devcontainer-resources/agent branch 5 times, most recently from 7f95be2 to 9643b75 Compare February 3, 2026 15:51

DanielleMaywood changed the title ~~feat(agent): support terraform-defined subagent ids~~ feat(coder/agentapi): support terraform-defined subagent ids Feb 3, 2026

DanielleMaywood changed the title ~~feat(coder/agentapi): support terraform-defined subagent ids~~ feat(coderd/agentapi): support terraform-defined subagent ids Feb 3, 2026

feat(coder/agentapi): populate subagent ID for terraform-defined devc…

ca54e30

…ontainers

DanielleMaywood force-pushed the danielle/devcontainer-resources/agent branch from 9643b75 to ca54e30 Compare February 3, 2026 16:49

DanielleMaywood marked this pull request as ready for review February 3, 2026 16:59

DanielleMaywood requested review from johnstcn and mafredri February 3, 2026 17:00

coder-tasks bot reviewed Feb 3, 2026

View reviewed changes

coderd/agentapi/subagent.go Show resolved Hide resolved

coderd/agentapi/subagent.go Show resolved Hide resolved

coderd/agentapi/subagent.go Show resolved Hide resolved

mafredri reviewed Feb 4, 2026

View reviewed changes

coderd/agentapi/subagent.go Outdated Show resolved Hide resolved

mafredri reviewed Feb 4, 2026

View reviewed changes

coderd/agentapi/subagent.go Show resolved Hide resolved

coderd/agentapi/subagent_test.go Outdated Show resolved Hide resolved

coderd/database/dbauthz/dbauthz.go Outdated Show resolved Hide resolved

johnstcn reviewed Feb 4, 2026

View reviewed changes

DanielleMaywood added 3 commits February 4, 2026 12:38

chore: add WorkspaceUpdateAgent policy

663a813

chore: slightly re-word error message

da403d6

chore: rename childAgent to baseChildAgent

b0e6002

DanielleMaywood requested a review from Emyrk as a code owner February 4, 2026 12:42

DanielleMaywood force-pushed the danielle/devcontainer-resources/agent branch from 4960519 to cf851aa Compare February 4, 2026 13:27

DanielleMaywood added 2 commits February 4, 2026 13:33

chore: create db migration and run make gen

aab7c43

chore: fix wantErr expectation

4e92897

DanielleMaywood force-pushed the danielle/devcontainer-resources/agent branch from cf851aa to 4e92897 Compare February 4, 2026 13:33

Emyrk reviewed Feb 4, 2026

View reviewed changes

johnstcn approved these changes Feb 4, 2026

View reviewed changes

mafredri approved these changes Feb 4, 2026

View reviewed changes

DanielleMaywood merged commit af0e171 into main Feb 4, 2026
30 of 32 checks passed

DanielleMaywood deleted the danielle/devcontainer-resources/agent branch February 4, 2026 15:33

github-actions bot locked and limited conversation to collaborators Feb 4, 2026

Conversation

DanielleMaywood commented Feb 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coder-tasks bot left a comment

Choose a reason for hiding this comment

Code Review

Summary

General Comments

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mafredri left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DanielleMaywood commented Feb 2, 2026 •

edited

Loading