-
Notifications
You must be signed in to change notification settings - Fork 923
feat(agent/agentcontainers): support agent name in customization #18451
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
This PR supports specifying a name that will be used for the devcontainer agent in the customizations section of the devcontainer.json configuration file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for specifying a custom agent name through the devcontainer.json configuration.
- Adds a new Name field to CoderCustomization in devcontainercli.go.
- Updates the API logic to use the agent name from the last customization layer.
- Enhances tests in api_test.go to verify that only the last customization layer's name is applied.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
agent/agentcontainers/devcontainercli.go | Adds a new Name field to the customization struct |
agent/agentcontainers/api.go | Implements logic to select the agent name from the last layer |
agent/agentcontainers/api_test.go | Introduces tests validating the correct agent name behavior |
Comments suppressed due to low confidence (1)
agent/agentcontainers/api_test.go:1765
- The test 'NameIsOnlyUsedWhenInLastLayer' currently verifies that the sub-agent name does not equal "custom-name" but does not assert the expected fallback name. Consider asserting an explicit default value to clarify the intended behavior.
require.NotEqual(t, "custom-name", subAgent.Name)
Technically this doesn't really test anything new but it is good to have an explicit test for anyways.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need some input validation but approach seems good to me otherwise 👍🏻
We want to be able to pick customizations that are only specified in the top-level customizations stanza. That is, we do not want to accidentally pick something specified in a feature (such as the agent name). This refactor picks the agent name from configuration instead of the merged configuration to ensure a feature cannot influence it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid! 👍🏻
Relates to coder/internal#732
This PR supports specifying a name that will be used for the devcontainer agent in the customizations section of the devcontainer.json configuration file.
A follow up PR will introduce a fallback that names agents based on the folder name before falling back to the container name.