Skip to content

RSDK-13909: Update CLI machine shell for windows#6030

Open
allisonschiang wants to merge 4 commits into
viamrobotics:mainfrom
allisonschiang:cli-fix-shell-stty-windows
Open

RSDK-13909: Update CLI machine shell for windows#6030
allisonschiang wants to merge 4 commits into
viamrobotics:mainfrom
allisonschiang:cli-fix-shell-stty-windows

Conversation

@allisonschiang
Copy link
Copy Markdown
Member

@allisonschiang allisonschiang commented May 20, 2026

Currently, viam machines part shell doesn't work on windows because the implementation uses stty, which doesn't work on windows. I am adding a new implementation that should work on all OSs

Testing:
Windows -> mac works
Windows -> linux works
mac -> linux works
mac -> windows doesn't work (as expected) but updated it to warn instead of silently failing
linux -> mac works

The setRaw helper in startRobotPartShell shelled out to stty to put
the local terminal into raw mode for an interactive remote shell. stty
isn't available on Windows, so `viam machine part shell` failed with
"exec: stty: executable file not found in %PATH%".

Replace exec.Command("stty", ...) with term.MakeRaw / term.Restore from
golang.org/x/term, which is already a dependency (used by cli/auth.go)
and handles both POSIX termios and the Windows console API natively.
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label May 20, 2026
@viambot viambot added safe to test This pull request is marked safe to test from a trusted zone and removed safe to test This pull request is marked safe to test from a trusted zone labels May 20, 2026
Comment thread cli/client.go
return rawMode.Run()
}
if err := setRaw(true); err != nil {
stdinFd := int(os.Stdin.Fd())
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stty doesn't work on windows - this alternative does

Comment thread services/shell/client.go
for {
resp, err := client.Recv()
if err != nil {
errMsg := ""
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

error msg would not propogate when trying to connect to windows and it would just silently fail, so I added this here so our "windows isn't supported" error we already have is visible

@allisonschiang allisonschiang marked this pull request as ready for review May 20, 2026 17:35
@allisonschiang allisonschiang requested a review from a team as a code owner May 20, 2026 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

safe to test This pull request is marked safe to test from a trusted zone

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants