feat(agent/agentcontainers): support displayApps from devcontainer config #18342

DanielleMaywood · 2025-06-12T09:45:10Z

This updates the agent injection routine to read the dev container's configuration so we can add display apps to the sub agent.

Example

{
        "name": "Sample Dev Container",
        "image": "mcr.microsoft.com/devcontainers/base:ubuntu",
        "customizations": {
                "com.coder": {
                        "displayApps": ["vscode", "web_terminal"]
                }
        }
}

With this devcontainer.json, the agent that is created for this dev container will have the vscode and web_terminal display apps.

…nfig

Copilot

Pull Request Overview

This PR adds support for configuring sub agent display apps through the dev container configuration. Key changes include:

Exposing a new GetSubAgentDisplayApps method on the client and fake agent API.
Passing converted display apps from dev container configuration into sub agent creation.
Enhancing tests and mocking to validate different display apps scenarios.

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
agent/agenttest/client.go	Added GetSubAgentDisplayApps method to client and fake API.
agent/agentcontainers/subagent.go	Converted coder display apps to proto display apps in sub agent creation.
agent/agentcontainers/subagent_test.go	Added tests to cover various display app scenarios.
agent/agentcontainers/devcontainercli.go	Introduced ReadConfig functionality and updated parsing logic.
agent/agentcontainers/devcontainercli_test.go	Included tests for reading and parsing the dev container configuration.
agent/agentcontainers/api.go	Integrated display app configuration into sub agent creation.
agent/agentcontainers/api_test.go	Updated API tests to accommodate the new display apps configuration.
agent/agentcontainers/acmock/acmock.go	Added ReadConfig mock support.

Comments suppressed due to low confidence (3)

agent/agentcontainers/subagent.go:100

In the display apps conversion switch-case, consider documenting or handling unsupported DisplayApp values more gracefully, especially as new display apps may be added in the future.

default:

agent/agentcontainers/api.go:1104

When failing to read the dev container config, the error is logged and DisplayApps is left empty. Consider whether this fallback behavior is intended or if additional error handling or documentation is needed to clarify the decision.

if config, err := api.dccli.ReadConfig(ctx, dc.WorkspaceFolder, dc.ConfigPath); err != nil {

agent/agentcontainers/devcontainercli.go:299

The updated generic function no longer returns the parsed result using result.Err(); please verify that this change in error propagation is intentional and ensure any required post-unmarshal validations are performed elsewhere if needed.

func parseDevcontainerCLILastLine[T any](ctx context.Context, logger slog.Logger, p []byte, result T) error {

agent/agentcontainers/devcontainercli.go

mafredri · 2025-06-12T16:46:59Z

agent/agentcontainers/subagent.go

+			return SubAgent{}, xerrors.Errorf("unexpected codersdk.DisplayApp: %#v", displayApp)
+		}
+
+		displayApps = append(displayApps, app)


I've been thinking what the default behavior here should be. If no customization is set we could potentially inherit parent apps or defaults. It won't be the best experience if there are no apps by default.

(Currently in my other feature branch I default to using parent agent apps as a placeholder.)

Yeah, I thought about this as well. I think inheriting parent display apps (not coder_apps) is definitely an option, although we could also have a default of "vscode", "web_terminal", and "ssh_helper"? That should cover a basic use-case.

As for inheriting "apps", I think that wouldn't work as many (or all?) work based on agent name, so copying it 1:1 would mean the app is visible but would just connect you to the parent agent instead of the sub agent.

Yeah, sorry for being unclear. I was only referring to display apps here, not other apps 👍🏻

agent/agentcontainers/devcontainercli.go

mafredri · 2025-06-12T16:52:18Z

agent/agentcontainers/devcontainercli.go

 )

+// DevcontainerConfig is a wrapper around the output from `read-configuration`.
+// Unfortunately we cannot make use of `dcspec` as the output doesn't appear to
+// match.


This is unfortunate. Any particular thing it choked on?

Btw, I notice there's no "merged" configuration. I suppose we won't get customizations added by features etc here?

They aren't 1:1 on schema.

{ "configuration": { "name": "Development environments on your infrastructure", "image": "codercom/oss-dogfood:latest", "features": { "ghcr.io/devcontainers/features/docker-in-docker:2": { "moby": "false" } }, "runArgs": [ "--cap-add=SYS_PTRACE" ], "customizations": { "vscode": { "extensions": [ "biomejs.biome" ] } }, "configFilePath": { "$mid": 1, "fsPath": "/home/coder/coder/.devcontainer/devcontainer.json", "path": "/home/coder/coder/.devcontainer/devcontainer.json", "scheme": "vscode-fileHost" } }, "workspace": { "workspaceFolder": "/workspaces/coder", "workspaceMount": "type=bind,source=/home/coder/coder,target=/workspaces/coder" } }

