- Notifications
You must be signed in to change notification settings - Fork917
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 ourterms of service andprivacy 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 thedevcontainer agent in the customizations section of thedevcontainer.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)Uh oh!
There was an error while loading.Please reload this page.
Technically this doesn't really test anything new but it is good to havean 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 👍🏻
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
We want to be able to pick customizations that are only specified in thetop-level customizations stanza. That is, we do not want to accidentallypick something specified in a feature (such as the agent name).This refactor picks the agent name from configuration instead of themerged 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! 👍🏻
Uh oh!
There was an error while loading.Please reload this page.
a04268a intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Relates tocoder/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.