-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: add ability to attach to devcontainers #463
Conversation
Adds the ability to connect to a running devcontainer inside a Coder Worksapce.
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 functionality to attach to a running devcontainer within a Coder Workspace. The changes include introducing a new utility to build remote authority strings, updating the extension to handle a new "/openDevContainer" path and register a corresponding command, and implementing the openDevContainer command in the commands module.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/util.ts | Adds toRemoteAuthority to centralize remote authority string logic. |
src/extension.ts | Introduces new query parameter processing and command invocation for devcontainers. |
src/commands.ts | Implements openDevContainer command and refactors openWorkspace to leverage toRemoteAuthority. |
const workspaceAgent = undefined // args[2] is reserved, but we do not support multiple agents yet. | ||
const devContainerName = args[3] as string |
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.
The extension passes a workspaceAgent parameter for the openDevContainer command, but this parameter is being discarded by explicitly setting workspaceAgent to undefined. Consider either using args[2] if provided or updating the extension to no longer supply the parameter.
const workspaceAgent = undefined // args[2] is reserved, but we do not support multiple agents yet. | |
const devContainerName = args[3] as string | |
const workspaceAgent = args[2] as WorkspaceAgent | undefined // Use args[2] if provided. |
Copilot is powered by AI, so mistakes are possible. Review output carefully before use.
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.
We already have precedent for doing this with the openWorkspace
flow
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.
Slick! I only tested VS Code (not Cursor, etc), but it looks good.
const devContainerFolder = params.get("devContainerFolder") | ||
|
||
if (!workspaceOwner) { | ||
throw new Error("workspace owner must be specified as a query parameter") |
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.
No need to change in this PR since this is a pattern that already existed, but at some point I wonder if we should catch handler errors and show them in an error notification. Currently the only way to see them seems to be to go to the extension's features > runtime status > uncaught errors.
Relates to coder/coder#16426
Adds the ability to connect to a running devcontainer inside a Coder Workspace.
We add a new handler to handle the URI
With the following query params:
url
: The URL of the deploymenttoken
: The API keyowner
: The workspace ownerworkspace
: The workspace namedevContainerName
: The name of the dev containerdevContainerFolder
: The folder inside the dev container