If I run cat agent/agentcontainers/dcspec/devContainer.base.schema.json | rg 'configFilePath', it shows no results. Maybe it is just that field but it is definitely a little unfortunate.

I don't see that configFilePath is needed for anything since we already know it, or am I missing something?

mafredri · 2025-06-12T16:53:29Z

agent/agentcontainers/devcontainercli.go

+	}
+	if configPath != "" {
+		args = append(args, "--config", configPath)
+	}


Did you consider container ID and include merged?

I suppose there is no harm in adding container ID, or including merged, but I don't think they're strictly needed.

From our slack discussion it appears we need to use on-disk configuration to have all the variables substituted correctly, and this on-disk configuration takes priority over the container's configuration. For this reason I decided against using the container ID.

I suppose we could make use of merged? For now I've only implemented the absolute minimum we need, but maybe there is a use-case for merged config that I've missed?

If we're only considering display apps, then the only use-case I can think of is having a Coder base-feature for all your devcontainers that sets some default customizations.

Considering merged gives us both the on-disk config and potential additions from container/features, it seems to me like the best way to handle all potential configuration needs, rather than occasionally using merged and other times not.

mafredri

Looks good! 👍🏻 I'm fine with merging as-is. I think we may still need to introduce the --container-id for something in the future but we can cross that bridge when we come to it.

I recall we talked about caching the JSON in memory (vs reading every time we inject). But if we need to, that's simple enough to address later.

And the final point about using dcspec or not, also fine to defer for later.

github-actions bot assigned DanielleMaywood Jun 12, 2025

DanielleMaywood added 2 commits June 12, 2025 11:09

feat(agent/agentcontainers): support displayApps from devcontainer co…

3b44a89

…nfig

chore: appease linter and remove duplicated test

e12ccc7

DanielleMaywood force-pushed the dm-sub-agent-apps-configuration branch from 6036ba2 to e12ccc7 Compare June 12, 2025 11:09

DanielleMaywood requested a review from Copilot June 12, 2025 11:09

Copilot AI reviewed Jun 12, 2025

View reviewed changes

DanielleMaywood marked this pull request as ready for review June 12, 2025 11:21

DanielleMaywood requested review from mafredri and johnstcn June 12, 2025 11:21

johnstcn reviewed Jun 12, 2025

View reviewed changes

agent/agentcontainers/devcontainercli.go Outdated Show resolved Hide resolved

chore: listen to feedback

4b6209e

johnstcn approved these changes Jun 12, 2025

View reviewed changes

mafredri reviewed Jun 12, 2025

View reviewed changes

chore: include merged, com.coder to coder, UnmarshallJSON

aa77e9e

mafredri approved these changes Jun 12, 2025

View reviewed changes

DanielleMaywood merged commit dd15026 into main Jun 12, 2025
34 checks passed

DanielleMaywood deleted the dm-sub-agent-apps-configuration branch June 12, 2025 22:36

github-actions bot locked and limited conversation to collaborators Jun 12, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

feat(agent/agentcontainers): support displayApps from devcontainer config #18342

feat(agent/agentcontainers): support displayApps from devcontainer config #18342

Uh oh!

DanielleMaywood commented Jun 12, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

mafredri Jun 12, 2025

Uh oh!

DanielleMaywood Jun 12, 2025

Uh oh!

mafredri Jun 12, 2025

Uh oh!

Uh oh!

mafredri Jun 12, 2025

Uh oh!

DanielleMaywood Jun 12, 2025

Uh oh!

mafredri Jun 12, 2025

Uh oh!

mafredri Jun 12, 2025

Uh oh!

DanielleMaywood Jun 12, 2025

Uh oh!

mafredri Jun 12, 2025

Uh oh!

mafredri left a comment

Uh oh!

Uh oh!

Uh oh!

feat(agent/agentcontainers): support displayApps from devcontainer config #18342

feat(agent/agentcontainers): support displayApps from devcontainer config #18342

Uh oh!

Conversation

DanielleMaywood commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Example

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

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!

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!

Uh oh!

Uh oh!

DanielleMaywood commented Jun 12, 2025 •

edited

Loading