feat(coderd/provisionerdserver): insert sub agent resource by DanielleMaywood · Pull Request #21699 · coder/coder

feat(coderd/provisionerdserver): insert sub agent resource by DanielleMaywood · Pull Request #21699 · coder/coder

Update provisionerdserver to handle the changes introduced to provisionerd in #21602

We create a new relationship between workspace_agent_devcontainers and workspace_agents with the newly created subagent_id.

Base automatically changed from danielle/devcontainer-resources/provisioner to main

January 29, 2026 00:01

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quick review to flag a few things. I'll do a more thorough one later.

		dc.Id = uuid.New().String()

		if dc.GetSubagentId() != "" {
		subAgentID := uuid.New()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this work with the agent id from proto option used elsewhere in the code?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I fully understand the question

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think this is probably fine, just a very confusing pattern since we're overwriting IDs supplied by the provider. Perhaps a comment is warranted on why we do it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a small comment in f8ad555, although not sure how useful it is.

I've tried to fully understand the surrounding context and why it was needed (without it the added test would fail) but I'll be honest it is slightly going over my head 😅 .

		dc.Id = uuid.New().String()

		if dc.GetSubagentId() != "" {
		subAgentID := uuid.New()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think this is probably fine, just a very confusing pattern since we're overwriting IDs supplied by the provider. Perhaps a comment is warranted on why we do it?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few final things, but other than that, looks good. I don't need to re-review.


		// Subagents in devcontainers can also have apps that need
		// tracking for task linking, just like the parent agent's
		// apps above.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't really explain why we replace the subagent ID. But considering it's a common theme throughout the provisioner implementation, I won't push for it here. We should look at improving the reasoning here or refactoring though, because it's almost like an arcane art. 😄

		scriptRunOnStart = append(scriptRunOnStart, true)
		scriptRunOnStop = append(scriptRunOnStop, false)
		scriptsParams.ScriptRunOnStart = append(scriptsParams.ScriptRunOnStart, true)
		scriptsParams.ScriptRunOnStop = append(scriptsParams.ScriptRunOnStop, false)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I spent some time scratching my head about this because I had forgotten why I added it, and of course didn't read the comments haha. Not required now, but this should really be broken out into it's own function with a very explicit name.

DanielleMaywood deleted the danielle/devcontainer-resources/provisionerdserver branch

January 30, 2026 17:19