- Notifications
You must be signed in to change notification settings - Fork1k
feat(cli): add dynamic completions for ssh command#20171
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
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Adds CompletionHandler to the ssh command that dynamically suggestsworkspace and agent targets based on the user's running workspaces.Features:- Suggests workspace name for single-agent workspaces- Suggests agent.workspace format for all agents in multi-agent workspaces- Only shows running workspaces (matches immediate availability)- Alphabetically sorted completions for better UX- Graceful error handling (returns error strings for debugging)Tests cover single-agent, multi-agent, and network error scenarios.Amp-Thread-ID:https://ampcode.com/threads/T-d137d343-53f3-4ece-be5a-584249bbd9e8Co-authored-by: Amp <amp@ampcode.com>
Verify that multi-agent workspaces do not suggest the bare workspacename (only agent.workspace format should be suggested).Amp-Thread-ID:https://ampcode.com/threads/T-d137d343-53f3-4ece-be5a-584249bbd9e8Co-authored-by: Amp <amp@ampcode.com>
Amp-Thread-ID:https://ampcode.com/threads/T-d137d343-53f3-4ece-be5a-584249bbd9e8Co-authored-by: Amp <amp@ampcode.com>
code-asher left a comment• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
This is really cool, been hoping we could get this since the wildcard switch 😎
Do you know how this is supposed to work in practice? I triedcoder completion and it generates a bashrc that saysSetup Bash to use the function for completions for '' which seems weird (the empty string) and neitherssh <tab> norcoder ssh <tab> are showing my workspaces. I tried manually sourcing the bashrc but it looks like the call tocomplete is giving an error.
The code looks good here, so I wonder if it is a problem with Serpent.
Edit: sorry, I see this is in draft, I got overly excited.
| ifws.LatestBuild.Status!=codersdk.WorkspaceStatusRunning { | ||
| continue | ||
| } |
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.
Can we show workspaces that are not running? Could be convenient, otherwise you have to start the workspace you want first, and thenssh (see next comment too).
| varagents []codersdk.WorkspaceAgent | ||
| for_,resource:=rangews.LatestBuild.Resources { | ||
| agents=append(agents,resource.Agents...) | ||
| } |
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.
If we decide to allow workspaces that are not running, since those will not have agents in the response we would have to add a call to template version resources. We could take inspiration from the function we used to have inconfigssh.go:
Lines 147 to 195 in0b2ba96
| funcsshFetchWorkspaceConfigs(ctx context.Context,client*codersdk.Client) ([]sshWorkspaceConfig,error) { | |
| res,err:=client.Workspaces(ctx, codersdk.WorkspaceFilter{ | |
| Owner:codersdk.Me, | |
| }) | |
| iferr!=nil { | |
| returnnil,err | |
| } | |
| varerrGroup errgroup.Group | |
| workspaceConfigs:=make([]sshWorkspaceConfig,len(res.Workspaces)) | |
| fori,workspace:=rangeres.Workspaces { | |
| i:=i | |
| workspace:=workspace | |
| errGroup.Go(func()error { | |
| resources,err:=client.TemplateVersionResources(ctx,workspace.LatestBuild.TemplateVersionID) | |
| iferr!=nil { | |
| returnerr | |
| } | |
| wc:=sshWorkspaceConfig{Name:workspace.Name} | |
| varagents []codersdk.WorkspaceAgent | |
| for_,resource:=rangeresources { | |
| ifresource.Transition!=codersdk.WorkspaceTransitionStart { | |
| continue | |
| } | |
| agents=append(agents,resource.Agents...) | |
| } | |
| // handle both WORKSPACE and WORKSPACE.AGENT syntax | |
| iflen(agents)==1 { | |
| wc.Hosts=append(wc.Hosts,workspace.Name) | |
| } | |
| for_,agent:=rangeagents { | |
| hostname:=workspace.Name+"."+agent.Name | |
| wc.Hosts=append(wc.Hosts,hostname) | |
| } | |
| workspaceConfigs[i]=wc | |
| returnnil | |
| }) | |
| } | |
| err=errGroup.Wait() | |
| iferr!=nil { | |
| returnnil,err | |
| } | |
| returnworkspaceConfigs,nil | |
| } |
| } | ||
| iflen(agents)==1 { | ||
| completions=append(completions,ws.Name) |
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.
What do you think about always adding this completion? Without the agentssh will use the first one and it could be convenient if a workspace has multiple agents that you can use the shorter syntax.
| } | ||
| res,err:=client.Workspaces(inv.Context(), codersdk.WorkspaceFilter{ | ||
| Owner:"me", |
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.
Could beOwner: codersdk.Me I think
| err:=inv.WithContext(ctx).Run() | ||
| // Completion handlers may fail gracefully, so we don't assert error | ||
| _=err |
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 am not quite following, why can we not require.NoError(err)? Looks like the handler never errors, but if it did that would be a problem we should catch right?
| err:=inv.WithContext(ctx).Run() | ||
| // Completion handlers may fail gracefully, so we don't assert error | ||
| _=err |
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.
Same here
| err:=inv.WithContext(ctx).Run() | ||
| _=err |
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.
And here
Uh oh!
There was an error while loading.Please reload this page.
Adds CompletionHandler to the ssh command that dynamically suggests workspace and agent targets based on the user's running workspaces.
Features:
Tests cover single-agent, multi-agent, and network error scenarios.
Amp-Thread-ID:https://ampcode.com/threads/T-d137d343-53f3-4ece-be5a-584249bbd9e8
closes#20158